Preserve context when compressing tool results - #15
Open
wangkailang wants to merge 1 commit into
Open
Conversation
Co-authored-by: multica-agent <github@multica.ai>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates tool-result compression so large tool outputs retain useful leading/trailing context instead of being replaced by a fully generic placeholder. This improves downstream model behavior when recent tool outputs contain critical identifiers, headers/footers, or “BEGIN/END”-style delimiters.
Changes:
- Replace the old “compressed tool result” placeholder with a head/tail snippet strategy (with an explicit compression notice).
- Add helpers to extract tool-result text consistently and build a compressed snippet with omitted-length metadata.
- Update the unit test to assert that compressed outputs preserve important boundary context while being shorter than the original.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main/ai/token-budget.ts | Implements head/tail snippet compression for large tool results and centralizes tool-result text extraction. |
| src/main/ai/tests/token-budget.test.ts | Strengthens compression test to require preserved semantic context (BEGIN/END markers) rather than a pure placeholder. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wangkailang
added a commit
that referenced
this pull request
May 23, 2026
#6 + #14 useBranchDiff races: generation token discards stale-path responses (path change mid-flight no longer commits to new key); refresh() now passes force=true so it bypasses the inflight guard. #7 + #9 + #11 + #15 git-diff-handler robustness: - guard `parsed.hunks ?? []` so a single malformed parsePatch entry (binary patch, weird header) doesn't tank the whole diff. Per-entry try/catch + console.warn instead of bubbling. - check `nameStatus.exitCode` before parsing; tolerate failure by degrading renames to add+delete. - detached HEAD (`rev-parse --abbrev-ref` returns literal "HEAD") falls back to the short SHA so the drawer title doesn't read "HEAD vs main". - truncated hunk runs now recount added/removed against the kept text rather than the original line count. #8 + #10 approval-batcher cancellation race: - enrichAndSend iterates a snapshot of buf.entries so a concurrent removeEntry splice can't perturb the loop. - re-check `pendingBatches.has(batchId)` after the preview Promise.all so a cancellation during the await window no longer produces a ghost approval card whose buttons no-op. #12 BranchDiffPanel drag cleanup: track active mousemove/mouseup handlers in a ref; useEffect destructor removes them and resets body cursor/userSelect if the panel unmounts mid-drag. #13 invalidator wire-up: App subscribes ai:stream-tool-result and bumps `diffInvalidator` for destructive tools (writeFile, moveFile, deleteFile, createDirectory, runCommand); Sidebar and BranchDiffPanel both consume it, so the pill and panel reflect post-write state without the 30 s TTL. Tests: 773 passing, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wangkailang
added a commit
that referenced
this pull request
May 23, 2026
…ff panel (#94) * feat(chat,git-diff): codex-style pre-execute preview gate + branch diff panel Two related capabilities for surfacing destructive changes: 1. Pre-execute preview in the approval card. Main process generates a structured preview for writeFile / moveFile / deleteFile / createDirectory / runCommand before the tool runs and ships it via the existing ai:stream-tool-batch-approval IPC. Snapshot is reused after apply so the post-execute tool card matches what the user approved without re-reading disk. Plan-approved fast path also captures a preview snapshot for the result card. 2. Branch diff panel on the right. Persistent in-flow panel (not a modal) showing current branch vs main, including uncommitted working-tree edits (single-arg `git diff <merge-base>`, not the three-dot form that misses uncommitted work). Files render as ai-elements InlineCitation cards with click-to-expand bodies. Left-edge resize handle, width persisted to localStorage. Misc: - Drop nested max-h-96 overflow in writeFile diff body, removes the inner scrollbar. - Local InlineCitation* component family ported from ai-elements (Context-driven, no Radix/Floating-UI dep). - Regen i18n types for 14 new keys across zh-CN / en / ja. Tests: +24 preview-generator units, +4 git-diff handler integration; total 773 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat,git-diff): address top-5 code-review findings 1. WriteFilePreviewCard fallback messages were hard-coded English; replace with the matching `LL.preview_*` keys so zh-CN/ja users see localized text in the binary / too-large / truncated / no-changes branches. 2. jsonl-store.saveMessages now strips `ToolPart.previewSnapshot` and `BatchApprovalEntry.preview` before write. Both were commented "not persisted" but actually were — would bloat session files and hydrate stale diffs on reload. 3. plan-runner and fork-skill-runner now call `consumePreview` on `tool_execution_start` and ship `previewSnapshot` in the IPC payload, matching ai-handlers. Without this, plan/skill-fork approvals populated the snapshot LRU but the renderer never received it → post-execute tool card showed no diff. 4. delete-file childCount cap loop: the old `if (childCount >= CAP) break;` check was dead because childCount never mutated inside the loop. Rewrite as a `for (let i = 0; i < min(entries.length, CAP); i++)` so the byte sum matches the displayed `+N` label for dirs with >5000 entries. 5. useBranchDiff: add `currentBranch` to the cache key so BranchSwitcher checkouts invalidate immediately instead of waiting for the 30 s TTL. Sidebar and BranchDiffPanel both thread the live branch through. Tests: 773 passing, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat,git-diff): address remaining 10 code-review findings #6 + #14 useBranchDiff races: generation token discards stale-path responses (path change mid-flight no longer commits to new key); refresh() now passes force=true so it bypasses the inflight guard. #7 + #9 + #11 + #15 git-diff-handler robustness: - guard `parsed.hunks ?? []` so a single malformed parsePatch entry (binary patch, weird header) doesn't tank the whole diff. Per-entry try/catch + console.warn instead of bubbling. - check `nameStatus.exitCode` before parsing; tolerate failure by degrading renames to add+delete. - detached HEAD (`rev-parse --abbrev-ref` returns literal "HEAD") falls back to the short SHA so the drawer title doesn't read "HEAD vs main". - truncated hunk runs now recount added/removed against the kept text rather than the original line count. #8 + #10 approval-batcher cancellation race: - enrichAndSend iterates a snapshot of buf.entries so a concurrent removeEntry splice can't perturb the loop. - re-check `pendingBatches.has(batchId)` after the preview Promise.all so a cancellation during the await window no longer produces a ghost approval card whose buttons no-op. #12 BranchDiffPanel drag cleanup: track active mousemove/mouseup handlers in a ref; useEffect destructor removes them and resets body cursor/userSelect if the panel unmounts mid-drag. #13 invalidator wire-up: App subscribes ai:stream-tool-result and bumps `diffInvalidator` for destructive tools (writeFile, moveFile, deleteFile, createDirectory, runCommand); Sidebar and BranchDiffPanel both consume it, so the pill and panel reflect post-write state without the 30 s TTL. Tests: 773 passing, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(git-diff): origin baseline + ahead/behind/uncommitted badges User feedback: after `git push`, the diff panel still showed the same files even though they were already on the remote — the panel had no signal to communicate "this is shipped, just waiting on merge." codex CLI sidesteps this entirely (no branch-level aggregator). We follow the GitHub PR Files convention instead: - Baseline is `origin/<baseBranch>` whenever the remote tracking ref exists (probed via `rev-parse --verify --quiet`). Falls back to the local branch when the repo has no origin (eg fresh init). The drawer title now reads `<head> vs origin/main` to make the basis explicit. Diff content unchanged after push — that's intentional; PR merge is what clears it. - Header gets three status pills so the user can tell *where* the changes are: * `+N -N` aggregate (existing) * `N uncommitted` — amber, count of `git status --porcelain` rows * `N unpushed` — emerald, ahead of `origin/<currentBranch>` * `N behind` — red, behind upstream (needs pull) All three are derived from extra lightweight git invocations (Promise.all). Detached HEAD or branches without upstream silently omit ahead/behind. i18n: +3 keys × 3 locales. Tests: 773 passing, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(git-diff): uncommitted skips untracked + binary detection User reported the "未提交 2" pill showing 2 even after committing everything — the two were just `.filework/` editor cache and a Vite build artifact, both untracked. Filter `??` rows out of collectUncommitted so the pill reflects what the user means by "uncommitted" (modified / staged / deleted tracked files). Also: binary files (eg PNG/JPG screenshots) were rendering as `+0 -0` cards with empty bodies because parsePatch returns ParsedFile with valid filenames but `hunks: []` for `Binary files ... differ` entries. We now set `isBinary: true` when hunks is empty and the status isn't a pure rename — the existing BranchDiffFileCard already renders a "(diff) binary" placeholder for that case. Tests: 773 passing, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(branch-diff): refresh spinner gets stuck on stale generation Root cause: the finally block only reset loading when `myGen === generation.current`. If a refresh (force=true) bumped generation while a previous fetch was still in flight, the old fetch's finally would see a mismatch and skip the reset. If the new fetch then itself became stale via a third bump, no one ever cleared loading and the spinner span forever. Replace the loadingRef boolean with an `inflight` counter: - enter: inflight++ and setLoading(true) - exit (any path, stale or not): inflight-- and only setLoading(false) when inflight reaches 0 Stale-write protection on `setData`/`setError` is unchanged — those still gate on `myGen === generation.current`, so a stale fetch still can't overwrite fresher data. Only the spinner is decoupled. Tests: 773 passing, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(branch-diff): stop refetch loop after first invalidator bump The previous spinner-stuck fix decoupled `loading` from staleness but missed the actual driver: the main effect's short-circuit checked `invalidator === 0`, but invalidator is a monotonic counter living in App state — once it's bumped (any destructive tool completes), it's never 0 again, so the short-circuit became permanently false and every re-render re-issued a fetch. setData(result) caused a re-render, which re-ran the effect, which fetched again, which re-rendered… Replace the literal-zero check with a ref tracking the last invalidator value the hook actually acted on. Refetch only when that ref changes. The TTL fast-path now works as intended. Tests: 773 passing, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(branch-diff): satisfy biome lint (CI green) CI biome check flagged 4 errors + 1 info that local `pnpm typecheck` didn't catch: 1. useBranchDiff.ts reset effect — biome's useExhaustiveDependencies doesn't accept `[path, baseBranch, currentBranch]` as deps when the effect body doesn't read them. Compose them into a single `cacheKey` string and gate the reset on `lastCacheKey !== cacheKey`. Same semantic (key change → reset), but deps array is 1 item the body actually reads. 2. BranchDiffPanel.tsx resize handle — `role="separator"` triggers useAriaPropsForRole (needs aria-valuenow), useSemanticElements (separator → real <hr>), and useFocusableInteractive. The handle is a CSS-only mouse-drag affordance with no keyboard story, so drop the role/aria-label and add `aria-hidden="true"` instead. Keyboard a11y for resize is a separate enhancement. 3. BranchDiffPanel.tsx useless fragment — `<>{items}</>` around an already-keyed ReactNode[] is redundant; React renders arrays directly. 4. BranchDiffPanel.tsx useOptionalChain — `data && data.headBranch` → `data?.headBranch`. `pnpm lint && pnpm typecheck && pnpm test` all green locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.