Warehouse improvements #3
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR improves warehouse selection and display functionality by unifying parameter names (replacing "warehouse_id" with "warehouse"), adding fuzzy matching for warehouse names, and enhancing the TUI display behavior using conditional display.
- Updates test cases to reflect parameter renaming and improved verification for warehouse selection
- Modifies the warehouse commands (list and select) to support fuzzy matching and conditional display
- Updates UI and command registry to support the new “conditional” agent_display mode
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_tui_display.py | Added extensive tests for basic display, styling, empty list, type conversion, and current warehouse message |
| tests/test_agent_tools.py | Updated handler assertions to include tool_output_callback in the call |
| tests/test_agent_tool_display_routing.py | Adjusted routing tests for conditional display in "list-warehouses" command |
| tests/fixtures.py | Updated add_warehouse fixture to accept additional parameters |
| tests/commands/test_warehouse_selection.py | Renamed tests and updated expectations for warehouse selection |
| tests/commands/test_list_warehouses.py | Introduced new tests for warehouse listing and display parameters |
| src/ui/tui.py | Enhanced TUI display functions to support full table display and styling |
| src/commands/warehouse_selection.py | Refactored warehouse selection to use fuzzy matching and updated parameter naming |
| src/commands/list_warehouses.py | Adjusted list warehouses command to handle display parameter and conditional output |
| src/command_registry.py | Updated CommandDefinition documentation for new agent_display values |
| src/agent/tool_executor.py | Added passing of output_callback in tool_args |
| src/agent/prompts/default_system_prompt.py | Improved the system prompt with updated instructions for warehouse commands |
Comments suppressed due to low confidence (2)
src/commands/warehouse_selection.py:68
- The error message 'warehouse parameter is required.' is generic. Consider providing additional context to distinguish between a missing parameter and a situation where the provided warehouse value does not match any warehouse.
if not warehouse:
src/commands/warehouse_selection.py:41
- The immediate return on substring match may preempt a better fuzzy match. Consider evaluating all candidates to select the best match based on the similarity score.
if target_lower in warehouse_lower or warehouse_lower.startswith(target_lower):
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Improve the ease of use for selecting and viewing warehouses