Unify tool output views - #880
Conversation
WalkthroughTool results now expose finalized model, human, artifact, and diagnostic views. Execution paths finalize outcomes after budgeting and rebudgeting. Agent, CLI, and TUI consumers use the appropriate derived view. ChangesTool outcome finalization
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant ToolRegistry
participant OutcomeFinalizer
participant AgentLoop
participant CLIWriter
participant TUI
ToolRegistry->>OutcomeFinalizer: processed output and boundary output
OutcomeFinalizer-->>ToolRegistry: finalized ToolOutcome
ToolRegistry->>AgentLoop: finalized tool result
AgentLoop->>AgentLoop: select ModelOutput
AgentLoop->>CLIWriter: model and human views
AgentLoop->>TUI: finalized tool result
TUI->>TUI: select HumanDisplay or ModelOutput
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/tools/types.go`:
- Around line 139-154: Persist the finalized state of ToolOutcome across JSON
serialization by exposing a serialized marker or implementing custom
marshaling/unmarshaling for finalized. Ensure restored outcomes make Finalized()
return true so HumanDisplay(), ModelOutput(), and toolResultDetail() retain
reduced error evidence; add a JSON round-trip regression covering a reduced
error outcome.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ab240c0-d033-48f8-b39f-39a06d4a229c
📒 Files selected for processing (10)
internal/agent/loop.gointernal/agent/types.gointernal/cli/exec_writer.gointernal/tools/output_boundary.gointernal/tools/registry.gointernal/tools/tool_outcome.gointernal/tools/tool_outcome_test.gointernal/tools/types.gointernal/tui/model.gointernal/tui/tool_outcome_test.go
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/tui/tool_outcome_test.go`:
- Around line 52-54: Update TestToolResultDetailSurvivesOutcomeJSONRoundTrip to
set TMP and TEMP to t.TempDir() in addition to TMPDIR, ensuring os.TempDir uses
the test-owned directory on Windows for exec_command output spills.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 887dc319-f869-40f9-81e8-f05b45d5f520
📒 Files selected for processing (2)
internal/tools/tool_outcome.gointernal/tui/tool_outcome_test.go
What changed
ToolOutcomethat separates model-facing output, human-facing detail, recoverable artifacts, and reduction diagnosticsWhy
Tool output previously relied on several loosely related fields and metadata conventions. This made it easy for model context, user-visible detail, and recoverable raw output to drift or accidentally use the wrong representation.
The unified boundary keeps model context compact while retaining useful detailed output for users and exact redacted artifacts for recovery and inspection.
Measured behavior
The corpus boundaries differ slightly, so the primary claim is preservation of the existing token reduction with improved human detail and recoverability, rather than a standalone 2-point performance gain.
Validation
make fmt-checkgo vet ./...internal/tools,internal/agent,internal/tui, andinternal/clitestsgo run ./cmd/zero-release buildgo run ./cmd/zero-release smokemake lint-staticmake vulncheckgit diff HEAD --checkThe full local suite also exercised all affected packages successfully. Existing environment/baseline failures remain in doctor connectivity, worktree-trust end-to-end, and daemon end-to-end tests; this change does not touch those paths.
Summary by CodeRabbit
New Features
Bug Fixes