🤖 feat: add full-width chat transcript setting#3350
Conversation
|
/coder-agents-review |
|
@codex review
|
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ 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". |
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, well-scoped single-feature PR. The vertical slice is tight: schema, config persistence, router handler, API definition, types, hook, component integration, settings UI, Storybook mock, and four test files all in agreement. The "only persist when enabled" serialization is a good choice. Tests are genuine: schema validation rejects string coercion, config tests inspect raw JSON for key presence, router test does a full enable/disable round-trip.
4 P3, 6 nits, 1 note below.
The P3s cluster around the hook and the width scope. DEREM-5 (state reset on disconnect) is the most actionable: one-line fix, real visual flash. DEREM-3 (input/transcript width mismatch) is a design call; the setting name scopes to "transcript" but the visual step-down may surprise users. DEREM-4 (mount flash from async init) follows the established pattern but affects a more prominent layout region than existing hooks.
Duplication (Robin flagged hook + GeneralSection + API schema copies) is real but pre-existing. The extraction would touch 5+ files outside this PR. Downgraded to P4 and not posted.
Process note: The agent replicated the structure of useProvidersConfig (the one sibling without tests) rather than useRouting or useContextSwitchWarning (which have dedicated test files). Something to watch in future PRs.
"This creates a full-width to narrow to full-width flash during reconnection." (Zoro)
src/browser/components/ChatPane/ChatPane.tsx:1226
Note [DEREM-14] TranscriptOnlyNoticePane also keeps max-w-4xl when full-width is enabled. Lower impact since transcript-only workspaces are a niche state, but if DEREM-3 is addressed, this should follow. (Nami)
🤖
🤖 This review was automatically generated with Coder Agents.
ibetitsmike
force-pushed
the
mike/chat-transcript-full-width
branch
from
8969579 to
e56b428
Compare
coder-agents-review responses:
|
|
/coder-agents-review |
|
@codex review
|
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ 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". |
There was a problem hiding this comment.
R1 findings addressed thoroughly. All 11 fixes verified against the code: test coverage added (DEREM-1), persisted state mirror prevents mount flash (DEREM-4), state preserved through disconnect (DEREM-5), nits corrected (DEREM-8-13), BooleanToggleEndpoint extraction de-duplicates schemas (DEREM-11).
DEREM-3 (transcript full-width but input stays narrow) was contested by the author and evaluated by the full panel. 8/8 reviewers accept the defense. The setting name and description scope explicitly to the transcript. Narrow input below wide reading area is a standard chat UI pattern (Claude, ChatGPT). Finding closed.
1 P2, 3 nits below. The P2 is a fix regression: the DEREM-8 fix replaced a verbose comment with a misleading one that could recreate the exact bug DEREM-5 was about.
The test infrastructure is well done. The createDeferred/createConfigEventStream helpers give tests deterministic control over async resolution order without wall-clock timing. The stale-fetch test in particular is a good model for testing version-nonce mechanisms.
"A maintainer reading this could add setSyncedValue(false) in the catch to 'ensure' the fallback, breaking graceful degradation for users who had full-width enabled during a transient API failure." (Gon)
🤖 This review was automatically generated with Coder Agents.
ibetitsmike
force-pushed
the
mike/chat-transcript-full-width
branch
from
e56b428 to
7abde90
Compare
coder-agents-review follow-up responses:
|
|
/coder-agents-review |
|
@codex review
|
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ 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". |
There was a problem hiding this comment.
All prior findings resolved. R2 fixes verified by Netero and the full panel: catch comment now accurately describes behavior (DEREM-19), Promise.withResolvers replaces hand-rolled deferred (DEREM-20), JSDoc focuses on the non-obvious cache rationale (DEREM-21), and booleanToggleRoute follows camelCase convention (DEREM-22).
1 nit below. Otherwise clean.
Three rounds, 25 findings tracked. 15 fixed by author, 1 contested and closed by panel (8/8), 2 acknowledged, 7 dropped by orchestrator. The author addressed every actionable finding, including generalizing DEREM-11's fix to both endpoints and synthesizing test patterns from multiple siblings rather than copying the untested one. Well-executed feature addition.
"That test would catch a real bug if someone broke the fetchVersion guard." (Bisky on the stale-fetch race test)
🤖 This review was automatically generated with Coder Agents.
ibetitsmike
force-pushed
the
mike/chat-transcript-full-width
branch
from
7abde90 to
bcef17d
Compare
coder-agents-review follow-up response:
|
|
/coder-agents-review |
|
@codex review
|
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ 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". |
There was a problem hiding this comment.
DEREM-25 fix verified (import type React). No new findings from Netero or the panel. All 25 findings across four rounds resolved: 16 fixed by author, 1 contested and closed by panel (8/8), 2 acknowledged, 7 dropped by orchestrator. Clean.
🤖 This review was automatically generated with Coder Agents.
ibetitsmike
force-pushed
the
mike/chat-transcript-full-width
branch
from
bcef17d to
8e6821e
Compare
|
/coder-agents-review |
|
@codex review
|
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ 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". |
There was a problem hiding this comment.
New work in this round: BashToolCall.tsx responsive layout (removes fixed max-widths, adds flex and @container(min-width:900px) breakpoint calibrated to the 896px readable column width), and a void runSubscription() cleanup replacing the redundant .catch(). Both changes are clean.
1 P3 below. The rest of the PR remains solid.
The @container breakpoint at 900px is well-calibrated: it activates just past the default max-w-4xl (896px), so the responsive bash summary widening only kicks in when full-width mode provides extra room. The void cleanup is correct since runSubscription catches internally.
"If someone refactors setSyncedValue to just call setChatTranscriptFullWidth(enabled) without the updatePersistedState call, the test suite stays green. On next mount, readPersistedState returns false, and the layout-flash bug is back." (Bisky)
🤖 This review was automatically generated with Coder Agents.
ibetitsmike
force-pushed
the
mike/chat-transcript-full-width
branch
from
8e6821e to
7eea3e0
Compare
coder-agents-review follow-up response:
|
|
/coder-agents-review |
|
/coder-agents-review |
|
@codex review
|
There was a problem hiding this comment.
Rebase brought the base forward. The transcript width feature remains clean. Two new concerns from the R8 delta.
1 P2, 1 nit below.
The P2 is the main issue: the "Collapsed bash summaries" display mode setting in GeneralSection was partially rolled back. The consumer side was removed (BashToolCall no longer reads the display mode; bashCollapsedSummary no longer accepts it as a parameter), but the producer side remains: ~80 lines of UI, constants, types, a state hook, a handler, and two tests for a dropdown that does nothing. grep -rn "TOOL_COLLAPSED" src/ returns hits only in GeneralSection and storage.ts. Either remove the dead settings UI or reconnect the consumer.
Process note: the collapsed bash summaries feature is not mentioned in the PR title, summary, or implementation bullets. The BashToolCall layout restructuring (horizontal to vertical, intent detail section in expanded view) and the ChatPane scroll hold refactoring are clean and work correctly, but bundling an undocumented, partially removed feature into an unrelated PR is how dead UI ships.
"When removing a feature, the agent should have grepped for all references to TOOL_COLLAPSED_DISPLAY_MODE_KEY and removed the orphaned code." (Mafu-san)
🤖 This review was automatically generated with Coder Agents.
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ 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". |
There was a problem hiding this comment.
DEREM-27 (P2, dead display mode setting) addressed: consumer reconnected in BashToolCall and bashCollapsedSummary, with a test for command-only mode.
DEREM-28 (Nit, createConfigEventStream duplicates createMockIterableSubscription from SshPromptDialog.test.tsx) has no author response. Further review is blocked until the author responds or pushes a fix. Options: extract to a shared test utility, acknowledge as out of scope, or explain why the duplication is intentional.
🤖 This review was automatically generated with Coder Agents.
coder-agents-review follow-up responses:
|
ibetitsmike
force-pushed
the
mike/chat-transcript-full-width
branch
from
7443449 to
e2f8bdf
Compare
|
/coder-agents-review |
|
@codex review
|
|
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". |
There was a problem hiding this comment.
DEREM-28 fix verified: createControllableAsyncIterable extracted to shared src/browser/testUtils.ts and reused in both useChatTranscriptFullWidth.test.tsx and SshPromptDialog.test.tsx. The SshPromptDialog test suite (8 tests) passes unchanged with the shared utility. Clean extraction, no semantic drift.
No new findings from Netero or the panel. All 28 findings across ten rounds resolved: 19 fixed by author, 1 contested and closed by panel (8/8), 2 acknowledged, 7 dropped by orchestrator. 0 open.
🤖 This review was automatically generated with Coder Agents.
|
@codex review
|
|
/coder-agents-review |
Add a persisted appearance setting that lets users expand chat transcripts to the full chat pane width, with backend config persistence, ORPC wiring, settings UI, and layout usage. --- _Generated with `mux` • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `$37.62`_ <!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=37.62 -->
--- _Generated with `mux` • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `$97.01`_ <!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=97.01 -->
![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: f35ec6144a
ℹ️ 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".
There was a problem hiding this comment.
Test setup switched from inline GlobalWindow to the project's shared installDom() utility with proper cleanup. CI green. No new findings from Netero or the panel.
All 28 findings across eleven rounds resolved: 19 fixed by author, 1 contested and closed by panel (8/8), 2 acknowledged, 7 dropped by orchestrator. 0 open.
🤖 This review was automatically generated with Coder Agents.
ibetitsmike
force-pushed
the
mike/chat-transcript-full-width
branch
from
f35ec61 to
fa369a0
Compare
|
@codex review
|
|
/coder-agents-review |
|
@codex review
Addressed the stale local toggle race by invalidating older config fetch versions when the persisted transcript width preference changes locally. Added a regression test covering an older backend fetch resolving after a local toggle. |
|
/coder-agents-review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d6392bee6
ℹ️ 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".
--- _Generated with `mux` • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `$97.01`_ <!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=97.01 -->
ibetitsmike
force-pushed
the
mike/chat-transcript-full-width
branch
from
2d6392b to
7472960
Compare
|
@codex review
Addressed the backend refresh ordering follow-up by marking backend-applied persisted writes separately from local setting writes. Added a regression test that accepts a newer backend refresh after a backend-driven cache update. |
|
/coder-agents-review |
There was a problem hiding this comment.
Display mode refactored cleanly: renamed from ToolCollapsedDisplayMode to BashCollapsedSummaryMode, new "intent" mode added as a third option, mode delivery moved from per-component usePersistedState to a BashCollapsedSummaryModeContext provider in ChatPane. The BASH_COLLAPSED_SUMMARY_MODES array drives both the type guard and the settings dropdown options, so adding a mode is a one-line change. Test updated to verify localStorage persistence directly.
CI failure is "Codex Comments" (external service), not related to this PR's code.
No new findings from Netero or the panel. All 28 findings across twelve rounds resolved. 0 open.
🤖 This review was automatically generated with Coder Agents.
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ 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". |
There was a problem hiding this comment.
Race condition fix verified: persistedValueRef and backendPersistedValueRef now detect when a local Settings toggle arrives while a backend fetch is in flight and invalidate the stale fetch by incrementing fetchVersionRef. Two new tests cover both directions: backend refresh accepted after a backend-driven cache update, and local persisted update preserved when an older backend fetch resolves with a conflicting value.
CI failure is "Test / Windows", not related to this PR's changes.
No new findings from Netero or the panel. All 28 findings across thirteen rounds resolved. 0 open.
🤖 This review was automatically generated with Coder Agents.
1 participant
Summary
Adds an appearance setting that lets users expand chat transcripts to the full chat pane width instead of the default readable column.
Implementation
chatTranscriptFullWidthin app config only when enabled.config.updateChatTranscriptFullWidthand returns the effective boolean fromconfig.getConfig.Validation
bun test src/browser/hooks/useChatTranscriptFullWidth.test.tsx src/browser/features/Settings/Sections/GeneralSection.test.tsx src/browser/features/Tools/bashCollapsedSummary.test.ts src/browser/stores/GitStatusStore.test.ts src/common/config/schemas/appConfigOnDisk.test.ts src/node/config.test.ts src/node/orpc/router.test.tsMUX_ESLINT_CONCURRENCY=2 make static-checkmake testReview notes
Generated with
mux• Model:openai:gpt-5.5• Thinking:xhigh• Cost:$97.01