Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 199 additions & 0 deletions apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
/**
* Transcript markdown rhythm contract.
*
* The defect this pins was not a wrong number — it was a wrong ORDER. Under
* `density="compact"` the transcript spaced list items ~10px apart (Astryx's
* List control row padding, which `density` cannot reach from outside) while
* paragraphs sat 4px apart, so items at the same level read as further apart
* than separate paragraphs. Visual distance stopped tracking semantic
* distance.
*
* So the invariant is the ladder's ORDER, not its values: retuning 8px to 10px
* is a design decision and should stay green here; making list gaps meet or
* exceed block gaps is the regression, and must fail. A screenshot cannot lock
* that — it fixes one rendering of one sample rather than the relation — which
* is why AGENTS.md asks for a computed-style or text contract on cascade and
* layout invariants. The visual half lives in the `TranscriptTurn` story
* (packages/ui/stories/markdown.stories.tsx).
*
* Scoped to the compact surface only. Document mode is Astryx's own rhythm and
* the Daily Review renders through it, so nothing here should constrain it.
*/
import { strict as assert } from 'node:assert';
import { describe, it } from 'node:test';
import { readAllRendererCss, stripCssComments } from './css-test-helpers.js';

/** The compact ladder, ordered from tightest (same level) to widest (chapter). */
const LADDER = ['--md-gap-list', '--md-gap-block', '--md-gap-section', '--md-gap-chapter'] as const;

/** `var(--space-N)` → N, the 4px-grid step count. Non-scale values return null. */
function spaceSteps(value: string): number | null {
const match = /^var\(\s*--space-(\d+)\s*\)$/.exec(value.trim());
return match ? Number(match[1]) : null;
}

describe('transcript markdown rhythm', () => {
it('declares the compact ladder in strictly increasing order', async () => {
const css = stripCssComments(await readAllRendererCss());
const block = /\.astryx-markdown\[data-density="compact"\]\s*\{([^}]*)\}/.exec(css);
assert.ok(block, 'no `.astryx-markdown[data-density="compact"]` custom-property block found');

const declared = new Map<string, string>();
for (const m of block[1].matchAll(/(--md-gap-[\w-]+)\s*:\s*([^;]+);/g)) {
declared.set(m[1], m[2].trim());
}

const steps = LADDER.map((name) => {
const value = declared.get(name);
assert.ok(value, `${name} is not declared on the compact surface`);
const n = spaceSteps(value);
assert.ok(
n !== null,
`${name} is \`${value}\`; the ladder must stay on the --space-* scale so the ` +
'order is comparable and a bare px value cannot drift off the 4px grid',
);
return { name, steps: n };
});

for (let i = 1; i < steps.length; i += 1) {
const prev = steps[i - 1];
const cur = steps[i];
assert.ok(
cur.steps > prev.steps,
`${cur.name} (--space-${cur.steps}) must be strictly wider than ${prev.name} ` +
`(--space-${prev.steps}). Visual distance has to rise with semantic distance: ` +
'same-level list items closer than blocks, blocks closer than section breaks.',
);
}
});

it('expresses every block gap as an adjacent-sibling relation', async () => {
const css = stripCssComments(await readAllRendererCss());
const compactBlockRules = [
...css.matchAll(
/(\[data-maka-contract="markdown"\]\s*\.astryx-markdown\[data-density="compact"\]\s*>[^{]*)\{([^}]*)\}/g,
),
].filter(([, , body]) => /margin-block-start\s*:/.test(body));

assert.ok(compactBlockRules.length > 0, 'no compact block-gap rules found');

for (const [, selector, body] of compactBlockRules) {
assert.match(
selector,
/>[^{]*\+/,
'every block gap must be an adjacent-sibling rule (`> … + …`). A gap is a relation ' +
'between two blocks, so the first block should match no gap rule at all. The ' +
'`> *` + `> :first-child` reset form looks equivalent but loses on specificity: ' +
`:first-child scores (0,4,0) against the heading rules' (0,5,0), so a turn opening ` +
'with a heading keeps a chapter gap above its first line and pushes off the top of ' +
`the bubble. Offending rule: \`${selector.trim()}\` { ${body.trim()} }`,
);
}

assert.doesNotMatch(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*:first-child/,
'the `:first-child` gap reset is back. It cannot beat the heading rules on ' +
'specificity — use adjacent-sibling gap rules so the first block is never matched.',
);

// The other half of "the distance between two blocks is the value declared
// here": without it Astryx's own per-element end margins survive in the
// earlier layer and collapse against these gaps. An `hr` carries 12px in
// compact, so dropping the reset silently widens the block step after one.
assert.match(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\{[^}]*margin-block\s*:\s*0/,
'the blanket `> * { margin-block: 0 }` reset is gone. The gap rules only set ' +
'margin-block-START, so without it Astryx\'s end margins survive and collapse ' +
'against them — the declared ladder stops being the spacing you get.',
);

// Declaration and usage are separate failures: the ladder can stay ordered
// while the rules that spend it are hardcoded, which turns the table into
// decoration and the first test into a tautology.
for (const [, selector, body] of compactBlockRules) {
assert.match(
body,
/margin-block-start\s*:\s*var\(\s*--md-gap-[\w-]+\s*\)/,
'a compact gap rule sets a literal instead of a `--md-gap-*` variable. The ladder ' +
'test above only checks that the variables are declared in order; a hardcoded ' +
`value leaves it green while the rendered spacing ignores it. Rule: \`${selector.trim()}\` ` +
`{ ${body.trim()} }`,
);
}
});

it('keeps a typed `hr` wider than the block step', async () => {
const css = stripCssComments(await readAllRendererCss());
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\+\s*hr\s*,[^{]*\{([^}]*)\}`,
).exec(css);
assert.ok(
rule,
'the compact surface no longer gives `hr` its own step. An `hr` is the only break ' +
'the author typed by hand; left on the generic block gap it reads no wider than ' +
'the paragraph boundary above it, so writing one changes nothing.',
);

const used = /margin-block-start\s*:\s*var\(\s*(--md-gap-[\w-]+)\s*\)/.exec(rule[1])?.[1];
assert.ok(used, `the \`hr\` rule must spend a --md-gap-* variable. Found: { ${rule[1].trim()} }`);

const block = LADDER.indexOf('--md-gap-block');
const rung = LADDER.indexOf(used as (typeof LADDER)[number]);
assert.ok(
rung > block,
`\`hr\` spends ${used}, which is not above --md-gap-block on the ladder. Which rung ` +
'it takes is a design call — section today, chapter would be fine — but it has to ' +
'outrank the ordinary block step or the separator carries no meaning.',
);
});

it('spends the list-item row padding it re-spaces as prose', async () => {
const css = stripCssComments(await readAllRendererCss());
// Astryx's ListItem carries the control-row padding that inverted the
// ladder. Zeroing it is what makes --md-gap-list the whole distance between
// two items; leave it in and the gap variable understates the real spacing.
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list-item\s*\{([^}]*)\}`,
).exec(css);
assert.ok(rule, 'the compact surface no longer neutralizes `.astryx-list-item` padding');
assert.match(
rule[1],
/padding-block\s*:\s*0/,
'ListItem block padding must be zeroed on the compact surface, otherwise the real ' +
'list-item gap is padding + gap and the ladder above is not the spacing you get',
);

const listGap = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list\s*\{([^}]*)\}`,
).exec(css);
assert.ok(listGap, 'the compact surface no longer sets a `.astryx-list` row gap');
assert.match(
listGap[1],
/gap\s*:\s*var\(\s*--md-gap-list\s*\)/,
'the list gap must spend `--md-gap-list`. A literal here detaches the tightest rung ' +
'from the ladder the first test orders, so an inversion could be reintroduced ' +
'without failing anything',
);
});

it('keeps two heading size steps on the compact surface', async () => {
const css = stripCssComments(await readAllRendererCss());
const fonts = [
...css.matchAll(
/\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-markdown-heading[^{]*\{([^}]*)\}/g,
),
]
.map((m) => /font\s*:\s*var\(\s*(--maka-text-heading-\d)\s*\)/.exec(m[1])?.[1])
.filter((tier): tier is string => tier !== undefined);

assert.ok(
new Set(fonts).size >= 2,
'the transcript heading scale collapsed to one size tier. Flattening Astryx\'s ' +
'document ladder is deliberate, but flattening it to ZERO steps is the bug this ' +
'replaced: h2 and h3 then differ in nothing — not size, weight, or colour. ' +
`Found tiers: ${JSON.stringify(fonts)}`,
);
});
});
43 changes: 10 additions & 33 deletions apps/desktop/src/renderer/styles/chat-message.css
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,39 +212,16 @@
independent instead of keyed on a `[data-role="assistant"]` descendant. */

/* --- Conversation heading scale ------------------------------------------- */
/* Astryx's heading ladder (20/18/16/14…) is a DOCUMENT scale, and the
transcript is not a document. An agent turn emits `##` freely, so a ladder
that steps 1.4× per level shreds one reply into visually unrelated slabs.
Flatten it to two steps — h1 for a genuine section break, everything below
at body size — and let weight 600 plus colour carry the hierarchy, which is
what the rest of the conversation already does.
Cursor (1.43/1.32/1.21), Claude Code (1.14/1.07/1.0) and Codex
(1.71/1.43/1.21) all flatten here relative to their own document styles;
Claude Code goes furthest and merges h4–h6 into `strong`. Two steps sits
inside that range.
Every level shares the transcript baseline so headings do not add rhythm of
their own.
Scoped to `.maka-turn`, not to the Markdown contract alone. The contract is
emitted by the shared MarkdownBody, whose other caller is the Daily Review
panel — and a review report IS a document, so the argument above does not
apply to it. It keeps Astryx's full ladder.
Bare tags are the whole story: Astryx's Markdown renderer emits plain
h1–h6 with stylex props (Markdown.js `const Tag = \`h${level}\``). The
.astryx-heading.level-N classes come from the standalone Heading
component, which Markdown never mounts. */
/* The one heading in a turn that is not at the body size, and so the one that
cannot take the body baseline with it: at 16px, --maka-line-body's 20px is
on the 4px grid but is the 14px tier's line box, one step tighter than the
tier this heading actually sits at. It takes its own tier's leading (24px);
the turn still reads as one rhythm because 24 is the next step of the same
grid, not a different one. */
.maka-turn [data-maka-contract="markdown"] h1 {
font: var(--maka-text-heading-3);
}

.maka-turn [data-maka-contract="markdown"] :is(h2, h3, h4, h5, h6) {
font: var(--maka-text-heading-4);
}
/* Moved to the transcript rhythm table in packages/ui/src/styles.css. The
argument for flattening the document ladder is unchanged; what changed is
where it is expressed. These rules keyed on a `.maka-turn` ancestor, which
made the heading scale a function of DOM position: the same MarkdownBody
rendered document-sized headings in the Daily Review and flat ones in a
turn, and no Storybook story could reproduce the transcript without
remembering to wrap one. The rhythm table keys on Astryx's own
`data-density` instead, so the scale follows the `density` prop that already
distinguishes the two surfaces. It also keeps two size steps rather than
one — h2 and h3 used to be identical in size, weight and colour. */

.maka-chat-message pre:not([data-maka-contract="markdown"] pre) {
margin: 0;
Expand Down
141 changes: 141 additions & 0 deletions packages/ui/src/__tests__/markdown-rhythm-dom-contract.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
/**
* The DOM half of the transcript rhythm contract.
*
* The rhythm table in styles.css keys entirely on DOM that Astryx generates at
* runtime — `data-density` on the document root, `astryx-markdown-heading` plus
* `data-level` on headings, `astryx-list`/`astryx-list-item` inside a list.
* None of those names appear in Maka source as literals; they come from
* Astryx's `themeProps()`, whose prefix has a single upstream owner
* (`naming.ts`). So an Astryx bump that renames one of them makes every
* selector in the table silently stop matching, and the spacing regresses to
* the inverted ladder with no failing check anywhere: the CSS contract test
* (apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts) reads the
* stylesheet as text and stays green against DOM it never sees, and the
* `check-dead-css` entries are an allowlist that marks these classes live
* rather than asserting they exist.
*
* This pins the other half: the hooks the stylesheet selects on are really
* emitted. Together the two tests fail on either side of the join.
*/
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { readdir, readFile } from 'node:fs/promises';
import { join, relative, resolve } from 'node:path';
import { renderToStaticMarkup } from 'react-dom/server';
import { MarkdownBody } from '../markdown-body.js';

const UI_SRC = resolve(import.meta.dirname, '..', '..', 'src');

async function tsxFiles(dir: string): Promise<string[]> {
const out: string[] = [];
for (const entry of await readdir(dir, { withFileTypes: true })) {
const path = join(dir, entry.name);
if (entry.isDirectory()) out.push(...(await tsxFiles(path)));
else if (entry.name.endsWith('.tsx') && !entry.name.includes('.test.')) out.push(path);
}
return out;
}

const SAMPLE = ['## Heading two', '', 'A paragraph.', '', '1. First item', '2. Second item'].join('\n');

function compactMarkup(): string {
return renderToStaticMarkup(<MarkdownBody text={SAMPLE} density="compact" />);
}

describe('transcript markdown rhythm — DOM hooks', () => {
it('emits the density attribute the rhythm table scopes on', () => {
const markup = compactMarkup();
// Every rule in the table is prefixed with this, so it is the one hook whose
// two halves — selector prefix and runtime attribute — nothing else joins.
// Rename it and the CSS contract still passes on text it never renders.
assert.match(
markup,
/data-maka-contract="markdown"/,
'the `data-maka-contract="markdown"` wrapper is gone. Every rule in the rhythm ' +
'table is scoped on it, so all compact prose spacing and the heading scale are ' +
'now dead — and the stylesheet-side contract cannot see it.',
);
assert.match(
markup,
/<div[^>]*role="document"[^>]*data-density="compact"|<div[^>]*data-density="compact"[^>]*role="document"/,
'the document root no longer carries `data-density="compact"`. Every rule in the ' +
'rhythm table is scoped on it, so all compact prose spacing is now dead.',
);
assert.match(
markup,
/class="[^"]*\bastryx-markdown\b/,
'the document root no longer carries the `astryx-markdown` class the table selects on',
);
});

it('emits the heading hooks the two-step scale selects on', () => {
const markup = compactMarkup();
assert.match(
markup,
/<h2[^>]*class="[^"]*\bastryx-markdown-heading\b/,
'headings no longer carry `astryx-markdown-heading`; the transcript heading scale is dead',
);
assert.match(
markup,
/<h2[^>]*data-level="2"/,
'headings no longer carry `data-level`. The table splits h1/h2 from h3-h6 on it, so ' +
'without it every heading collapses to one tier — the exact defect this branch replaced.',
);
});

/**
* The rhythm table carries the transcript's heading typography, not just its
* spacing, and keys both on `density`. That is a deliberate bet, and it is a
* bet: Astryx's own density RFC (facebook/astryx#839) draws the line at
* "density shifts heights and spacing, not typography", so the key is
* honest only while `compact` and "transcript" are the same set. They are
* today — the transcript is the only caller that asks for compact, and the
* Daily Review renders a document at the default.
*
* A separate surface attribute would decouple them, but nothing needs it
* yet and it would add a prop to a public component for a caller that does
* not exist. Guard the assumption instead: the moment a second surface asks
* for compact markdown it silently inherits transcript heading sizes and
* dimmed deep headings, and this test is what tells whoever adds it that
* they have to choose — inherit deliberately, or split the key then.
*/
it('keeps compact markdown a transcript-only surface', async () => {
const callers: string[] = [];
for (const file of await tsxFiles(UI_SRC)) {
const source = await readFile(file, 'utf8');
// `<Markdown …>` opening tags only — not MarkdownBody's internal plumbing
// and not the density Maka hands its own code-block renderers.
for (const tag of source.matchAll(/<Markdown\s[^>]*?\/?>/gs)) {
if (/density=(["'])compact\1/.test(tag[0])) callers.push(relative(UI_SRC, file));
}
}

assert.deepEqual(
[...new Set(callers)].sort(),
['chat-turn.tsx'],
'a new caller renders markdown at compact density. The rhythm table treats ' +
'`density="compact"` as "this is a transcript" and gives it flattened heading sizes ' +
'plus secondary-colour h4-h6 — typography, which Astryx\'s density is explicitly not ' +
'supposed to carry (facebook/astryx#839). Either that is what the new surface wants, ' +
'and this list grows, or the typography rules need their own key. ' +
`Found: ${JSON.stringify([...new Set(callers)].sort())}`,
);
});

it('emits the list hooks whose control padding the table neutralizes', () => {
const markup = compactMarkup();
assert.match(
markup,
/class="[^"]*\bastryx-list\b/,
'the markdown list no longer renders through Astryx `List`. If it stopped being a ' +
'control list that is good news, but the padding-zeroing rule is now dead and the ' +
'list rhythm needs re-measuring rather than silently inheriting whatever replaced it.',
);
assert.match(
markup,
/class="[^"]*\bastryx-list-item\b/,
'list rows no longer carry `astryx-list-item`; the rule that spends their control-row ' +
'padding no longer applies and list items revert to sitting wider apart than paragraphs',
);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(ui): restore the transcript's markdown rhythm by Astro-Han · Pull Request #2348 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 199 additions & 0 deletions apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
/**
* Transcript markdown rhythm contract.
*
* The defect this pins was not a wrong number — it was a wrong ORDER. Under
* `density="compact"` the transcript spaced list items ~10px apart (Astryx's
* List control row padding, which `density` cannot reach from outside) while
* paragraphs sat 4px apart, so items at the same level read as further apart
* than separate paragraphs. Visual distance stopped tracking semantic
* distance.
*
* So the invariant is the ladder's ORDER, not its values: retuning 8px to 10px
* is a design decision and should stay green here; making list gaps meet or
* exceed block gaps is the regression, and must fail. A screenshot cannot lock
* that — it fixes one rendering of one sample rather than the relation — which
* is why AGENTS.md asks for a computed-style or text contract on cascade and
* layout invariants. The visual half lives in the `TranscriptTurn` story
* (packages/ui/stories/markdown.stories.tsx).
*
* Scoped to the compact surface only. Document mode is Astryx's own rhythm and
* the Daily Review renders through it, so nothing here should constrain it.
*/
import { strict as assert } from 'node:assert';
import { describe, it } from 'node:test';
import { readAllRendererCss, stripCssComments } from './css-test-helpers.js';

/** The compact ladder, ordered from tightest (same level) to widest (chapter). */
const LADDER = ['--md-gap-list', '--md-gap-block', '--md-gap-section', '--md-gap-chapter'] as const;

/** `var(--space-N)` → N, the 4px-grid step count. Non-scale values return null. */
function spaceSteps(value: string): number | null {
const match = /^var\(\s*--space-(\d+)\s*\)$/.exec(value.trim());
return match ? Number(match[1]) : null;
}

describe('transcript markdown rhythm', () => {
it('declares the compact ladder in strictly increasing order', async () => {
const css = stripCssComments(await readAllRendererCss());
const block = /\.astryx-markdown\[data-density="compact"\]\s*\{([^}]*)\}/.exec(css);
assert.ok(block, 'no `.astryx-markdown[data-density="compact"]` custom-property block found');

const declared = new Map<string, string>();
for (const m of block[1].matchAll(/(--md-gap-[\w-]+)\s*:\s*([^;]+);/g)) {
declared.set(m[1], m[2].trim());
}

const steps = LADDER.map((name) => {
const value = declared.get(name);
assert.ok(value, `${name} is not declared on the compact surface`);
const n = spaceSteps(value);
assert.ok(
n !== null,
`${name} is \`${value}\`; the ladder must stay on the --space-* scale so the ` +
'order is comparable and a bare px value cannot drift off the 4px grid',
);
return { name, steps: n };
});

for (let i = 1; i < steps.length; i += 1) {
const prev = steps[i - 1];
const cur = steps[i];
assert.ok(
cur.steps > prev.steps,
`${cur.name} (--space-${cur.steps}) must be strictly wider than ${prev.name} ` +
`(--space-${prev.steps}). Visual distance has to rise with semantic distance: ` +
'same-level list items closer than blocks, blocks closer than section breaks.',
);
}
});

it('expresses every block gap as an adjacent-sibling relation', async () => {
const css = stripCssComments(await readAllRendererCss());
const compactBlockRules = [
...css.matchAll(
/(\[data-maka-contract="markdown"\]\s*\.astryx-markdown\[data-density="compact"\]\s*>[^{]*)\{([^}]*)\}/g,
),
].filter(([, , body]) => /margin-block-start\s*:/.test(body));

assert.ok(compactBlockRules.length > 0, 'no compact block-gap rules found');

for (const [, selector, body] of compactBlockRules) {
assert.match(
selector,
/>[^{]*\+/,
'every block gap must be an adjacent-sibling rule (`> … + …`). A gap is a relation ' +
'between two blocks, so the first block should match no gap rule at all. The ' +
'`> *` + `> :first-child` reset form looks equivalent but loses on specificity: ' +
`:first-child scores (0,4,0) against the heading rules' (0,5,0), so a turn opening ` +
'with a heading keeps a chapter gap above its first line and pushes off the top of ' +
`the bubble. Offending rule: \`${selector.trim()}\` { ${body.trim()} }`,
);
}

assert.doesNotMatch(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*:first-child/,
'the `:first-child` gap reset is back. It cannot beat the heading rules on ' +
'specificity — use adjacent-sibling gap rules so the first block is never matched.',
);

// The other half of "the distance between two blocks is the value declared
// here": without it Astryx's own per-element end margins survive in the
// earlier layer and collapse against these gaps. An `hr` carries 12px in
// compact, so dropping the reset silently widens the block step after one.
assert.match(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\{[^}]*margin-block\s*:\s*0/,
'the blanket `> * { margin-block: 0 }` reset is gone. The gap rules only set ' +
'margin-block-START, so without it Astryx\'s end margins survive and collapse ' +
'against them — the declared ladder stops being the spacing you get.',
);

// Declaration and usage are separate failures: the ladder can stay ordered
// while the rules that spend it are hardcoded, which turns the table into
// decoration and the first test into a tautology.
for (const [, selector, body] of compactBlockRules) {
assert.match(
body,
/margin-block-start\s*:\s*var\(\s*--md-gap-[\w-]+\s*\)/,
'a compact gap rule sets a literal instead of a `--md-gap-*` variable. The ladder ' +
'test above only checks that the variables are declared in order; a hardcoded ' +
`value leaves it green while the rendered spacing ignores it. Rule: \`${selector.trim()}\` ` +
`{ ${body.trim()} }`,
);
}
});

it('keeps a typed `hr` wider than the block step', async () => {
const css = stripCssComments(await readAllRendererCss());
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\+\s*hr\s*,[^{]*\{([^}]*)\}`,
).exec(css);
assert.ok(
rule,
'the compact surface no longer gives `hr` its own step. An `hr` is the only break ' +
'the author typed by hand; left on the generic block gap it reads no wider than ' +
'the paragraph boundary above it, so writing one changes nothing.',
);

const used = /margin-block-start\s*:\s*var\(\s*(--md-gap-[\w-]+)\s*\)/.exec(rule[1])?.[1];
assert.ok(used, `the \`hr\` rule must spend a --md-gap-* variable. Found: { ${rule[1].trim()} }`);

const block = LADDER.indexOf('--md-gap-block');
const rung = LADDER.indexOf(used as (typeof LADDER)[number]);
assert.ok(
rung > block,
`\`hr\` spends ${used}, which is not above --md-gap-block on the ladder. Which rung ` +
'it takes is a design call — section today, chapter would be fine — but it has to ' +
'outrank the ordinary block step or the separator carries no meaning.',
);
});

it('spends the list-item row padding it re-spaces as prose', async () => {
const css = stripCssComments(await readAllRendererCss());
// Astryx's ListItem carries the control-row padding that inverted the
// ladder. Zeroing it is what makes --md-gap-list the whole distance between
// two items; leave it in and the gap variable understates the real spacing.
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list-item\s*\{([^}]*)\}`,
).exec(css);
assert.ok(rule, 'the compact surface no longer neutralizes `.astryx-list-item` padding');
assert.match(
rule[1],
/padding-block\s*:\s*0/,
'ListItem block padding must be zeroed on the compact surface, otherwise the real ' +
'list-item gap is padding + gap and the ladder above is not the spacing you get',
);

const listGap = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list\s*\{([^}]*)\}`,
).exec(css);
assert.ok(listGap, 'the compact surface no longer sets a `.astryx-list` row gap');
assert.match(
listGap[1],
/gap\s*:\s*var\(\s*--md-gap-list\s*\)/,
'the list gap must spend `--md-gap-list`. A literal here detaches the tightest rung ' +
'from the ladder the first test orders, so an inversion could be reintroduced ' +
'without failing anything',
);
});

it('keeps two heading size steps on the compact surface', async () => {
const css = stripCssComments(await readAllRendererCss());
const fonts = [
...css.matchAll(
/\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-markdown-heading[^{]*\{([^}]*)\}/g,
),
]
.map((m) => /font\s*:\s*var\(\s*(--maka-text-heading-\d)\s*\)/.exec(m[1])?.[1])
.filter((tier): tier is string => tier !== undefined);

assert.ok(
new Set(fonts).size >= 2,
'the transcript heading scale collapsed to one size tier. Flattening Astryx\'s ' +
'document ladder is deliberate, but flattening it to ZERO steps is the bug this ' +
'replaced: h2 and h3 then differ in nothing — not size, weight, or colour. ' +
`Found tiers: ${JSON.stringify(fonts)}`,
);
});
});
43 changes: 10 additions & 33 deletions apps/desktop/src/renderer/styles/chat-message.css
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,39 +212,16 @@
independent instead of keyed on a `[data-role="assistant"]` descendant. */

/* --- Conversation heading scale ------------------------------------------- */
/* Astryx's heading ladder (20/18/16/14…) is a DOCUMENT scale, and the
transcript is not a document. An agent turn emits `##` freely, so a ladder
that steps 1.4× per level shreds one reply into visually unrelated slabs.
Flatten it to two steps — h1 for a genuine section break, everything below
at body size — and let weight 600 plus colour carry the hierarchy, which is
what the rest of the conversation already does.
Cursor (1.43/1.32/1.21), Claude Code (1.14/1.07/1.0) and Codex
(1.71/1.43/1.21) all flatten here relative to their own document styles;
Claude Code goes furthest and merges h4–h6 into `strong`. Two steps sits
inside that range.
Every level shares the transcript baseline so headings do not add rhythm of
their own.
Scoped to `.maka-turn`, not to the Markdown contract alone. The contract is
emitted by the shared MarkdownBody, whose other caller is the Daily Review
panel — and a review report IS a document, so the argument above does not
apply to it. It keeps Astryx's full ladder.
Bare tags are the whole story: Astryx's Markdown renderer emits plain
h1–h6 with stylex props (Markdown.js `const Tag = \`h${level}\``). The
.astryx-heading.level-N classes come from the standalone Heading
component, which Markdown never mounts. */
/* The one heading in a turn that is not at the body size, and so the one that
cannot take the body baseline with it: at 16px, --maka-line-body's 20px is
on the 4px grid but is the 14px tier's line box, one step tighter than the
tier this heading actually sits at. It takes its own tier's leading (24px);
the turn still reads as one rhythm because 24 is the next step of the same
grid, not a different one. */
.maka-turn [data-maka-contract="markdown"] h1 {
font: var(--maka-text-heading-3);
}

.maka-turn [data-maka-contract="markdown"] :is(h2, h3, h4, h5, h6) {
font: var(--maka-text-heading-4);
}
/* Moved to the transcript rhythm table in packages/ui/src/styles.css. The
argument for flattening the document ladder is unchanged; what changed is
where it is expressed. These rules keyed on a `.maka-turn` ancestor, which
made the heading scale a function of DOM position: the same MarkdownBody
rendered document-sized headings in the Daily Review and flat ones in a
turn, and no Storybook story could reproduce the transcript without
remembering to wrap one. The rhythm table keys on Astryx's own
`data-density` instead, so the scale follows the `density` prop that already
distinguishes the two surfaces. It also keeps two size steps rather than
one — h2 and h3 used to be identical in size, weight and colour. */

.maka-chat-message pre:not([data-maka-contract="markdown"] pre) {
margin: 0;
Expand Down
141 changes: 141 additions & 0 deletions packages/ui/src/__tests__/markdown-rhythm-dom-contract.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
/**
* The DOM half of the transcript rhythm contract.
*
* The rhythm table in styles.css keys entirely on DOM that Astryx generates at
* runtime — `data-density` on the document root, `astryx-markdown-heading` plus
* `data-level` on headings, `astryx-list`/`astryx-list-item` inside a list.
* None of those names appear in Maka source as literals; they come from
* Astryx's `themeProps()`, whose prefix has a single upstream owner
* (`naming.ts`). So an Astryx bump that renames one of them makes every
* selector in the table silently stop matching, and the spacing regresses to
* the inverted ladder with no failing check anywhere: the CSS contract test
* (apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts) reads the
* stylesheet as text and stays green against DOM it never sees, and the
* `check-dead-css` entries are an allowlist that marks these classes live
* rather than asserting they exist.
*
* This pins the other half: the hooks the stylesheet selects on are really
* emitted. Together the two tests fail on either side of the join.
*/
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { readdir, readFile } from 'node:fs/promises';
import { join, relative, resolve } from 'node:path';
import { renderToStaticMarkup } from 'react-dom/server';
import { MarkdownBody } from '../markdown-body.js';

const UI_SRC = resolve(import.meta.dirname, '..', '..', 'src');

async function tsxFiles(dir: string): Promise<string[]> {
const out: string[] = [];
for (const entry of await readdir(dir, { withFileTypes: true })) {
const path = join(dir, entry.name);
if (entry.isDirectory()) out.push(...(await tsxFiles(path)));
else if (entry.name.endsWith('.tsx') && !entry.name.includes('.test.')) out.push(path);
}
return out;
}

const SAMPLE = ['## Heading two', '', 'A paragraph.', '', '1. First item', '2. Second item'].join('\n');

function compactMarkup(): string {
return renderToStaticMarkup(<MarkdownBody text={SAMPLE} density="compact" />);
}

describe('transcript markdown rhythm — DOM hooks', () => {
it('emits the density attribute the rhythm table scopes on', () => {
const markup = compactMarkup();
// Every rule in the table is prefixed with this, so it is the one hook whose
// two halves — selector prefix and runtime attribute — nothing else joins.
// Rename it and the CSS contract still passes on text it never renders.
assert.match(
markup,
/data-maka-contract="markdown"/,
'the `data-maka-contract="markdown"` wrapper is gone. Every rule in the rhythm ' +
'table is scoped on it, so all compact prose spacing and the heading scale are ' +
'now dead — and the stylesheet-side contract cannot see it.',
);
assert.match(
markup,
/<div[^>]*role="document"[^>]*data-density="compact"|<div[^>]*data-density="compact"[^>]*role="document"/,
'the document root no longer carries `data-density="compact"`. Every rule in the ' +
'rhythm table is scoped on it, so all compact prose spacing is now dead.',
);
assert.match(
markup,
/class="[^"]*\bastryx-markdown\b/,
'the document root no longer carries the `astryx-markdown` class the table selects on',
);
});

it('emits the heading hooks the two-step scale selects on', () => {
const markup = compactMarkup();
assert.match(
markup,
/<h2[^>]*class="[^"]*\bastryx-markdown-heading\b/,
'headings no longer carry `astryx-markdown-heading`; the transcript heading scale is dead',
);
assert.match(
markup,
/<h2[^>]*data-level="2"/,
'headings no longer carry `data-level`. The table splits h1/h2 from h3-h6 on it, so ' +
'without it every heading collapses to one tier — the exact defect this branch replaced.',
);
});

/**
* The rhythm table carries the transcript's heading typography, not just its
* spacing, and keys both on `density`. That is a deliberate bet, and it is a
* bet: Astryx's own density RFC (facebook/astryx#839) draws the line at
* "density shifts heights and spacing, not typography", so the key is
* honest only while `compact` and "transcript" are the same set. They are
* today — the transcript is the only caller that asks for compact, and the
* Daily Review renders a document at the default.
*
* A separate surface attribute would decouple them, but nothing needs it
* yet and it would add a prop to a public component for a caller that does
* not exist. Guard the assumption instead: the moment a second surface asks
* for compact markdown it silently inherits transcript heading sizes and
* dimmed deep headings, and this test is what tells whoever adds it that
* they have to choose — inherit deliberately, or split the key then.
*/
it('keeps compact markdown a transcript-only surface', async () => {
const callers: string[] = [];
for (const file of await tsxFiles(UI_SRC)) {
const source = await readFile(file, 'utf8');
// `<Markdown …>` opening tags only — not MarkdownBody's internal plumbing
// and not the density Maka hands its own code-block renderers.
for (const tag of source.matchAll(/<Markdown\s[^>]*?\/?>/gs)) {
if (/density=(["'])compact\1/.test(tag[0])) callers.push(relative(UI_SRC, file));
}
}

assert.deepEqual(
[...new Set(callers)].sort(),
['chat-turn.tsx'],
'a new caller renders markdown at compact density. The rhythm table treats ' +
'`density="compact"` as "this is a transcript" and gives it flattened heading sizes ' +
'plus secondary-colour h4-h6 — typography, which Astryx\'s density is explicitly not ' +
'supposed to carry (facebook/astryx#839). Either that is what the new surface wants, ' +
'and this list grows, or the typography rules need their own key. ' +
`Found: ${JSON.stringify([...new Set(callers)].sort())}`,
);
});

it('emits the list hooks whose control padding the table neutralizes', () => {
const markup = compactMarkup();
assert.match(
markup,
/class="[^"]*\bastryx-list\b/,
'the markdown list no longer renders through Astryx `List`. If it stopped being a ' +
'control list that is good news, but the padding-zeroing rule is now dead and the ' +
'list rhythm needs re-measuring rather than silently inheriting whatever replaced it.',
);
assert.match(
markup,
/class="[^"]*\bastryx-list-item\b/,
'list rows no longer carry `astryx-list-item`; the rule that spends their control-row ' +
'padding no longer applies and list items revert to sitting wider apart than paragraphs',
);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(ui): restore the transcript's markdown rhythm by Astro-Han · Pull Request #2348 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 199 additions & 0 deletions apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
/**
* Transcript markdown rhythm contract.
*
* The defect this pins was not a wrong number — it was a wrong ORDER. Under
* `density="compact"` the transcript spaced list items ~10px apart (Astryx's
* List control row padding, which `density` cannot reach from outside) while
* paragraphs sat 4px apart, so items at the same level read as further apart
* than separate paragraphs. Visual distance stopped tracking semantic
* distance.
*
* So the invariant is the ladder's ORDER, not its values: retuning 8px to 10px
* is a design decision and should stay green here; making list gaps meet or
* exceed block gaps is the regression, and must fail. A screenshot cannot lock
* that — it fixes one rendering of one sample rather than the relation — which
* is why AGENTS.md asks for a computed-style or text contract on cascade and
* layout invariants. The visual half lives in the `TranscriptTurn` story
* (packages/ui/stories/markdown.stories.tsx).
*
* Scoped to the compact surface only. Document mode is Astryx's own rhythm and
* the Daily Review renders through it, so nothing here should constrain it.
*/
import { strict as assert } from 'node:assert';
import { describe, it } from 'node:test';
import { readAllRendererCss, stripCssComments } from './css-test-helpers.js';

/** The compact ladder, ordered from tightest (same level) to widest (chapter). */
const LADDER = ['--md-gap-list', '--md-gap-block', '--md-gap-section', '--md-gap-chapter'] as const;

/** `var(--space-N)` → N, the 4px-grid step count. Non-scale values return null. */
function spaceSteps(value: string): number | null {
const match = /^var\(\s*--space-(\d+)\s*\)$/.exec(value.trim());
return match ? Number(match[1]) : null;
}

describe('transcript markdown rhythm', () => {
it('declares the compact ladder in strictly increasing order', async () => {
const css = stripCssComments(await readAllRendererCss());
const block = /\.astryx-markdown\[data-density="compact"\]\s*\{([^}]*)\}/.exec(css);
assert.ok(block, 'no `.astryx-markdown[data-density="compact"]` custom-property block found');

const declared = new Map<string, string>();
for (const m of block[1].matchAll(/(--md-gap-[\w-]+)\s*:\s*([^;]+);/g)) {
declared.set(m[1], m[2].trim());
}

const steps = LADDER.map((name) => {
const value = declared.get(name);
assert.ok(value, `${name} is not declared on the compact surface`);
const n = spaceSteps(value);
assert.ok(
n !== null,
`${name} is \`${value}\`; the ladder must stay on the --space-* scale so the ` +
'order is comparable and a bare px value cannot drift off the 4px grid',
);
return { name, steps: n };
});

for (let i = 1; i < steps.length; i += 1) {
const prev = steps[i - 1];
const cur = steps[i];
assert.ok(
cur.steps > prev.steps,
`${cur.name} (--space-${cur.steps}) must be strictly wider than ${prev.name} ` +
`(--space-${prev.steps}). Visual distance has to rise with semantic distance: ` +
'same-level list items closer than blocks, blocks closer than section breaks.',
);
}
});

it('expresses every block gap as an adjacent-sibling relation', async () => {
const css = stripCssComments(await readAllRendererCss());
const compactBlockRules = [
...css.matchAll(
/(\[data-maka-contract="markdown"\]\s*\.astryx-markdown\[data-density="compact"\]\s*>[^{]*)\{([^}]*)\}/g,
),
].filter(([, , body]) => /margin-block-start\s*:/.test(body));

assert.ok(compactBlockRules.length > 0, 'no compact block-gap rules found');

for (const [, selector, body] of compactBlockRules) {
assert.match(
selector,
/>[^{]*\+/,
'every block gap must be an adjacent-sibling rule (`> … + …`). A gap is a relation ' +
'between two blocks, so the first block should match no gap rule at all. The ' +
'`> *` + `> :first-child` reset form looks equivalent but loses on specificity: ' +
`:first-child scores (0,4,0) against the heading rules' (0,5,0), so a turn opening ` +
'with a heading keeps a chapter gap above its first line and pushes off the top of ' +
`the bubble. Offending rule: \`${selector.trim()}\` { ${body.trim()} }`,
);
}

assert.doesNotMatch(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*:first-child/,
'the `:first-child` gap reset is back. It cannot beat the heading rules on ' +
'specificity — use adjacent-sibling gap rules so the first block is never matched.',
);

// The other half of "the distance between two blocks is the value declared
// here": without it Astryx's own per-element end margins survive in the
// earlier layer and collapse against these gaps. An `hr` carries 12px in
// compact, so dropping the reset silently widens the block step after one.
assert.match(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\{[^}]*margin-block\s*:\s*0/,
'the blanket `> * { margin-block: 0 }` reset is gone. The gap rules only set ' +
'margin-block-START, so without it Astryx\'s end margins survive and collapse ' +
'against them — the declared ladder stops being the spacing you get.',
);

// Declaration and usage are separate failures: the ladder can stay ordered
// while the rules that spend it are hardcoded, which turns the table into
// decoration and the first test into a tautology.
for (const [, selector, body] of compactBlockRules) {
assert.match(
body,
/margin-block-start\s*:\s*var\(\s*--md-gap-[\w-]+\s*\)/,
'a compact gap rule sets a literal instead of a `--md-gap-*` variable. The ladder ' +
'test above only checks that the variables are declared in order; a hardcoded ' +
`value leaves it green while the rendered spacing ignores it. Rule: \`${selector.trim()}\` ` +
`{ ${body.trim()} }`,
);
}
});

it('keeps a typed `hr` wider than the block step', async () => {
const css = stripCssComments(await readAllRendererCss());
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\+\s*hr\s*,[^{]*\{([^}]*)\}`,
).exec(css);
assert.ok(
rule,
'the compact surface no longer gives `hr` its own step. An `hr` is the only break ' +
'the author typed by hand; left on the generic block gap it reads no wider than ' +
'the paragraph boundary above it, so writing one changes nothing.',
);

const used = /margin-block-start\s*:\s*var\(\s*(--md-gap-[\w-]+)\s*\)/.exec(rule[1])?.[1];
assert.ok(used, `the \`hr\` rule must spend a --md-gap-* variable. Found: { ${rule[1].trim()} }`);

const block = LADDER.indexOf('--md-gap-block');
const rung = LADDER.indexOf(used as (typeof LADDER)[number]);
assert.ok(
rung > block,
`\`hr\` spends ${used}, which is not above --md-gap-block on the ladder. Which rung ` +
'it takes is a design call — section today, chapter would be fine — but it has to ' +
'outrank the ordinary block step or the separator carries no meaning.',
);
});

it('spends the list-item row padding it re-spaces as prose', async () => {
const css = stripCssComments(await readAllRendererCss());
// Astryx's ListItem carries the control-row padding that inverted the
// ladder. Zeroing it is what makes --md-gap-list the whole distance between
// two items; leave it in and the gap variable understates the real spacing.
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list-item\s*\{([^}]*)\}`,
).exec(css);
assert.ok(rule, 'the compact surface no longer neutralizes `.astryx-list-item` padding');
assert.match(
rule[1],
/padding-block\s*:\s*0/,
'ListItem block padding must be zeroed on the compact surface, otherwise the real ' +
'list-item gap is padding + gap and the ladder above is not the spacing you get',
);

const listGap = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list\s*\{([^}]*)\}`,
).exec(css);
assert.ok(listGap, 'the compact surface no longer sets a `.astryx-list` row gap');
assert.match(
listGap[1],
/gap\s*:\s*var\(\s*--md-gap-list\s*\)/,
'the list gap must spend `--md-gap-list`. A literal here detaches the tightest rung ' +
'from the ladder the first test orders, so an inversion could be reintroduced ' +
'without failing anything',
);
});

it('keeps two heading size steps on the compact surface', async () => {
const css = stripCssComments(await readAllRendererCss());
const fonts = [
...css.matchAll(
/\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-markdown-heading[^{]*\{([^}]*)\}/g,
),
]
.map((m) => /font\s*:\s*var\(\s*(--maka-text-heading-\d)\s*\)/.exec(m[1])?.[1])
.filter((tier): tier is string => tier !== undefined);

assert.ok(
new Set(fonts).size >= 2,
'the transcript heading scale collapsed to one size tier. Flattening Astryx\'s ' +
'document ladder is deliberate, but flattening it to ZERO steps is the bug this ' +
'replaced: h2 and h3 then differ in nothing — not size, weight, or colour. ' +
`Found tiers: ${JSON.stringify(fonts)}`,
);
});
});
43 changes: 10 additions & 33 deletions apps/desktop/src/renderer/styles/chat-message.css
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,39 +212,16 @@
independent instead of keyed on a `[data-role="assistant"]` descendant. */

/* --- Conversation heading scale ------------------------------------------- */
/* Astryx's heading ladder (20/18/16/14…) is a DOCUMENT scale, and the
transcript is not a document. An agent turn emits `##` freely, so a ladder
that steps 1.4× per level shreds one reply into visually unrelated slabs.
Flatten it to two steps — h1 for a genuine section break, everything below
at body size — and let weight 600 plus colour carry the hierarchy, which is
what the rest of the conversation already does.
Cursor (1.43/1.32/1.21), Claude Code (1.14/1.07/1.0) and Codex
(1.71/1.43/1.21) all flatten here relative to their own document styles;
Claude Code goes furthest and merges h4–h6 into `strong`. Two steps sits
inside that range.
Every level shares the transcript baseline so headings do not add rhythm of
their own.
Scoped to `.maka-turn`, not to the Markdown contract alone. The contract is
emitted by the shared MarkdownBody, whose other caller is the Daily Review
panel — and a review report IS a document, so the argument above does not
apply to it. It keeps Astryx's full ladder.
Bare tags are the whole story: Astryx's Markdown renderer emits plain
h1–h6 with stylex props (Markdown.js `const Tag = \`h${level}\``). The
.astryx-heading.level-N classes come from the standalone Heading
component, which Markdown never mounts. */
/* The one heading in a turn that is not at the body size, and so the one that
cannot take the body baseline with it: at 16px, --maka-line-body's 20px is
on the 4px grid but is the 14px tier's line box, one step tighter than the
tier this heading actually sits at. It takes its own tier's leading (24px);
the turn still reads as one rhythm because 24 is the next step of the same
grid, not a different one. */
.maka-turn [data-maka-contract="markdown"] h1 {
font: var(--maka-text-heading-3);
}

.maka-turn [data-maka-contract="markdown"] :is(h2, h3, h4, h5, h6) {
font: var(--maka-text-heading-4);
}
/* Moved to the transcript rhythm table in packages/ui/src/styles.css. The
argument for flattening the document ladder is unchanged; what changed is
where it is expressed. These rules keyed on a `.maka-turn` ancestor, which
made the heading scale a function of DOM position: the same MarkdownBody
rendered document-sized headings in the Daily Review and flat ones in a
turn, and no Storybook story could reproduce the transcript without
remembering to wrap one. The rhythm table keys on Astryx's own
`data-density` instead, so the scale follows the `density` prop that already
distinguishes the two surfaces. It also keeps two size steps rather than
one — h2 and h3 used to be identical in size, weight and colour. */

.maka-chat-message pre:not([data-maka-contract="markdown"] pre) {
margin: 0;
Expand Down
141 changes: 141 additions & 0 deletions packages/ui/src/__tests__/markdown-rhythm-dom-contract.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
/**
* The DOM half of the transcript rhythm contract.
*
* The rhythm table in styles.css keys entirely on DOM that Astryx generates at
* runtime — `data-density` on the document root, `astryx-markdown-heading` plus
* `data-level` on headings, `astryx-list`/`astryx-list-item` inside a list.
* None of those names appear in Maka source as literals; they come from
* Astryx's `themeProps()`, whose prefix has a single upstream owner
* (`naming.ts`). So an Astryx bump that renames one of them makes every
* selector in the table silently stop matching, and the spacing regresses to
* the inverted ladder with no failing check anywhere: the CSS contract test
* (apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts) reads the
* stylesheet as text and stays green against DOM it never sees, and the
* `check-dead-css` entries are an allowlist that marks these classes live
* rather than asserting they exist.
*
* This pins the other half: the hooks the stylesheet selects on are really
* emitted. Together the two tests fail on either side of the join.
*/
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { readdir, readFile } from 'node:fs/promises';
import { join, relative, resolve } from 'node:path';
import { renderToStaticMarkup } from 'react-dom/server';
import { MarkdownBody } from '../markdown-body.js';

const UI_SRC = resolve(import.meta.dirname, '..', '..', 'src');

async function tsxFiles(dir: string): Promise<string[]> {
const out: string[] = [];
for (const entry of await readdir(dir, { withFileTypes: true })) {
const path = join(dir, entry.name);
if (entry.isDirectory()) out.push(...(await tsxFiles(path)));
else if (entry.name.endsWith('.tsx') && !entry.name.includes('.test.')) out.push(path);
}
return out;
}

const SAMPLE = ['## Heading two', '', 'A paragraph.', '', '1. First item', '2. Second item'].join('\n');

function compactMarkup(): string {
return renderToStaticMarkup(<MarkdownBody text={SAMPLE} density="compact" />);
}

describe('transcript markdown rhythm — DOM hooks', () => {
it('emits the density attribute the rhythm table scopes on', () => {
const markup = compactMarkup();
// Every rule in the table is prefixed with this, so it is the one hook whose
// two halves — selector prefix and runtime attribute — nothing else joins.
// Rename it and the CSS contract still passes on text it never renders.
assert.match(
markup,
/data-maka-contract="markdown"/,
'the `data-maka-contract="markdown"` wrapper is gone. Every rule in the rhythm ' +
'table is scoped on it, so all compact prose spacing and the heading scale are ' +
'now dead — and the stylesheet-side contract cannot see it.',
);
assert.match(
markup,
/<div[^>]*role="document"[^>]*data-density="compact"|<div[^>]*data-density="compact"[^>]*role="document"/,
'the document root no longer carries `data-density="compact"`. Every rule in the ' +
'rhythm table is scoped on it, so all compact prose spacing is now dead.',
);
assert.match(
markup,
/class="[^"]*\bastryx-markdown\b/,
'the document root no longer carries the `astryx-markdown` class the table selects on',
);
});

it('emits the heading hooks the two-step scale selects on', () => {
const markup = compactMarkup();
assert.match(
markup,
/<h2[^>]*class="[^"]*\bastryx-markdown-heading\b/,
'headings no longer carry `astryx-markdown-heading`; the transcript heading scale is dead',
);
assert.match(
markup,
/<h2[^>]*data-level="2"/,
'headings no longer carry `data-level`. The table splits h1/h2 from h3-h6 on it, so ' +
'without it every heading collapses to one tier — the exact defect this branch replaced.',
);
});

/**
* The rhythm table carries the transcript's heading typography, not just its
* spacing, and keys both on `density`. That is a deliberate bet, and it is a
* bet: Astryx's own density RFC (facebook/astryx#839) draws the line at
* "density shifts heights and spacing, not typography", so the key is
* honest only while `compact` and "transcript" are the same set. They are
* today — the transcript is the only caller that asks for compact, and the
* Daily Review renders a document at the default.
*
* A separate surface attribute would decouple them, but nothing needs it
* yet and it would add a prop to a public component for a caller that does
* not exist. Guard the assumption instead: the moment a second surface asks
* for compact markdown it silently inherits transcript heading sizes and
* dimmed deep headings, and this test is what tells whoever adds it that
* they have to choose — inherit deliberately, or split the key then.
*/
it('keeps compact markdown a transcript-only surface', async () => {
const callers: string[] = [];
for (const file of await tsxFiles(UI_SRC)) {
const source = await readFile(file, 'utf8');
// `<Markdown …>` opening tags only — not MarkdownBody's internal plumbing
// and not the density Maka hands its own code-block renderers.
for (const tag of source.matchAll(/<Markdown\s[^>]*?\/?>/gs)) {
if (/density=(["'])compact\1/.test(tag[0])) callers.push(relative(UI_SRC, file));
}
}

assert.deepEqual(
[...new Set(callers)].sort(),
['chat-turn.tsx'],
'a new caller renders markdown at compact density. The rhythm table treats ' +
'`density="compact"` as "this is a transcript" and gives it flattened heading sizes ' +
'plus secondary-colour h4-h6 — typography, which Astryx\'s density is explicitly not ' +
'supposed to carry (facebook/astryx#839). Either that is what the new surface wants, ' +
'and this list grows, or the typography rules need their own key. ' +
`Found: ${JSON.stringify([...new Set(callers)].sort())}`,
);
});

it('emits the list hooks whose control padding the table neutralizes', () => {
const markup = compactMarkup();
assert.match(
markup,
/class="[^"]*\bastryx-list\b/,
'the markdown list no longer renders through Astryx `List`. If it stopped being a ' +
'control list that is good news, but the padding-zeroing rule is now dead and the ' +
'list rhythm needs re-measuring rather than silently inheriting whatever replaced it.',
);
assert.match(
markup,
/class="[^"]*\bastryx-list-item\b/,
'list rows no longer carry `astryx-list-item`; the rule that spends their control-row ' +
'padding no longer applies and list items revert to sitting wider apart than paragraphs',
);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(ui): restore the transcript's markdown rhythm by Astro-Han · Pull Request #2348 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 199 additions & 0 deletions apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
/**
* Transcript markdown rhythm contract.
*
* The defect this pins was not a wrong number — it was a wrong ORDER. Under
* `density="compact"` the transcript spaced list items ~10px apart (Astryx's
* List control row padding, which `density` cannot reach from outside) while
* paragraphs sat 4px apart, so items at the same level read as further apart
* than separate paragraphs. Visual distance stopped tracking semantic
* distance.
*
* So the invariant is the ladder's ORDER, not its values: retuning 8px to 10px
* is a design decision and should stay green here; making list gaps meet or
* exceed block gaps is the regression, and must fail. A screenshot cannot lock
* that — it fixes one rendering of one sample rather than the relation — which
* is why AGENTS.md asks for a computed-style or text contract on cascade and
* layout invariants. The visual half lives in the `TranscriptTurn` story
* (packages/ui/stories/markdown.stories.tsx).
*
* Scoped to the compact surface only. Document mode is Astryx's own rhythm and
* the Daily Review renders through it, so nothing here should constrain it.
*/
import { strict as assert } from 'node:assert';
import { describe, it } from 'node:test';
import { readAllRendererCss, stripCssComments } from './css-test-helpers.js';

/** The compact ladder, ordered from tightest (same level) to widest (chapter). */
const LADDER = ['--md-gap-list', '--md-gap-block', '--md-gap-section', '--md-gap-chapter'] as const;

/** `var(--space-N)` → N, the 4px-grid step count. Non-scale values return null. */
function spaceSteps(value: string): number | null {
const match = /^var\(\s*--space-(\d+)\s*\)$/.exec(value.trim());
return match ? Number(match[1]) : null;
}

describe('transcript markdown rhythm', () => {
it('declares the compact ladder in strictly increasing order', async () => {
const css = stripCssComments(await readAllRendererCss());
const block = /\.astryx-markdown\[data-density="compact"\]\s*\{([^}]*)\}/.exec(css);
assert.ok(block, 'no `.astryx-markdown[data-density="compact"]` custom-property block found');

const declared = new Map<string, string>();
for (const m of block[1].matchAll(/(--md-gap-[\w-]+)\s*:\s*([^;]+);/g)) {
declared.set(m[1], m[2].trim());
}

const steps = LADDER.map((name) => {
const value = declared.get(name);
assert.ok(value, `${name} is not declared on the compact surface`);
const n = spaceSteps(value);
assert.ok(
n !== null,
`${name} is \`${value}\`; the ladder must stay on the --space-* scale so the ` +
'order is comparable and a bare px value cannot drift off the 4px grid',
);
return { name, steps: n };
});

for (let i = 1; i < steps.length; i += 1) {
const prev = steps[i - 1];
const cur = steps[i];
assert.ok(
cur.steps > prev.steps,
`${cur.name} (--space-${cur.steps}) must be strictly wider than ${prev.name} ` +
`(--space-${prev.steps}). Visual distance has to rise with semantic distance: ` +
'same-level list items closer than blocks, blocks closer than section breaks.',
);
}
});

it('expresses every block gap as an adjacent-sibling relation', async () => {
const css = stripCssComments(await readAllRendererCss());
const compactBlockRules = [
...css.matchAll(
/(\[data-maka-contract="markdown"\]\s*\.astryx-markdown\[data-density="compact"\]\s*>[^{]*)\{([^}]*)\}/g,
),
].filter(([, , body]) => /margin-block-start\s*:/.test(body));

assert.ok(compactBlockRules.length > 0, 'no compact block-gap rules found');

for (const [, selector, body] of compactBlockRules) {
assert.match(
selector,
/>[^{]*\+/,
'every block gap must be an adjacent-sibling rule (`> … + …`). A gap is a relation ' +
'between two blocks, so the first block should match no gap rule at all. The ' +
'`> *` + `> :first-child` reset form looks equivalent but loses on specificity: ' +
`:first-child scores (0,4,0) against the heading rules' (0,5,0), so a turn opening ` +
'with a heading keeps a chapter gap above its first line and pushes off the top of ' +
`the bubble. Offending rule: \`${selector.trim()}\` { ${body.trim()} }`,
);
}

assert.doesNotMatch(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*:first-child/,
'the `:first-child` gap reset is back. It cannot beat the heading rules on ' +
'specificity — use adjacent-sibling gap rules so the first block is never matched.',
);

// The other half of "the distance between two blocks is the value declared
// here": without it Astryx's own per-element end margins survive in the
// earlier layer and collapse against these gaps. An `hr` carries 12px in
// compact, so dropping the reset silently widens the block step after one.
assert.match(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\{[^}]*margin-block\s*:\s*0/,
'the blanket `> * { margin-block: 0 }` reset is gone. The gap rules only set ' +
'margin-block-START, so without it Astryx\'s end margins survive and collapse ' +
'against them — the declared ladder stops being the spacing you get.',
);

// Declaration and usage are separate failures: the ladder can stay ordered
// while the rules that spend it are hardcoded, which turns the table into
// decoration and the first test into a tautology.
for (const [, selector, body] of compactBlockRules) {
assert.match(
body,
/margin-block-start\s*:\s*var\(\s*--md-gap-[\w-]+\s*\)/,
'a compact gap rule sets a literal instead of a `--md-gap-*` variable. The ladder ' +
'test above only checks that the variables are declared in order; a hardcoded ' +
`value leaves it green while the rendered spacing ignores it. Rule: \`${selector.trim()}\` ` +
`{ ${body.trim()} }`,
);
}
});

it('keeps a typed `hr` wider than the block step', async () => {
const css = stripCssComments(await readAllRendererCss());
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\+\s*hr\s*,[^{]*\{([^}]*)\}`,
).exec(css);
assert.ok(
rule,
'the compact surface no longer gives `hr` its own step. An `hr` is the only break ' +
'the author typed by hand; left on the generic block gap it reads no wider than ' +
'the paragraph boundary above it, so writing one changes nothing.',
);

const used = /margin-block-start\s*:\s*var\(\s*(--md-gap-[\w-]+)\s*\)/.exec(rule[1])?.[1];
assert.ok(used, `the \`hr\` rule must spend a --md-gap-* variable. Found: { ${rule[1].trim()} }`);

const block = LADDER.indexOf('--md-gap-block');
const rung = LADDER.indexOf(used as (typeof LADDER)[number]);
assert.ok(
rung > block,
`\`hr\` spends ${used}, which is not above --md-gap-block on the ladder. Which rung ` +
'it takes is a design call — section today, chapter would be fine — but it has to ' +
'outrank the ordinary block step or the separator carries no meaning.',
);
});

it('spends the list-item row padding it re-spaces as prose', async () => {
const css = stripCssComments(await readAllRendererCss());
// Astryx's ListItem carries the control-row padding that inverted the
// ladder. Zeroing it is what makes --md-gap-list the whole distance between
// two items; leave it in and the gap variable understates the real spacing.
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list-item\s*\{([^}]*)\}`,
).exec(css);
assert.ok(rule, 'the compact surface no longer neutralizes `.astryx-list-item` padding');
assert.match(
rule[1],
/padding-block\s*:\s*0/,
'ListItem block padding must be zeroed on the compact surface, otherwise the real ' +
'list-item gap is padding + gap and the ladder above is not the spacing you get',
);

const listGap = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list\s*\{([^}]*)\}`,
).exec(css);
assert.ok(listGap, 'the compact surface no longer sets a `.astryx-list` row gap');
assert.match(
listGap[1],
/gap\s*:\s*var\(\s*--md-gap-list\s*\)/,
'the list gap must spend `--md-gap-list`. A literal here detaches the tightest rung ' +
'from the ladder the first test orders, so an inversion could be reintroduced ' +
'without failing anything',
);
});

it('keeps two heading size steps on the compact surface', async () => {
const css = stripCssComments(await readAllRendererCss());
const fonts = [
...css.matchAll(
/\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-markdown-heading[^{]*\{([^}]*)\}/g,
),
]
.map((m) => /font\s*:\s*var\(\s*(--maka-text-heading-\d)\s*\)/.exec(m[1])?.[1])
.filter((tier): tier is string => tier !== undefined);

assert.ok(
new Set(fonts).size >= 2,
'the transcript heading scale collapsed to one size tier. Flattening Astryx\'s ' +
'document ladder is deliberate, but flattening it to ZERO steps is the bug this ' +
'replaced: h2 and h3 then differ in nothing — not size, weight, or colour. ' +
`Found tiers: ${JSON.stringify(fonts)}`,
);
});
});
43 changes: 10 additions & 33 deletions apps/desktop/src/renderer/styles/chat-message.css
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,39 +212,16 @@
independent instead of keyed on a `[data-role="assistant"]` descendant. */

/* --- Conversation heading scale ------------------------------------------- */
/* Astryx's heading ladder (20/18/16/14…) is a DOCUMENT scale, and the
transcript is not a document. An agent turn emits `##` freely, so a ladder
that steps 1.4× per level shreds one reply into visually unrelated slabs.
Flatten it to two steps — h1 for a genuine section break, everything below
at body size — and let weight 600 plus colour carry the hierarchy, which is
what the rest of the conversation already does.
Cursor (1.43/1.32/1.21), Claude Code (1.14/1.07/1.0) and Codex
(1.71/1.43/1.21) all flatten here relative to their own document styles;
Claude Code goes furthest and merges h4–h6 into `strong`. Two steps sits
inside that range.
Every level shares the transcript baseline so headings do not add rhythm of
their own.
Scoped to `.maka-turn`, not to the Markdown contract alone. The contract is
emitted by the shared MarkdownBody, whose other caller is the Daily Review
panel — and a review report IS a document, so the argument above does not
apply to it. It keeps Astryx's full ladder.
Bare tags are the whole story: Astryx's Markdown renderer emits plain
h1–h6 with stylex props (Markdown.js `const Tag = \`h${level}\``). The
.astryx-heading.level-N classes come from the standalone Heading
component, which Markdown never mounts. */
/* The one heading in a turn that is not at the body size, and so the one that
cannot take the body baseline with it: at 16px, --maka-line-body's 20px is
on the 4px grid but is the 14px tier's line box, one step tighter than the
tier this heading actually sits at. It takes its own tier's leading (24px);
the turn still reads as one rhythm because 24 is the next step of the same
grid, not a different one. */
.maka-turn [data-maka-contract="markdown"] h1 {
font: var(--maka-text-heading-3);
}

.maka-turn [data-maka-contract="markdown"] :is(h2, h3, h4, h5, h6) {
font: var(--maka-text-heading-4);
}
/* Moved to the transcript rhythm table in packages/ui/src/styles.css. The
argument for flattening the document ladder is unchanged; what changed is
where it is expressed. These rules keyed on a `.maka-turn` ancestor, which
made the heading scale a function of DOM position: the same MarkdownBody
rendered document-sized headings in the Daily Review and flat ones in a
turn, and no Storybook story could reproduce the transcript without
remembering to wrap one. The rhythm table keys on Astryx's own
`data-density` instead, so the scale follows the `density` prop that already
distinguishes the two surfaces. It also keeps two size steps rather than
one — h2 and h3 used to be identical in size, weight and colour. */

.maka-chat-message pre:not([data-maka-contract="markdown"] pre) {
margin: 0;
Expand Down
141 changes: 141 additions & 0 deletions packages/ui/src/__tests__/markdown-rhythm-dom-contract.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
/**
* The DOM half of the transcript rhythm contract.
*
* The rhythm table in styles.css keys entirely on DOM that Astryx generates at
* runtime — `data-density` on the document root, `astryx-markdown-heading` plus
* `data-level` on headings, `astryx-list`/`astryx-list-item` inside a list.
* None of those names appear in Maka source as literals; they come from
* Astryx's `themeProps()`, whose prefix has a single upstream owner
* (`naming.ts`). So an Astryx bump that renames one of them makes every
* selector in the table silently stop matching, and the spacing regresses to
* the inverted ladder with no failing check anywhere: the CSS contract test
* (apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts) reads the
* stylesheet as text and stays green against DOM it never sees, and the
* `check-dead-css` entries are an allowlist that marks these classes live
* rather than asserting they exist.
*
* This pins the other half: the hooks the stylesheet selects on are really
* emitted. Together the two tests fail on either side of the join.
*/
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { readdir, readFile } from 'node:fs/promises';
import { join, relative, resolve } from 'node:path';
import { renderToStaticMarkup } from 'react-dom/server';
import { MarkdownBody } from '../markdown-body.js';

const UI_SRC = resolve(import.meta.dirname, '..', '..', 'src');

async function tsxFiles(dir: string): Promise<string[]> {
const out: string[] = [];
for (const entry of await readdir(dir, { withFileTypes: true })) {
const path = join(dir, entry.name);
if (entry.isDirectory()) out.push(...(await tsxFiles(path)));
else if (entry.name.endsWith('.tsx') && !entry.name.includes('.test.')) out.push(path);
}
return out;
}

const SAMPLE = ['## Heading two', '', 'A paragraph.', '', '1. First item', '2. Second item'].join('\n');

function compactMarkup(): string {
return renderToStaticMarkup(<MarkdownBody text={SAMPLE} density="compact" />);
}

describe('transcript markdown rhythm — DOM hooks', () => {
it('emits the density attribute the rhythm table scopes on', () => {
const markup = compactMarkup();
// Every rule in the table is prefixed with this, so it is the one hook whose
// two halves — selector prefix and runtime attribute — nothing else joins.
// Rename it and the CSS contract still passes on text it never renders.
assert.match(
markup,
/data-maka-contract="markdown"/,
'the `data-maka-contract="markdown"` wrapper is gone. Every rule in the rhythm ' +
'table is scoped on it, so all compact prose spacing and the heading scale are ' +
'now dead — and the stylesheet-side contract cannot see it.',
);
assert.match(
markup,
/<div[^>]*role="document"[^>]*data-density="compact"|<div[^>]*data-density="compact"[^>]*role="document"/,
'the document root no longer carries `data-density="compact"`. Every rule in the ' +
'rhythm table is scoped on it, so all compact prose spacing is now dead.',
);
assert.match(
markup,
/class="[^"]*\bastryx-markdown\b/,
'the document root no longer carries the `astryx-markdown` class the table selects on',
);
});

it('emits the heading hooks the two-step scale selects on', () => {
const markup = compactMarkup();
assert.match(
markup,
/<h2[^>]*class="[^"]*\bastryx-markdown-heading\b/,
'headings no longer carry `astryx-markdown-heading`; the transcript heading scale is dead',
);
assert.match(
markup,
/<h2[^>]*data-level="2"/,
'headings no longer carry `data-level`. The table splits h1/h2 from h3-h6 on it, so ' +
'without it every heading collapses to one tier — the exact defect this branch replaced.',
);
});

/**
* The rhythm table carries the transcript's heading typography, not just its
* spacing, and keys both on `density`. That is a deliberate bet, and it is a
* bet: Astryx's own density RFC (facebook/astryx#839) draws the line at
* "density shifts heights and spacing, not typography", so the key is
* honest only while `compact` and "transcript" are the same set. They are
* today — the transcript is the only caller that asks for compact, and the
* Daily Review renders a document at the default.
*
* A separate surface attribute would decouple them, but nothing needs it
* yet and it would add a prop to a public component for a caller that does
* not exist. Guard the assumption instead: the moment a second surface asks
* for compact markdown it silently inherits transcript heading sizes and
* dimmed deep headings, and this test is what tells whoever adds it that
* they have to choose — inherit deliberately, or split the key then.
*/
it('keeps compact markdown a transcript-only surface', async () => {
const callers: string[] = [];
for (const file of await tsxFiles(UI_SRC)) {
const source = await readFile(file, 'utf8');
// `<Markdown …>` opening tags only — not MarkdownBody's internal plumbing
// and not the density Maka hands its own code-block renderers.
for (const tag of source.matchAll(/<Markdown\s[^>]*?\/?>/gs)) {
if (/density=(["'])compact\1/.test(tag[0])) callers.push(relative(UI_SRC, file));
}
}

assert.deepEqual(
[...new Set(callers)].sort(),
['chat-turn.tsx'],
'a new caller renders markdown at compact density. The rhythm table treats ' +
'`density="compact"` as "this is a transcript" and gives it flattened heading sizes ' +
'plus secondary-colour h4-h6 — typography, which Astryx\'s density is explicitly not ' +
'supposed to carry (facebook/astryx#839). Either that is what the new surface wants, ' +
'and this list grows, or the typography rules need their own key. ' +
`Found: ${JSON.stringify([...new Set(callers)].sort())}`,
);
});

it('emits the list hooks whose control padding the table neutralizes', () => {
const markup = compactMarkup();
assert.match(
markup,
/class="[^"]*\bastryx-list\b/,
'the markdown list no longer renders through Astryx `List`. If it stopped being a ' +
'control list that is good news, but the padding-zeroing rule is now dead and the ' +
'list rhythm needs re-measuring rather than silently inheriting whatever replaced it.',
);
assert.match(
markup,
/class="[^"]*\bastryx-list-item\b/,
'list rows no longer carry `astryx-list-item`; the rule that spends their control-row ' +
'padding no longer applies and list items revert to sitting wider apart than paragraphs',
);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix(ui): restore the transcript's markdown rhythm by Astro-Han · Pull Request #2348 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 199 additions & 0 deletions apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
/**
* Transcript markdown rhythm contract.
*
* The defect this pins was not a wrong number — it was a wrong ORDER. Under
* `density="compact"` the transcript spaced list items ~10px apart (Astryx's
* List control row padding, which `density` cannot reach from outside) while
* paragraphs sat 4px apart, so items at the same level read as further apart
* than separate paragraphs. Visual distance stopped tracking semantic
* distance.
*
* So the invariant is the ladder's ORDER, not its values: retuning 8px to 10px
* is a design decision and should stay green here; making list gaps meet or
* exceed block gaps is the regression, and must fail. A screenshot cannot lock
* that — it fixes one rendering of one sample rather than the relation — which
* is why AGENTS.md asks for a computed-style or text contract on cascade and
* layout invariants. The visual half lives in the `TranscriptTurn` story
* (packages/ui/stories/markdown.stories.tsx).
*
* Scoped to the compact surface only. Document mode is Astryx's own rhythm and
* the Daily Review renders through it, so nothing here should constrain it.
*/
import { strict as assert } from 'node:assert';
import { describe, it } from 'node:test';
import { readAllRendererCss, stripCssComments } from './css-test-helpers.js';

/** The compact ladder, ordered from tightest (same level) to widest (chapter). */
const LADDER = ['--md-gap-list', '--md-gap-block', '--md-gap-section', '--md-gap-chapter'] as const;

/** `var(--space-N)` → N, the 4px-grid step count. Non-scale values return null. */
function spaceSteps(value: string): number | null {
const match = /^var\(\s*--space-(\d+)\s*\)$/.exec(value.trim());
return match ? Number(match[1]) : null;
}

describe('transcript markdown rhythm', () => {
it('declares the compact ladder in strictly increasing order', async () => {
const css = stripCssComments(await readAllRendererCss());
const block = /\.astryx-markdown\[data-density="compact"\]\s*\{([^}]*)\}/.exec(css);
assert.ok(block, 'no `.astryx-markdown[data-density="compact"]` custom-property block found');

const declared = new Map<string, string>();
for (const m of block[1].matchAll(/(--md-gap-[\w-]+)\s*:\s*([^;]+);/g)) {
declared.set(m[1], m[2].trim());
}

const steps = LADDER.map((name) => {
const value = declared.get(name);
assert.ok(value, `${name} is not declared on the compact surface`);
const n = spaceSteps(value);
assert.ok(
n !== null,
`${name} is \`${value}\`; the ladder must stay on the --space-* scale so the ` +
'order is comparable and a bare px value cannot drift off the 4px grid',
);
return { name, steps: n };
});

for (let i = 1; i < steps.length; i += 1) {
const prev = steps[i - 1];
const cur = steps[i];
assert.ok(
cur.steps > prev.steps,
`${cur.name} (--space-${cur.steps}) must be strictly wider than ${prev.name} ` +
`(--space-${prev.steps}). Visual distance has to rise with semantic distance: ` +
'same-level list items closer than blocks, blocks closer than section breaks.',
);
}
});

it('expresses every block gap as an adjacent-sibling relation', async () => {
const css = stripCssComments(await readAllRendererCss());
const compactBlockRules = [
...css.matchAll(
/(\[data-maka-contract="markdown"\]\s*\.astryx-markdown\[data-density="compact"\]\s*>[^{]*)\{([^}]*)\}/g,
),
].filter(([, , body]) => /margin-block-start\s*:/.test(body));

assert.ok(compactBlockRules.length > 0, 'no compact block-gap rules found');

for (const [, selector, body] of compactBlockRules) {
assert.match(
selector,
/>[^{]*\+/,
'every block gap must be an adjacent-sibling rule (`> … + …`). A gap is a relation ' +
'between two blocks, so the first block should match no gap rule at all. The ' +
'`> *` + `> :first-child` reset form looks equivalent but loses on specificity: ' +
`:first-child scores (0,4,0) against the heading rules' (0,5,0), so a turn opening ` +
'with a heading keeps a chapter gap above its first line and pushes off the top of ' +
`the bubble. Offending rule: \`${selector.trim()}\` { ${body.trim()} }`,
);
}

assert.doesNotMatch(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*:first-child/,
'the `:first-child` gap reset is back. It cannot beat the heading rules on ' +
'specificity — use adjacent-sibling gap rules so the first block is never matched.',
);

// The other half of "the distance between two blocks is the value declared
// here": without it Astryx's own per-element end margins survive in the
// earlier layer and collapse against these gaps. An `hr` carries 12px in
// compact, so dropping the reset silently widens the block step after one.
assert.match(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\{[^}]*margin-block\s*:\s*0/,
'the blanket `> * { margin-block: 0 }` reset is gone. The gap rules only set ' +
'margin-block-START, so without it Astryx\'s end margins survive and collapse ' +
'against them — the declared ladder stops being the spacing you get.',
);

// Declaration and usage are separate failures: the ladder can stay ordered
// while the rules that spend it are hardcoded, which turns the table into
// decoration and the first test into a tautology.
for (const [, selector, body] of compactBlockRules) {
assert.match(
body,
/margin-block-start\s*:\s*var\(\s*--md-gap-[\w-]+\s*\)/,
'a compact gap rule sets a literal instead of a `--md-gap-*` variable. The ladder ' +
'test above only checks that the variables are declared in order; a hardcoded ' +
`value leaves it green while the rendered spacing ignores it. Rule: \`${selector.trim()}\` ` +
`{ ${body.trim()} }`,
);
}
});

it('keeps a typed `hr` wider than the block step', async () => {
const css = stripCssComments(await readAllRendererCss());
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\+\s*hr\s*,[^{]*\{([^}]*)\}`,
).exec(css);
assert.ok(
rule,
'the compact surface no longer gives `hr` its own step. An `hr` is the only break ' +
'the author typed by hand; left on the generic block gap it reads no wider than ' +
'the paragraph boundary above it, so writing one changes nothing.',
);

const used = /margin-block-start\s*:\s*var\(\s*(--md-gap-[\w-]+)\s*\)/.exec(rule[1])?.[1];
assert.ok(used, `the \`hr\` rule must spend a --md-gap-* variable. Found: { ${rule[1].trim()} }`);

const block = LADDER.indexOf('--md-gap-block');
const rung = LADDER.indexOf(used as (typeof LADDER)[number]);
assert.ok(
rung > block,
`\`hr\` spends ${used}, which is not above --md-gap-block on the ladder. Which rung ` +
'it takes is a design call — section today, chapter would be fine — but it has to ' +
'outrank the ordinary block step or the separator carries no meaning.',
);
});

it('spends the list-item row padding it re-spaces as prose', async () => {
const css = stripCssComments(await readAllRendererCss());
// Astryx's ListItem carries the control-row padding that inverted the
// ladder. Zeroing it is what makes --md-gap-list the whole distance between
// two items; leave it in and the gap variable understates the real spacing.
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list-item\s*\{([^}]*)\}`,
).exec(css);
assert.ok(rule, 'the compact surface no longer neutralizes `.astryx-list-item` padding');
assert.match(
rule[1],
/padding-block\s*:\s*0/,
'ListItem block padding must be zeroed on the compact surface, otherwise the real ' +
'list-item gap is padding + gap and the ladder above is not the spacing you get',
);

const listGap = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list\s*\{([^}]*)\}`,
).exec(css);
assert.ok(listGap, 'the compact surface no longer sets a `.astryx-list` row gap');
assert.match(
listGap[1],
/gap\s*:\s*var\(\s*--md-gap-list\s*\)/,
'the list gap must spend `--md-gap-list`. A literal here detaches the tightest rung ' +
'from the ladder the first test orders, so an inversion could be reintroduced ' +
'without failing anything',
);
});

it('keeps two heading size steps on the compact surface', async () => {
const css = stripCssComments(await readAllRendererCss());
const fonts = [
...css.matchAll(
/\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-markdown-heading[^{]*\{([^}]*)\}/g,
),
]
.map((m) => /font\s*:\s*var\(\s*(--maka-text-heading-\d)\s*\)/.exec(m[1])?.[1])
.filter((tier): tier is string => tier !== undefined);

assert.ok(
new Set(fonts).size >= 2,
'the transcript heading scale collapsed to one size tier. Flattening Astryx\'s ' +
'document ladder is deliberate, but flattening it to ZERO steps is the bug this ' +
'replaced: h2 and h3 then differ in nothing — not size, weight, or colour. ' +
`Found tiers: ${JSON.stringify(fonts)}`,
);
});
});
43 changes: 10 additions & 33 deletions apps/desktop/src/renderer/styles/chat-message.css
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,39 +212,16 @@
independent instead of keyed on a `[data-role="assistant"]` descendant. */

/* --- Conversation heading scale ------------------------------------------- */
/* Astryx's heading ladder (20/18/16/14…) is a DOCUMENT scale, and the
transcript is not a document. An agent turn emits `##` freely, so a ladder
that steps 1.4× per level shreds one reply into visually unrelated slabs.
Flatten it to two steps — h1 for a genuine section break, everything below
at body size — and let weight 600 plus colour carry the hierarchy, which is
what the rest of the conversation already does.
Cursor (1.43/1.32/1.21), Claude Code (1.14/1.07/1.0) and Codex
(1.71/1.43/1.21) all flatten here relative to their own document styles;
Claude Code goes furthest and merges h4–h6 into `strong`. Two steps sits
inside that range.
Every level shares the transcript baseline so headings do not add rhythm of
their own.
Scoped to `.maka-turn`, not to the Markdown contract alone. The contract is
emitted by the shared MarkdownBody, whose other caller is the Daily Review
panel — and a review report IS a document, so the argument above does not
apply to it. It keeps Astryx's full ladder.
Bare tags are the whole story: Astryx's Markdown renderer emits plain
h1–h6 with stylex props (Markdown.js `const Tag = \`h${level}\``). The
.astryx-heading.level-N classes come from the standalone Heading
component, which Markdown never mounts. */
/* The one heading in a turn that is not at the body size, and so the one that
cannot take the body baseline with it: at 16px, --maka-line-body's 20px is
on the 4px grid but is the 14px tier's line box, one step tighter than the
tier this heading actually sits at. It takes its own tier's leading (24px);
the turn still reads as one rhythm because 24 is the next step of the same
grid, not a different one. */
.maka-turn [data-maka-contract="markdown"] h1 {
font: var(--maka-text-heading-3);
}

.maka-turn [data-maka-contract="markdown"] :is(h2, h3, h4, h5, h6) {
font: var(--maka-text-heading-4);
}
/* Moved to the transcript rhythm table in packages/ui/src/styles.css. The
argument for flattening the document ladder is unchanged; what changed is
where it is expressed. These rules keyed on a `.maka-turn` ancestor, which
made the heading scale a function of DOM position: the same MarkdownBody
rendered document-sized headings in the Daily Review and flat ones in a
turn, and no Storybook story could reproduce the transcript without
remembering to wrap one. The rhythm table keys on Astryx's own
`data-density` instead, so the scale follows the `density` prop that already
distinguishes the two surfaces. It also keeps two size steps rather than
one — h2 and h3 used to be identical in size, weight and colour. */

.maka-chat-message pre:not([data-maka-contract="markdown"] pre) {
margin: 0;
Expand Down
141 changes: 141 additions & 0 deletions packages/ui/src/__tests__/markdown-rhythm-dom-contract.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
/**
* The DOM half of the transcript rhythm contract.
*
* The rhythm table in styles.css keys entirely on DOM that Astryx generates at
* runtime — `data-density` on the document root, `astryx-markdown-heading` plus
* `data-level` on headings, `astryx-list`/`astryx-list-item` inside a list.
* None of those names appear in Maka source as literals; they come from
* Astryx's `themeProps()`, whose prefix has a single upstream owner
* (`naming.ts`). So an Astryx bump that renames one of them makes every
* selector in the table silently stop matching, and the spacing regresses to
* the inverted ladder with no failing check anywhere: the CSS contract test
* (apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts) reads the
* stylesheet as text and stays green against DOM it never sees, and the
* `check-dead-css` entries are an allowlist that marks these classes live
* rather than asserting they exist.
*
* This pins the other half: the hooks the stylesheet selects on are really
* emitted. Together the two tests fail on either side of the join.
*/
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { readdir, readFile } from 'node:fs/promises';
import { join, relative, resolve } from 'node:path';
import { renderToStaticMarkup } from 'react-dom/server';
import { MarkdownBody } from '../markdown-body.js';

const UI_SRC = resolve(import.meta.dirname, '..', '..', 'src');

async function tsxFiles(dir: string): Promise<string[]> {
const out: string[] = [];
for (const entry of await readdir(dir, { withFileTypes: true })) {
const path = join(dir, entry.name);
if (entry.isDirectory()) out.push(...(await tsxFiles(path)));
else if (entry.name.endsWith('.tsx') && !entry.name.includes('.test.')) out.push(path);
}
return out;
}

const SAMPLE = ['## Heading two', '', 'A paragraph.', '', '1. First item', '2. Second item'].join('\n');

function compactMarkup(): string {
return renderToStaticMarkup(<MarkdownBody text={SAMPLE} density="compact" />);
}

describe('transcript markdown rhythm — DOM hooks', () => {
it('emits the density attribute the rhythm table scopes on', () => {
const markup = compactMarkup();
// Every rule in the table is prefixed with this, so it is the one hook whose
// two halves — selector prefix and runtime attribute — nothing else joins.
// Rename it and the CSS contract still passes on text it never renders.
assert.match(
markup,
/data-maka-contract="markdown"/,
'the `data-maka-contract="markdown"` wrapper is gone. Every rule in the rhythm ' +
'table is scoped on it, so all compact prose spacing and the heading scale are ' +
'now dead — and the stylesheet-side contract cannot see it.',
);
assert.match(
markup,
/<div[^>]*role="document"[^>]*data-density="compact"|<div[^>]*data-density="compact"[^>]*role="document"/,
'the document root no longer carries `data-density="compact"`. Every rule in the ' +
'rhythm table is scoped on it, so all compact prose spacing is now dead.',
);
assert.match(
markup,
/class="[^"]*\bastryx-markdown\b/,
'the document root no longer carries the `astryx-markdown` class the table selects on',
);
});

it('emits the heading hooks the two-step scale selects on', () => {
const markup = compactMarkup();
assert.match(
markup,
/<h2[^>]*class="[^"]*\bastryx-markdown-heading\b/,
'headings no longer carry `astryx-markdown-heading`; the transcript heading scale is dead',
);
assert.match(
markup,
/<h2[^>]*data-level="2"/,
'headings no longer carry `data-level`. The table splits h1/h2 from h3-h6 on it, so ' +
'without it every heading collapses to one tier — the exact defect this branch replaced.',
);
});

/**
* The rhythm table carries the transcript's heading typography, not just its
* spacing, and keys both on `density`. That is a deliberate bet, and it is a
* bet: Astryx's own density RFC (facebook/astryx#839) draws the line at
* "density shifts heights and spacing, not typography", so the key is
* honest only while `compact` and "transcript" are the same set. They are
* today — the transcript is the only caller that asks for compact, and the
* Daily Review renders a document at the default.
*
* A separate surface attribute would decouple them, but nothing needs it
* yet and it would add a prop to a public component for a caller that does
* not exist. Guard the assumption instead: the moment a second surface asks
* for compact markdown it silently inherits transcript heading sizes and
* dimmed deep headings, and this test is what tells whoever adds it that
* they have to choose — inherit deliberately, or split the key then.
*/
it('keeps compact markdown a transcript-only surface', async () => {
const callers: string[] = [];
for (const file of await tsxFiles(UI_SRC)) {
const source = await readFile(file, 'utf8');
// `<Markdown …>` opening tags only — not MarkdownBody's internal plumbing
// and not the density Maka hands its own code-block renderers.
for (const tag of source.matchAll(/<Markdown\s[^>]*?\/?>/gs)) {
if (/density=(["'])compact\1/.test(tag[0])) callers.push(relative(UI_SRC, file));
}
}

assert.deepEqual(
[...new Set(callers)].sort(),
['chat-turn.tsx'],
'a new caller renders markdown at compact density. The rhythm table treats ' +
'`density="compact"` as "this is a transcript" and gives it flattened heading sizes ' +
'plus secondary-colour h4-h6 — typography, which Astryx\'s density is explicitly not ' +
'supposed to carry (facebook/astryx#839). Either that is what the new surface wants, ' +
'and this list grows, or the typography rules need their own key. ' +
`Found: ${JSON.stringify([...new Set(callers)].sort())}`,
);
});

it('emits the list hooks whose control padding the table neutralizes', () => {
const markup = compactMarkup();
assert.match(
markup,
/class="[^"]*\bastryx-list\b/,
'the markdown list no longer renders through Astryx `List`. If it stopped being a ' +
'control list that is good news, but the padding-zeroing rule is now dead and the ' +
'list rhythm needs re-measuring rather than silently inheriting whatever replaced it.',
);
assert.match(
markup,
/class="[^"]*\bastryx-list-item\b/,
'list rows no longer carry `astryx-list-item`; the rule that spends their control-row ' +
'padding no longer applies and list items revert to sitting wider apart than paragraphs',
);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(ui): restore the transcript's markdown rhythm by Astro-Han · Pull Request #2348 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 199 additions & 0 deletions apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
/**
* Transcript markdown rhythm contract.
*
* The defect this pins was not a wrong number — it was a wrong ORDER. Under
* `density="compact"` the transcript spaced list items ~10px apart (Astryx's
* List control row padding, which `density` cannot reach from outside) while
* paragraphs sat 4px apart, so items at the same level read as further apart
* than separate paragraphs. Visual distance stopped tracking semantic
* distance.
*
* So the invariant is the ladder's ORDER, not its values: retuning 8px to 10px
* is a design decision and should stay green here; making list gaps meet or
* exceed block gaps is the regression, and must fail. A screenshot cannot lock
* that — it fixes one rendering of one sample rather than the relation — which
* is why AGENTS.md asks for a computed-style or text contract on cascade and
* layout invariants. The visual half lives in the `TranscriptTurn` story
* (packages/ui/stories/markdown.stories.tsx).
*
* Scoped to the compact surface only. Document mode is Astryx's own rhythm and
* the Daily Review renders through it, so nothing here should constrain it.
*/
import { strict as assert } from 'node:assert';
import { describe, it } from 'node:test';
import { readAllRendererCss, stripCssComments } from './css-test-helpers.js';

/** The compact ladder, ordered from tightest (same level) to widest (chapter). */
const LADDER = ['--md-gap-list', '--md-gap-block', '--md-gap-section', '--md-gap-chapter'] as const;

/** `var(--space-N)` → N, the 4px-grid step count. Non-scale values return null. */
function spaceSteps(value: string): number | null {
const match = /^var\(\s*--space-(\d+)\s*\)$/.exec(value.trim());
return match ? Number(match[1]) : null;
}

describe('transcript markdown rhythm', () => {
it('declares the compact ladder in strictly increasing order', async () => {
const css = stripCssComments(await readAllRendererCss());
const block = /\.astryx-markdown\[data-density="compact"\]\s*\{([^}]*)\}/.exec(css);
assert.ok(block, 'no `.astryx-markdown[data-density="compact"]` custom-property block found');

const declared = new Map<string, string>();
for (const m of block[1].matchAll(/(--md-gap-[\w-]+)\s*:\s*([^;]+);/g)) {
declared.set(m[1], m[2].trim());
}

const steps = LADDER.map((name) => {
const value = declared.get(name);
assert.ok(value, `${name} is not declared on the compact surface`);
const n = spaceSteps(value);
assert.ok(
n !== null,
`${name} is \`${value}\`; the ladder must stay on the --space-* scale so the ` +
'order is comparable and a bare px value cannot drift off the 4px grid',
);
return { name, steps: n };
});

for (let i = 1; i < steps.length; i += 1) {
const prev = steps[i - 1];
const cur = steps[i];
assert.ok(
cur.steps > prev.steps,
`${cur.name} (--space-${cur.steps}) must be strictly wider than ${prev.name} ` +
`(--space-${prev.steps}). Visual distance has to rise with semantic distance: ` +
'same-level list items closer than blocks, blocks closer than section breaks.',
);
}
});

it('expresses every block gap as an adjacent-sibling relation', async () => {
const css = stripCssComments(await readAllRendererCss());
const compactBlockRules = [
...css.matchAll(
/(\[data-maka-contract="markdown"\]\s*\.astryx-markdown\[data-density="compact"\]\s*>[^{]*)\{([^}]*)\}/g,
),
].filter(([, , body]) => /margin-block-start\s*:/.test(body));

assert.ok(compactBlockRules.length > 0, 'no compact block-gap rules found');

for (const [, selector, body] of compactBlockRules) {
assert.match(
selector,
/>[^{]*\+/,
'every block gap must be an adjacent-sibling rule (`> … + …`). A gap is a relation ' +
'between two blocks, so the first block should match no gap rule at all. The ' +
'`> *` + `> :first-child` reset form looks equivalent but loses on specificity: ' +
`:first-child scores (0,4,0) against the heading rules' (0,5,0), so a turn opening ` +
'with a heading keeps a chapter gap above its first line and pushes off the top of ' +
`the bubble. Offending rule: \`${selector.trim()}\` { ${body.trim()} }`,
);
}

assert.doesNotMatch(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*:first-child/,
'the `:first-child` gap reset is back. It cannot beat the heading rules on ' +
'specificity — use adjacent-sibling gap rules so the first block is never matched.',
);

// The other half of "the distance between two blocks is the value declared
// here": without it Astryx's own per-element end margins survive in the
// earlier layer and collapse against these gaps. An `hr` carries 12px in
// compact, so dropping the reset silently widens the block step after one.
assert.match(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\{[^}]*margin-block\s*:\s*0/,
'the blanket `> * { margin-block: 0 }` reset is gone. The gap rules only set ' +
'margin-block-START, so without it Astryx\'s end margins survive and collapse ' +
'against them — the declared ladder stops being the spacing you get.',
);

// Declaration and usage are separate failures: the ladder can stay ordered
// while the rules that spend it are hardcoded, which turns the table into
// decoration and the first test into a tautology.
for (const [, selector, body] of compactBlockRules) {
assert.match(
body,
/margin-block-start\s*:\s*var\(\s*--md-gap-[\w-]+\s*\)/,
'a compact gap rule sets a literal instead of a `--md-gap-*` variable. The ladder ' +
'test above only checks that the variables are declared in order; a hardcoded ' +
`value leaves it green while the rendered spacing ignores it. Rule: \`${selector.trim()}\` ` +
`{ ${body.trim()} }`,
);
}
});

it('keeps a typed `hr` wider than the block step', async () => {
const css = stripCssComments(await readAllRendererCss());
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\+\s*hr\s*,[^{]*\{([^}]*)\}`,
).exec(css);
assert.ok(
rule,
'the compact surface no longer gives `hr` its own step. An `hr` is the only break ' +
'the author typed by hand; left on the generic block gap it reads no wider than ' +
'the paragraph boundary above it, so writing one changes nothing.',
);

const used = /margin-block-start\s*:\s*var\(\s*(--md-gap-[\w-]+)\s*\)/.exec(rule[1])?.[1];
assert.ok(used, `the \`hr\` rule must spend a --md-gap-* variable. Found: { ${rule[1].trim()} }`);

const block = LADDER.indexOf('--md-gap-block');
const rung = LADDER.indexOf(used as (typeof LADDER)[number]);
assert.ok(
rung > block,
`\`hr\` spends ${used}, which is not above --md-gap-block on the ladder. Which rung ` +
'it takes is a design call — section today, chapter would be fine — but it has to ' +
'outrank the ordinary block step or the separator carries no meaning.',
);
});

it('spends the list-item row padding it re-spaces as prose', async () => {
const css = stripCssComments(await readAllRendererCss());
// Astryx's ListItem carries the control-row padding that inverted the
// ladder. Zeroing it is what makes --md-gap-list the whole distance between
// two items; leave it in and the gap variable understates the real spacing.
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list-item\s*\{([^}]*)\}`,
).exec(css);
assert.ok(rule, 'the compact surface no longer neutralizes `.astryx-list-item` padding');
assert.match(
rule[1],
/padding-block\s*:\s*0/,
'ListItem block padding must be zeroed on the compact surface, otherwise the real ' +
'list-item gap is padding + gap and the ladder above is not the spacing you get',
);

const listGap = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list\s*\{([^}]*)\}`,
).exec(css);
assert.ok(listGap, 'the compact surface no longer sets a `.astryx-list` row gap');
assert.match(
listGap[1],
/gap\s*:\s*var\(\s*--md-gap-list\s*\)/,
'the list gap must spend `--md-gap-list`. A literal here detaches the tightest rung ' +
'from the ladder the first test orders, so an inversion could be reintroduced ' +
'without failing anything',
);
});

it('keeps two heading size steps on the compact surface', async () => {
const css = stripCssComments(await readAllRendererCss());
const fonts = [
...css.matchAll(
/\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-markdown-heading[^{]*\{([^}]*)\}/g,
),
]
.map((m) => /font\s*:\s*var\(\s*(--maka-text-heading-\d)\s*\)/.exec(m[1])?.[1])
.filter((tier): tier is string => tier !== undefined);

assert.ok(
new Set(fonts).size >= 2,
'the transcript heading scale collapsed to one size tier. Flattening Astryx\'s ' +
'document ladder is deliberate, but flattening it to ZERO steps is the bug this ' +
'replaced: h2 and h3 then differ in nothing — not size, weight, or colour. ' +
`Found tiers: ${JSON.stringify(fonts)}`,
);
});
});
43 changes: 10 additions & 33 deletions apps/desktop/src/renderer/styles/chat-message.css
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,39 +212,16 @@
independent instead of keyed on a `[data-role="assistant"]` descendant. */

/* --- Conversation heading scale ------------------------------------------- */
/* Astryx's heading ladder (20/18/16/14…) is a DOCUMENT scale, and the
transcript is not a document. An agent turn emits `##` freely, so a ladder
that steps 1.4× per level shreds one reply into visually unrelated slabs.
Flatten it to two steps — h1 for a genuine section break, everything below
at body size — and let weight 600 plus colour carry the hierarchy, which is
what the rest of the conversation already does.
Cursor (1.43/1.32/1.21), Claude Code (1.14/1.07/1.0) and Codex
(1.71/1.43/1.21) all flatten here relative to their own document styles;
Claude Code goes furthest and merges h4–h6 into `strong`. Two steps sits
inside that range.
Every level shares the transcript baseline so headings do not add rhythm of
their own.
Scoped to `.maka-turn`, not to the Markdown contract alone. The contract is
emitted by the shared MarkdownBody, whose other caller is the Daily Review
panel — and a review report IS a document, so the argument above does not
apply to it. It keeps Astryx's full ladder.
Bare tags are the whole story: Astryx's Markdown renderer emits plain
h1–h6 with stylex props (Markdown.js `const Tag = \`h${level}\``). The
.astryx-heading.level-N classes come from the standalone Heading
component, which Markdown never mounts. */
/* The one heading in a turn that is not at the body size, and so the one that
cannot take the body baseline with it: at 16px, --maka-line-body's 20px is
on the 4px grid but is the 14px tier's line box, one step tighter than the
tier this heading actually sits at. It takes its own tier's leading (24px);
the turn still reads as one rhythm because 24 is the next step of the same
grid, not a different one. */
.maka-turn [data-maka-contract="markdown"] h1 {
font: var(--maka-text-heading-3);
}

.maka-turn [data-maka-contract="markdown"] :is(h2, h3, h4, h5, h6) {
font: var(--maka-text-heading-4);
}
/* Moved to the transcript rhythm table in packages/ui/src/styles.css. The
argument for flattening the document ladder is unchanged; what changed is
where it is expressed. These rules keyed on a `.maka-turn` ancestor, which
made the heading scale a function of DOM position: the same MarkdownBody
rendered document-sized headings in the Daily Review and flat ones in a
turn, and no Storybook story could reproduce the transcript without
remembering to wrap one. The rhythm table keys on Astryx's own
`data-density` instead, so the scale follows the `density` prop that already
distinguishes the two surfaces. It also keeps two size steps rather than
one — h2 and h3 used to be identical in size, weight and colour. */

.maka-chat-message pre:not([data-maka-contract="markdown"] pre) {
margin: 0;
Expand Down
141 changes: 141 additions & 0 deletions packages/ui/src/__tests__/markdown-rhythm-dom-contract.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
/**
* The DOM half of the transcript rhythm contract.
*
* The rhythm table in styles.css keys entirely on DOM that Astryx generates at
* runtime — `data-density` on the document root, `astryx-markdown-heading` plus
* `data-level` on headings, `astryx-list`/`astryx-list-item` inside a list.
* None of those names appear in Maka source as literals; they come from
* Astryx's `themeProps()`, whose prefix has a single upstream owner
* (`naming.ts`). So an Astryx bump that renames one of them makes every
* selector in the table silently stop matching, and the spacing regresses to
* the inverted ladder with no failing check anywhere: the CSS contract test
* (apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts) reads the
* stylesheet as text and stays green against DOM it never sees, and the
* `check-dead-css` entries are an allowlist that marks these classes live
* rather than asserting they exist.
*
* This pins the other half: the hooks the stylesheet selects on are really
* emitted. Together the two tests fail on either side of the join.
*/
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { readdir, readFile } from 'node:fs/promises';
import { join, relative, resolve } from 'node:path';
import { renderToStaticMarkup } from 'react-dom/server';
import { MarkdownBody } from '../markdown-body.js';

const UI_SRC = resolve(import.meta.dirname, '..', '..', 'src');

async function tsxFiles(dir: string): Promise<string[]> {
const out: string[] = [];
for (const entry of await readdir(dir, { withFileTypes: true })) {
const path = join(dir, entry.name);
if (entry.isDirectory()) out.push(...(await tsxFiles(path)));
else if (entry.name.endsWith('.tsx') && !entry.name.includes('.test.')) out.push(path);
}
return out;
}

const SAMPLE = ['## Heading two', '', 'A paragraph.', '', '1. First item', '2. Second item'].join('\n');

function compactMarkup(): string {
return renderToStaticMarkup(<MarkdownBody text={SAMPLE} density="compact" />);
}

describe('transcript markdown rhythm — DOM hooks', () => {
it('emits the density attribute the rhythm table scopes on', () => {
const markup = compactMarkup();
// Every rule in the table is prefixed with this, so it is the one hook whose
// two halves — selector prefix and runtime attribute — nothing else joins.
// Rename it and the CSS contract still passes on text it never renders.
assert.match(
markup,
/data-maka-contract="markdown"/,
'the `data-maka-contract="markdown"` wrapper is gone. Every rule in the rhythm ' +
'table is scoped on it, so all compact prose spacing and the heading scale are ' +
'now dead — and the stylesheet-side contract cannot see it.',
);
assert.match(
markup,
/<div[^>]*role="document"[^>]*data-density="compact"|<div[^>]*data-density="compact"[^>]*role="document"/,
'the document root no longer carries `data-density="compact"`. Every rule in the ' +
'rhythm table is scoped on it, so all compact prose spacing is now dead.',
);
assert.match(
markup,
/class="[^"]*\bastryx-markdown\b/,
'the document root no longer carries the `astryx-markdown` class the table selects on',
);
});

it('emits the heading hooks the two-step scale selects on', () => {
const markup = compactMarkup();
assert.match(
markup,
/<h2[^>]*class="[^"]*\bastryx-markdown-heading\b/,
'headings no longer carry `astryx-markdown-heading`; the transcript heading scale is dead',
);
assert.match(
markup,
/<h2[^>]*data-level="2"/,
'headings no longer carry `data-level`. The table splits h1/h2 from h3-h6 on it, so ' +
'without it every heading collapses to one tier — the exact defect this branch replaced.',
);
});

/**
* The rhythm table carries the transcript's heading typography, not just its
* spacing, and keys both on `density`. That is a deliberate bet, and it is a
* bet: Astryx's own density RFC (facebook/astryx#839) draws the line at
* "density shifts heights and spacing, not typography", so the key is
* honest only while `compact` and "transcript" are the same set. They are
* today — the transcript is the only caller that asks for compact, and the
* Daily Review renders a document at the default.
*
* A separate surface attribute would decouple them, but nothing needs it
* yet and it would add a prop to a public component for a caller that does
* not exist. Guard the assumption instead: the moment a second surface asks
* for compact markdown it silently inherits transcript heading sizes and
* dimmed deep headings, and this test is what tells whoever adds it that
* they have to choose — inherit deliberately, or split the key then.
*/
it('keeps compact markdown a transcript-only surface', async () => {
const callers: string[] = [];
for (const file of await tsxFiles(UI_SRC)) {
const source = await readFile(file, 'utf8');
// `<Markdown …>` opening tags only — not MarkdownBody's internal plumbing
// and not the density Maka hands its own code-block renderers.
for (const tag of source.matchAll(/<Markdown\s[^>]*?\/?>/gs)) {
if (/density=(["'])compact\1/.test(tag[0])) callers.push(relative(UI_SRC, file));
}
}

assert.deepEqual(
[...new Set(callers)].sort(),
['chat-turn.tsx'],
'a new caller renders markdown at compact density. The rhythm table treats ' +
'`density="compact"` as "this is a transcript" and gives it flattened heading sizes ' +
'plus secondary-colour h4-h6 — typography, which Astryx\'s density is explicitly not ' +
'supposed to carry (facebook/astryx#839). Either that is what the new surface wants, ' +
'and this list grows, or the typography rules need their own key. ' +
`Found: ${JSON.stringify([...new Set(callers)].sort())}`,
);
});

it('emits the list hooks whose control padding the table neutralizes', () => {
const markup = compactMarkup();
assert.match(
markup,
/class="[^"]*\bastryx-list\b/,
'the markdown list no longer renders through Astryx `List`. If it stopped being a ' +
'control list that is good news, but the padding-zeroing rule is now dead and the ' +
'list rhythm needs re-measuring rather than silently inheriting whatever replaced it.',
);
assert.match(
markup,
/class="[^"]*\bastryx-list-item\b/,
'list rows no longer carry `astryx-list-item`; the rule that spends their control-row ' +
'padding no longer applies and list items revert to sitting wider apart than paragraphs',
);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(ui): restore the transcript's markdown rhythm by Astro-Han · Pull Request #2348 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 199 additions & 0 deletions apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
/**
* Transcript markdown rhythm contract.
*
* The defect this pins was not a wrong number — it was a wrong ORDER. Under
* `density="compact"` the transcript spaced list items ~10px apart (Astryx's
* List control row padding, which `density` cannot reach from outside) while
* paragraphs sat 4px apart, so items at the same level read as further apart
* than separate paragraphs. Visual distance stopped tracking semantic
* distance.
*
* So the invariant is the ladder's ORDER, not its values: retuning 8px to 10px
* is a design decision and should stay green here; making list gaps meet or
* exceed block gaps is the regression, and must fail. A screenshot cannot lock
* that — it fixes one rendering of one sample rather than the relation — which
* is why AGENTS.md asks for a computed-style or text contract on cascade and
* layout invariants. The visual half lives in the `TranscriptTurn` story
* (packages/ui/stories/markdown.stories.tsx).
*
* Scoped to the compact surface only. Document mode is Astryx's own rhythm and
* the Daily Review renders through it, so nothing here should constrain it.
*/
import { strict as assert } from 'node:assert';
import { describe, it } from 'node:test';
import { readAllRendererCss, stripCssComments } from './css-test-helpers.js';

/** The compact ladder, ordered from tightest (same level) to widest (chapter). */
const LADDER = ['--md-gap-list', '--md-gap-block', '--md-gap-section', '--md-gap-chapter'] as const;

/** `var(--space-N)` → N, the 4px-grid step count. Non-scale values return null. */
function spaceSteps(value: string): number | null {
const match = /^var\(\s*--space-(\d+)\s*\)$/.exec(value.trim());
return match ? Number(match[1]) : null;
}

describe('transcript markdown rhythm', () => {
it('declares the compact ladder in strictly increasing order', async () => {
const css = stripCssComments(await readAllRendererCss());
const block = /\.astryx-markdown\[data-density="compact"\]\s*\{([^}]*)\}/.exec(css);
assert.ok(block, 'no `.astryx-markdown[data-density="compact"]` custom-property block found');

const declared = new Map<string, string>();
for (const m of block[1].matchAll(/(--md-gap-[\w-]+)\s*:\s*([^;]+);/g)) {
declared.set(m[1], m[2].trim());
}

const steps = LADDER.map((name) => {
const value = declared.get(name);
assert.ok(value, `${name} is not declared on the compact surface`);
const n = spaceSteps(value);
assert.ok(
n !== null,
`${name} is \`${value}\`; the ladder must stay on the --space-* scale so the ` +
'order is comparable and a bare px value cannot drift off the 4px grid',
);
return { name, steps: n };
});

for (let i = 1; i < steps.length; i += 1) {
const prev = steps[i - 1];
const cur = steps[i];
assert.ok(
cur.steps > prev.steps,
`${cur.name} (--space-${cur.steps}) must be strictly wider than ${prev.name} ` +
`(--space-${prev.steps}). Visual distance has to rise with semantic distance: ` +
'same-level list items closer than blocks, blocks closer than section breaks.',
);
}
});

it('expresses every block gap as an adjacent-sibling relation', async () => {
const css = stripCssComments(await readAllRendererCss());
const compactBlockRules = [
...css.matchAll(
/(\[data-maka-contract="markdown"\]\s*\.astryx-markdown\[data-density="compact"\]\s*>[^{]*)\{([^}]*)\}/g,
),
].filter(([, , body]) => /margin-block-start\s*:/.test(body));

assert.ok(compactBlockRules.length > 0, 'no compact block-gap rules found');

for (const [, selector, body] of compactBlockRules) {
assert.match(
selector,
/>[^{]*\+/,
'every block gap must be an adjacent-sibling rule (`> … + …`). A gap is a relation ' +
'between two blocks, so the first block should match no gap rule at all. The ' +
'`> *` + `> :first-child` reset form looks equivalent but loses on specificity: ' +
`:first-child scores (0,4,0) against the heading rules' (0,5,0), so a turn opening ` +
'with a heading keeps a chapter gap above its first line and pushes off the top of ' +
`the bubble. Offending rule: \`${selector.trim()}\` { ${body.trim()} }`,
);
}

assert.doesNotMatch(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*:first-child/,
'the `:first-child` gap reset is back. It cannot beat the heading rules on ' +
'specificity — use adjacent-sibling gap rules so the first block is never matched.',
);

// The other half of "the distance between two blocks is the value declared
// here": without it Astryx's own per-element end margins survive in the
// earlier layer and collapse against these gaps. An `hr` carries 12px in
// compact, so dropping the reset silently widens the block step after one.
assert.match(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\{[^}]*margin-block\s*:\s*0/,
'the blanket `> * { margin-block: 0 }` reset is gone. The gap rules only set ' +
'margin-block-START, so without it Astryx\'s end margins survive and collapse ' +
'against them — the declared ladder stops being the spacing you get.',
);

// Declaration and usage are separate failures: the ladder can stay ordered
// while the rules that spend it are hardcoded, which turns the table into
// decoration and the first test into a tautology.
for (const [, selector, body] of compactBlockRules) {
assert.match(
body,
/margin-block-start\s*:\s*var\(\s*--md-gap-[\w-]+\s*\)/,
'a compact gap rule sets a literal instead of a `--md-gap-*` variable. The ladder ' +
'test above only checks that the variables are declared in order; a hardcoded ' +
`value leaves it green while the rendered spacing ignores it. Rule: \`${selector.trim()}\` ` +
`{ ${body.trim()} }`,
);
}
});

it('keeps a typed `hr` wider than the block step', async () => {
const css = stripCssComments(await readAllRendererCss());
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\+\s*hr\s*,[^{]*\{([^}]*)\}`,
).exec(css);
assert.ok(
rule,
'the compact surface no longer gives `hr` its own step. An `hr` is the only break ' +
'the author typed by hand; left on the generic block gap it reads no wider than ' +
'the paragraph boundary above it, so writing one changes nothing.',
);

const used = /margin-block-start\s*:\s*var\(\s*(--md-gap-[\w-]+)\s*\)/.exec(rule[1])?.[1];
assert.ok(used, `the \`hr\` rule must spend a --md-gap-* variable. Found: { ${rule[1].trim()} }`);

const block = LADDER.indexOf('--md-gap-block');
const rung = LADDER.indexOf(used as (typeof LADDER)[number]);
assert.ok(
rung > block,
`\`hr\` spends ${used}, which is not above --md-gap-block on the ladder. Which rung ` +
'it takes is a design call — section today, chapter would be fine — but it has to ' +
'outrank the ordinary block step or the separator carries no meaning.',
);
});

it('spends the list-item row padding it re-spaces as prose', async () => {
const css = stripCssComments(await readAllRendererCss());
// Astryx's ListItem carries the control-row padding that inverted the
// ladder. Zeroing it is what makes --md-gap-list the whole distance between
// two items; leave it in and the gap variable understates the real spacing.
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list-item\s*\{([^}]*)\}`,
).exec(css);
assert.ok(rule, 'the compact surface no longer neutralizes `.astryx-list-item` padding');
assert.match(
rule[1],
/padding-block\s*:\s*0/,
'ListItem block padding must be zeroed on the compact surface, otherwise the real ' +
'list-item gap is padding + gap and the ladder above is not the spacing you get',
);

const listGap = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list\s*\{([^}]*)\}`,
).exec(css);
assert.ok(listGap, 'the compact surface no longer sets a `.astryx-list` row gap');
assert.match(
listGap[1],
/gap\s*:\s*var\(\s*--md-gap-list\s*\)/,
'the list gap must spend `--md-gap-list`. A literal here detaches the tightest rung ' +
'from the ladder the first test orders, so an inversion could be reintroduced ' +
'without failing anything',
);
});

it('keeps two heading size steps on the compact surface', async () => {
const css = stripCssComments(await readAllRendererCss());
const fonts = [
...css.matchAll(
/\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-markdown-heading[^{]*\{([^}]*)\}/g,
),
]
.map((m) => /font\s*:\s*var\(\s*(--maka-text-heading-\d)\s*\)/.exec(m[1])?.[1])
.filter((tier): tier is string => tier !== undefined);

assert.ok(
new Set(fonts).size >= 2,
'the transcript heading scale collapsed to one size tier. Flattening Astryx\'s ' +
'document ladder is deliberate, but flattening it to ZERO steps is the bug this ' +
'replaced: h2 and h3 then differ in nothing — not size, weight, or colour. ' +
`Found tiers: ${JSON.stringify(fonts)}`,
);
});
});
43 changes: 10 additions & 33 deletions apps/desktop/src/renderer/styles/chat-message.css
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,39 +212,16 @@
independent instead of keyed on a `[data-role="assistant"]` descendant. */

/* --- Conversation heading scale ------------------------------------------- */
/* Astryx's heading ladder (20/18/16/14…) is a DOCUMENT scale, and the
transcript is not a document. An agent turn emits `##` freely, so a ladder
that steps 1.4× per level shreds one reply into visually unrelated slabs.
Flatten it to two steps — h1 for a genuine section break, everything below
at body size — and let weight 600 plus colour carry the hierarchy, which is
what the rest of the conversation already does.
Cursor (1.43/1.32/1.21), Claude Code (1.14/1.07/1.0) and Codex
(1.71/1.43/1.21) all flatten here relative to their own document styles;
Claude Code goes furthest and merges h4–h6 into `strong`. Two steps sits
inside that range.
Every level shares the transcript baseline so headings do not add rhythm of
their own.
Scoped to `.maka-turn`, not to the Markdown contract alone. The contract is
emitted by the shared MarkdownBody, whose other caller is the Daily Review
panel — and a review report IS a document, so the argument above does not
apply to it. It keeps Astryx's full ladder.
Bare tags are the whole story: Astryx's Markdown renderer emits plain
h1–h6 with stylex props (Markdown.js `const Tag = \`h${level}\``). The
.astryx-heading.level-N classes come from the standalone Heading
component, which Markdown never mounts. */
/* The one heading in a turn that is not at the body size, and so the one that
cannot take the body baseline with it: at 16px, --maka-line-body's 20px is
on the 4px grid but is the 14px tier's line box, one step tighter than the
tier this heading actually sits at. It takes its own tier's leading (24px);
the turn still reads as one rhythm because 24 is the next step of the same
grid, not a different one. */
.maka-turn [data-maka-contract="markdown"] h1 {
font: var(--maka-text-heading-3);
}

.maka-turn [data-maka-contract="markdown"] :is(h2, h3, h4, h5, h6) {
font: var(--maka-text-heading-4);
}
/* Moved to the transcript rhythm table in packages/ui/src/styles.css. The
argument for flattening the document ladder is unchanged; what changed is
where it is expressed. These rules keyed on a `.maka-turn` ancestor, which
made the heading scale a function of DOM position: the same MarkdownBody
rendered document-sized headings in the Daily Review and flat ones in a
turn, and no Storybook story could reproduce the transcript without
remembering to wrap one. The rhythm table keys on Astryx's own
`data-density` instead, so the scale follows the `density` prop that already
distinguishes the two surfaces. It also keeps two size steps rather than
one — h2 and h3 used to be identical in size, weight and colour. */

.maka-chat-message pre:not([data-maka-contract="markdown"] pre) {
margin: 0;
Expand Down
141 changes: 141 additions & 0 deletions packages/ui/src/__tests__/markdown-rhythm-dom-contract.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
/**
* The DOM half of the transcript rhythm contract.
*
* The rhythm table in styles.css keys entirely on DOM that Astryx generates at
* runtime — `data-density` on the document root, `astryx-markdown-heading` plus
* `data-level` on headings, `astryx-list`/`astryx-list-item` inside a list.
* None of those names appear in Maka source as literals; they come from
* Astryx's `themeProps()`, whose prefix has a single upstream owner
* (`naming.ts`). So an Astryx bump that renames one of them makes every
* selector in the table silently stop matching, and the spacing regresses to
* the inverted ladder with no failing check anywhere: the CSS contract test
* (apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts) reads the
* stylesheet as text and stays green against DOM it never sees, and the
* `check-dead-css` entries are an allowlist that marks these classes live
* rather than asserting they exist.
*
* This pins the other half: the hooks the stylesheet selects on are really
* emitted. Together the two tests fail on either side of the join.
*/
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { readdir, readFile } from 'node:fs/promises';
import { join, relative, resolve } from 'node:path';
import { renderToStaticMarkup } from 'react-dom/server';
import { MarkdownBody } from '../markdown-body.js';

const UI_SRC = resolve(import.meta.dirname, '..', '..', 'src');

async function tsxFiles(dir: string): Promise<string[]> {
const out: string[] = [];
for (const entry of await readdir(dir, { withFileTypes: true })) {
const path = join(dir, entry.name);
if (entry.isDirectory()) out.push(...(await tsxFiles(path)));
else if (entry.name.endsWith('.tsx') && !entry.name.includes('.test.')) out.push(path);
}
return out;
}

const SAMPLE = ['## Heading two', '', 'A paragraph.', '', '1. First item', '2. Second item'].join('\n');

function compactMarkup(): string {
return renderToStaticMarkup(<MarkdownBody text={SAMPLE} density="compact" />);
}

describe('transcript markdown rhythm — DOM hooks', () => {
it('emits the density attribute the rhythm table scopes on', () => {
const markup = compactMarkup();
// Every rule in the table is prefixed with this, so it is the one hook whose
// two halves — selector prefix and runtime attribute — nothing else joins.
// Rename it and the CSS contract still passes on text it never renders.
assert.match(
markup,
/data-maka-contract="markdown"/,
'the `data-maka-contract="markdown"` wrapper is gone. Every rule in the rhythm ' +
'table is scoped on it, so all compact prose spacing and the heading scale are ' +
'now dead — and the stylesheet-side contract cannot see it.',
);
assert.match(
markup,
/<div[^>]*role="document"[^>]*data-density="compact"|<div[^>]*data-density="compact"[^>]*role="document"/,
'the document root no longer carries `data-density="compact"`. Every rule in the ' +
'rhythm table is scoped on it, so all compact prose spacing is now dead.',
);
assert.match(
markup,
/class="[^"]*\bastryx-markdown\b/,
'the document root no longer carries the `astryx-markdown` class the table selects on',
);
});

it('emits the heading hooks the two-step scale selects on', () => {
const markup = compactMarkup();
assert.match(
markup,
/<h2[^>]*class="[^"]*\bastryx-markdown-heading\b/,
'headings no longer carry `astryx-markdown-heading`; the transcript heading scale is dead',
);
assert.match(
markup,
/<h2[^>]*data-level="2"/,
'headings no longer carry `data-level`. The table splits h1/h2 from h3-h6 on it, so ' +
'without it every heading collapses to one tier — the exact defect this branch replaced.',
);
});

/**
* The rhythm table carries the transcript's heading typography, not just its
* spacing, and keys both on `density`. That is a deliberate bet, and it is a
* bet: Astryx's own density RFC (facebook/astryx#839) draws the line at
* "density shifts heights and spacing, not typography", so the key is
* honest only while `compact` and "transcript" are the same set. They are
* today — the transcript is the only caller that asks for compact, and the
* Daily Review renders a document at the default.
*
* A separate surface attribute would decouple them, but nothing needs it
* yet and it would add a prop to a public component for a caller that does
* not exist. Guard the assumption instead: the moment a second surface asks
* for compact markdown it silently inherits transcript heading sizes and
* dimmed deep headings, and this test is what tells whoever adds it that
* they have to choose — inherit deliberately, or split the key then.
*/
it('keeps compact markdown a transcript-only surface', async () => {
const callers: string[] = [];
for (const file of await tsxFiles(UI_SRC)) {
const source = await readFile(file, 'utf8');
// `<Markdown …>` opening tags only — not MarkdownBody's internal plumbing
// and not the density Maka hands its own code-block renderers.
for (const tag of source.matchAll(/<Markdown\s[^>]*?\/?>/gs)) {
if (/density=(["'])compact\1/.test(tag[0])) callers.push(relative(UI_SRC, file));
}
}

assert.deepEqual(
[...new Set(callers)].sort(),
['chat-turn.tsx'],
'a new caller renders markdown at compact density. The rhythm table treats ' +
'`density="compact"` as "this is a transcript" and gives it flattened heading sizes ' +
'plus secondary-colour h4-h6 — typography, which Astryx\'s density is explicitly not ' +
'supposed to carry (facebook/astryx#839). Either that is what the new surface wants, ' +
'and this list grows, or the typography rules need their own key. ' +
`Found: ${JSON.stringify([...new Set(callers)].sort())}`,
);
});

it('emits the list hooks whose control padding the table neutralizes', () => {
const markup = compactMarkup();
assert.match(
markup,
/class="[^"]*\bastryx-list\b/,
'the markdown list no longer renders through Astryx `List`. If it stopped being a ' +
'control list that is good news, but the padding-zeroing rule is now dead and the ' +
'list rhythm needs re-measuring rather than silently inheriting whatever replaced it.',
);
assert.match(
markup,
/class="[^"]*\bastryx-list-item\b/,
'list rows no longer carry `astryx-list-item`; the rule that spends their control-row ' +
'padding no longer applies and list items revert to sitting wider apart than paragraphs',
);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); fix(ui): restore the transcript's markdown rhythm by Astro-Han · Pull Request #2348 · apache/maka · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 199 additions & 0 deletions apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
/**
* Transcript markdown rhythm contract.
*
* The defect this pins was not a wrong number — it was a wrong ORDER. Under
* `density="compact"` the transcript spaced list items ~10px apart (Astryx's
* List control row padding, which `density` cannot reach from outside) while
* paragraphs sat 4px apart, so items at the same level read as further apart
* than separate paragraphs. Visual distance stopped tracking semantic
* distance.
*
* So the invariant is the ladder's ORDER, not its values: retuning 8px to 10px
* is a design decision and should stay green here; making list gaps meet or
* exceed block gaps is the regression, and must fail. A screenshot cannot lock
* that — it fixes one rendering of one sample rather than the relation — which
* is why AGENTS.md asks for a computed-style or text contract on cascade and
* layout invariants. The visual half lives in the `TranscriptTurn` story
* (packages/ui/stories/markdown.stories.tsx).
*
* Scoped to the compact surface only. Document mode is Astryx's own rhythm and
* the Daily Review renders through it, so nothing here should constrain it.
*/
import { strict as assert } from 'node:assert';
import { describe, it } from 'node:test';
import { readAllRendererCss, stripCssComments } from './css-test-helpers.js';

/** The compact ladder, ordered from tightest (same level) to widest (chapter). */
const LADDER = ['--md-gap-list', '--md-gap-block', '--md-gap-section', '--md-gap-chapter'] as const;

/** `var(--space-N)` → N, the 4px-grid step count. Non-scale values return null. */
function spaceSteps(value: string): number | null {
const match = /^var\(\s*--space-(\d+)\s*\)$/.exec(value.trim());
return match ? Number(match[1]) : null;
}

describe('transcript markdown rhythm', () => {
it('declares the compact ladder in strictly increasing order', async () => {
const css = stripCssComments(await readAllRendererCss());
const block = /\.astryx-markdown\[data-density="compact"\]\s*\{([^}]*)\}/.exec(css);
assert.ok(block, 'no `.astryx-markdown[data-density="compact"]` custom-property block found');

const declared = new Map<string, string>();
for (const m of block[1].matchAll(/(--md-gap-[\w-]+)\s*:\s*([^;]+);/g)) {
declared.set(m[1], m[2].trim());
}

const steps = LADDER.map((name) => {
const value = declared.get(name);
assert.ok(value, `${name} is not declared on the compact surface`);
const n = spaceSteps(value);
assert.ok(
n !== null,
`${name} is \`${value}\`; the ladder must stay on the --space-* scale so the ` +
'order is comparable and a bare px value cannot drift off the 4px grid',
);
return { name, steps: n };
});

for (let i = 1; i < steps.length; i += 1) {
const prev = steps[i - 1];
const cur = steps[i];
assert.ok(
cur.steps > prev.steps,
`${cur.name} (--space-${cur.steps}) must be strictly wider than ${prev.name} ` +
`(--space-${prev.steps}). Visual distance has to rise with semantic distance: ` +
'same-level list items closer than blocks, blocks closer than section breaks.',
);
}
});

it('expresses every block gap as an adjacent-sibling relation', async () => {
const css = stripCssComments(await readAllRendererCss());
const compactBlockRules = [
...css.matchAll(
/(\[data-maka-contract="markdown"\]\s*\.astryx-markdown\[data-density="compact"\]\s*>[^{]*)\{([^}]*)\}/g,
),
].filter(([, , body]) => /margin-block-start\s*:/.test(body));

assert.ok(compactBlockRules.length > 0, 'no compact block-gap rules found');

for (const [, selector, body] of compactBlockRules) {
assert.match(
selector,
/>[^{]*\+/,
'every block gap must be an adjacent-sibling rule (`> … + …`). A gap is a relation ' +
'between two blocks, so the first block should match no gap rule at all. The ' +
'`> *` + `> :first-child` reset form looks equivalent but loses on specificity: ' +
`:first-child scores (0,4,0) against the heading rules' (0,5,0), so a turn opening ` +
'with a heading keeps a chapter gap above its first line and pushes off the top of ' +
`the bubble. Offending rule: \`${selector.trim()}\` { ${body.trim()} }`,
);
}

assert.doesNotMatch(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*:first-child/,
'the `:first-child` gap reset is back. It cannot beat the heading rules on ' +
'specificity — use adjacent-sibling gap rules so the first block is never matched.',
);

// The other half of "the distance between two blocks is the value declared
// here": without it Astryx's own per-element end margins survive in the
// earlier layer and collapse against these gaps. An `hr` carries 12px in
// compact, so dropping the reset silently widens the block step after one.
assert.match(
css,
/\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\{[^}]*margin-block\s*:\s*0/,
'the blanket `> * { margin-block: 0 }` reset is gone. The gap rules only set ' +
'margin-block-START, so without it Astryx\'s end margins survive and collapse ' +
'against them — the declared ladder stops being the spacing you get.',
);

// Declaration and usage are separate failures: the ladder can stay ordered
// while the rules that spend it are hardcoded, which turns the table into
// decoration and the first test into a tautology.
for (const [, selector, body] of compactBlockRules) {
assert.match(
body,
/margin-block-start\s*:\s*var\(\s*--md-gap-[\w-]+\s*\)/,
'a compact gap rule sets a literal instead of a `--md-gap-*` variable. The ladder ' +
'test above only checks that the variables are declared in order; a hardcoded ' +
`value leaves it green while the rendered spacing ignores it. Rule: \`${selector.trim()}\` ` +
`{ ${body.trim()} }`,
);
}
});

it('keeps a typed `hr` wider than the block step', async () => {
const css = stripCssComments(await readAllRendererCss());
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\]\s*>\s*\*\s*\+\s*hr\s*,[^{]*\{([^}]*)\}`,
).exec(css);
assert.ok(
rule,
'the compact surface no longer gives `hr` its own step. An `hr` is the only break ' +
'the author typed by hand; left on the generic block gap it reads no wider than ' +
'the paragraph boundary above it, so writing one changes nothing.',
);

const used = /margin-block-start\s*:\s*var\(\s*(--md-gap-[\w-]+)\s*\)/.exec(rule[1])?.[1];
assert.ok(used, `the \`hr\` rule must spend a --md-gap-* variable. Found: { ${rule[1].trim()} }`);

const block = LADDER.indexOf('--md-gap-block');
const rung = LADDER.indexOf(used as (typeof LADDER)[number]);
assert.ok(
rung > block,
`\`hr\` spends ${used}, which is not above --md-gap-block on the ladder. Which rung ` +
'it takes is a design call — section today, chapter would be fine — but it has to ' +
'outrank the ordinary block step or the separator carries no meaning.',
);
});

it('spends the list-item row padding it re-spaces as prose', async () => {
const css = stripCssComments(await readAllRendererCss());
// Astryx's ListItem carries the control-row padding that inverted the
// ladder. Zeroing it is what makes --md-gap-list the whole distance between
// two items; leave it in and the gap variable understates the real spacing.
const rule = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list-item\s*\{([^}]*)\}`,
).exec(css);
assert.ok(rule, 'the compact surface no longer neutralizes `.astryx-list-item` padding');
assert.match(
rule[1],
/padding-block\s*:\s*0/,
'ListItem block padding must be zeroed on the compact surface, otherwise the real ' +
'list-item gap is padding + gap and the ladder above is not the spacing you get',
);

const listGap = new RegExp(
String.raw`\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-list\s*\{([^}]*)\}`,
).exec(css);
assert.ok(listGap, 'the compact surface no longer sets a `.astryx-list` row gap');
assert.match(
listGap[1],
/gap\s*:\s*var\(\s*--md-gap-list\s*\)/,
'the list gap must spend `--md-gap-list`. A literal here detaches the tightest rung ' +
'from the ladder the first test orders, so an inversion could be reintroduced ' +
'without failing anything',
);
});

it('keeps two heading size steps on the compact surface', async () => {
const css = stripCssComments(await readAllRendererCss());
const fonts = [
...css.matchAll(
/\.astryx-markdown\[data-density="compact"\][^{]*\.astryx-markdown-heading[^{]*\{([^}]*)\}/g,
),
]
.map((m) => /font\s*:\s*var\(\s*(--maka-text-heading-\d)\s*\)/.exec(m[1])?.[1])
.filter((tier): tier is string => tier !== undefined);

assert.ok(
new Set(fonts).size >= 2,
'the transcript heading scale collapsed to one size tier. Flattening Astryx\'s ' +
'document ladder is deliberate, but flattening it to ZERO steps is the bug this ' +
'replaced: h2 and h3 then differ in nothing — not size, weight, or colour. ' +
`Found tiers: ${JSON.stringify(fonts)}`,
);
});
});
43 changes: 10 additions & 33 deletions apps/desktop/src/renderer/styles/chat-message.css
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,39 +212,16 @@
independent instead of keyed on a `[data-role="assistant"]` descendant. */

/* --- Conversation heading scale ------------------------------------------- */
/* Astryx's heading ladder (20/18/16/14…) is a DOCUMENT scale, and the
transcript is not a document. An agent turn emits `##` freely, so a ladder
that steps 1.4× per level shreds one reply into visually unrelated slabs.
Flatten it to two steps — h1 for a genuine section break, everything below
at body size — and let weight 600 plus colour carry the hierarchy, which is
what the rest of the conversation already does.
Cursor (1.43/1.32/1.21), Claude Code (1.14/1.07/1.0) and Codex
(1.71/1.43/1.21) all flatten here relative to their own document styles;
Claude Code goes furthest and merges h4–h6 into `strong`. Two steps sits
inside that range.
Every level shares the transcript baseline so headings do not add rhythm of
their own.
Scoped to `.maka-turn`, not to the Markdown contract alone. The contract is
emitted by the shared MarkdownBody, whose other caller is the Daily Review
panel — and a review report IS a document, so the argument above does not
apply to it. It keeps Astryx's full ladder.
Bare tags are the whole story: Astryx's Markdown renderer emits plain
h1–h6 with stylex props (Markdown.js `const Tag = \`h${level}\``). The
.astryx-heading.level-N classes come from the standalone Heading
component, which Markdown never mounts. */
/* The one heading in a turn that is not at the body size, and so the one that
cannot take the body baseline with it: at 16px, --maka-line-body's 20px is
on the 4px grid but is the 14px tier's line box, one step tighter than the
tier this heading actually sits at. It takes its own tier's leading (24px);
the turn still reads as one rhythm because 24 is the next step of the same
grid, not a different one. */
.maka-turn [data-maka-contract="markdown"] h1 {
font: var(--maka-text-heading-3);
}

.maka-turn [data-maka-contract="markdown"] :is(h2, h3, h4, h5, h6) {
font: var(--maka-text-heading-4);
}
/* Moved to the transcript rhythm table in packages/ui/src/styles.css. The
argument for flattening the document ladder is unchanged; what changed is
where it is expressed. These rules keyed on a `.maka-turn` ancestor, which
made the heading scale a function of DOM position: the same MarkdownBody
rendered document-sized headings in the Daily Review and flat ones in a
turn, and no Storybook story could reproduce the transcript without
remembering to wrap one. The rhythm table keys on Astryx's own
`data-density` instead, so the scale follows the `density` prop that already
distinguishes the two surfaces. It also keeps two size steps rather than
one — h2 and h3 used to be identical in size, weight and colour. */

.maka-chat-message pre:not([data-maka-contract="markdown"] pre) {
margin: 0;
Expand Down
141 changes: 141 additions & 0 deletions packages/ui/src/__tests__/markdown-rhythm-dom-contract.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
/**
* The DOM half of the transcript rhythm contract.
*
* The rhythm table in styles.css keys entirely on DOM that Astryx generates at
* runtime — `data-density` on the document root, `astryx-markdown-heading` plus
* `data-level` on headings, `astryx-list`/`astryx-list-item` inside a list.
* None of those names appear in Maka source as literals; they come from
* Astryx's `themeProps()`, whose prefix has a single upstream owner
* (`naming.ts`). So an Astryx bump that renames one of them makes every
* selector in the table silently stop matching, and the spacing regresses to
* the inverted ladder with no failing check anywhere: the CSS contract test
* (apps/desktop/src/main/__tests__/markdown-rhythm-contract.test.ts) reads the
* stylesheet as text and stays green against DOM it never sees, and the
* `check-dead-css` entries are an allowlist that marks these classes live
* rather than asserting they exist.
*
* This pins the other half: the hooks the stylesheet selects on are really
* emitted. Together the two tests fail on either side of the join.
*/
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { readdir, readFile } from 'node:fs/promises';
import { join, relative, resolve } from 'node:path';
import { renderToStaticMarkup } from 'react-dom/server';
import { MarkdownBody } from '../markdown-body.js';

const UI_SRC = resolve(import.meta.dirname, '..', '..', 'src');

async function tsxFiles(dir: string): Promise<string[]> {
const out: string[] = [];
for (const entry of await readdir(dir, { withFileTypes: true })) {
const path = join(dir, entry.name);
if (entry.isDirectory()) out.push(...(await tsxFiles(path)));
else if (entry.name.endsWith('.tsx') && !entry.name.includes('.test.')) out.push(path);
}
return out;
}

const SAMPLE = ['## Heading two', '', 'A paragraph.', '', '1. First item', '2. Second item'].join('\n');

function compactMarkup(): string {
return renderToStaticMarkup(<MarkdownBody text={SAMPLE} density="compact" />);
}

describe('transcript markdown rhythm — DOM hooks', () => {
it('emits the density attribute the rhythm table scopes on', () => {
const markup = compactMarkup();
// Every rule in the table is prefixed with this, so it is the one hook whose
// two halves — selector prefix and runtime attribute — nothing else joins.
// Rename it and the CSS contract still passes on text it never renders.
assert.match(
markup,
/data-maka-contract="markdown"/,
'the `data-maka-contract="markdown"` wrapper is gone. Every rule in the rhythm ' +
'table is scoped on it, so all compact prose spacing and the heading scale are ' +
'now dead — and the stylesheet-side contract cannot see it.',
);
assert.match(
markup,
/<div[^>]*role="document"[^>]*data-density="compact"|<div[^>]*data-density="compact"[^>]*role="document"/,
'the document root no longer carries `data-density="compact"`. Every rule in the ' +
'rhythm table is scoped on it, so all compact prose spacing is now dead.',
);
assert.match(
markup,
/class="[^"]*\bastryx-markdown\b/,
'the document root no longer carries the `astryx-markdown` class the table selects on',
);
});

it('emits the heading hooks the two-step scale selects on', () => {
const markup = compactMarkup();
assert.match(
markup,
/<h2[^>]*class="[^"]*\bastryx-markdown-heading\b/,
'headings no longer carry `astryx-markdown-heading`; the transcript heading scale is dead',
);
assert.match(
markup,
/<h2[^>]*data-level="2"/,
'headings no longer carry `data-level`. The table splits h1/h2 from h3-h6 on it, so ' +
'without it every heading collapses to one tier — the exact defect this branch replaced.',
);
});

/**
* The rhythm table carries the transcript's heading typography, not just its
* spacing, and keys both on `density`. That is a deliberate bet, and it is a
* bet: Astryx's own density RFC (facebook/astryx#839) draws the line at
* "density shifts heights and spacing, not typography", so the key is
* honest only while `compact` and "transcript" are the same set. They are
* today — the transcript is the only caller that asks for compact, and the
* Daily Review renders a document at the default.
*
* A separate surface attribute would decouple them, but nothing needs it
* yet and it would add a prop to a public component for a caller that does
* not exist. Guard the assumption instead: the moment a second surface asks
* for compact markdown it silently inherits transcript heading sizes and
* dimmed deep headings, and this test is what tells whoever adds it that
* they have to choose — inherit deliberately, or split the key then.
*/
it('keeps compact markdown a transcript-only surface', async () => {
const callers: string[] = [];
for (const file of await tsxFiles(UI_SRC)) {
const source = await readFile(file, 'utf8');
// `<Markdown …>` opening tags only — not MarkdownBody's internal plumbing
// and not the density Maka hands its own code-block renderers.
for (const tag of source.matchAll(/<Markdown\s[^>]*?\/?>/gs)) {
if (/density=(["'])compact\1/.test(tag[0])) callers.push(relative(UI_SRC, file));
}
}

assert.deepEqual(
[...new Set(callers)].sort(),
['chat-turn.tsx'],
'a new caller renders markdown at compact density. The rhythm table treats ' +
'`density="compact"` as "this is a transcript" and gives it flattened heading sizes ' +
'plus secondary-colour h4-h6 — typography, which Astryx\'s density is explicitly not ' +
'supposed to carry (facebook/astryx#839). Either that is what the new surface wants, ' +
'and this list grows, or the typography rules need their own key. ' +
`Found: ${JSON.stringify([...new Set(callers)].sort())}`,
);
});

it('emits the list hooks whose control padding the table neutralizes', () => {
const markup = compactMarkup();
assert.match(
markup,
/class="[^"]*\bastryx-list\b/,
'the markdown list no longer renders through Astryx `List`. If it stopped being a ' +
'control list that is good news, but the padding-zeroing rule is now dead and the ' +
'list rhythm needs re-measuring rather than silently inheriting whatever replaced it.',
);
assert.match(
markup,
/class="[^"]*\bastryx-list-item\b/,
'list rows no longer carry `astryx-list-item`; the rule that spends their control-row ' +
'padding no longer applies and list items revert to sitting wider apart than paragraphs',
);
});
});
Loading
Loading