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
78 changes: 78 additions & 0 deletions apps/desktop/e2e/markdown-defer.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { test, expect, type ElectronApplication, type Page } from "@playwright/test";
import fs from "node:fs";
import path from "node:path";
import { items, launch, seedHome, tid } from "./support";

/**
* Long threads parse only the Markdown near the view (`DeferMarkdown`, Markdown.tsx): the newest
* turns at once, older answers as written until they scroll near, then parsed for good. Parsing
* above the view must not move what the reader is looking at.
*/
let app: ElectronApplication | undefined;
let home = "", repo = "";

test.afterEach(async () => {
await app?.close().catch(() => {});
app = undefined;
for (const dir of [home, repo]) if (dir) fs.rmSync(dir, { recursive: true, force: true });
});

const TURNS = 40;

async function openLongThread(): Promise<Page> {
({ home, repo } = seedHome());
const threadId = "long";
const at = new Date(Date.now() - TURNS * 60_000).toISOString();
const saved: Record<string, unknown>[] = [];
for (let turn = 0; turn < TURNS; turn++) {
const origin = { thread: threadId, turn: `u-${turn}`, attempt: `a-${turn}`, backend: "codex", native: { runtime: "R", thread: "nt", turn: `nu-${turn}`, item: `f-${turn}` } };
saved.push({ id: `u-${turn}`, kind: "user", text: `Request ${turn}`, at });
saved.push({ id: `f-${turn}`, kind: "assistant", text: `Answer **${turn}**.\n\n${"A paragraph of explanation that wraps. ".repeat(12)}\n\n\`\`\`ts\nexport const value${turn} = ${turn};\n\`\`\``,
phase: "final", nativePhase: "final_answer", outcome: "succeeded", origin, at });
}
fs.writeFileSync(path.join(home, "app", "threads", `${threadId}.json`), JSON.stringify(saved));
const statePath = path.join(home, "app", "state.json");
const state = JSON.parse(fs.readFileSync(statePath, "utf8"));
const now = new Date().toISOString();
state.threads = [{ id: threadId, projectId: "p1", title: "Long", titleSource: "manual", createdAt: now, updatedAt: now, cwd: repo, backend: "codex", mode: "chat", plan: false, model: "", status: "idle" }];
fs.writeFileSync(statePath, JSON.stringify(state));
let page: Page;
({ app, page } = await launch(home));
await page.locator(`[data-testid="thread-row"][data-thread-id="${threadId}"] button.thread-main`).click();
await expect(items(page, "assistant")).toHaveCount(TURNS);
return page;
}

test("a long thread parses its newest answers at once and older ones as they near the view", async () => {
const page = await openLongThread();
const answers = items(page, "assistant");
// The newest answer is parsed Markdown, not text as written.
await expect(answers.last().locator("strong")).toHaveText(String(TURNS - 1));
await expect(answers.last().locator("[data-markdown-deferred]")).toHaveCount(0);
// The oldest is far above the view: its text is all there, as written, until it nears.
const first = answers.first();
await expect(first.locator("[data-markdown-deferred]")).toHaveCount(1);
await expect(first).toContainText("Answer **0**.");
// Scrolling to the top parses it, and it stays parsed after scrolling back down.
await tid(page, "transcript").evaluate((el) => { el.scrollTop = 0; });
await expect(first.locator("strong")).toHaveText("0");
await expect(first.locator("pre code")).toContainText("export const value0 = 0;");
await tid(page, "transcript").evaluate((el) => { el.scrollTop = el.scrollHeight; });
await expect(first.locator("[data-markdown-deferred]")).toHaveCount(0);
});

test("answers parsing above the view do not move the answer being read", async () => {
const page = await openLongThread();
const answers = items(page, "assistant");
// Bring a still-deferred middle answer into view; the answers around it parse as it arrives.
const middle = answers.nth(Math.floor(TURNS / 3));
await expect(middle.locator("[data-markdown-deferred]")).toHaveCount(1);
// Its position is read in the same task as the scroll, before any of those parses run.
const before = await middle.evaluate((el) => { el.scrollIntoView({ block: "start" }); return el.getBoundingClientRect().top; });
await expect(middle.locator("[data-markdown-deferred]")).toHaveCount(0);
// The answers above it parse too, and grow; the reader's answer stays where it was.
await expect(answers.nth(Math.floor(TURNS / 3) - 3).locator("[data-markdown-deferred]")).toHaveCount(0);
await page.waitForTimeout(300);
const after = await middle.evaluate((el) => el.getBoundingClientRect().top);
expect(Math.abs(after - before), `moved from ${before} to ${after}`).toBeLessThanOrEqual(2);
});
3 changes: 2 additions & 1 deletion apps/desktop/e2e/transcript-measure.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ async function measureTranscript(testInfo: TestInfo, launchNumber: number): Prom
// Item markup both this build and the pre-#273 baseline share, so the same probe measures either.
const answers = transcript ? [...transcript.querySelectorAll<HTMLElement>('[data-testid="item"][data-item-kind="assistant"]')] : [];
const last = answers.at(-1);
if (last?.textContent?.includes(marker)) {
// Its Markdown must be parsed, not shown as written while deferred (DeferMarkdown).
if (last?.textContent?.includes(marker) && !last.querySelector("[data-markdown-deferred]")) {
const box = last.getBoundingClientRect(), view = transcript!.getBoundingClientRect();
if (box.top < view.bottom && box.bottom > view.top) return performance.now() - start;
}
Expand Down
25 changes: 24 additions & 1 deletion apps/desktop/src/renderer/components/Markdown.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { cloneElement, isValidElement, memo, useEffect, useState, type ReactNode } from "react";
import { cloneElement, createContext, isValidElement, memo, useContext, useEffect, useRef, useState, type ReactNode, type RefObject } from "react";
import type { Definition, ListItem, Nodes, Parents, PhrasingContent, RootContent, TableRow } from "mdast";
import { fromMarkdown } from "mdast-util-from-markdown";
import { gfmFromMarkdown } from "mdast-util-gfm";
Expand All @@ -22,6 +22,20 @@ export const Markdown = memo(function Markdown({ text }: { text: string }) {
const timer = setTimeout(() => setSettled(text), STREAM_SETTLE_MS);
return () => clearTimeout(timer);
}, [text, settled]);
// Far from the newest turns, text shows as written until it nears the transcript's view, then
// parses once and stays parsed. Text parsed earlier in this session renders at once.
const root = useContext(DeferMarkdown);
const [near, setNear] = useState(() => !root || settledRenders.has(text));
const deferred = useRef<HTMLDivElement>(null);
useEffect(() => {
const el = deferred.current;
if (near || !root || !el) return;
const observer = new IntersectionObserver((entries) => { if (entries.some((entry) => entry.isIntersecting)) setNear(true); },
{ root: root.current, rootMargin: DEFER_MARGIN });
observer.observe(el);
return () => observer.disconnect();
}, [near, root]);
if (!near) return <div className="md md-deferred" ref={deferred} data-markdown-deferred="">{text}</div>;
if (text === settled) return <div className="md">{settledBlocks(text)}</div>;
// Keep the same top-level React structure during streaming and settling. Wrapping chunks
// in component boundaries remounted every code block on the first delta and at settlement,
Expand All @@ -34,6 +48,14 @@ export const Markdown = memo(function Markdown({ text }: { text: string }) {
/** How long text must stop changing before it is parsed whole again. */
const STREAM_SETTLE_MS = 300;

/**
* Provided around transcript turns far from the newest, with the transcript's scroller: Markdown
* inside parses only once it comes within `DEFER_MARGIN` of that view. Opening a long thread then
* parses what is on screen, not every answer above it.
*/
export const DeferMarkdown = createContext<RefObject<HTMLElement | null> | null>(null);
const DEFER_MARGIN = "1500px 0px";

function render(text: string): ReactNode[] {
const tree = fromMarkdown(text, { extensions: [gfm()], mdastExtensions: [gfmFromMarkdown()] });
const definitions = new Map<string, Definition>();
Expand All @@ -46,6 +68,7 @@ export class RenderCache {
private readonly kept = new Map<string, unknown>();
constructor(private readonly entries: number, private readonly maxChars: number) {}
get size(): number { return this.kept.size; }
has(text: string): boolean { return this.kept.has(text); }
get<T>(text: string, make: () => T): T {
if (text.length > this.maxChars) return make();
if (this.kept.has(text)) {
Expand Down
7 changes: 6 additions & 1 deletion apps/desktop/src/renderer/components/ThreadView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type { Attachment } from "../../shared/attachments";
import { bridge } from "../bridge";
import { AttachmentChips } from "./Attachments";
import { Composer } from "./Composer";
import { Markdown } from "./Markdown";
import { DeferMarkdown, Markdown } from "./Markdown";
import { Icon } from "./ui/Icon";
import { ErrorNotice } from "./ui/ErrorNotice";
import { TurnContent, groupTurns } from "./transcript/TurnContent";
Expand All @@ -26,6 +26,9 @@ import { PatchCompare, TranscriptThread } from "./transcript/PatchCompare";

export { formatElapsed } from "./transcript/TurnContent";

/** The newest turns parse their Markdown at once; older ones when they near the view (`DeferMarkdown`). */
const EAGER_TURNS = 8;

interface Props {
thread: Thread;
project: Project;
Expand Down Expand Up @@ -219,9 +222,11 @@ export function ThreadView({ thread, project, items, text, onText, attachments,
</div>
)}
{turns.map((turn, i) => (
<DeferMarkdown.Provider key={turn.user?.id ?? `pre-${i}`} value={i < turns.length - EAGER_TURNS ? scroller : null}>
<TurnContent key={turn.user?.id ?? `pre-${i}`} turn={turn} status={i === lastUser && busy ? thread.status : undefined}
footer={(t) => <><TaskNoPlanLine items={t.rest} /><TaskVerificationLine items={t.rest} /><TaskContextLine items={t.rest} cwd={thread.cwd} /><SaveTaskNote threadId={thread.id} turnId={t.user!.id} saved={Boolean(thread.taskNotes?.[t.user!.id])} /></>}
render={(item, live) => <Item key={item.id} item={item} live={live} onAnswer={onAnswer} onAnswerQuestion={onAnswerQuestion} onPolicy={(approvals) => onUpdate({ approvals })} onFix={onFix} onRetry={!thread.retired && !busy && item.id === retryId ? onRetry : undefined} onReply={!thread.retired && !busy && item.id === askId ? (text) => void onSend(text) : undefined} />} />
</DeferMarkdown.Provider>
))}
</div>
</TranscriptThread.Provider>
Expand Down
2 changes: 2 additions & 0 deletions apps/desktop/src/renderer/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,8 @@ a.thread-pr:hover { text-decoration: underline; }
.msg.user { display: flex; flex-direction: column; align-items: flex-end; }
.msg.user .bubble { max-width: 70%; padding: 8px 16px; border: 1px solid var(--bubble-border); border-radius: 12px; background: var(--bg-user-bubble); white-space: pre-wrap; overflow-wrap: anywhere; }
.md p { margin: 0 0 12px; }
/* Text as written until it nears the view and parses (Markdown.tsx, DeferMarkdown). */
.md-deferred { white-space: pre-wrap; overflow-wrap: anywhere; }
.md h2, .md h3, .md h4, .md h5, .md h6 { margin: 18px 0 8px; font-weight: var(--weight-semibold); }
.md h5, .md h6 { font-size: var(--text-md); }
.md ul, .md ol { margin: 0 0 12px; padding-left: 28px; }
Expand Down
16 changes: 15 additions & 1 deletion apps/desktop/test/markdown-render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ test("rendering a reply piece by piece shows the same text as rendering it whole

test("the settled cache keeps recent renders, drops the oldest past its limit, and never holds very long text", async () => {
await render("");
const { RenderCache } = await server!.ssrLoadModule("/src/renderer/components/Markdown.tsx") as { RenderCache: new (entries: number, maxChars: number) => { get<T>(text: string, make: () => T): T; size: number } };
const { RenderCache } = await server!.ssrLoadModule("/src/renderer/components/Markdown.tsx") as { RenderCache: new (entries: number, maxChars: number) => { get<T>(text: string, make: () => T): T; has(text: string): boolean; size: number } };
const cache = new RenderCache(2, 10);
let made = 0;
const make = () => ++made;
Expand All @@ -153,6 +153,8 @@ test("the settled cache keeps recent renders, drops the oldest past its limit, a
cache.get("a", make); // "a" is now the most recent
cache.get("c", make); // past the limit: "b", the least recent, goes
assert.equal(cache.size, 2);
assert.equal(cache.has("b"), false, "a dropped text is not reported as kept");
assert.equal(cache.has("c"), true);
assert.equal(cache.get("a", make), 1);
assert.equal(cache.get("b", make), 4, "b was rendered again");
const long = "x".repeat(11);
Expand All @@ -161,6 +163,18 @@ test("the settled cache keeps recent renders, drops the oldest past its limit, a
assert.equal(cache.size, 2);
});

test("deferred Markdown shows its text as written until it nears the view, and text parsed before renders at once", async () => {
await render("");
const { DeferMarkdown } = await server!.ssrLoadModule("/src/renderer/components/Markdown.tsx") as { DeferMarkdown: import("react").Context<unknown> };
const deferred = (text: string) => renderToStaticMarkup(createElement(DeferMarkdown.Provider, { value: { current: null } }, createElement(Markdown, { text })));
const fresh = "Not parsed **yet** <b>raw</b>";
assert.equal(deferred(fresh), '<div class="md md-deferred" data-markdown-deferred="">Not parsed **yet** &lt;b&gt;raw&lt;/b&gt;</div>');
// Rendered (and kept) once outside a deferred turn, the same text parses at once inside one.
const seen = "Already **parsed**";
await render(seen);
assert.match(deferred(seen), /^<div class="md"><p>Already <strong>parsed<\/strong><\/p><\/div>$/);
});

test("a code block names its language and offers Copy", async () => {
const html = await render("```ts\nconst a = 1;\n```\n\n```\nplain\n```");
assert.match(html, /<span class="code-lang">ts<\/span><button type="button" class="code-copy" data-testid="code-copy" aria-label="Copy ts code">Copy<\/button><\/div><pre class="code" data-lang="ts"[^>]*><code>const a = 1;<\/code><\/pre>/);
Expand Down
53 changes: 53 additions & 0 deletions docs/reviews/2026-10-30-deferred-markdown.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Deferred Markdown for long threads, 2026-10-30

Why the approved 500 ms cold-open gate (`e2e/transcript-measure.spec.ts`, Val, 2026-10-22) failed on hosted CI, and the change Val chose on 2026-10-30 ("Defer offscreen answers"). The gate, its limit and its five-launch method are unchanged.

## Hosted variance, before the change

The `[transcript-budget]` line of every completed hosted `unit + e2e (macOS)` job from 2026-10-30 08:41 to 17:29 UTC (34 jobs; one printed no line), all Apple M1 (Virtual), 3 cores, except one Apple M2 Pro (Virtual), 5 cores:

- 33 medians: median 377.7 ms, minimum 222.8 ms, maximum 781.9 ms; three above 450 ms, one above 500 ms.
- The failure, merge-queue run [41056164788](https://github.com/TypeSafeAI/modex/actions/runs/41056164788) on `b033809`, measured 782, 1022, 822, 720 and 729 ms. `b033809` is `9f85d61` plus one CI-only change (#456, the Linux KWallet5 pin). Run [41055938841](https://github.com/TypeSafeAI/modex/actions/runs/41055938841) on `9f85d61`, started two minutes earlier, measured a 436.1 ms median. The application under test was identical, so the spike was the runner, not the code.
- Earlier recorded failures: 971.8 ms (#431's queue run) and 2019.2 ms (run 40764795777).

The typical hosted median sat about 120 ms under the limit, so ordinary runner variance was enough to fail it.

## Where a cold open went

Local, Apple M5 Max (18 cores), from a Chromium trace of the first open in each launch: the 7 ms `thread:items` read was negligible. One renderer task of 150–180 ms held the cost, about 110 ms of it parsing Markdown for the fixture's 200 visible final answers (`fromMarkdown` from mdast). Two cheaper candidates were measured and rejected:

- Mounting never-shown folded work only on first expand cut rendered items from 1,400 to 400 but left the open at about 176 ms: folded work was not the cost.
- Creating the GFM extensions once instead of per parse made no difference beyond run-to-run noise (Node, cold, four paired runs: 110/104, 176/237, 154/121, 127/128 ms).

## The change

`DeferMarkdown` (`Markdown.tsx`) is provided around every transcript turn before the newest eight (`EAGER_TURNS`, `ThreadView.tsx`). Markdown inside one shows its text as written (`.md-deferred`, `data-markdown-deferred`) until it comes within 1,500 px of the transcript's view; it then parses once and stays parsed. Text already parsed this session renders at once. The newest turns, including any streaming one, parse as before. Chromium's scroll anchoring keeps the reader's place while answers above it parse and grow.

The budget probe now also waits until the last answer is parsed Markdown, so a deferred answer cannot count as opened.

## Results

Throttled cold opens (a local probe that applies `Emulation.setCPUThrottlingRate` 3 to the first open of each launch; three five-launch runs each):

| | medians (ms) |
| --- | --- |
| before | 806, 834, 870 |
| after | 327, 312, 340 |
| before, unthrottled | 197 (a single five-launch run) |
| after, unthrottled | 101, 102 |

First-pass `pnpm build && pnpm test && pnpm test:e2e` on this branch (local, M5 Max): build and unit tests passed. E2E finished with 477 passed, 7 skipped and 1 failed. The five-launch median was 87.7 ms. The failure was `e2e/companion.spec.ts:44`: the copied pairing link read `192.168.2.1-rotated` where `10.0.0.1-rotated` was expected, from the system clipboard. It passed when rerun alone, together with the new spec (3 passed). It is unrelated to the transcript and is recorded as observed.

New coverage:

- `e2e/markdown-defer.spec.ts`: the newest answer parses at once; the oldest shows as written, parses when scrolled to, and stays parsed. A middle answer brought into view stays within 2 px while the answers above it parse. As a negative control, with `overflow-anchor: none` the second case failed ("moved from 45.2 to -50.7").
- `test/markdown-render.test.ts`: deferred text renders as written, escaped; text parsed before renders parsed inside a deferred turn; `RenderCache.has`.

## Not covered

- Assistive technology reaching a deferred answer without scrolling near it would read the Markdown as written. VoiceOver moves the view as it navigates, which should parse the answer first. This is untested by a person, and rows C1–C5 remain deferred.
- Copying a deferred answer's text copies it as written. That is the source Markdown, not the rendered text.

## Hosted CI on the PR, 2026-10-31

[Run 41163979727](https://github.com/TypeSafeAI/modex/actions/runs/41163979727/job/124427436348) on `#463`, Apple M1 (Virtual), 3 cores, Node v24.22.1, passed on its first attempt. Desktop E2E had 481 passed and 4 skipped in 28.4 min, and site E2E had 7 passed. The five-launch samples were 129.9, 126.2, 129.4, 136.1 and 181.1 ms, a median of 129.9 ms. Hosted runs on the same runner type before the change had a median of 377.7 ms over 33 runs. One run is not a variance study; later queue runs add to it.
Loading
Loading