Misc improvements #1152
Draft
Misc improvements #1152
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
Reduce boilerplate across tool components by introducing a reusable base component that handles common functionality: - Add toolcommon.Base component with shared Init/Update/SetSize/View logic - Add SimpleRenderer and SimpleRendererWithResult helpers for common patterns - Consolidate ShortenPath function into toolcommon package (was duplicated 4x) - Refactor 10 tool components to use base: shell, writefile, allowed, readfile, listdirectory, searchfiles, api, readmultiplefiles, editfile, defaulttool This reduces ~400 lines of duplicated boilerplate while maintaining all existing functionality. Assisted-By: cagent
Convert handoff, transfertask, and todotool components to use the toolcommon.Base component pattern, reducing ~86 lines of boilerplate. Assisted-By: cagent
Extract common dialog centering logic into CenterPosition helper, simplifying 6 Position() implementations and reducing ~38 lines. Assisted-By: cagent
Introduce ParseArgs[T] and ExtractField[T] generic functions to toolcommon package, simplifying 6 tool component argument extractors. Assisted-By: cagent
Extract common fake proxy setup pattern into setupFakeProxy function, removing duplicate code from run.go and api.go. Assisted-By: cagent
Extract repeated Unicode-aware text truncation pattern into TruncateText function in toolcommon package. Used by: - sidebar.go (agent description) - todotool/sidebar.go (todo descriptions) - commands.go (command palette descriptions) Assisted-By: cagent
Signed-off-by: David Gageot <david.gageot@docker.com>
- Add BaseDialog struct in pkg/tui/dialog/base.go with common patterns: - Size management (width, height, SetSize, Width, Height methods) - Dialog width/content width calculations - Centered positioning helper - ConfirmKeyMap for standard Yes/No dialogs - Render helpers: RenderTitle, RenderSeparator, RenderOptions, RenderHelp - HandleQuit and HandleConfirmKeys for common key handling - Update dialogs to use BaseDialog: - max_iterations.go: uses BaseDialog, ConfirmKeyMap, render helpers - oauth_authorization.go: uses BaseDialog, HandleConfirmKeys - command_palette.go: uses BaseDialog, render helpers - mcp_prompt_input.go: uses BaseDialog, render helpers - attachment_preview.go: uses BaseDialog.SetSize, RenderSeparator - Simplify tool factory registration: - Add toolRegistration struct for declarative tool registration - Group tools with same builder (e.g., all todo tools together) - Improve listdirectory pluralization: - Replace pluralize/pluralizeDirectory with formatCount(singular, plural) - More readable and self-documenting Assisted-By: cagent
- Add pkg/tui/core/events.go with reusable event patterns: - ScrollDirection enum and KeyScrollMap for scroll key detection - GetScrollDirection() to check if a key is a scroll key - IsQuitKey() and IsEscapeKey() for common key checks - MouseWheelDirection() for mouse wheel event handling - IsNavigationKey() for common navigation keys - Update tui.go to use IsNavigationKey() for completion handling - Update tool_confirmation.go to use: - HandleQuit() instead of manual ctrl+c check - GetScrollDirection() instead of key string switch Assisted-By: cagent
Remove the type aliases that re-exported message types from the messages package. These aliases served no purpose as they were only used within the same file. Now using the messages types directly improves clarity. Assisted-By: cagent
The MouseWheelDirection function in core/events.go was never used. All mouse wheel handling is done inline in the components that need it. Assisted-By: cagent
These utility functions in core/events.go were never used. Components use key.Matches() with their own KeyMap structures instead, which provides better consistency with the bubbles library patterns. Assisted-By: cagent
Convert toolConfirmationDialog to embed BaseDialog instead of manually managing width/height fields. This provides consistency with other dialogs and reuses the common SetSize, Width(), and Height() methods. Assisted-By: cagent
Replace manual for loop with slices.Contains for better readability and to address gopls hint about loop simplification. Assisted-By: cagent
Simplify the runtime package by reducing code duplication in two areas: 1. Tool error response methods: Consolidated three nearly identical methods (addToolValidationErrorResponse, addToolRejectedResponse, addToolCancelledResponse) into a single addToolErrorResponse method that accepts the error message as a parameter. 2. Tool execution logic: Created a shared executeToolWithHandler helper that handles the common patterns between runTool and runAgentTool: - OpenTelemetry span creation - Event emission (ToolCall, ToolCallResponse) - Error handling with context cancellation detection - Session message creation and storage These changes reduce approximately 80-100 lines of duplicated code while maintaining full functionality and all existing tests pass. Assisted-By: cagent
- Add getAgentModelID helper to reduce repeated model ID extraction pattern - Simplify discoverMCPPrompts: pre-size collections, remove nil checks on slices - Use slices.ContainsFunc in handleHandoff instead of manual loop - Remove redundant GetAgentName methods from events that embed AgentContext Assisted-By: cagent
Signed-off-by: David Gageot <david.gageot@docker.com>
Signed-off-by: David Gageot <david.gageot@docker.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.