fix(ui): restore the transcript's markdown rhythm - #2348
Merged
Conversation
Under `density="compact"` the transcript's markdown spacing was out of order: list items sat 10px apart while paragraphs sat 4px apart, so items at the same level read as further apart than separate paragraphs. Astryx's `density` only reaches the blocks Markdown renders itself. Lists are delegated to the List control, and Markdown hands it a hardcoded `density="compact"` in both modes — so a markdown list is spaced as a clickable row (4px block padding) rather than as prose, and no density value can change that from the outside. Own the compact prose rhythm in one table instead: 4px between list items, 8px between blocks, 16px before a section heading, 24px before a chapter heading. Gaps are margin-top only, so there is no collapsing to reason about and the gap between two blocks is the declared value. Two scoping choices: - Only `[data-density="compact"]`. Document mode is not broken (10px list items against 12px paragraphs is already in order) and the Daily Review renders through it, so it stays out of the blast radius. - Keyed on Astryx's own `data-density` rather than a `.maka-turn` ancestor. `themeProps()` reflects visual props as data attributes for exactly this, so the rhythm follows the `density` prop instead of where the markdown happens to sit in the DOM. That second point retires the position coupling in chat-message.css: the heading scale was a function of DOM ancestry, which is why no Storybook story could reproduce the transcript. The scale keeps its two size steps (the flattening argument is unchanged) and moves into the same table. Add the TranscriptTurn story that the old scoping made impossible, and correct the two existing stories' `Real path` annotations — both named chat, but both render document density, which is the Daily Review's mode.
h4-h6 already render at body size in a transcript, so the size ladder has nothing left to give them — and growing them back would rebuild the visual slabs the flattening exists to prevent. Use the one axis that costs no vertical space: the deepest levels step down to `--foreground-secondary`. A turn can then carry four levels of structure inside two type sizes, where before h3 and h4 were indistinguishable. opencode makes the same call at the same point, dropping h4-h6 to a muted colour once its 17/15/13 ladder runs out.
The rhythm table declared block gaps as `> *` plus a `> :first-child` reset. Those look equivalent to an adjacent-sibling rule and are not: `:first-child` scores (0,4,0) against the heading rules' (0,5,0), so the reset lost. A turn that OPENS with a heading — which is most of them, since models reach for `##` on the first line — kept the 24px chapter gap above its first line and pushed itself off the top of the bubble. Measured 26px from the bubble edge to the first heading where 2px was intended. Express the gaps as what they are instead: a gap is a relation BETWEEN two blocks, so every gap rule is an adjacent-sibling rule over a blanket `margin-block: 0`. The first block then matches no gap rule at all and needs no reset, which makes the defect unrepresentable rather than merely fixed — there is no longer a rule whose job is to lose gracefully. Pin the rhythm invariants while the reasoning is fresh, following the CSS contract tests already in this directory. The ladder assertion checks the ORDER of the four gaps rather than their values, because the order is the invariant — retuning 8px to 10px is a design call and stays green, while list gaps meeting block gaps is the regression this branch exists to fix. Every assertion was mutation-tested against the defect it describes. Found by adversarial review of the branch.
The CSS contract added with the first-block fix reads the stylesheet as text, so it stays green against DOM it never sees. The whole rhythm table keys on runtime Astryx output — `data-density`, `astryx-markdown-heading`, `data-level`, `astryx-list-item` — whose names have a single upstream owner and appear in Maka source only inside selectors. An Astryx bump that renames one makes every rule stop matching, the inverted ladder returns, and nothing fails: the check-dead-css entries are an allowlist that marks those classes live, not an assertion that they exist. Pin the other side of the join by rendering MarkdownBody and asserting the hooks are really emitted. Each assertion was mutation-tested against the break it describes. Also guard the one bet the table makes. Its heading rules are typography, not spacing, and Astryx's density RFC (facebook/astryx#839) draws the line at "density shifts heights and spacing, not typography" — so keying them on `density` is honest only while `compact` and "transcript" name the same set. They do today. Rather than add a surface prop for a caller that does not exist, assert the set: a second compact caller fails the test and is told what the choice is. The comment now records this as a risk rather than only as a decision. Addresses the two P2 findings from an adversarial review of the branch.
The rhythm table's gap rules use the child combinator, so they only reach the document root's own children: a paragraph inside a list item or a blockquote keeps Astryx's compact 4px rather than the table's 8px block gap. That is deliberate — a list item reads as one unit, so blocks inside it belong closer together than blocks in the transcript, and the ladder extends downward instead of inverting — but nothing said so, which left the tier difference looking like an oversight in the scoping.
The rhythm table put every top-level gap on one 8px step, which swept the `hr` in with it — an explicit separator ended up reading no wider than the paragraph boundary above it, so typing one changed nothing. That is the same failure as the flattened heading scale, in a different place: structure the author expressed, rendered as no structure at all. Give it the section step instead. A following h1/h2 still wins on specificity ((0,5,0) over (0,3,1)) and keeps its chapter gap, so an `hr` before a major heading is unaffected. The ladder stays monotonic: 4 list / 8 block / 16 section and hr / 24 chapter. Measured against a real transcript: paragraph -> hr was 12px on main, 8px before this commit, 16px now.
Four mutations survived the contract as written, each of them a way to
keep the tests green while the rendered ladder stops being what the table
declares:
- deleting the blanket `> * { margin-block: 0 }` reset, after which
Astryx's own end margins survive in the earlier layer and collapse
against the gaps
- hardcoding a gap value, which leaves the ordered `--md-gap-*` ladder
declared but unspent — the first test then asserts nothing about the
spacing anyone sees
- hardcoding the list gap, detaching the tightest rung, so an inversion
could be reintroduced without failing anything
- deleting or demoting the `hr` step
Close all four, and pin `data-maka-contract="markdown"` on the DOM side:
every rule in the table is prefixed with it, and it was the one hook
whose two halves nothing joined — rename it and both suites stayed green
while all compact spacing died.
The hr assertion checks the rung outranks --md-gap-block rather than
naming one, so moving it to chapter stays a design call. Each assertion
was verified to fail against the mutation it describes.
Astro-Han
force-pushed
the
fix/markdown-transcript-rhythm
branch
from
August 6, 2026 18:38
4d58394 to
739643f
Compare
Astryx's `Dialog` does not portal, so the Mermaid fullscreen overlay renders as a direct child of the markdown document while being `position: fixed; inset: 0`. The compact adjacent-sibling gap rules matched it and gave it an 8px margin-block-start, which pushed the viewport-filling overlay 8px down instead of spacing anything — the Mermaid E2E journey measured exactly that. A gap is a relation between two blocks in the document flow, and an overlay is not one, so exclude it on the side that receives the margin. Sitting between two blocks as the `+` antecedent stays fine.
Astro-Han
marked this pull request as ready for review
August 7, 2026 03:35
This was referenced Aug 21, 2026
Astro-Han
added a commit
that referenced
this pull request
Aug 21, 2026
Two paragraphs in a chat turn sat 8px apart against 20px body leading, so a paragraph break was 0.4 of a line — narrower than the line break inside a paragraph, which is the one vertical distance a reader already knows. A long answer read as one slab. Take the block rung to 12px, which is Astryx's own paragraph rhythm at document density (spacingParagraphDefault, --spacing-3). The transcript keeps its own heading scale and its own 4px list rung and only stops being denser than the design system between two paragraphs. Where the fix does NOT belong, because the obvious reading is wrong: `density="compact"` in chat-turn.tsx has not owned transcript block spacing since #2348. That table zeroes every Astryx margin on the document's children and declares the four gaps itself, in @layer components — a Markdown density token, or a makaTheme.ts override of `astryx-markdown-paragraph`, lands in an earlier layer and is inert. `--md-gap-block` is the only authority, so the call site now says so rather than leaving the next reader to re-derive it from commit history. One rung moves, so code blocks and blockquotes move with it: they are already carried by the same `> * + *` step (Astryx skips its codeblock wrapper whenever `components.code` is set, which Maka always does), and splitting a per-element rung out would re-introduce the multi-value spacing this table exists to collapse. What that costs is contrast at the top of the ladder — the section step falls from 2x the block gap to 1.33x — and the heading rungs stay put anyway, because heading spacing is #1857's decision and not this change's business. Measured in Storybook against live computed styles, before and after, on Product/Markdown → TranscriptTurn and Product/Shell Official AppShell → Native Conversation: list rows 4px 4px unchanged blocks 8px 12px paragraph, list, quote, code block section 16px 16px unchanged (h3-h6) chapter 24px 24px unchanged (h1, h2) Daily Review renders at density="default" and is out of the selector, so its ladder is byte-for-byte what it was: 12/12 paragraphs, 16/16 quote and table, straight from Astryx. Generated-by: Claude Code
Astro-Han
added a commit
that referenced
this pull request
Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by `__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in claiming a guard that does not exist — the worst state to leave an assumption in, since the next person reads the citation and stops looking. Restore the two assertions the comment is actually about, plus the ladder order #2348 exists to protect, in ONE file. Split across two they each stayed green against the half they could not see, which is how the stylesheet half got deleted without anything noticing: - the ladder is declared on the --space-* scale in strictly increasing order (retuning a rung is a design call and stays green; list gaps meeting block gaps fails) - every rung declared is actually spent by a rule, so the ladder cannot become decoration while literals do the spacing - the runtime hooks the selectors need are really emitted by MarkdownBody — data-maka-contract, data-density, astryx-markdown-heading, data-level, astryx-list-item. These names have a single upstream owner in Astryx and appear in Maka only inside selectors, so a rename kills every rule at once with nothing failing - compact markdown still has exactly one caller, which is the bet the comment names: the table carries heading TYPOGRAPHY on a density key, and Astryx's own RFC says density must not do that Why these are not what #2425/#2462 retired. Those PRs removed markup/copy pins, duplicate presentation assertions and CSS-structure suites that asserted how a rule was written. Nothing here asserts an appearance or a rule's shape: three of the four are relations that must survive any retune, and the fourth renders the real component and checks the join between Astryx's output and Maka's selectors. Deliberately left unpinned, because they ARE how the table is written: the adjacent-sibling gap form, the `hr` rung, the ListItem padding reset, and the two heading size tiers. The stylesheet-reading half no longer revives the deleted desktop css-test-helpers module or the renderer-wide CSS aggregate it read; it reads packages/ui/src/styles.css directly, in the package that owns it. Seven mutations, each verified to fail the assertion that describes it: inverting the ladder; taking a rung off the --space-* grid; hardcoding a rung's usage; hardcoding it while naming the rung in a comment (this is why comments are stripped first); renaming the contract wrapper; cutting density off before it reaches Astryx; and adding a second compact caller. Also state in the comment what the contract does and does not hold, so the next reader does not have to open the test to find out. Generated-by: Claude Code
Astro-Han
added a commit
that referenced
this pull request
Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by `__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in #2462 and its stylesheet-side sibling in #2425, so the comment has been citing a guard that does not exist — the worst state to leave an assumption in, since the next person reads the citation and stops looking. Restore only what cannot be noticed without a test. Both failure modes here are silent: no error, no failing check, just spacing that quietly stops being what the table declares, because a CSS selector that matches nothing never complains. - The table selects entirely on DOM Astryx generates at runtime — data-density, astryx-markdown-heading + data-level, astryx-list-item. Those names have one upstream owner and appear in Maka only inside selectors, so a rename kills every rule at once. Astryx is bumped regularly (0.4.0 in #2983, 0.4.3 in flight, plus the Dependabot minor group), so this is a recurring event, not a hypothetical. - Astryx's ListItem carries CONTROL row padding that `density` cannot reach from outside. That padding is what inverted the ladder in #2348 — list items ~10px apart against 4px paragraphs — and one rule neutralizes it. Lose the rule and the original defect returns, silently. - Plus the assumption the comment actually names: compact markdown still has exactly one caller. The table carries heading TYPOGRAPHY on a density key, which Astryx's own RFC says density must not do, so the key is honest only while `compact` and "transcript" are the same set. Deliberately NOT pinned: the ladder's declared values and their order. A first draft asserted them, and it was the wrong instinct — a reversed ladder has to be typed on purpose into four adjacent lines under a comment explaining the order, and it is visible the moment anyone looks at a transcript. It also would not have caught the defect it claimed to guard: #2348's inversion came from the ListItem padding above, which a test reading only the declared variables cannot see. Same for the adjacent-sibling gap form, the `hr` rung and the two heading size tiers — those are how the table is written, not what it promises. Why this is not what #2425/#2462 retired. Those PRs removed markup/copy pins, duplicate presentation assertions, and CSS-structure suites that asserted how a rule was written. Two of the three assertions here render the real component and check the join between Astryx's output and Maka's selectors; the third asserts an outcome the table promises, not a syntax. The stylesheet-reading half does not revive the deleted desktop css-test-helpers module or the renderer-wide CSS aggregate it read — it reads packages/ui/src/styles.css directly, in the package that owns it. Six mutations, each verified to fail the assertion that describes it: deleting the ListItem padding reset; un-zeroing it; un-zeroing it while naming the reset in a comment (this is why comments are stripped first); renaming the contract wrapper; cutting density off before it reaches Astryx; and adding a second compact caller. Generated-by: Claude Code
Astro-Han
added a commit
that referenced
this pull request
Aug 21, 2026
Transcript paragraphs sat 8px apart against 20px body leading — a paragraph break narrower than the line break inside a paragraph. Raise the block rung to 12px, which is Astryx's own `spacingParagraphDefault`, so the transcript stops being denser than the design system between two paragraphs while keeping its own heading scale (#1857) and list rung untouched. The spacing is owned by the compact rhythm table in packages/ui/src/styles.css, not by Astryx's density tokens: the table zeroes Astryx's margins outright and declares every gap itself, so retuning `--md-gap-block` is the only lever. Adds two contract assertions for the ways that table can break silently — the runtime DOM hooks its selectors need still being emitted, and the ListItem control padding still being neutralized (the mechanism that inverted the ladder in #2348). Declared values are deliberately not pinned; a wrong number is visible the moment anyone looks at a transcript. Review caught a third assertion claiming compact markdown was transcript-only. It was already false — the desktop Artifact Preview has rendered `<MarkdownBody density="compact">` since #2506 — and passed anyway. Removed rather than widened, since holding it means a library test grepping application source. The shared surface and the unguarded typography coupling are documented next to the heading rules instead. Generated-by: Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Under
density="compact"the transcript's markdown spacing was out of order: list items sat 10px apart while paragraphs sat 4px apart, so items at the same level read as further apart than separate paragraphs. Visual distance stopped tracking semantic distance, which is what made a turn read as an undifferentiated block.The cause is not a wrong number. Astryx's
densityonly reaches the blocks Markdown renders itself — lists are delegated to theListcontrol, and Markdown hands it a hardcodeddensity="compact"in both modes (Markdown.js:883,908), so a markdown list is spaced as a clickable row (4px block padding + 2px list gap = 10px) rather than as prose. Nodensityvalue can change that from the outside, so the prose rhythm could not close on Astryx's side.This owns the compact prose rhythm in one table:
Every gap is an adjacent-sibling rule over a blanket
margin-block: 0. A gap is a relation between two blocks, so the first block matches no gap rule and needs no reset. That is not cosmetic: the reset form (> *plus> :first-child) loses on specificity —:first-childscores (0,4,0) against the heading rules' (0,5,0) — so a turn that opens with a heading kept the 24px chapter gap above its first line and pushed 26px off the top of the bubble where 2px was intended. Most turns open with a heading.Two scoping choices worth reviewing:
[data-density="compact"]. Document mode is not broken (10px list items against 12px paragraphs is already in order) and the Daily Review renders through it, so it stays out of the blast radius.data-density, not a.maka-turnancestor.themeProps()reflects visual props as data attributes for exactly this ("consumers target stable data-attribute selectors"), so the rhythm follows thedensityprop instead of where the markdown sits in the DOM. That retires the position coupling inchat-message.css, which is why no Storybook story could reproduce the transcript before.The heading scale keeps its two size steps — the argument for flattening Astryx's document ladder is unchanged — and moves into the same table. A separate commit adds what the old rule had no room for: h4–h6 step down in colour, because below h3 the size ladder is spent and growing it back would rebuild the slabs the flattening exists to prevent. Previously h2 and h3 were identical in size, weight and colour, so three levels of structure rendered as three identical bold lines.
Reviewed against opencode (17/15/13 + muted h4–h6), kimi-code (+3/+2/+1 + muted h4) and Astryx's own document mode. All three keep list items tighter than paragraphs; this branch was the only one with the order inverted.
Verification
Computed styles from the live Storybook,
Product/Markdown— the layout invariant is the contract here, so the numbers are the evidence.TranscriptTurn(new story,density="compact")--foreground--foreground-secondaryOrder restored:
4 < 8 < 16 < 24, each step ~2×.RichAssistantAnswer(document mode) — zero regression, re-measured after the change: h2 18px, list item padding 4px, list gap 2px, blocks 12/16px — identical tomain.Contracts. Two tests, both halves of the same join, every assertion mutation-tested against the defect it describes:
apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts— the CSS side. Asserts the ladder's order rather than its values (retuning 8px to 10px is a design call and stays green; list gaps meeting block gaps is the regression), that every gap rule is adjacent-sibling, that ListItem padding is spent, and that two heading size steps survive.packages/ui/src/__tests__/markdown-rhythm-dom-contract.test.tsx— the DOM side. The table keys entirely on runtime Astryx output whose names have a single upstream owner, so a rename would make every rule stop matching with nothing failing. This rendersMarkdownBodyand asserts the hooks are really emitted.Ran: both contract suites,
check-dead-css --check,check-story-annotations,node --test scripts/check-dead-css.test.mjs scripts/check-story-annotations.test.mjs(21/21),@maka/uitypecheck,typecheck:stories,lint,format:check.Not run: full
npm testand E2E — this changes CSS, stories, tests and one allowlist, with no runtime behaviour touched.Review focus
A known bet, held by a test. The table's heading rules are typography, not spacing, and Astryx's density RFC (facebook/astryx#839) draws the line at "density shifts heights and spacing, not typography". Keying them on
data-densityis honest only whilecompactand "transcript" name the same set — true today, since the transcript is the only caller asking for compact. A surface prop would decouple them, but nothing needs it yet, so the assumption is asserted instead: a second compact caller failskeeps compact markdown a transcript-only surfaceand is told what the choice is. Worth a second opinion on whether that trade is right.TranscriptTurnis the story that could not exist before. The two existing stories'Real pathannotations were both wrong — they named chat, but render document density, which is the Daily Review's mode — and are corrected here.