Uh oh!
There was an error while loading. Please reload this page.
fix(tool-trow): stabilize concurrent tool summary and settle seam - #721
Merged
Conversation
… jitter A multi-tool running trow showed the active tool's description as the summary line, cycling through each tool's intent as tools started/finished in parallel — the 1234567 jitter. Switch to the whole-group bucket aggregation with a "正在" prefix (e.g. "正在读取 7 个文件,搜索 2 次"), counting the whole group including settled tools so the summary does not decrement as tools finish in batches. Single-tool rows keep the tool's own description (locked by existing tests, and the "what exactly is running" signal is useful when there is one). Running summary omits the failed count (it changes mid-group as tools error); errored tools still force-open their disclosure (trowNeedsAttention), so the failure signal is not lost — it just stops jittering the summary line.
…tack A tool row settled with a one-shot opacity-0→1 fade (SETTLE_FADE). When parallel tools finished together, N fades stacked into the 1234567 jitter. The row now settles by its shimmer stopping — the same seam as the 深度思考 disclosure title (light band → static muted text), no opacity fade — so a batch settle is N light bands dropping, not N fades stacking. Removes the now-dead deriveToolRowMotion / ToolRowMotion (the group summary keeps its own settle fade as the whole-group signal; the row no longer tracks everRunning) and the two settle-fade contract tests that locked the removed behavior. isToolRowRunning / isToolRowSettled stay (the row still uses them).
…review) Addresses Codex review findings on #721. P3 — the group summary icon still picked the active tool's kind, so a mixed-kind group (e.g. Read running + Grep running) flipped its icon as the active tool changed mid-run even though the aggregated summary text never moved. The icon now stays on the first bucket's kind (the same first-seen order the summary clauses use), so the whole summary row is stable. P2 — adds the contracts the prior commits lacked: - summarizeTrowTools({ live: true }) omits the failed count (it changes mid-group); settled includes it. - rendered rows never carry the settle-fade animation (the per-row seam is a light-band stop). A full running→settled rerender contract needs dynamic test infrastructure packages/ui lacks; this static contract locks the rendered output and the rerender case is tracked separately.
…JSDoc (re-review) Addresses Codex re-review findings on #721. P2 — the prior row no-fade test only did a terminal first-mount SSR, so the old everRunning-gated fade implementation also passed it; it didn't lock the real running→settled regression. Replace it with a source contract over tool-activity.tsx: the row never reintroduces the per-row settle fade (SETTLE_FADE stays at 2 — the group summary's define + use) or the motion abstraction (deriveToolRowMotion / motion.* / settleFade). The full dynamic running→settled rerender contract is still tracked separately (packages/ui has only renderToStaticMarkup). P3 — two JSDoc blocks in trow-summary.ts no longer matched the behavior: summarizeTrowTools now has a { live } mode (running "正在" prefix + suppressed failed count), and activeTrowTool's description drives the shimmering summary only for a single-tool group (multi-tool uses the aggregation). Updated both.
…r (review P3) The multi-tool summary aggregates the whole group, the group icon follows the first bucket, and disclosure attention is overridden by the whole-group trowNeedsAttention — so ToolTrowGroup's active-tool lookup had no remaining effect (a single-tool group's active tool is items[0] anyway). Replace it with firstPresentation = deriveToolActivityPresentation(items[0]!), used for the icon, the single-tool summary, and the disclosure base. Removes the now-unused activeTrowTool helper, its barrel export, and the activeTrowTool assertions in the desktop trow-summary test (the isTrowRunning classification tests stay). No behavior change: single-tool active === items[0], and the multi-tool path never read activePresentation.summary/needsAttention.
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 freeto 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.
Summary
Concurrent (parallel) tool calls made the trow jitter badly: the summary line cycled through each tool's description as tools started/finished (the "1234567" jump), and when parallel results returned together, N per-row opacity-0→1 settle fades stacked into a big shake.
Why
Refs #546 (Phase B PR6 tool rendering polish), #646 (real-time status language — tool run→done seam). Manual testing flagged parallel tool execution as a poor experience.
Scope
deriveToolRowMotion/ToolRowMotionand the two contract tests that locked the removed behavior;isToolRowRunning/isToolRowSettledstay.Not included (tracked separately under #546 Phase B/C):
ToolCardBody→ToolTrowRowlist)overlayLiveTurnstep regroup)Verification
npm run -w @maka/ui test— 101 pass, 0 fail (newtool-trow-summary.test.ts: running aggregation + whole-group count; existing presentation/stability tests green).npm run -w @maka/desktop test— 2310 pass, 0 fail (updatedtool-row-motion.test.tskeeps theisToolRowRunning/isToolRowSettledclassification; removed the two settle-fade tests that locked the removed behavior).npm run typecheck— clean across all workspaces.connectOverCDP): injected the post-changeToolTrowrunning render, confirmed the summary shows "正在读取 2 个文件", themaka-text-shimmerkeyframe runs (animationPlayState: running) with adequate light-band vs base contrast across light + dark themes (catppuccin-mocha / nord / tokyo-night / mono) — so "shimmer seems missing" was the pre-change summary jitter breaking the sweep's continuity, not a contrast/keyframe fault.User-facing impact
Concurrent tool calls no longer make the summary line jump through each tool's description, and parallel results returning together no longer stack N settle fades into a shake. The working signal stays the 深度思考-style light band, now continuous because the summary text is stable. No schema, migration, or config changes.
Reviewer notes
SETTLE_FADE) as the whole-group signal; only the per-row fade is removed, so a batch settle is N light bands dropping, not N fades stacking.deriveToolRowMotion/ToolRowMotionremoved as dead code (the row no longer trackseverRunning; the group computes its ownsettled/settling). The two desktop contract tests that locked the removed fade-gating behavior are removed; the classification tests stay.Checklist