feat(tui): improve tool event handling with fallback matching and upd…#40
Merged
Conversation
Reviewer's GuideAdds fallback matching of tool result events by tool name when call IDs are missing or mismatched, and ensures standalone tool result log entries use the new ToolEntry-based rendering instead of the legacy emoji format. Sequence diagram for tool result event handling with ID and name fallbacksequenceDiagram
participant EventSource
participant TeaProgram
participant Model
participant ToolEntryMap as ToolCallEntries
participant LogEntries
EventSource->>TeaProgram: tool_result MessageEvent
TeaProgram->>Model: Update(msg)
Model->>Model: getToolCallIDFromEventContent(msg.event.Content)
alt call_id_found_and_matches_running_entry
Model->>ToolEntryMap: lookup by call_id
Model->>ToolEntryMap: update ToolEntry.SetResult(ToolResultInfo)
Model->>LogEntries: findLogEntryByToolCallID
Model->>LogEntries: update content and isToolRunning
Model->>ToolEntryMap: delete entry by call_id
Model->>Model: updateActiveAnim()
Model->>Model: buildViewportContent()
Model-->>TeaProgram: updated model, listenForEvents, tickCmd
else no_matching_call_id
Model->>Model: getToolNameFromEventContent(msg.event.Content)
alt tool_name_found_and_running_entry_exists
Model->>ToolEntryMap: findRunningEntryByName(tool_name)
Model->>ToolEntryMap: update matchedEntry.SetResult(ToolResultInfo)
Model->>LogEntries: findLogEntryByToolCallID(matchedID)
Model->>LogEntries: update content and isToolRunning
Model->>ToolEntryMap: delete entry by matchedID
Model->>Model: updateActiveAnim()
Model->>Model: buildViewportContent()
Model-->>TeaProgram: updated model, listenForEvents, tickCmd
else no_tool_name_match
Model->>Model: formatEventAsEntry(event)
Model-->>TeaProgram: updated model, listenForEvents, tickCmd
end
end
Updated class diagram for tool event and log entry handlingclassDiagram
class Model {
map~string,ToolEntry~ toolCallEntries
slice~LogEntry~ logEntries
Update(msg tea_Msg) tea_Model
updateActiveAnim()
buildViewportContent()
}
class LogEntry {
string content
bool isToolRunning
string rendered
string toolCallID
string toolName
ToolEntry toolEntry
}
class ToolEntry {
ToolCallInfo Call
ToolStatus Status
SetResult(result ToolResultInfo)
}
class ToolCallInfo {
string ID
string Name
string Summary
}
class ToolResultInfo {
string ToolCallID
string Name
string Content
bool IsError
}
class ToolStatus {
<<enum>>
ToolStatusRunning
ToolStatusOther
}
Model "*" --> "*" LogEntry : maintains
Model "*" --> "*" ToolEntry : tracks_running
LogEntry --> ToolEntry : optional_toolEntry
ToolEntry --> ToolCallInfo : has_call
ToolEntry --> ToolResultInfo : uses_result
class Helpers {
getToolCallIDFromEventContent(content interface_any) string
getToolNameFromEventContent(content interface_any) string
findRunningEntryByName(entries map_string_ToolEntry, toolName string) string_ToolEntry
getResultFromEventContent(content interface_any) string
formatEventAsEntry(event MessageEvent) LogEntry
findLogEntryByToolCallID(entries slice_LogEntry, id string) int
}
Helpers ..> Model : used_by
Helpers ..> LogEntry : creates_and_updates
Helpers ..> ToolEntry : queries_and_updates
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 comment on
findRunningEntryByNamesays it returns the "most recently-added" entry, but iteration over amapis not ordered; if recency matters, consider tracking insertion order (e.g., a slice or timestamp) or adjust the comment to match the actual behavior. - The
ToolResultInfoconstruction informatEventAsEntryduplicates the same struct literal for the error and non-error cases; you could build it once and setIsErrorfromisErrto reduce repetition and keep the two branches in sync.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The comment on `findRunningEntryByName` says it returns the "most recently-added" entry, but iteration over a `map` is not ordered; if recency matters, consider tracking insertion order (e.g., a slice or timestamp) or adjust the comment to match the actual behavior.
- The `ToolResultInfo` construction in `formatEventAsEntry` duplicates the same struct literal for the error and non-error cases; you could build it once and set `IsError` from `isErr` to reduce repetition and keep the two branches in sync.
## Individual Comments
### Comment 1
<location path="tui.go" line_range="1236-1242" />
<code_context>
+ // Create a ToolEntry for standalone results so they use new-style
+ // rendering (✓ read_file · /path/to/file) instead of the legacy
+ // emoji path (✅ read_file).
+ isErr := strings.HasPrefix(entry.content, "Error:")
+ entry.toolEntry = tui.NewToolEntry(tui.ToolCallInfo{
+ ID: entry.toolCallID,
+ Name: entry.toolName,
+ Summary: extractToolSummary(entry.toolName, ""),
+ })
+ if isErr {
+ entry.toolEntry.SetResult(tui.ToolResultInfo{
+ ToolCallID: entry.toolCallID,
</code_context>
<issue_to_address>
**suggestion:** The `ToolResultInfo` construction is duplicated and can be simplified using the existing `isErr` variable.
Since `isErr` is already computed, you can avoid duplicating the `ToolResultInfo` literal in both branches by constructing it once, setting `IsError: isErr`, and calling `SetResult` a single time. This removes repetition and makes future changes to the struct safer.
</issue_to_address>
### Comment 2
<location path="tui.go" line_range="1863-1866" />
<code_context>
+ return ""
+}
+
+// findRunningEntryByName finds the most recently-added running entry with the
+// given tool name in the toolCallEntries map. Returns the call ID and the entry.
+func findRunningEntryByName(entries map[string]*tui.ToolEntry, toolName string) (string, *tui.ToolEntry) {
+ for id, entry := range entries {
+ if entry.Call.Name == toolName && entry.Status == tui.ToolStatusRunning {
+ return id, entry
</code_context>
<issue_to_address>
**issue (bug_risk):** Iterating a map does not guarantee returning the most recently-added running entry, conflicting with the function’s documented behavior.
Because Go map iteration order is random, `findRunningEntryByName` may return any matching running entry when multiple calls with the same tool name are active. If true recency is required, consider tracking order explicitly (e.g., an ordered slice of IDs or a timestamp on `ToolEntry`) so you can deterministically select the newest entry, or adjust the contract/callers if any matching running entry is acceptable.
</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
+1236
to
+1242
| isErr := strings.HasPrefix(entry.content, "Error:") | ||
| entry.toolEntry = tui.NewToolEntry(tui.ToolCallInfo{ | ||
| ID: entry.toolCallID, | ||
| Name: entry.toolName, | ||
| Summary: extractToolSummary(entry.toolName, ""), | ||
| }) | ||
| if isErr { |
There was a problem hiding this comment.
suggestion: The ToolResultInfo construction is duplicated and can be simplified using the existing isErr variable.
Since isErr is already computed, you can avoid duplicating the ToolResultInfo literal in both branches by constructing it once, setting IsError: isErr, and calling SetResult a single time. This removes repetition and makes future changes to the struct safer.
Comment on lines
+1863
to
+1866
| // findRunningEntryByName finds the most recently-added running entry with the | ||
| // given tool name in the toolCallEntries map. Returns the call ID and the entry. | ||
| func findRunningEntryByName(entries map[string]*tui.ToolEntry, toolName string) (string, *tui.ToolEntry) { | ||
| for id, entry := range entries { |
There was a problem hiding this comment.
issue (bug_risk): Iterating a map does not guarantee returning the most recently-added running entry, conflicting with the function’s documented behavior.
Because Go map iteration order is random, findRunningEntryByName may return any matching running entry when multiple calls with the same tool name are active. If true recency is required, consider tracking order explicitly (e.g., an ordered slice of IDs or a timestamp on ToolEntry) so you can deterministically select the newest entry, or adjust the contract/callers if any matching running entry is acceptable.
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.
…ated formatting
Summary by Sourcery
Improve TUI handling and rendering of tool events, adding fallback matching by tool name and modernizing standalone tool result display.
New Features:
Enhancements: