fix: bound tool input projections - #952
Conversation
📝 WalkthroughWalkthroughAdds recursive tool-input truncation and a ChangesTool Input Truncation and UI Display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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.
Code Review
This pull request introduces truncation for long tool inputs during session compaction to prevent excessive token usage, setting a limit of 2,000 characters. It also refactors the UI to display concise tool summaries based on item counts (such as the number of questions or to-dos) rather than raw contents or schema errors. The review feedback notes that the subtitle for TOOL_TODOWRITE uses hardcoded English strings instead of localized keys, and suggests using i18n.t for proper internationalization.
Perf delta summaryComparator: pass
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/opencode/src/session/message-v2.ts`:
- Around line 94-102: The function truncateToolInput doesn't consistently honor
zero/negative maxChars for non-string inputs; add an explicit check early (after
the null check) that if maxChars <= 0 returns toolInputTruncatedMarker(maxChars)
so non-string paths behave the same as the string path (which uses
truncateToolInputString). Update truncateToolInput to return the marker for
maxChars <= 0 before calling serializedToolInputLength or processing objects;
keep existing calls to truncateToolInputString, serializedToolInputLength, and
toolInputTruncatedMarker unchanged otherwise.
- Around line 86-91: The current loop in appendToolInputMarkerToObject produces
a nested _truncated object (e.g., { _truncated: { _truncated: "..." } }) because
withMarker is built as { ...output, _truncated: marker } while output may
already contain a _truncated object; fix this by ensuring the final _truncated
value is the marker string itself (not an object) before measuring size: when
constructing withMarker, set _truncated to marker (a string) unconditionally or
replace any existing output._truncated with the marker string, then call
serializedToolInputLength(withMarker); update the logic around keys, output,
marker, withMarker and serializedToolInputLength to avoid nesting and to only
drop fields as needed.
🪄 Autofix (Beta)
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: 64f88b83-0aa4-474c-ba05-6fcc42ceed06
📒 Files selected for processing (4)
packages/opencode/src/session/message-v2.tspackages/opencode/test/session/message-v2.test.tspackages/ui/src/components/message-part/context-tool-helpers.test.tspackages/ui/src/components/message-part/context-tool-helpers.ts
Summary
Fixes a repeated projection-boundary bug where long or invalid tool inputs could leak into compact UI summaries or model replay during compaction. There is no linked issue; this came from a user-reported long-question failure that also matched an earlier apply_patch-class failure shape.
Why
Tool rows and compaction were treating raw tool state as presentation/replay data. That made one malformed or very long tool input able to break the summarization surfaces that are supposed to make the transcript smaller and safer to replay.
Related Issue
No issue. User-reported regression in the long-question tool path.
Human Review Status
Pending
Review Focus
Please focus on whether the input cap is applied at the correct projection boundary, without changing persisted tool state or normal tool execution behavior.
Risk Notes
Low to moderate. The raw tool input remains stored for lifecycle/debug/export paths; truncation is only applied when callers opt into model-message projection limits. The visible UI change affects compact tool row summary copy. The platform/CI surface is limited to desktop-smoke: the new Electron repair script is only called from
.github/workflows/desktop-smoke.ymlto restore Electron install metadata and verify the macOS framework before the raw CI smoke launch. It does not change packaged app runtime behavior, updater, signing, user paths, shell behavior, or permissions.How To Verify
Screenshots or Recordings
Visible check performed with
bun run snap session-trow; reviewed the generateddocs/design/preview/screenshots/session-trow.pnglocally for the compact tool row surface.Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
Bug Fixes
New Features
Tests