Skip to content

feat(assistant): auto-follow transcript, collapse tool spam, one-click continue#162

Merged
Tjemmmic merged 5 commits into
mainfrom
feat/assistant-transcript-autofollow
Jul 2, 2026
Merged

feat(assistant): auto-follow transcript, collapse tool spam, one-click continue#162
Tjemmmic merged 5 commits into
mainfrom
feat/assistant-transcript-autofollow

Conversation

@Tjemmmic

@Tjemmmic Tjemmmic commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

The chat-UI half of a batch of fixes found while testing workflows on the Tangle platform (the platform-side changes are in the agent-dev-container repo). All three are in the assistant transcript.

Auto-follow the transcript

There was no auto-scroll on the conversation container at all, so a streaming response ran below the fold and the user had to scroll by hand to watch it. Added a bottom-stick auto-follow: it pins to the newest content as tokens stream (a pre-paint useLayoutEffect keyed to the streamed length, so content never flashes above the fold), yet yields the moment the user scrolls up to read and re-arms when they return to the bottom or a new turn/thread starts.

Collapse tool-call spam

A multi-step turn floods the transcript with a card per tool call. Now a settled run of 3+ consecutive tool calls collapses into one "Worked through N steps" disclosure. While a turn is streaming nothing is grouped (live activity is what the user wants to watch), and text runs (the answer) always render in full.

One-click Continue

A turn that hit the per-turn step limit told the user to type "continue" with no affordance. Now the capped state is tracked and a Continue button renders (the thread keeps full context, so it resumes where it left off), and the status copy is softened from "I reached the step limit for this turn" to a calmer "Paused after a lot of steps."

Tests

Updated reducer tests for the capped flag (set on a capped turn, cleared when the next turn starts) and the softened copy. Full assistant + web-react suites green (243 tests); changed files typecheck clean.

Verified locally

Linked into the platform web app and driven with deepseek: the transcript followed the streaming response to the bottom and yielded correctly when scrolled up mid-stream; a multi-step authoring turn rendered cleanly.

…click continue

Three chat UX problems surfaced while driving the assistant:

The transcript never auto-scrolled, so a streaming response ran below the
fold and the user had to scroll by hand to watch it. Add a bottom-stick
auto-follow on the conversation container: it pins to the newest content
as tokens stream (a pre-paint layout effect keyed to the streamed length),
yet yields the moment the user scrolls up to read and re-arms when they
return to the bottom or a new turn/thread starts.

A multi-step turn floods the transcript with a card per tool call. Group a
SETTLED run of 3+ consecutive tool calls into one "Worked through N steps"
disclosure; while streaming nothing is grouped (live activity is what the
user wants to watch), and text runs (the answer) always render in full.

A turn that hit the per-turn step limit told the user to type "continue".
Track the capped state and render a one-click Continue button (the thread
keeps full context, so it resumes where it left off), and soften the
status copy from "I reached the step limit" to a calmer "Paused after a
lot of steps."
@Tjemmmic

Tjemmmic commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI Code Review (ensemble)

Resolution (0b99efc): 4 addressed.

Summary

The PR adds auto-follow scrolling, tool-call collapsing for settled turns, and a Continue button for capped turns with good reducer test coverage. However, the tool-collapsing logic has a serious UX bug (hiding failed/actionable tool calls behind a generic summary) and both significant UI behaviors lack rendering/component tests. Given 4 P2s, the PR needs another pass before merge.

Issues Found

4 total — 0 P1 (blocking) · 4 P2 (should fix) · 0 P3 (nice to have)

⚠️ P2 — Collapsed tool runs can hide failed or actionable tool resultsAddressed

Note: A settled run containing a failed tool or a proposal awaiting approval is no longer collapsed (isImportantTool gate in SegmentedBody), so a failure/blocked turn can't hide behind the summary; covered by a test. (0b99efc)

  • File: src/web-react/index.tsx:840-851
  • Problem: Every settled run of 3+ consecutive tool calls collapses into a closed <details> block regardless of each call's state. If a tool card contains an error, approval UI, or action-required state, it disappears behind a generic 'Worked through N steps' summary as soon as streaming ends, making a failed or blocked turn look successful.
  • Fix: Skip collapsing groups that contain important tools (errors, action-required, approvals), or render <details open> for such groups. Derive hasImportantTool from call status/type and use it to gate the collapse behavior.

⚠️ P2 — New auto-follow behavior has no regression coverageAddressed

Note: Auto-follow extracted into a useStickToBottom hook with unit tests covering follow-while-pinned, stop-on-scroll-up, re-arm on new turn/thread, and disabled. (0b99efc)

  • File: src/assistant/AssistantPanel.tsx:223-268
  • Problem: The PR adds stateful scroll-follow logic that changes behavior based on streaming state, thread changes, and user scroll position, but only reducer tests were added. Programmatic and user scroll events both mutate stickToBottomRef.current, and there's no test asserting follow-while-pinned, stop-on-scroll-up, and re-arm-on-new-turn/thread behavior.
  • Fix: Add a component test that mounts AssistantPanel with a mocked scroll container, verifying: new streamed content scrolls to bottom while pinned; scrolling up disables following; a new streamingId or threadId re-enables following.

⚠️ P2 — Tool-run collapsing lacks tests for transcript rendering contractAddressed

Note: Added SegmentedBody rendering tests: <3 tools inline, 3+ settled collapse, a streaming run stays inline, ordering preserved, and important-tool runs never collapse. (0b99efc)

  • File: src/web-react/index.tsx:815-860
  • Problem: The PR changes how settled tool segments render by grouping consecutive tools into a <details> disclosure at threshold 3, but there is no test covering the threshold, the streaming exception, or that text/tool ordering is preserved. A regression could silently hide tool cards or collapse live activity during streaming.
  • Fix: Add rendering tests for SegmentedBody covering: <3 tool calls render inline; 3+ consecutive settled tools collapse; streaming tool runs remain inline; interleaved text preserves order; leftover tool calls still render.

⚠️ P2 — Continue button relies on untested 'continue' string for backend resumeAddressed

Note: Documented at the call site: 'continue' is the automated form of the instruction the prior capped-turn copy gave the user, and resume works from the thread history the server replays each turn — no special backend handling, so it's not a magic string the backend must decode. (0b99efc)

  • File: src/assistant/AssistantPanel.tsx:556
  • Problem: The Continue button calls chat.send('continue') which sends the literal string 'continue' as a new user message. There is no test or documentation confirming the backend treats this as a resume rather than a fresh prompt. The UX promise of 'pick up where I left off' depends on undocumented backend behavior.
  • Fix: Add a test or documentation confirming 'continue' after a capped turn triggers resume on the backend, or change the client to send a dedicated signal (e.g., resume action type) rather than relying on magic string handling.

❌ CHANGES REQUESTEDResolved — no blocking concerns remain

Status: all 4 P2 items addressed in 0b99efc (fix + tests + documentation). No P1/blocking concerns were raised.

The PR has 4 P2 issues: the tool-collapse logic can hide failed/actionable tool results behind a generic summary (a significant UX regression risk), and two substantial new UI behaviors (auto-follow scrolling and tool-run collapsing) lack component/rendering tests that could silently regress. While the reducer path is well-tested, the UI contract gaps and the collapse-hiding-failures risk warrant another pass before merge.

Quick Reference

  • P2: Collapsed tool runs can hide failed or actionable tool results
  • P2: New auto-follow behavior has no regression coverage
  • P2: Tool-run collapsing lacks tests for transcript rendering contract
  • P2: Continue button relies on untested 'continue' string for backend resume

Synthesized by Sokuza AI from multiple independent reviewers

…er scroll + collapse

Addresses the review on the transcript changes:

- A settled tool run is no longer collapsed when it contains a failed tool or a
  proposal awaiting approval, so a failure or a blocked turn can never hide
  behind a "Worked through N steps" summary and read as successful.
- Extract the auto-follow logic into a `useStickToBottom` hook and unit-test the
  follow / yield-on-scroll-up / re-arm-on-new-turn / re-arm-on-thread-change /
  disabled contract (jsdom can't compute scroll geometry, so the hook test mocks
  the element's scrollHeight/clientHeight/scrollTop).
- Add SegmentedBody rendering tests: <3 tools render inline, 3+ settled collapse,
  a streaming run stays inline, and a run with a failed or approval-pending tool
  is never collapsed.
- Document why the Continue button sends the literal "continue": it's the
  automated form of the instruction the prior capped-turn copy gave the user, and
  resume works from the thread history the server replays each turn — no special
  backend handling.
@Tjemmmic

Tjemmmic commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI Code Review (ensemble)

Resolution (c54c9a1): 1 addressed.

Summary

The PR adds auto-follow transcript scrolling, collapsible tool-run grouping, and a one-click Continue button for capped turns. While the reducer changes and UI affordances are well-tested, the new auto-follow hook has a significant ordering bug that defeats its purpose on the common thread-switch path.

Issues Found

1 total — 0 P1 (blocking) · 1 P2 (should fix) · 0 P3 (nice to have)

⚠️ P2 — Thread switch re-arm happens too late to scroll newly loaded contentAddressed

Note: Re-arm folded into the same useLayoutEffect as the scroll (before the stuck check), so a thread switch that updates threadId and loads content in one render now scrolls; the turn/thread re-arm tests were strengthened to change the id and content in a single rerender (the failing path). (c54c9a1)

  • File: src/assistant/use-stick-to-bottom.ts:49-70
  • Problem: The hook sets stuckRef.current = true inside a passive useEffect (L62) on a thread or streaming id change, but the actual scroll-to-bottom logic runs earlier inside a useLayoutEffect (L72). If the user has scrolled up (stuckRef.current === false) and switches threads, the new thread's messages may load synchronously in the same render (updating both threadId and contentSignature). The layout effect will see stuckRef.current as false and skip the scroll. The passive effect will subsequently re-arm the ref, but there will be no further contentSignature change to trigger another scroll, leaving the user at the old scroll position instead of the newest message.
  • Fix: Move the re-arm logic directly into the useLayoutEffect before checking stuckRef.current, or combine the logic into a single effect. Add a regression test that changes threadId and contentSignature in the exact same rerender while the user is unstuck to ensure the scroll executes correctly.

✅ APPROVE

There are no P1 issues and only 1 P2 issue, falling below the threshold to block the PR. However, the thread-switch auto-scroll bug defeats the hook's core purpose on a common path and should ideally be fixed before merge. The rest of the PR is solid, with excellent test coverage for the reducer, scroll edge cases, and tool-run grouping.

Quick Reference

  • P2: Thread switch re-arm happens too late to scroll newly loaded content

Synthesized by Sokuza AI from multiple independent reviewers

…ad switch scrolls

The re-arm ran in a passive effect that fires AFTER the scroll's layout effect,
so switching threads while scrolled up — which updates threadId and loads the
new thread's content in the same render — skipped the scroll (the layout effect
saw the stale unstuck ref) and left the user at the old position. Fold the
re-arm into the same layout effect, before the stuck check, so the re-arm and
scroll are synchronous. Strengthen the turn/thread re-arm tests to change the id
and content in one rerender (the real path), which fails on the old ordering.
@Tjemmmic

Tjemmmic commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI Code Review (ensemble)

Resolution (a79d157): 4 addressed.

Summary

The PR adds auto-follow transcript scrolling, collapsible tool call runs, and a one-click Continue button for capped turns. It is well-structured and includes comprehensive test coverage for the new logic. However, there are subtle UI regressions where tool-only transcript updates bypass the auto-follow mechanism, and the scroll hook incorrectly tracks the user's position while disabled, which directly undermines the feature.

Issues Found

4 total — 0 P1 (blocking) · 3 P2 (should fix) · 1 P3 (nice to have)

⚠️ P2 — Auto-follow does not react to tool-only transcript updatesAddressed

Note: The content signature that drives auto-follow now includes each tool message's status, so a turn streaming tool cards before any text (or a card changing status) scrolls. (a79d157)

  • File: src/assistant/AssistantPanel.tsx:L223-L229
  • Problem: contentSignature only includes total message text length, reasoning length, message count, and status. Tool-call segments can be appended or have their status change without changing any of those values, so useStickToBottom will not run when a turn streams several tool cards before any text.
  • Fix: Include the rendered tool/segment shape in the signature, or drive the hook from a reducer-level monotonically increasing transcript revision that changes for every visible transcript mutation.

⚠️ P2 — Scrolling the disabled history view can permanently unstick the chat transcriptAddressed

Note: onScroll now returns early while the hook is disabled, so scrolling the shared history-view container can't clear the chat's stuck ref. (a79d157)

  • File: src/assistant/use-stick-to-bottom.ts:L44-L49
  • Problem: onScroll updates stuckRef even when enabled is false. Because AssistantPanel attaches the returned handler to the scroll container unconditionally while enabled is only view === "chat", scrolling the history view away from its bottom can set stuckRef.current = false; switching back to the same chat view then skips the scroll.
  • Fix: Ignore scroll events while disabled, and include enabled in the callback dependency: if (!enabled) return; at the top of onScroll.

⚠️ P2 — Collapsed tool runs can hide failed outcomes reported with ok:falseAddressed

Note: isImportantTool now also treats a done tool with an ok:false outcome as non-collapsible (same predicate as ToolCallCard), so it never hides behind the summary. (a79d157)

  • File: src/web-react/index.tsx:L782-L786
  • Problem: isImportantTool only treats call.status === 'error' and pending approvals as non-collapsible. However, ToolCallCard marks a call as failed when toolOutcomeOf(call)?.ok === false even if call.status is 'done'. A settled run containing such a failed tool will collapse, making the turn look successful unless the user expands it.
  • Fix: Use the same failure predicate as ToolCallCard when deciding whether a run is important: return call.status === 'error' || toolOutcomeOf(call)?.ok === false || pendingApprovalOf(call) !== null.

ℹ️ P3 — Collapse threshold may hide intermediate tool results the user needs to inspectAddressed

Note: isImportantTool now also treats a still-running tool in a settled turn (stuck/timed out) as non-collapsible. (a79d157)

  • File: src/web-react/index.tsx:L789-L791
  • Problem: The collapse logic groups ALL consecutive tool segments, including those with status: 'running' when not streaming. If a settled turn has a running tool (e.g., a tool that never completed due to a timeout), it would be collapsed along with the others, hiding a potentially stuck tool.
  • Fix: Also check that no tool in the group has status === 'running' before collapsing, similar to the isImportantTool check: !g.calls.some((c) => c.status === 'running' || isImportantTool(c)).

✅ APPROVE

No P1 issues were found, and there are exactly three P2 issues, which meets the stated approval threshold. The identified P2s should definitely be addressed before merging as they directly undermine the new auto-follow and collapse behaviors in common UI paths, but they do not warrant blocking the draft.

Quick Reference

  • P2: Auto-follow does not react to tool-only transcript updates
  • P2: Scrolling the disabled history view can permanently unstick the chat transcript
  • P2: Collapsed tool runs can hide failed outcomes reported with ok:false
  • P3: Collapse threshold may hide intermediate tool results the user needs to inspect

Synthesized by Sokuza AI from multiple independent reviewers

…'t collapse failed/stuck tools

Addresses the re-review:

- Auto-follow now reacts to tool-card updates: tool activity rides on separate
  `tool` messages, so the content signature includes each tool's status — a turn
  that streams tool cards before any answer text still scrolls.
- `onScroll` is ignored while the hook is disabled, so scrolling the (shared)
  history-view container can't unstick the chat transcript.
- `isImportantTool` now also treats a done tool with an `ok:false` outcome
  (matching ToolCallCard's own failure predicate) and a still-`running` tool in a
  settled turn (stuck / timed out) as non-collapsible, so neither hides behind the
  "Worked through N steps" summary. Tests cover both.
@Tjemmmic

Tjemmmic commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI Code Review (ensemble)

Resolution (b5f0281): 2 addressed · 1 not an issue.

Summary

The PR introduces transcript auto-follow, tool spam collapsing, and a one-click Continue button for capped turns. The implementation is robust overall, with comprehensive tests for the new useStickToBottom hook and tool collapse edge cases, but contains a real runtime crash risk in the auto-follow signature computation.

Issues Found

3 total — 0 P1 (blocking) · 1 P2 (should fix) · 2 P3 (nice to have)

⚠️ P2 — contentSignature may crash on messages with undefined textAddressed

Note: Guards m.text?.length ?? 0 so a malformed message (e.g. from external thread history) can't throw, even though the type says text: string. (b5f0281)

  • File: src/assistant/AssistantPanel.tsx:229-235
  • Problem: The contentSignature useMemo accesses m.text.length inside a loop over state.messages. If any message in the array (e.g., a status message, tool-only message, or edge-case event) lacks a text property, this will throw a TypeError at runtime, crashing the entire AssistantPanel.
  • Fix: Use optional chaining or nullish coalescing to guard against undefined text: sig += \|${m.text?.length ?? 0}`;. Alternatively, verify that textis always a non-nullable string in theAssistantMessage` type definition and all code paths producing messages.

ℹ️ P3 — Auto-follow ignores proposal cards added outside messagesAddressed

Note: The content signature now includes pending-proposal call ids, so a newly-inserted approval card scrolls into view while pinned. (b5f0281)

  • File: src/assistant/AssistantPanel.tsx:227-234
  • Problem: contentSignature only tracks reasoning, status, and state.messages, but the visible transcript also renders pending proposal cards from state.pendingProposals. When a tool_proposal event adds a proposal while the user is pinned, no dependency value changes, so useStickToBottom may not scroll to reveal the newly inserted approval card until a later event fires.
  • Fix: Include pending proposal identity/state in the signature and dependency list, e.g., append state.pendingProposals.map((p) => \${p.callId}:${p.retryError ?? ''}`).join('|')` or at minimum the count plus call ids.

ℹ️ P3 — Continue button uses hardcoded English stringsNot an issue

Note: The panel has no i18n layer — every string (empty state, credit nudge, etc.) is inline English — so the Continue label/message follow the same convention; the review's own note says this is adequate absent i18n. (b5f0281)

  • File: src/assistant/AssistantPanel.tsx:527-535
  • Problem: The Continue button sends the hardcoded string 'continue' via chat.send('continue'), and the labels 'Continue' and 'Paused after a lot of steps.' are hardcoded English strings with no i18n consideration. If the app supports localization or the backend expects a specific locale/command format, this bypasses that layer.
  • Fix: If i18n is a concern, route these strings through the translation layer. If not, the inline comment already documents that 'continue' is intentionally a raw user-equivalent message, not a magic command — this is adequate.

✅ APPROVE

The PR is well-architected with thorough test coverage for the new hook and tool-collapse edge cases. The identified runtime crash risk (undefined m.text access in contentSignature) is a legitimate P2 concern that should be addressed before or shortly after merge, but it doesn't rise to a blocking P1 since the AssistantMessage type may already guarantee text as a non-nullable string. No P1 issues and fewer than 3 P2 issues means this meets the approval bar.

Quick Reference

  • P2: contentSignature may crash on messages with undefined text
  • P3: Auto-follow ignores proposal cards added outside messages
  • P3: Continue button uses hardcoded English strings

Synthesized by Sokuza AI from multiple independent reviewers

Addresses the re-review: the auto-follow content signature now includes pending
proposal identities, so a newly-inserted approval card scrolls into view while
pinned; and it guards `m.text?.length` so a malformed message from external
thread history can't throw and blank the panel.
@Tjemmmic

Tjemmmic commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI Code Review (ensemble)

Resolution (b5f0281): 1 not an issue · 1 acknowledged.

Summary

This PR adds auto-follow transcript scrolling, collapsing of consecutive settled tool calls, and a one-click Continue button for capped turns, backed by thorough tests. The implementation is solid overall, but there is a rendering stability issue in the tool collapse logic where historical messages depend on a global streaming flag rather than their own state.

Issues Found

2 total — 0 P1 (blocking) · 1 P2 (should fix) · 1 P3 (nice to have)

⚠️ P2 — Tool collapse uses global streaming flag causing rendering instability for historical messagesNot an issue

Note: The premise that streaming is derived globally is incorrect. It is computed per-message at src/web-react/index.tsx:1155 (!!loading && msg.id === messages[messages.length - 1]?.id) and threaded into SegmentedBody (line 994). A historical, non-last message therefore always evaluates streaming as false and stays collapsed. Sending a new prompt appends a fresh user+assistant pair, so the NEW assistant message becomes the last one — the historical message is never the streaming target, so it does not retroactively expand or flicker.

  • File: src/web-react/index.tsx:855-857
  • Problem: The collapse condition checks !streaming, where streaming is derived globally from loading && lastMessage. If a user has a past, fully-settled message with 3+ successful tool calls, that message will evaluate streaming as false and collapse retroactively when the user is not actively loading a new message. However, it will immediately expand the moment the user sends a new prompt and the component re-renders with streaming set to true. This coupling to a global flag causes visual flickering and unpredictable rendering for historical messages depending on the current global chat state.
  • Fix: Determine streaming status per-message (e.g., by checking if the message id matches the active streaming turn ID) rather than relying on a global streaming boolean. This ensures historical message rendering is stable regardless of the current loading state.

ℹ️ P3 — Reducer test asserts on brittle copy ("continue") instead of semantic state ℹ️ Acknowledged

Note: The test already asserts the semantic markers the fix asks for: note?.role === "status" (reducer.test.ts:121) and s.capped === true (line 123). The toLowerCase().toContain("continue") check (line 122) is a deliberate supplementary guard that the user-facing affordance actually references continuing — not the sole assertion. Left as-is.

  • File: src/assistant/reducer.test.ts:124
  • Problem: The updated test asserts note?.text.toLowerCase()).toContain('continue'). Because the user-facing copy in reducer.ts includes the word "continue", this test passes, but it couples the test to exact wording rather than verifying the semantic status note or the partial-reply semantics.
  • Fix: Assert on a semantic marker of a capped/partial turn rather than a specific word in human-readable copy. For example, check that s.capped is true and that the status note role is present, rather than scanning the copy text.

✅ APPROVE

The PR introduces well-tested features with careful edge-case handling (especially around tool collapsing and scroll behavior). The identified P2 issue regarding the global streaming flag dependency for tool collapse is a valid concern for historical message rendering stability, but since it does not violate correctness, security, or API contracts and doesn't meet the threshold of 3 P2s to block the PR, we can approve. The author should consider addressing the P2 in a follow-up.

Quick Reference

  • P2: Tool collapse uses global streaming flag causing rendering instability for historical messages
  • P3: Reducer test asserts on brittle copy ("continue") instead of semantic state

Synthesized by Sokuza AI from multiple independent reviewers

@Tjemmmic Tjemmmic marked this pull request as ready for review July 2, 2026 00:10

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Value Audit — sound

Verdict sound
Concerns 1 (1 low)
Heuristic 0.0s
Duplication 0.1s
Interrogation 136.1s (2 bridge agents)
Total 136.2s

💰 Value — sound

Adds three coherent chat UX improvements—transcript auto-follow, settled tool-run collapse, and a one-click Continue for capped turns—built in the existing reducer-and-web-react component grain.

  • What it does: 1) Pins the assistant transcript scroll to the bottom as content streams, yielding when the user scrolls up and re-arming on return-to-bottom or a new turn/thread. 2) Collapses settled runs of 3+ consecutive tool calls into a 'Worked through N steps' disclosure, while leaving live turns, text answers, failed/stuck tools, and approvals fully visible. 3) Tracks a capped state when a turn hits the
  • Goals it achieves: Keeps streaming responses in view without manual scrolling, reduces visual spam from multi-step agent turns, and removes the friction of typing 'continue' to resume a capped turn.
  • Assessment: Good change. The hook is extracted and unit-tested independent of the DOM, the reducer state change is minimal and tested, the collapse logic correctly excludes failures/running/approval states, and the pre-paint useLayoutEffect timing is deliberately chosen to handle thread switches correctly.
  • Better / existing approach: none — no existing auto-scroll, stick-to-bottom, or tool-collapse capability exists in the repo (git grep found only an unrelated reasoning-box scroll at src/web-react/index.tsx:956). The new hook is generic but correctly colocated with its only consumer; moving it to web-react would be speculative until another surface needs it.
  • Model: opencode/kimi-for-coding/k2p7
  • Bridge attempts: 1

🎯 Usefulness — sound

Three coherent, correctly-wired UI polish changes (auto-follow, tool-spam collapse, one-click Continue) that each fill a real gap in the existing assistant transcript with no dead or competing surface.

  • Integration: All three are reachable on every applicable render. capped is fully wired backend→client.ts:271→reducer.ts:326→AssistantPanel.tsx:525, and the Continue button reuses the composer's chat.send path (useAssistantChat.ts:59,275). useStickToBottom is instantiated on the live logRef container (AssistantPanel.tsx:245) with onScroll attached at :440. The collapse runs inside the existing `Segmen
  • Fit with existing patterns: Each change extends an established pattern rather than competing with one. The capped flag follows the exact shape of sibling AssistantState fields (usage/error/reasoning). The collapse adds a conditional wrapper to the existing consecutive-tool grouping in SegmentedBody. Auto-follow targets a container (logRef) that had NO prior auto-scroll — confirmed by grep: the only other chat-path `s
  • Real-world viability: Edge cases are handled. useStickToBottom uses a pre-paint useLayoutEffect with slack-based pin detection (48px), re-arms on new-turn/thread transitions, and ignores scrolls while disabled (history view) — all covered by jsdom tests. The collapse's isImportantTool (web-react/index.tsx:784) keeps failed/error/running/approval-pending tools visible so a stuck or blocked turn never hides behind
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added src/assistant/use-stick-to-bottom.test.ts

+function makeEl(scrollHeight = 1000, clientHeight = 400, scrollTop = 0): MockScrollEl {


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260702T001603Z

@tangletools

Copy link
Copy Markdown

✅ No Blockers — b5f02817

Review health 100/100 · Reviewer score 79/100 · Confidence 70/100 · 9 findings (1 medium, 8 low)

opencode-kimi glm deepseek aggregate
Readiness 79 83 89 79
Confidence 70 70 70 70
Correctness 79 83 89 79
Security 79 83 89 79
Testing 79 83 89 79
Architecture 79 83 89 79

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 2/2 planned shots over 7 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 7 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 7 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM Continue affordance is lost after history restore / page reload — src/assistant/reducer.ts

restore_history restores messages and pendingProposals but never sets capped. Because initialAssistantState() (used by switch_thread and hydrate) initializes capped: false, a reloaded thread that ended with a capped turn still shows the status note in messages but no Continue button. Concrete: render a capped turn, reload → state.capped is false → AssistantPanel line 525 condition is false. Fix: either include capped in the restore_history payload from the server, or derive it client-side by scanning the restored messages for the cap note.

🟡 LOW Continue button UI has no component test — src/assistant/AssistantPanel.tsx

The new Continue button (lines 525-553) is a user-facing affordance that gates on state.capped, state.status === 'idle', !chat.restoring, and view === 'chat', and on click calls chat.send('continue'). AssistantPanel.test.tsx was not updated to exercise this branch, so regressions in visibility logic or the send call are not caught. Fix: add a component test that renders AssistantPanel with capped: true, asserts the button appears, clicks it, and verifies chat.send was called with 'continue'.

🟡 LOW contentSignature misses proposal retry-error updates — src/assistant/AssistantPanel.tsx

The signature includes p.callId for pending proposals but not p.retryError. When proposal_retry_failed adds a retry error to an existing proposal, the card expands to show the error, but contentSignature does not move and the layout effect will not auto-scroll a pinned view to reveal the new content. Fix: append p.retryError (or a hash of it) to the proposal segment of the signature.

🟡 LOW Cap note renders inside awaiting_confirm, where 'continue when you're ready' is unreachable — src/assistant/reducer.ts

When a turn is both capped and leaves a pending proposal (reachable: a tool_proposal emitted before the server applies the step limit), done appends the status note 'Paused after a lot of steps — continue when you're ready…' AND sets status to awaiting_confirm (reducer.ts:342) because pendingProposals is non-empty. The AssistantPanel Continue button is correctly hidden (it requires status==='idle'), but the note still shows in the transcript telling the user to 'continue' while the composer is disabled and the only valid actions are confirm/cancel. Fix: either suppress the cap note when proposals are pending, or reword it so it doesn't prescribe an action the current status blocks.

🟡 LOW Continue button absent after thread restore/reload — src/assistant/reducer.ts

The capped flag is set only by the done stream event. When a thread is restored via restore_history or hydrate, no stream events replay, so capped stays false. The capped-status note persists in messages (and renders), but the one-click Continue button won't appear after reload — the user must type 'continue' manually. This is the intentional session-only design, but the copy on the status note says 'continue when you're ready' which implies the button should be there. Consider either: (a) including a capped flag in the restore_history payload so the button re-appears, or (b) adjusting the status-note copy on reload to not suggest a one-click path exists.

🟡 LOW capped flag and cap status note are session-only; reload drops the Continue affordance and the paused indicator — src/assistant/reducer.ts

capped lives only in reducer state — loadThread (useAssistantChat.ts:82) persists/loads just threadId, and restore_history (reducer.ts:484-515) never sets capped, so after a reload the Continue button never renders even if the last turn was capped. The cap status note (id cap-${turnId}, reducer.ts:331) is client-authored, not a server message, so restore_history doesn't bring it back either — meaning a reloaded capped turn shows the partial reply with zero indication it was paused, reading as complete (the exact misread the note exists to prevent). Not a regression vs. pre-PR (there was no Continue button before), but the feature's value is limited to the live session. Worth either persisting capped/deriving it from server transcript metadata, or accepting the limitation expl

🟡 LOW Missing test: approved proposal within settled run should collapse — src/web-react/chat-messages-segments.test.tsx

The test for pending-approval-prevention (line 292) proves that queued_for_approval blocks collapse. The first collapse test (line 226) proves 3+ done generic tools collapse. But no test verifies that an approved/acted-upon proposal (status != 'queued_for_approval') within a 3+ settled run collapses as expected. The code path is correct — pendingApprovalOf returns null for non-pending statuses, so isImportantTool returns false — but covering this variant explicitly would strengthen the contract.

🟡 LOW No test for multiple independent collapsible runs in one message — src/web-react/chat-messages-segments.test.tsx

The grouping logic creates a separate group per maximal run of consecutive tools, and each group is independently collapse-eligible. No test exercises a message like [text, tool×3, text, tool×3] to confirm two independent disclosures render with correct counts. The grouping loop (lines 843-853) is straightforward and single-run cases are covered, so risk is low, but a multi-group test would lock in the per-group independence.

🟡 LOW Successful schedule_followup / render_ui cards collapse inside a 3+ generic-tool run — src/web-react/index.tsx

isImportantTool only flags error/running/ok:false/pendingApproval. A successfully-completed schedule_followup (renders as a distinct FollowupCard at line 661) or render_ui inside a 3+ run of generic tools will be folded behind 'Worked through N steps'. The user must expand the disclosure to discover a scheduled follow-up or rendered view. Defensible by design (these are completed actions), but a scheduled follow-up is arguably a user-actionable artifact worth surfacing. If the product wants these always-visible, add || blockKindOf(call) === 'followup' to isImportantTool. No correctness bug — content is accessible on expand.


tangletools · 2026-07-02T00:21:33Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 9 non-blocking findings — b5f02817

Full multi-shot audit completed 2/2 planned shots over 7 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 7 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 7 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-07-02T00:21:33Z · immutable trace

@Tjemmmic Tjemmmic merged commit 31981ce into main Jul 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants