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
97 changes: 97 additions & 0 deletions apps/desktop/src/main/__tests__/markdown-prose-contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,3 +408,100 @@ describe('TABLE-A11Y-SEMANTICS-0 contract (#618 item 5)', () => {
assert.match(wrapper!.decls, /max-width:\s*100%/, 'the wrapper caps at the prose measure so wide tables scroll instead of stretching the bubble');
});
});

describe('MARKDOWN-PROSE-HEADING-HIERARCHY-0 contract (#739)', () => {
// Issue #739: the #546 Phase B ladder (1.4615 / 1.2308 / 1.0769 / 1) chased
// integer px targets at the 13px base, but h3 (1.0769em, +8% over body) was
// too close to body copy — long answers relied on weight alone to
// distinguish H3 sections. #739 re-pins the ladder to a uniform 0.15em
// step (1.45 / 1.30 / 1.15 / 1): each tier clears the one below by a fixed
// 0.15em increment (≈2px at the 13px base; the relative gap widens per tier
// — +15% over body for h3, +13% over h3 for h2, +12% over h2 for h1),
// CDP-verified as the minimum distinguishable step.
//
// This contract locks the STEP, not specific em values: a future patch
// may widen the step (e.g. 0.20em for a roomier ladder), but must not
// narrow it back to the #546 shape where h3-h4 = 0.08em and hierarchy
// collapses back onto weight. h4 stays at 1em (GitHub's 1em h4
// convention: a heading via weight + secondary color, not size).
//
// Source of truth: issue #739 acceptance ("H2/H3 sections clearly
// distinguishable from body copy") + CDP comparison of +8% / +15% / +23%
// h3 tiers — the 0.15em step was the smallest gap that read as a section.
const HEADING_STEP_EM = 0.15;

const headingEm = (blocks: ReturnType<typeof cssBlocks>, sel: string): number => {
const b = blocks.find(({ selectors }) => selectors === sel);
assert.ok(b, `expected ${sel} rule in prose.css`);
const m = b!.decls.match(/font-size:\s*([\d.]+)em/);
assert.ok(m, `expected a font-size em declaration on ${sel}; got ${b!.decls}`);
return parseFloat(m![1]);
};

it('each heading tier clears the one below by >=0.15em so hierarchy reads by size, not weight alone', async () => {
const css = stripCssComments(await readFile(PROSE_CSS, 'utf8'));
const blocks = cssBlocks(css);
const h1 = headingEm(blocks, '.maka-prose h1');
const h2 = headingEm(blocks, '.maka-prose h2');
const h3 = headingEm(blocks, '.maka-prose h3');
const h4 = headingEm(blocks, '.maka-prose h4');
// Compare via toFixed(4) — exact enough to reject a 0.1451em gap, but
// robust to float noise (1.15 - 1.0 = 0.14999... rounds to 0.1500). A
// centi-em Math.round(x*100) check would let 1.1451em through (115 vs
// 100, gap "15"), understating the threshold.
const gap = (a: number, b: number) => parseFloat((a - b).toFixed(4));
assert.equal(h4, 1, 'h4 sits at 1em and reads as a heading via weight + secondary color (GitHub 1em h4 convention)');
assert.ok(gap(h3, h4) >= HEADING_STEP_EM, `h3 (${h3}em) must clear body by >=${HEADING_STEP_EM}em so it reads as a section, not body; got ${gap(h3, h4)}em (the #546 ladder's 0.0769em gap was the #739 defect)`);
assert.ok(gap(h2, h3) >= HEADING_STEP_EM, `h2 (${h2}em) must clear h3 by >=${HEADING_STEP_EM}em; got ${gap(h2, h3)}em`);
assert.ok(gap(h1, h2) >= HEADING_STEP_EM, `h1 (${h1}em) must clear h2 by >=${HEADING_STEP_EM}em; got ${gap(h1, h2)}em`);
});
});

describe('MARKDOWN-PROSE-RENDER-OWNER-0 contract (#739)', () => {
// Streamdown's default components tag every markdown element with Tailwind
// utility classes (h1 "text-3xl", h3 "text-xl", th/td "px-4 py-2 text-sm",
// thead "bg-muted/80", tbody "divide-y", blockquote "border-l-4", ul
// "list-disc", ...). Those sit in the `utilities` cascade layer and override
// prose.css's `components`-layer markdown rules, so the .maka-prose layer
// never reached the rendered DOM — the heading ladder and table padding
// declared in prose.css were silently overwritten (#739). markdown-body.tsx
// renders bare semantic elements via bareElement so prose.css owns all
// markdown typography. This contract locks the bare override for the
// heading + table-structure elements so a future patch does not silently
// drop it and let Streamdown's utilities back in (which would re-break the
// heading ladder and table fit the moment Tailwind generates those classes
// from elsewhere in the codebase).
const BARE_OVERRIDDEN = ['h1', 'h2', 'h3', 'h4', 'blockquote', 'ul', 'li', 'thead', 'tbody', 'tr', 'th', 'td'] as const;
const NOT_OVERRIDDEN = ['p', 'ol', 'section', 'h5', 'h6'] as const;

it('markdown-body overrides heading + table-structure elements with bareElement so prose.css reaches the DOM', async () => {
const src = await readFile(MARKDOWN_BODY, 'utf8');
assert.match(src, /function bareElement/, 'markdown-body.tsx must define a bareElement helper that strips Streamdown utility classes so prose.css reaches the rendered markdown (#739)');
for (const tag of BARE_OVERRIDDEN) {
assert.match(
src,
new RegExp(`\\b${tag}:\\s*bareElement\\(['"]${tag}['"]\\)`),
`markdown-body.tsx components prop must override ${tag} with bareElement('${tag}') so prose.css rules reach the rendered ${tag} instead of Streamdown's Tailwind utilities`,
);
}
});

it('markdown-body does NOT override p/ol/section/h5/h6 (functional logic or no prose.css rule)', async () => {
const src = await readFile(MARKDOWN_BODY, 'utf8');
for (const tag of NOT_OVERRIDDEN) {
assert.ok(
!new RegExp(`\\b${tag}:\\s*bareElement\\(['"]${tag}['"]\\)`).test(src),
`markdown-body.tsx must NOT override ${tag} with bareElement — p/ol/section carry Streamdown functional logic (p unwraps a lone image; ol/section clean streaming footnotes), and h5/h6 have no prose.css rule so stripping would lose Streamdown's heading styling`,
);
}
});

it('Daily Review section-body carries .maka-prose so heading/table get a prose-css owner outside the chat bubble', async () => {
const src = await readFile(resolve(REPO_ROOT, 'packages', 'ui', 'src', 'daily-review-panel.tsx'), 'utf8');
assert.match(
src,
/maka-daily-review-archive-section-body[^"]* maka-prose|maka-prose[^"]* maka-daily-review-archive-section-body/,
'daily-review-panel.tsx must put .maka-prose on the section-body div so heading/table/blockquote get prose.css styling — they have no Streamdown utility fallback now that markdown-body strips utilities, and Daily Review has no chat-bubble .maka-prose ancestor (#739)',
);
});
});
27 changes: 18 additions & 9 deletions apps/desktop/src/renderer/styles/prose.css
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,24 @@
.maka-prose h2 {
letter-spacing: var(--tracking-normal);
}
/* Heading ladder re-pinned for the 13px body base (#546 Phase B): the old
em values were derived for 15px (22/19/16/14) and landed on fractional
pixels at 13 (19.07/16.47/13.87/12.13), with h3 only 0.87px above body
and h4 *below* body. Integer targets now: 19 / 16 / 14 / 13. h4 sits at
1em and reads as a heading via weight + secondary color (same convention
as GitHub's 1em h4). */
.maka-prose h1 { font-size: 1.4615em; }
.maka-prose h2 { font-size: 1.2308em; }
.maka-prose h3 { font-size: 1.0769em; }
/* Heading ladder pinned to a 0.15em step for the 13px body base (#739).
#546 Phase B derived the ladder from the old 15px scale and chased
integer px targets 19 / 16 / 14 / 13 — but at the 13px body base h3
(1.0769em, +8% over body) was too close to body copy, so long answers
relied on weight alone to distinguish H3 sections (#739).
Re-pinned to a uniform 0.15em step (1.45 / 1.30 / 1.15 / 1) so each
tier clears the one below by a fixed 0.15em step (≈2px at the 13px
body base; the relative gap widens per tier — +15% over body for h3,
+13% over h3 for h2, +12% over h2 for h1), CDP-verified as the minimum
distinguishable step — and hierarchy reads by size, not weight alone.
The em values are design ratios (not px/13 rounding), so a future
body-base change does not require re-pinning the ladder the way the
#546 / #616 integer-px chase did. h4 stays at 1em and reads as a heading
via weight + secondary color (GitHub's 1em h4 convention).
MARKDOWN-PROSE-HEADING-HIERARCHY-0 locks the step. */
.maka-prose h1 { font-size: 1.45em; }
.maka-prose h2 { font-size: 1.30em; }
.maka-prose h3 { font-size: 1.15em; }
.maka-prose h4 { font-size: 1em; color: var(--foreground-secondary); }
.maka-prose ul,
.maka-prose ol {
Expand Down
14 changes: 14 additions & 0 deletions packages/ui/src/__tests__/markdown-body.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,17 @@ it('preserves allowlisted Maka navigation links through sanitization', () => {
assert.match(markup, /data-maka-uri-kind="settings"/);
assert.doesNotMatch(markup, /Blocked URL/);
});

it('preserves GFM task-list HAST classes so prose.css task-list rules match (#739 round-2 guard)', () => {
const markup = renderToStaticMarkup(createElement(MarkdownBody, {
text: '- [x] done\n- [ ] todo',
}));
// bareElement must preserve the HAST className react-markdown forwards
// (remark-gfm's contains-task-list / task-list-item). Dropping it — the
// round-2 regression — makes prose.css's `.maka-prose ul.contains-task-list`
// rules stop matching, so task-list checkboxes fall back to the UA default
// and list markers reappear. This render assertion locks the HAST-className
// contract that the text-shape contract alone cannot catch.
assert.match(markup, /class="contains-task-list"/, 'bareElement must preserve the HAST className remark-gfm sets on the task-list <ul>; dropping it (round-2 regression) makes prose.css .maka-prose ul.contains-task-list rules stop matching');
assert.match(markup, /class="task-list-item"/, 'bareElement must preserve the HAST className remark-gfm sets on task-list <li> items');
});
2 changes: 1 addition & 1 deletion packages/ui/src/daily-review-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ function DailyReviewArchiveBody(props: { archive: DailyReviewArchive | null; loa
{/* Reports are LLM-generated markdown — bullet lists and
inline code rendered as flat pre-wrap text read as mush.
Reuse the shared Markdown pipeline (same one chat uses). */}
<div className="maka-daily-review-archive-section-body">
<div className="maka-daily-review-archive-section-body maka-prose">
<Markdown text={section.content} />
</div>
</section>
Expand Down
58 changes: 56 additions & 2 deletions packages/ui/src/markdown-body.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import { useContext, type ReactNode } from 'react';
import * as React from 'react';
import { defaultRehypePlugins, defaultRemarkPlugins, Streamdown } from 'streamdown';
import { defaultRehypePlugins, defaultRemarkPlugins, Streamdown, type ExtraProps } from 'streamdown';
import rehypeHighlight from 'rehype-highlight';
import remarkBreaks from 'remark-breaks';
import { Check, Copy } from './icons.js';
Expand Down Expand Up @@ -61,6 +61,34 @@ const MARKDOWN_REHYPE_PLUGINS = [
],
];

/**
* Streamdown's default components merge Tailwind utility classes into every
* markdown element (h1 "text-3xl", h3 "text-xl", th/td "px-4 py-2 text-sm",
* thead "bg-muted/80", ul "list-disc", ...) via an internal `r(utility, t)`
* call. Those utilities sit in the `utilities` cascade layer and override
* prose.css's `components`-layer markdown rules, so the .maka-prose layer
* never reaches the rendered DOM (#739: the heading ladder and table padding
* declared in prose.css were silently overwritten).
*
* A `components` override REPLACES the default component, so Streamdown's
* internal utility merge never runs. The `className` react-markdown forwards
* here is the HAST node's semantic class (remark-gfm's `contains-task-list`/
* `task-list-item`, rehype-highlight's `language-*`) — NOT Streamdown's
* utilities. So render the element with its HAST className preserved and only
* `node` (the AST node, which would otherwise leak to the DOM as
* `node="[object Object]"`) dropped. prose.css then styles the bare element.
*
* Only apply this to elements whose ONLY Streamdown default is the utility
* merge — p, ol, and section carry functional logic (p unwraps a lone image,
* ol/section clean streaming footnotes) and must stay on Streamdown's default
* renderer; h5/h6 have no prose.css rule and would lose all heading styling
* if stripped. See the components prop below for the exact override set.
*/
function bareElement<K extends keyof React.JSX.IntrinsicElements>(Tag: K) {
return ({ node: _node, children, ...rest }: React.JSX.IntrinsicElements[K] & ExtraProps) =>
React.createElement(Tag, rest, children);
}

export function MarkdownBody(props: { text: string; streaming?: boolean }) {
return (
<Streamdown
Expand All @@ -87,7 +115,10 @@ export function MarkdownBody(props: { text: string; streaming?: boolean }) {
</MarkdownLink>
),
// Inline `code` keeps the bubble's foreground color; only block code
// gets the framed treatment via `pre > code` in CSS.
// gets the framed treatment via `pre > code` in CSS. react-markdown
// forwards the HAST className here (inline code has none; block code
// carries `language-*` from rehype-highlight), so passing it through
// styles block code for hljs and leaves inline code to prose.css.
code: ({ children, className, ...rest }) => (
<code {...rest} className={className}>
{children}
Expand All @@ -107,6 +138,29 @@ export function MarkdownBody(props: { text: string; streaming?: boolean }) {
<table {...rest}>{children}</table>
</div>
),
// #739: render bare heading + table-structure + list elements so
// prose.css (heading ladder, frameless table, cell padding, task-list)
// actually applies — Streamdown's default components tag them with
// Tailwind utilities in the `utilities` layer that override the
// `components`-layer prose.css rules. bareElement preserves the HAST
// className (task-list, language-*) so prose.css's `.maka-prose
// ul.contains-task-list` rules still match. p/ol/section are NOT
// overridden — Streamdown's default p unwraps a lone image, and
// ol/section clean streaming footnotes; stripping them breaks that
// logic. h5/h6 are NOT overridden — prose.css has no rule for them, so
// stripping would lose Streamdown's heading styling. See bareElement.
h1: bareElement('h1'),
h2: bareElement('h2'),
h3: bareElement('h3'),
h4: bareElement('h4'),
blockquote: bareElement('blockquote'),
ul: bareElement('ul'),
li: bareElement('li'),
thead: bareElement('thead'),
tbody: bareElement('tbody'),
tr: bareElement('tr'),
th: bareElement('th'),
td: bareElement('td'),
}}
>
{props.text}
Expand Down
Loading