feat: tui beautify & async lauch codebase#39
Merged
Merged
Conversation
Add list_dir to skipBodyTools so that its output body is not rendered in the TUI. Only the header (icon + name + directory path) will be shown, avoiding visual clutter from large directory listings.
Add search_by_regex to skipBodyTools to avoid rendering large result sets in the TUI. Only the header (icon + name + search query) will be shown.
- Lower log level from ERROR to WARN for better visibility of warnings - Make codebase health check asynchronous to avoid blocking TUI/HTTP startup - Increase health check timeout from 30s to 60s - Add warning message when codebase server fails to start - Upgrade codebase binary missing from WARN to ERROR level
- Add taskCompleteDialog overlay shown on task completion/failure - Success dialog displays "Task Completed" with OK button only - Failure dialog displays error message with OK button - Support Enter, Space, or Esc to dismiss the dialog - Use teal color theme to differentiate from auth confirmation dialog
Reviewer's GuideAdds a task-completion overlay dialog to the TUI, relaxes codebase server startup to use async health checks with clearer warnings, trims README feature marketing, and refines which tool results are body-less in TUI rendering. Sequence diagram for asynchronous codebase server startup and health checksequenceDiagram
participant Main
participant startCodebaseServer
participant FileSystem
participant CodebaseProcess
participant waitForCodebase
Main->>startCodebaseServer: startCodebaseServer(port, repoPath)
startCodebaseServer->>FileSystem: osStat(binPath)
alt binary_not_found
FileSystem-->>startCodebaseServer: osIsNotExist
startCodebaseServer->>Main: return nil
else binary_found
FileSystem-->>startCodebaseServer: ok
startCodebaseServer->>CodebaseProcess: execCommand(binPath, port, repoPath)
CodebaseProcess-->>startCodebaseServer: running_cmd
startCodebaseServer-->>Main: return cmd
startCodebaseServer->>waitForCodebase: go waitForCodebase(address, 60s)
note over waitForCodebase: Runs asynchronously
waitForCodebase-->>waitForCodebase: poll_health_until_timeout
alt health_check_fails
waitForCodebase->>CodebaseProcess: cmdProcessKill()
else health_check_succeeds
waitForCodebase-->>CodebaseProcess: no_action
end
end
Class diagram for updated TUI model and task completion dialogclassDiagram
class model {
bool showHistoryPanel
bool historyConfirmDelete
bool quitting
confirmDialog confirmDialog
taskCompleteDialog taskCompleteDialog
messagingMessagePublisher* publisher
chanMessagingMessagePublisher publisherCh
Update(msgTeaMsg) teaModel
View() string
renderConfirmDialog() string
renderTaskCompleteDialog() string
}
class confirmDialog {
bool open
int selectedOption
string question
}
class taskCompleteDialog {
bool open
string message
}
class lipglossStylesTaskComplete {
style taskCompleteBorderStyle
style taskCompleteTitleStyle
style taskCompleteMessageStyle
style taskCompleteButtonFocused
style taskCompleteButtonBlurred
}
model --> confirmDialog : uses
model --> taskCompleteDialog : uses
model --> lipglossStylesTaskComplete : styled_by
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new
taskCompleteDialogstruct and styles define amessagefield andtaskCompleteMessageStyle, butrenderTaskCompleteDialognever uses them, so the dialog content is always static; consider rendering the dialog’smessageand applying the corresponding style so success/failure details are actually shown. - The task completion dialog is always titled "Task Completed" and the help text omits
esceven though it’s handled inUpdate, which is confusing when displaying failure messages ("❌ Task Failed..."); consider deriving the title/help text from the dialog state or message so success and error cases are clearly distinguished and key hints match actual behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `taskCompleteDialog` struct and styles define a `message` field and `taskCompleteMessageStyle`, but `renderTaskCompleteDialog` never uses them, so the dialog content is always static; consider rendering the dialog’s `message` and applying the corresponding style so success/failure details are actually shown.
- The task completion dialog is always titled "Task Completed" and the help text omits `esc` even though it’s handled in `Update`, which is confusing when displaying failure messages ("❌ Task Failed..."); consider deriving the title/help text from the dialog state or message so success and error cases are clearly distinguished and key hints match actual behavior.
## Individual Comments
### Comment 1
<location path="tui.go" line_range="119-121" />
<code_context>
}
+// taskCompleteDialog holds the state of the task completion overlay dialog.
+type taskCompleteDialog struct {
+ open bool
+ message string
+}
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The `message` field on `taskCompleteDialog` is never used in rendering.
You set `taskCompleteDialog.message` with success/failure text, but `renderTaskCompleteDialog` hardcodes the title and never uses this field, so neither the error (`msg.err.Error()`) nor the success/failure copy is shown. Please render `m.taskCompleteDialog.message` in the dialog body, and optionally vary the title/styling based on success vs failure if that’s part of the message design.
Suggested implementation:
```golang
func (m model) renderTaskCompleteDialog() string {
return dialog.New().
Title("Task complete").
Body(m.taskCompleteDialog.message).
WithKeyMap(m.keyMap).
WithFocused(true).
String()
}
```
I only see the `taskCompleteDialog` struct, not the actual `renderTaskCompleteDialog` implementation, so you will need to align the `SEARCH` block with the real code. Specifically:
1. Locate `renderTaskCompleteDialog` in `tui.go`.
2. Replace the hardcoded body string (whatever literal is currently passed to `.Body(...)`) with `m.taskCompleteDialog.message` as shown.
3. If you also want to vary the title or styling by success/failure, you can:
- Either derive that from the contents of `m.taskCompleteDialog.message`, or
- Extend `taskCompleteDialog` with additional fields (e.g., `title string`, `isError bool`) and then conditionally set `.Title(...)` and any error styling based on those fields. If you add fields, update all `taskCompleteDialog{...}` composite literals to use named fields to avoid breakage.
</issue_to_address>
### Comment 2
<location path="tui.go" line_range="486-492" />
<code_context>
return m, nil
}
+ // Task complete dialog key handling
+ if m.taskCompleteDialog.open {
+ switch msg.String() {
+ case "enter", " ", "esc":
+ m.taskCompleteDialog.open = false
+ return m, nil
+ case "ctrl+c":
+ m.quitting = true
+ return m, tea.Quit
</code_context>
<issue_to_address>
**nitpick:** Key handling for the dialog doesn’t match the help text shown to the user.
The dialog currently accepts Enter, Space, Esc, and Ctrl+C, but `renderTaskCompleteDialog` only documents Enter and Space. Please either update the help text to include Esc/Ctrl+C or restrict the key handler to only the documented shortcuts to avoid surprising users.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+119
to
+121
| type taskCompleteDialog struct { | ||
| open bool | ||
| message string |
There was a problem hiding this comment.
suggestion (bug_risk): The message field on taskCompleteDialog is never used in rendering.
You set taskCompleteDialog.message with success/failure text, but renderTaskCompleteDialog hardcodes the title and never uses this field, so neither the error (msg.err.Error()) nor the success/failure copy is shown. Please render m.taskCompleteDialog.message in the dialog body, and optionally vary the title/styling based on success vs failure if that’s part of the message design.
Suggested implementation:
func (m model) renderTaskCompleteDialog() string {
return dialog.New().
Title("Task complete").
Body(m.taskCompleteDialog.message).
WithKeyMap(m.keyMap).
WithFocused(true).
String()
}I only see the taskCompleteDialog struct, not the actual renderTaskCompleteDialog implementation, so you will need to align the SEARCH block with the real code. Specifically:
- Locate
renderTaskCompleteDialogintui.go. - Replace the hardcoded body string (whatever literal is currently passed to
.Body(...)) withm.taskCompleteDialog.messageas shown. - If you also want to vary the title or styling by success/failure, you can:
- Either derive that from the contents of
m.taskCompleteDialog.message, or - Extend
taskCompleteDialogwith additional fields (e.g.,title string,isError bool) and then conditionally set.Title(...)and any error styling based on those fields. If you add fields, update alltaskCompleteDialog{...}composite literals to use named fields to avoid breakage.
- Either derive that from the contents of
Comment on lines
+486
to
+492
| // Task complete dialog key handling | ||
| if m.taskCompleteDialog.open { | ||
| switch msg.String() { | ||
| case "enter", " ", "esc": | ||
| m.taskCompleteDialog.open = false | ||
| return m, nil | ||
| case "ctrl+c": |
There was a problem hiding this comment.
nitpick: Key handling for the dialog doesn’t match the help text shown to the user.
The dialog currently accepts Enter, Space, Esc, and Ctrl+C, but renderTaskCompleteDialog only documents Enter and Space. Please either update the help text to include Esc/Ctrl+C or restrict the key handler to only the documented shortcuts to avoid surprising users.
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
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.
Summary by Sourcery
Add a task completion overlay dialog to the TUI and make the codebase server startup and health checks more user-visible and asynchronous.
New Features:
Enhancements: