fix(web): keep code block word wrap across virtualizer remounts - #8004

💭 I'm looking at how the change turns word wrap into a global setting rather than per-component state, so I need to check useSettings.ts for the useClientSettings and persistClientSettings functions.

1. browse_code — apps/web/src/hooks/useSettings.ts

2. browse_code — apps/web/src/components/ChatMarkdown.tsx

💭 I'm noticing that making word wrap a global setting via useWordWrapPreference changes prior behavior where each code block and table had independent local state — now toggling one block's wrap affects all of them and persists globally. I want to check if this is intentional given the PR's goal of surviving virtualizer remounts. Digging deeper, the table's "expand cells" toggle and the code block's "wrap" toggle now appear to share the same underlying setting, so expanding a table cell would also toggle code block wrapping and vice versa — that seems like a real regression. There's also an imperative column-width measurement that runs on table expand, which wouldn't fire correctly if expansion is triggered by a setting change from elsewhere, causing layout inconsistencies with unpinned min-widths. I also want to verify whether getClientSettings is still actually used after the import changes, since an unused import would break linting or type-checking. There's also some duplication between updateClientSettings and useUpdateClientSettings that could be refactored, plus a mismatch between a specific doc comment and the more generic function naming — minor points worth noting. Now let me check how getClientSettings is used in ChatMarkdown.

3. git_tools — getClientSettings|useClientSettings|wordWrap

4. git_tools — wordWrap

💭 I've confirmed the getClientSettings import in ChatMarkdown.tsx is now unused, which will trigger a lint error or build failure. I'm also noting a behavioral issue: the toggle changed from a local per-instance setting to a global persisted one, so all code blocks and tables now share a single value, and the settings panel's word-wrap toggle flips whenever a user clicks a code block. I'm digging into whether this is intended by the PR — the title and comments suggest the goal was persistence across virtualizer remounts, but coupling a per-block control to the global preference creates a broader consistency issue across DiffPanel and PullRequestCodeTab, since their initial state now shifts as a side-effect of unrelated toggles elsewhere. This looks like a genuine violation of preserving component-scoped behavior versus introducing a shared, unintended coupling. The MarkdownTable's expand toggle also pins column widths before expanding, but if expanded flips from an outside source that pinning never runs, causing layout jumps. There's also an unused import and a switch from functional state updates to direct negation, which could introduce stale closures under rapid interaction — worth noting but lower priority. I want to check the code block toggle region around lines 660-760 to see how it handles aria-pressed and related accessibility attributes.

5. browse_code — apps/web/src/components/ChatMarkdown.tsx

6. browse_code — apps/web/src/components/ChatMarkdown.tsx

💭 With the findings confirmed, I want to check whether this global coupling was actually intentional by looking at the PR description or prior discussion.

7. github_api_read_only — repos/{owner}/{repo}/pulls/{pull_number}

💭 I'm noticing the "expand table cells" toggle seems semantically distinct from word wrap—expanded likely means cells grow to fit content rather than wrapping, so I want to check the CSS to confirm what the data-expanded attribute actually controls.

8. git_tools — chat-markdown-table-container

💭 I'm noting the "expanded" table behavior is semantically consistent since both derive from wordWrap. But I'm flagging a real issue: the unused getClientSettings import would likely trigger a lint failure even if typecheck passes, so I need to check whether the ESLint config actually enforces no-unused-vars for imports. More importantly, since expanded is now global rather than per-table, tables can flip to expanded without the column-width pinning measurement that only ran when a table's own toggle was clicked — meaning other mounted tables now reflow inconsistently instead of guaranteeing measurement always precedes expansion, which is a real regression. There's also a lower-priority stale-closure concern with setExpanded(!expanded) computing from a captured render value, though useSyncExternalStore likely keeps it fresh enough to deprioritize. More significant is that clicking the code-block wrap chip now permanently rewrites the user's global "Word wrap" setting, affecting other panels' initial state — that's a control-semantics change where an ephemeral per-block toggle now behaves like a persistent global preference, which conflicts with keeping interaction semantics stable and changes as minimal as possible. I'm settling on three inline comments to post: the unused import at line 70, the global write-through concern around lines 430-439 with a suggestion for a module-level ephemeral store instead of overwriting user preferences, and the expansion/min-width pinning issue at line 471. I want to double-check the diff's line numbers align correctly before posting, and verify the eslint config actually flags unused imports.

9. browse_code — Does the ESLint or TypeScript config for apps/web flag unused imports (e.g. noUnusedLocals in tsconfig, or @typescript-eslint/no-unused-vars rule)? Show the relevant config snippets.

💭 The unused import is a minor lint issue worth noting briefly, and I've confirmed that toggling chat code block wrap does immediately affect file preview panel layout. Given these findings, the review should fail with three comments: the unused import, the global write-through scoping concern, and a measurement/pin issue. For the write-through concern, this is a real semantics change since a per-block chrome toggle is silently rewriting a user-facing Settings preference, which cascades to other surfaces like the FilePreviewPanel live view, the Settings "modified from default" badge, and DiffPanel initial state — plus it affects aria-pressed semantics across all blocks since a toolbar toggle labeled per-block now changes globally. I'll suggest fixing this with a module-level ephemeral store seeded from the setting rather than persisting to settings directly. For the third comment, there's no self-contained fix, so I'll suggest applying width pinning via an effect keyed on the expanded state so externally-driven expansions are handled properly. For the width-pinning issue, though, moving the measurement into an effect keyed on expanded state doesn't quite work because measurement needs to happen before expansion for accurate collapsed widths, but by the time an effect runs after the DOM has updated with the new expanded state, the layout has already reflowed. So a correct fix would require either per-instance state for the table's expand control or a layout effect that measures before flipping the expanded state — nontrivial enough that I'll flag it without providing a diff and let the author consider approaches. Time to post these comments.

10. modify_pr — UI consistency review found issues with the word-wrap ownership change in apps/web/src/components/ChatMarkdown.tsx. The fix for virtualizer remounts is sound in intent, but routing per-block chrome toggles straight into the persisted global wordWrap preference changes control semantics across several surfaces and breaks the table's column-width pinning invariant. Details inline.