Uh oh!
There was an error while loading. Please reload this page.
fix(desktop): give the chat surface one reading measure - #3997
Conversation
757f86f to
1d67f03Compare167ee41 to
bb568f1Compare08c256b to
86f33cfCompareThe turn column is `--maka-transcript-measure` (800px), and the tool cards, code blocks and the user bubble all reach it — but every paragraph stopped 120px short, leaving a band of whitespace down the right side of the transcript that grows with the window. The cause is two reading-measure authorities. Maka's column owns one, and Astryx's `Markdown` owns another: `contentWidth` defaults to 680px. #3617 raised the column from 680 to 800 so source evidence could be read, which made the second authority visible for the first time. `MarkdownBody` now passes `contentWidth: 100%`, which is a disclaimer rather than a width: Markdown holds no measure, and the container it lands in is the only one that does. That rule only works if every container has one, and the two document callers did not. Astryx renders a supplied `components.code` bare — no spacing, no width, no alignment — and Maka always supplies one, so on the Daily Review report, which sits on an uncapped module page, prose stopped at 680 while the code blocks inside it ran the width of the window. The report column holds the measure now, and it is the column and not the prose inside it: a section is a heading, a divider and its prose, so capping only the prose would have left the other two running past it. The Artifact Preview pane caps itself at 600px and so never showed this in the normal layout; it holds the measure for the stacked layout a narrow window switches to, where the workbar drops that cap. Also corrects the neighbouring `--maka-chat-measure` comment, which still named the conversation and tool output as its consumers; both moved to the transcript measure in #3617. Generated-by: Claude Code
86f33cf to
dde2322CompareTwenty-one custom properties in `maka-tokens.css` have no `var()` consumer anywhere — not in renderer CSS, not in `@maka/ui`, not in stories, and not in Astryx, which reads a handful of Maka properties through its own theme bridge and is checked separately here. A token nobody reads still has to be kept consistent with the palette, the theme blocks, and every audit that walks the file, so it costs maintenance and returns nothing. Removed: the checked-control colors (`--control`, `--control-foreground`), four unread wash tiers, `--border-width-accent`, `--foreground-alpha-10`, `--space-0`, `--opacity-pending`, `--ease-in-out-strong`, `--maka-text-display-2`, and `--h-control-xl` / `--h-control-2xl`. Two groups are more than single tokens: - The icon scale mirrored `ICON_SIZE` in `@maka/ui`'s `icons.tsx` by its own admission. `ICON_SIZE` has 209 consumers and the CSS mirror had two; the two CSS-clamped rungs stay and the rest go, including `--icon-size`, an alias marked "deprecated, one release", and `--icon-chrome`, whose only reader was that alias. - The z-index ladder kept four rungs no layer sits on. Reserving unused floors invites the next reader to pick by name rather than by stacking, so the ladder now lists what exists: sticky, titlebar, tooltip, overlay, plus the two micro tiers. `--h-control-xl` overrides a recorded decision rather than collecting an oversight, and should be read as one: #1879 found the rung had no reader, wrote that in the comment, and kept it anyway because Astryx defines it and a prominent button is a plausible future need. A rung held for a need nobody has yet is the same reserved floor the z-index ladder just lost, and it is one line to add back the day a call site wants it. Four comments that named a deleted token as live are corrected with it. Two survivors that look unread are not. `--elevation-raised` / `--elevation-drag` carry no consumer, but DESIGN.md §5 names the three-step tier as the vocabulary product CSS must use and the Floating Recipe requires `--elevation-overlay` by name; retiring part of a published tier is a design decision, not a sweep. `--foreground-8` stays beside `--foreground-alpha-*`: the first mixes into the background and is opaque, the second is a real alpha channel, and the file already records why the two cannot substitute for each other. Generated-by: Claude Code
`--maka-chat-measure` (680px) and `--maka-transcript-measure` (800px) split one column in two. #3617 introduced the second so transcript code, tables and tool output could be read, and every surface the user answers with — composer, header notices, hero, plan mode, WorkHub — stayed at the narrower one. The answer was therefore wider than the box you type into, and turn chrome (`.maka-turn-footer`, `.maka-turn-lineage-row`) stopped at 680 inside an 800px turn: three right edges down one conversation. `--maka-chat-measure` is now 800px and is the only measure. The transcript token is deleted, `.maka-turn` reads the shared one, and `@maka/ui`'s fallback follows it — that package does not load `maka-tokens.css`, so its fallback is the value that actually renders there. `.workhub-return` had written the old 680 as a literal, so unifying the tokens alone would have left it 120px narrower than the WorkHub panels above it. It reads the measure now, like its siblings. 800px because the alternative undoes #3617: at 680 the code, tables and tool output it widened go back into a column too tight to read. The composer, hero, notices, plan mode, WorkHub and the agent graph gain 120px. Generated-by: Claude Code
Four removals the audit of this branch turned up, three of them created by the branch itself. The assistant bubble set `width: 100%; max-width: none` from product CSS, in `chat-message.css` and again at higher specificity in `workhub.css`. That is what Astryx's published `width` prop does: it sets the width and drops the default `max(80%, 280px)` cap in one place, and both call sites already pass `variant="ghost"`, which its docs pair it with. The two call sites pass the prop now, the markdown stories pass it too, and the two CSS blocks go. Turn chrome carried `max-width: var(--maka-chat-measure, 800px)` with `margin-right: auto`. Those bound while the chrome was 680 inside an 800px turn; now that one measure governs both, a descendant of an 800px box cannot exceed 800, so the cap and the auto margins can never apply. The 800px fallback leaves with them, and with it the second place the number was written. The `Marker` doc comment that promised that geometry is corrected rather than kept. `chat-message.css` also held a documentation block with no rules under it, restating the bubble geometry already documented where those rules live. Two comments elsewhere stated the old split as current — the composer "stayed at 680px" in `chat-header.css`, and `shell-layout.css` pinned the first-run hero's content at "~680px" to explain a flex bug whose argument does not need the number. Both are corrected; a number repeated in prose is the same second authority in a form no search finds. Generated-by: Claude Code
`--maka-chat-measure` stopped being about chat two commits ago: it now governs the Daily Review report and the Artifact Preview as well, and its own comment calls it "one reading column, and the only one". A name that describes a subset of what a token governs is how the next surface concludes the token is not for it and writes a second one — the defect this branch exists to remove, one release later. It is `--maka-reading-measure` now, at all twenty-four call sites, and the two comments that said "the chat measure" in prose say the same thing the token does. The Artifact Preview comment loses a number for the same reason. It had written the workbar's 600px cap out as a literal, which is a second copy of a value `SESSION_WORKBAR_MAX_WIDTH` already owns and nothing keeps in sync; it names the constant instead. DESIGN.md gains the rule, in §7 because line length is a typographic decision and §7 is where the other two named typography rules live: one reading column, one token, held by the container and never by a component. §11's list of rulers nobody may duplicate gains "reading measure" beside spacing, radius, icons and the text axis, which is the sentence that would have stopped #3617 from adding the second one. Generated-by: Claude Code
f62b88b to
35f37e3Compare
YayoiNanoka
left a comment
There was a problem hiding this comment.
Approved — I found no reachable P0/P1 blocker at 35f37e327d7ee42544da0987f49111880867dad0.
The change removes the nested 680px prose authority and leaves reading measure ownership with the containing surface. The transcript, composer, WorkHub, Daily Review, and stacked Artifact Preview paths all retain an explicit bounded container. The deleted token names have no remaining consumers in apps, packages, native, or scripts.
Verification on a clean merge with current main:
@maka/uiand@maka/desktoptypechecks passed.- The renderer production build and third-party notice check passed.
e2e/transcript-measure.spec.tspassed in Electron and confirmed that assistant prose reaches the transcript edge.- No temporary test was committed or pushed; the original workspace was unchanged.
The hosted test was still pending when I reviewed; this approval records the requested P0/P1 code-risk threshold.
点击展开中文
已批准——在 35f37e327d7ee42544da0987f49111880867dad0 上未发现真实可达的 P0/P1 阻塞问题。
该改动移除了组件内部 680px 的第二重正文宽度权威,把阅读宽度的所有权留给承载它的容器。transcript、composer、WorkHub、Daily Review,以及堆叠布局下的 Artifact Preview 路径都仍有明确的有界容器。被删除的 token 名称在 apps、packages、native 和 scripts 中没有剩余消费者。
已在与当前 main 的干净合并结果上验证:
@maka/ui与@maka/desktoptypecheck 通过。- renderer 生产构建及第三方 notice 检查通过。
- Electron 中的
e2e/transcript-measure.spec.ts通过,确认 assistant 正文到达 transcript 右边界。 - 没有提交或推送临时测试;原工作区未改动。
审查时托管测试仍在等待;本次批准表示已满足要求的 P0/P1 代码风险阈值。
jackwener
left a comment
There was a problem hiding this comment.
Approving the code. No blocking issues found. One caveat about verification at the end, which is about this branch's CI rather than about the change.
The token sweep is clean — I checked it mechanically
The risk in a change like this is not the layout reasoning, it is a missed reference: a token whose definition is deleted but whose var() call site survives, resolving to nothing and silently dropping a declaration. So I enumerated every custom property whose definition this diff removes and never re-adds — 23 of them — and searched the whole tracked tree (.css, .ts, .tsx, .js, .mjs, .cjs, .html, .scss) for surviving consumers.
Zero surviving var() consumers, and zero textual mentions of --maka-chat-measure or --maka-transcript-measure anywhere in the tree. The rename across all call sites is complete.
One methodology note, because it nearly produced a false report on my side: a naive word-boundary search matches --space-0 inside var(--space-0-5) and --z-panel inside var(--z-panel-action), since - counts as a boundary. Both of those are live tokens that this PR does not touch. Anyone re-running this audit should anchor on "not followed by another name character or hyphen", or they will get three false positives.
Other things I verified
--maka-reading-measurehas exactly one definition (maka-tokens.css:914, 800px) and 24 consumers, all inapps/desktop. Nothing in@maka/uireads it, which is consistent with the description: the.maka-turn-footercap was deleted rather than renamed, so no stale680pxfallback is left behind in the package that does not loadmaka-tokens.css.- The
max-widthand margin removal lands on the shared.maka-turn-lineage-row, .maka-turn-footerrule, so it covers both rows the description names, not just the footer. - The remaining literal
680s in the renderer are unrelated — a dialog width inmcp-page.tsxand a dialog height insettings/form.css. Neither is a reading measure. - The new E2E test guards against a false pass properly. Asserting
turnWidth > 680beforerightGap <= 1means the probe is shown able to detect the regression before it is trusted to report its absence. A test that only asserted the gap would pass on a narrow viewport where both boxes are equally constrained.
The caveat: CI has not run on this head
35f37e3 has a single check, test, and it has been queued since 13:43Z — roughly eight hours as I write this, never started. Nothing has executed on this commit.
That matters here specifically because of what your Verification section defers:
Not run: the repository-wide suite and Storybook visual smoke. CI covers both.
Storybook visual smoke is exactly the check that would catch a regression in a change that moves eight surfaces from 680 to 800 and edits four stories. So the verification this PR relies on is currently missing, not merely pending review.
My approval covers the code review above and does not stand in for that run. GitHub still lists this as BLOCKED with the required check outstanding, so nothing can merge until test goes terminal green — please treat that as the real gate. If the queue is stuck rather than slow, it is worth re-running before reading the green as meaningful.
Posted by an automated review agent operated by @jackwener. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.
简体中文
本条评论由 @jackwener 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。
M4n5ter
left a comment
There was a problem hiding this comment.
Approved at exact head 35f37e327d7ee42544da0987f49111880867dad0: I found no P0–P3 issue.
This is a real simplification rather than a directory or token rename. The second reading-measure authority (--maka-chat-measure / --maka-transcript-measure) and duplicate assistant-bubble width override disappear; MarkdownBody no longer owns a hidden 680px cap, while each production caller is bounded by its containing surface. Exact-reference scans also found no live consumer for any of the 21 removed CSS tokens.
Verification on this head:
- Core, Storage, MCP, Runtime, Runtime Host, Computer Use, UI, and the complete Desktop main/preload/overlay/renderer build passed.
- The complete UI unit suite passed 230/230.
- The focused Electron geometry regression passed at a 1400px viewport and proved assistant prose reaches the turn edge beyond Astryx's former 680px cap.
- Biome and
git diff --checkpassed for the changed files. - The current-main synthetic merge was clean.
- Hosted exact-head checks are terminal green.
Posted by an automated review agent operated by @M4n5ter. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.
简体中文
本条评论由 @M4n5ter 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。
Uh oh!
There was an error while loading. Please reload this page.
An assistant turn had two right edges. The turn column was 800px and the tool cards, code blocks and user bubble all reached it, but every paragraph stopped 120px short: Astryx's `Markdown` carries its own `contentWidth`, defaulting to 680px, so Maka's column and the component each held a measure. apache#3617 raised the column from 680 to 800 so source evidence could be read, which made the second authority visible. `MarkdownBody` passes `contentWidth: 100%` — a disclaimer rather than a width. Markdown holds no measure; the container it lands in does. Two containers did not have one: Astryx renders a supplied `components.code` bare, and Maka always supplies one, so the Daily Review report showed prose at 680 with code blocks running the width of an uncapped module page. The report column and the Artifact Preview hold the measure now. The branch then takes the same defect to its root: - `--maka-chat-measure` (680px) and `--maka-transcript-measure` (800px) split one column in two, so the answer was wider than the box you type into and turn chrome stopped at 680 inside an 800px turn. One measure now, at 800px, because 680 would undo apache#3617. The composer, header notices, hero, plan mode, WorkHub, the agent graph and both document surfaces gain 120px. - Twenty-one custom properties with no `var()` consumer anywhere are removed. `--elevation-*` and `--foreground-8` stay, with the reasons recorded. - What the merge made redundant goes with it: the assistant bubble's duplicated width CSS (Astryx's `width` prop does it), the now-unreachable turn-chrome cap, a documentation block with no rules under it, and three comments stating the old split as current. - The token is `--maka-reading-measure`, since it stopped being about chat, and DESIGN.md §7 gains The One Measure Rule so the next surface reads the policy instead of inferring it from a name. Reported upstream as facebook/astryx#5598: Astryx already publishes an inheritable `--layout-content-width` in `Layout`, and neither `Markdown` nor `ChatLayout` participates. Refs apache#3617 Generated-by: Claude Code
Summary
An assistant turn had two right edges. The turn column is
--maka-transcript-measure(800px), and the tool cards, code blocks and the user bubble all reach it — but every paragraph stopped 120px short, leaving a band of whitespace down the right side of the transcript that is wider on a wider window.The cause is two reading-measure authorities. Maka's column owns one, and Astryx's
Markdownowns another:contentWidthdefaults to 680px withcontentAlign: 'start', while code and tables are left to fill the container. #3617 raised the column from 680 to 800 so source evidence could be read, which made the second authority visible for the first time.MarkdownBodynow passescontentWidth: 100%, which is a disclaimer rather than a width: Markdown holds no measure, and the container it lands in is the only one that does.That rule only works if every container has one, and the two document callers did not. Astryx renders a supplied
components.codebare — no spacing, no width, no alignment — and Maka always supplies one, so on the Daily Review report, which sits on an uncapped module page, prose stopped at 680 while the code blocks inside it ran the width of the window. That is the same two edges, one surface over, and it is live onmaintoday. The report column holds the measure now — the column and not the prose inside it, since a section is a heading, a divider and its prose, and capping only the prose would have left the other two running past it. The Artifact Preview pane caps itself at 600px and so never showed this in the normal layout; it holds the measure for the stacked layout a narrow window switches to, where the workbar drops that cap.The 680 is deliberately not mirrored into a Maka token. A token would make Astryx's default ours to keep in sync, which is the same two-authority defect wearing a token's name.
Three commits follow it, taking the same defect to its root.
One reading measure.
--maka-reading-measure(680px) and--maka-transcript-measure(800px) split one column in two, so the answer was wider than the box you type into, and turn chrome (.maka-turn-footer,.maka-turn-lineage-row) stopped at 680 inside an 800px turn — three right edges down one conversation.--maka-reading-measureis now 800px and is the only measure; the transcript token is deleted and@maka/ui's fallback follows it, since that package does not loadmaka-tokens.css..workhub-returnhad written the old 680 as a literal and reads the measure now, like the panels above it. 800px because the alternative undoes #3617: at 680 the code and tables it widened go back into a column too tight to read.This widens surfaces beyond the transcript by 18%: the composer, header notices, the hero, plan mode, WorkHub, the agent graph, the Daily Review report and the Artifact Preview all move from 680 to 800. That is the point rather than a side effect — those are the surfaces whose edge was failing to line up with the answer above them — but it is the visible part of this PR and the reason for the second screenshot.
Tokens nothing reads. The audit behind the measure merge also found 21 custom properties with no
var()consumer anywhere — not in renderer CSS,@maka/ui, stories, or Astryx, which reads some Maka properties through its own theme bridge and was checked separately. Two groups are more than single tokens: the CSS icon scale mirroredICON_SIZEinicons.tsx(209 consumers there, two here) and keeps only its two CSS-clamped rungs; the z-index ladder listed four rungs no layer sits on. Four comments naming a deleted token as live are corrected with it. The unread--elevation-raised/--elevation-dragstay: DESIGN.md §5 names the three-step tier as the vocabulary product CSS must use and the Floating Recipe requires--elevation-overlayby name, so retiring part of a published tier is a design decision rather than a sweep.--foreground-8stays beside--foreground-alpha-*— the first mixes into the background and is opaque, the second is a real alpha channel.What the merge made redundant. Two comments elsewhere stated the old split as current — the composer "stayed at 680px", and the first-run hero's content was pinned at "~680px" to explain a flex bug whose argument does not need the number; a number repeated in prose is the same second authority in a form no search finds. The assistant bubble set
width: 100%; max-width: nonefrom product CSS, inchat-message.cssand again at higher specificity inworkhub.css; Astryx's publishedwidthprop does exactly that, and both call sites already pass thevariant="ghost"its docs pair it with, so the two CSS blocks become two props, and the markdown stories pass it too. Turn chrome'smax-width: var(--maka-chat-measure, 800px)and its auto margins bound while the chrome was 680 inside an 800px turn; under one measure a descendant of an 800px box cannot exceed 800, so they can never apply, and the 800px fallback leaves with them, along with theMarkerdoc comment that promised that geometry. A documentation block inchat-message.csswith no rules under it, restating geometry documented where those rules live, goes too.One name.
--maka-chat-measurestopped being about chat once it governed the Daily Review report and the Artifact Preview too, and a name that describes a subset of what a token governs is how the next surface concludes the token is not for it and writes a second one. It is--maka-reading-measureat all twenty-four call sites. DESIGN.md gains the rule in §7, where the other two named typography rules live — one reading column, one token, held by the container and never by a component — and §11's list of rulers nobody may duplicate gains "reading measure" beside spacing, radius, icons and the text axis.Refs #3617
Screenshots
Same conversation, same 1400px viewport.
Before — three right edges: the paragraph wraps 120px inside the code card, and the composer stops 120px inside both:
After — one: prose, code card, user bubble and composer all stop at the same edge:
Verification
All of the following on the final tree.
e2e/transcript-measure.spec.ts,e2e/session-workbar.spec.ts— 8 passed. The second renders the WorkHub assistant bubble, whose product CSS this replaces with thewidthprop.e2e/composer-plus-menu-stability.spec.ts,e2e/prompt-rail.spec.ts,e2e/link-color-contract.spec.ts,e2e/settings-row-focus-ring.spec.ts— 19 passed. The first two cover the composer at the widened measure; the last two assert computed color and focus-ring geometry, the surfaces closest to the removed color and shadow tiers.apps/desktop/e2e/transcript-measure.spec.ts: sends a message at a 1400px viewport and asserts the assistant paragraph's right edge meets the turn column's right edge. RevertingcontentWidthto680fails it at exactly120.apps,packages,native,scripts(css/ts/tsx/js/mjs/html) plus@astryxdesign/core/distandtheme-neutral, excluding build output and caches; re-run after the deletions and after the rename, novar()reference and no bare mention of a removed or old name survives. Dynamic access checked separately: only--backgroundand--maka-drawer-tooltip-xgo throughsetProperty/getPropertyValue.@maka/uiand@maka/desktoptypecheck,npm --workspace @maka/desktop run build:with-deps, andnpm run format— all clean.transcript-measure.spec.tsasserts; losing a container cap degrades line length rather than breaking behavior, and driving E2E to the Daily Review report to assert it was judged not worth its cost.Root cause
The defect reproduces in Astryx alone, with no Maka code:
ChatLayout density="spacious"gives an 800px message area,ChatMessageBubble variant="ghost" width="100%"(the documented pattern for rich content) fills it, and a defaultMarkdowninside stops its prose at 680px while its code blocks fill the bubble.Astryx already solves exactly this in
Layout, which publishes itscontentWidthas the inheritable--layout-content-widthand hasLayoutHeader/LayoutFooterread it back asvar(--layout-content-width, none). NeitherMarkdownnorChatLayoutparticipates: both carry hardcoded values instead. Reported upstream as facebook/astryx#5598. If Astryx adopts the inheritance, this prop can be deleted.AI use
Select exactly one:
Tool(s) and scope: Claude Code — diagnosis, the fix, the token audit, the E2E, and this description. Reviewed and verified by the contributor before opening.
Checklist
Does this PR entail a change in behavior?