feat(chat): bound the reading measure — T1 of the typography plan - #71
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements T1 of the chat typography plan by constraining the transcript reading measure when the chat is opened as an editor tab, while keeping the sidebar rendering unchanged. It also adds regression tests to ensure the cap, centering, and width-gated rhythm don’t silently drift in future refactors.
Changes:
- Add a fixed prose column cap (
--prose-max: 680px) and shared centering for all#logchildren, plus width-gated vertical rhythm adjustments. - Introduce a new “AI: Open Chat in Editor” command and supporting “single live surface” hand-off behavior between sidebar and editor tab.
- Add targeted tests to guard the new CSS invariants and the sidebar⇄editor-tab surface wiring.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/levelcode-ai/media/chat.html | Adds bounded transcript measure and width-gated rhythm changes. |
| extensions/levelcode-ai/extension.js | Implements sidebar⇄editor-tab move semantics and detached “bring it back” card. |
| extensions/levelcode-ai/package.json | Contributes the new command and a view-title button for discoverability. |
| extensions/levelcode-ai/test/chatSurface.test.js | Adds tests asserting the “move not mirror” wiring and replay/restore behavior. |
| extensions/levelcode-ai/test/webviewCss.test.js | Adds CSS regression tests to pin the measure cap and width-gated rhythm. |
| docs/CHAT-TYPOGRAPHY.md | Introduces the 5-slice typography plan and documents the measured rationale for T1. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| This became urgent the moment the chat could open as an editor tab (#70). In a 380px sidebar the | ||
| line length is bounded by the container, so nothing looks badly wrong. At 900px it is unbounded, | ||
| and the same CSS produces ~130-character lines — roughly twice the readable measure. |
| function detachedHtml() { | ||
| const bg = 'var(--vscode-sideBar-background)', fg = 'var(--vscode-foreground)'; | ||
| return '<!DOCTYPE html><html><head><meta charset="utf-8">' | ||
| + '<style>' | ||
| + 'body{margin:0;padding:28px 22px;background:' + bg + ';color:' + fg + ';' | ||
| + 'font-family:var(--vscode-font-family);font-size:var(--vscode-font-size);text-align:center}' | ||
| + '.t{font-size:14px;font-weight:600;margin-bottom:6px}' | ||
| + '.s{opacity:.7;line-height:1.55;margin-bottom:18px}' | ||
| + 'button{width:100%;padding:7px 10px;border:1px solid var(--vscode-button-border,transparent);' | ||
| + 'border-radius:4px;background:var(--vscode-button-background);color:var(--vscode-button-foreground);' | ||
| + 'font:inherit;cursor:pointer}button:hover{background:var(--vscode-button-hoverBackground)}' | ||
| + '</style></head><body>' | ||
| + '<div class="t">Chat is open in the editor</div>' | ||
| + '<div class="s">The conversation moved to a tab so it has room. Closing that tab brings it back here.</div>' | ||
| + '<button id="b">Bring it back</button>' | ||
| + '<script>const v=acquireVsCodeApi();document.getElementById("b").onclick=()=>v.postMessage({type:"reattach"});</script>' | ||
| + '</body></html>'; | ||
| } |
| **T1 — measure + rhythm** *(S)*. D1 and D3. The largest perceptual change for the least code, and the | ||
| one that fixes the editor tab. Ships: a wrapper max-width, `em`-based prose spacing, wider `#log` | ||
| padding at editor width. **Exit:** a long answer in the editor tab holds ~72 characters per line, and | ||
| the sidebar renders byte-identically to today. |
| - **No test currently guards the measure.** `webviewCss.test.js` pins hidden-attribute defeats and the | ||
| session-card overflow; it should gain a guard that the prose column is bounded, or T1 will regress | ||
| silently the first time someone refactors the log container. |
Nothing constrained the transcript's line length. The twelve max-width rules in chat.html are all cards, dialogs and the empty state; the message column had none. That went unnoticed for as long as the chat only lived in a ~380px sidebar, where the container did the bounding — opening it as an editor tab (#70) put the same CSS at 900px and produced ~154-character lines. Now: every direct child of #log shares one 680px column, centred, behind a --prose-max custom property so T5 can hand it to a setting. Applied to every child rather than just .msg, or the cards and the activity timeline drift wider than the prose they sit between. MEASURED, not estimated — and the measurement corrected the plan. The doc's first draft said "~72ch, near 640-700px". Both halves were wrong: "0" (the ch unit) 8.13px real average prose char 5.86px `ch` is 39% wider than actual text, so a 72ch cap yields ~100 characters, not 72; and 72 REAL characters is a 422px column, narrower than the sidebar. The print range of 45-75 does not transfer to a chat carrying identifiers and file paths. 680px is ~116 characters, down from 154, and still wide enough that a fenced block does not wrap constantly. The doc now records the correction; a test asserts the cap stays an absolute length so nobody reaches for `ch` again. Rhythm (D3) is em-based and GATED to >=760px. T1's exit criterion is that a narrow panel renders exactly as before, and that is verified rather than assumed: computed styles at 520px against develop are identical across padding, gap, paragraph and heading margins, line-height and font-size. Rendered and measured at three widths: 520px column 496 · 81 chars · padding 12px (identical to develop) 900px column 680 · 115 chars · padding 24px 1400px column 680 · 115 chars · does not grow Two guards added to webviewCss, because this regression is invisible in a sidebar: whoever refactors the log container will not see it break, a user with the chat in an editor tab will. Verified non-vacuous five ways — remove the cap, scope it to .msg only, switch to ch, drop the width gate, drop the centring: 14-15/17 each. 33 suites green.
e5edf86 to
407512d
Compare
Review found CHAT-TYPOGRAPHY.md contradicting itself in three places. One cause: D1 was corrected after measuring, and the correction was never propagated. intro "~130-character lines" → ~154, the figure actually measured T1 exit "~72 characters" → ~116, the 680px cap D1 settled on risks "no test guards this" → closed by the guards in this PR The 130 and the 72 were both written BEFORE the measurement that produced 680px; the risk bullet was true when written and stopped being true in the same commit. Rather than proofreading it again, the number is now pinned to the code: a test reads `--prose-max` out of the stylesheet and asserts the doc names that exact cap, and that no stray reference to the disproved 72-character target survives. Prose drifts from code silently — this is the only claim in the doc that a test can hold, so it holds it. Caught both directions: change the CSS without the doc (15/18), the doc without the CSS (15/18), reintroduce the old figure (15/18). The fourth comment (detachedHtml has no CSP) was written against this branch before it was rebased, when it still carried #70's commits. extension.js is no longer in this PR's diff, and the CSP + nonce landed in #70 as dc91502. 33 suites green.
|
Three valid, one already resolved elsewhere. Fixed in 2d4964d. The doc contradicted itself in three places — one causeD1 was corrected after I measured, and the correction never propagated:
The 130 and the 72 were both written before the measurement that produced 680px. The risk bullet was true when written and stopped being true in the same commit that added the guards. Pinned to the code rather than proofread againProse drifts from code silently, so the number is no longer maintained by hand. A test reads
It catches drift in both directions, which is the property that matters — the failure here wasn't a typo, it was two sources of truth. On the CSP commentThat one was written against this branch before it was rebased, when it still carried #70's commits. 33 suites green. #72 has been restacked on this branch. |
Adds
docs/CHAT-TYPOGRAPHY.md(the full five-slice plan) and ships T1, the slice that fixes the editor tab.The finding
Nothing constrained the transcript's line length. The twelve
max-widthrules inchat.htmlare all cards, dialogs and the empty state — the message column had none.It went unnoticed because the chat only ever lived in a ~380px sidebar, where the container did the bounding. Opening it as an editor tab (#70) put the same CSS at 900px:
The measurement corrected the plan
My first draft of the doc said "~72ch, which lands near 640–700px". Both halves were wrong, and the correction is in the doc because the mistake is easy to repeat:
ch(the width of0)chis 39% wider than actual text, so a72chcap yields ~100 characters, not 72. And 72 real characters is a 422px column — narrower than the sidebar, and it would wrap every code block. The print-typography range of 45–75 doesn't transfer to a chat carrying identifiers and file paths.680px is chosen against the measurement, and a test now asserts the cap stays an absolute length so nobody reaches for
chagain.The sidebar is untouched — verified, not assumed
T1's exit criterion was "the sidebar renders byte-identically to today". Computed styles at 520px, this branch vs
develop:Identical. The cap is inert below 680px, and the looser rhythm is gated to ≥760px — so a user who upgrades and never opens the editor tab sees nothing move.
Rendered at three widths:
Guards
Two new tests in
webviewCss, because this regression is invisible in a sidebar — whoever refactors the log container won't see it break; a user with the chat in an editor tab will..msgonlychunits33 suites green.
What's deliberately not here
T2 (prose type scale — the one that diverges from
--vscode-font-size), T3 (code surfaces), T4 (speaker treatment), T5 (the settings escape hatch). T1 ships alone on purpose: it may be most of the perceived fix, and that's the cheapest way to find out before spending effort on the rest.