🤖 feat: sticky per-workspace auto-expand for thinking & tool blocks#3452
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 626c05230e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed both Codex P2 findings in
Added hook tests locking the behavior: "opens on a late forceExpanded trigger and never auto-collapses it (latched)" and "a user collapse still wins over an active forceExpanded signal". Full |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Fixed the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a16d31346
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Addressed the P2 "Preserve forced expansion for live advisor output" (commit
|
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Re-requesting review on the latest HEAD ( |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Replace bespoke per-block expand defaults and present-block-mutating auto behaviors with a single per-workspace, per-type auto-expand preference seeded once at mount (no layout flashes). Thinking defaults quiet; bash folds into the unified tools preference. Deletes useBashAutoExpand and useLatestStreamingBashId.
The reasoning block now starts collapsed (quiet default) instead of staying expanded after being displaced by answer text, so the e2e asserts the summary + ellipsis collapsed state and verifies expanding reveals the full body.
useStickyExpand now treats forceExpanded as a live, latched 'must open' signal layered above the seed-once preference/fallback, so a task error or failed task_await sub-task that arrives after mount still opens the row (instead of staying hidden behind the header). User toggle still wins; never auto-collapses.
…nd unit job useStickyExpand returned fresh setExpanded/toggleExpanded closures every render. Consumers pass setExpanded into effect deps — e.g. main's newer WorkflowRunToolCall lists it in a useLayoutEffect and a useEffect. The old useToolExpansion returned React's stable useState setter, so those effects only re-ran on real changes. bun test does NOT run the React Compiler, so the unstable identity re-fired the synchronous layout effect every commit and spun act() forever. This only manifested on the PR *merge* commit (this branch + newer main, which added WorkflowRunToolCall.test.tsx), hanging Test/Unit for 40+ min since the job had no timeout. - Stabilize setExpanded/toggleExpanded with useCallback (restore stable-setter contract; correctness, not perf memoization — tests lack the compiler). - Add timeout-minutes: 15 to the test-unit job (Test/Integration already caps); bounds any future hang with logs instead of stalling the runner ~6h.
Codex review: with a stored `tools: false` preference, the sticky tools default overrode the initialExpanded fallback that AdvisorToolCall used (useToolExpansion(isExecutingWithoutResult)) to keep streamed advice visible while executing — so a live advisor could mount collapsed and hide its output. Route these must-open states through forceExpanded (latched; the user can still collapse), matching the AskUserQuestion / TaskToolCall carve-outs. - AdvisorToolCall: forceExpanded while executing (settled rows still remount collapsed via the existing key swap). - TaskApplyGitPatchToolCall: forceExpanded when there is an error result.
ammar-agent
force-pushed
the
expand-state-2cdh
branch
from
08db34e to
173a8c8
Compare
2 participants
Summary
Makes transcript expand/collapse for thinking (reasoning) and tool blocks legible and customizable via a single per-workspace, per-type auto-expand preference. Expanding or collapsing any block of a type sets that workspace's preference for the type; newly arriving blocks inherit it. Crucially, a preference change never mutates the expand state of an already-mounted block, so there are no layout flashes.
Background
Expand stickiness felt random: thinking blocks expanded while streaming then auto-collapsed on finish, bash auto-expanded the latest streaming command and auto-collapsed it when superseded, and each tool had its own bespoke default. Several of these behaviors mutated present blocks (a visible height tear) and fought any notion of a remembered user choice.
Implementation
New hook
useStickyExpand(kind, fallbackExpanded, opts?)(src/browser/features/Messages/useStickyExpand.ts):auto-expand:{workspaceId}→{ thinking?, tools? }, read viareadPersistedState(no subscription). A later preference change therefore cannot retroactively expand/collapse a mounted block — only blocks mounting afterwards inherit it.workspaceIdfromMessageListContext(already wraps the whole transcript), so no prop-drilling; outside that context it degrades to local-only state.Wiring:
useToolExpansionis now a thin wrapper overuseStickyExpand("tools", …), so all ~46 existing tool call sites become sticky with no per-site edits (each tool's prior default becomes the fallback).ReasoningMessage: quiet default (collapsed, even while streaming), removed the auto-collapse-on-finishuseLayoutEffect, allowed expanding mid-stream, and made a collapsed streaming block hide its body while an expanded one stays height-uncontrolled (no clipping).FileEditToolCall/TaskToolCall: replaced bespoke?? defaultlocal state with the hook.BashToolCall: folded into the tools preference; deleteduseBashAutoExpand, theuseLatestStreamingBashIdstore hook, and the aggregator'sgetLatestStreamingBashToolCallId+ cache field. Live output still renders when expanded.AskUserQuestionToolCall:forceExpandedwhileexecutingso a live, blocking prompt is never hidden; follows the preference once answered.storage.ts: addedgetAutoExpandPrefsKey+AutoExpandPrefs, registered inPERSISTENT_WORKSPACE_KEY_FUNCTIONS(survives fork/rename).useStickyExpandreturnssetExpanded/toggleExpandedwith stable identities (useCallback), matching the olduseToolExpansion's stableuseStatesetter. Consumers passsetExpandedinto effect dependency arrays (e.g.WorkflowRunToolCall'suseLayoutEffect/useEffect), so an unstable identity would re-fire those effects every commit — see Pains.timeout-minutes: 15to theTest / Unitjob (Test / Integrationwas already capped) so a hung test fails fast with logs instead of stalling the runner for GitHub's ~6h default.Resolved UX decisions
Validation
useStickyExpand.test.tsxlocks the present-block invariant: an external preference change does not mutate a mounted block, but a freshly mounted block inherits it; plus per-workspace isolation, write-on-toggle,forceExpanded, and null-context fallback.ReasoningMessage.test.tsx(quiet default, no auto-collapse on completion, preference inheritance) andFileEditToolCall.ui.test.tsx(no mutation when a failed result arrives later).tests/e2e/scenarios/toolFlows.spec.tsfor the quiet default: a completed reasoning block now renders collapsed (summary + ellipsis), and the test verifies expanding reveals the full body. Verified locally under Electron/xvfb.Risks
AskUserQuestionwrites thetoolspreference; it only fires on a deliberate click, so impact is minimal.Pains
The first revision hung
Test / Unitfor 40+ min, and the hang only reproduced on the PR merge commit:mainhad since addedWorkflowRunToolCall.test.tsx, whose component listssetExpandedin auseLayoutEffectdependency array.useStickyExpandreturned a freshsetExpandedeach render, and becausebun testdoes not run the React Compiler (which memoizes it in the app), the synchronous layout effect re-fired every commit and spunact()forever — past bun's 5s per-test timeout, so the whole job stalled (which also had notimeout-minutes). Reproduced locally by checking outrefs/pull/3452/mergeand running the file in isolation, then fixed by stabilizing the callbacks. (The suite also has pre-existing, order-dependentmock.modulecross-file leaks unrelated to this PR —mainitself fails them in a sorted run — left untouched.)Generated with
mux• Model:anthropic:claude-opus-4-8• Thinking:max• Cost:$26.40