Skip to content
Open
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
115 changes: 115 additions & 0 deletions apps/ui/src/components/MarkdownText.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,121 @@ describe("MarkdownText — code highlighting with plain fallback", () => {
// The appended prose rendered too (the grow actually took effect).
expect(container.textContent).toContain("more prose below");
});

// RIG-1422: leading-edge first highlight. A settled (non-streaming) block —
// every historical message in a channel — must colorize on FIRST paint, not
// sit on the plain `<pre>` fallback for HIGHLIGHT_DEBOUNCE_MS. This is the
// cache-MISS counterpart to the R1 test above (which proves the sync-cache
// paint): here the (lang, code) pair is NOT cached (afterEach cleared it), so
// the highlight runs the async path — but on the leading edge, kicked
// immediately rather than gated behind the 150ms trailing timer. The proof is
// that the highlighter is asked, and its markup paints, WITHOUT ever elapsing
// `flushHighlightDebounce()`. A debounced-from-the-first-tick implementation
// would leave `asked` empty until the 150ms window passed.
test("a settled fence highlights on first paint, not gated behind the debounce (leading edge, cache miss)", async () => {
const HIGHLIGHTED =
'<pre class="shiki"><code><span style="color:green" data-le="hit">const x = 1;</span></code></pre>';
const asked: string[] = [];
mock.module("../markdown/highlighter", () => ({
...realHighlighter,
highlightToHtml: (code: string, lang: string) => {
asked.push(code);
return Promise.resolve(
lang === "ts" && code.includes("const x = 1;") ? HIGHLIGHTED : null,
);
},
}));
const { container } = render(() => (
<MarkdownText text={"```ts\nconst x = 1;\n```"} byHandle={byHandle()} />
));
// The highlight is kicked on the leading edge: the highlighter was asked
// synchronously at render, with NO debounce window elapsed. (A trailing-only
// debounce would have asked nothing yet.)
flush();
expect(asked.length).toBe(1);
expect(asked[0]).toContain("const x = 1;");
// And the resolved markup paints — still without ever elapsing the debounce
// flush, so no plain-fallback frame is gated behind the 150ms timer.
await waitFor(
() => expect(container.querySelector('[data-le="hit"]')).not.toBeNull(),
{ timeout: HIGHLIGHT_WAIT_MS },
);
expect(container.querySelector("pre.code-block")).toBeNull();
});

// RIG-1422: the leading-edge gate must NOT reintroduce the per-tick re-tokenize
// the debounce exists to collapse. A single fence grown across ticks that
// arrive inside the debounce window (no `flushHighlightDebounce` between them)
// must issue ONE leading-edge pass, then collapse the whole growth burst into
// exactly ONE trailing pass — not one per tick.
test("a fence's within-window growth ticks collapse to one trailing highlight", async () => {
const asked: string[] = [];
mock.module("../markdown/highlighter", () => ({
...realHighlighter,
// Never resolves: we count HOW MANY times the highlighter is asked, not
// what it returns.
highlightToHtml: (code: string) => {
asked.push(code);
return new Promise<string | null>(() => {});
},
}));
const [text, setText] = createSignal("```ts\nL0\n```");
render(() => <MarkdownText text={text()} byHandle={byHandle()} />);
flush();
// Leading edge: the fresh fence is asked once, immediately.
expect(asked.length).toBe(1);
// Three growth ticks inside the window (flush() is synchronous, far under
// HIGHLIGHT_DEBOUNCE_MS) — each is a growth tick, debounced, so no new ask.
setText("```ts\nL0\nL1\n```");
flush();
setText("```ts\nL0\nL1\nL2\n```");
flush();
setText("```ts\nL0\nL1\nL2\nL3\n```");
flush();
expect(asked.length).toBe(1);
// Once the window elapses, the burst collapses to exactly ONE trailing pass
// carrying the latest text — not one pass per tick.
await flushHighlightDebounce();
expect(asked.length).toBe(2);
expect(asked[1]).toContain("L3");
});

// RIG-1422 regression: the leading-edge gate is a MODULE-LEVEL record, so a
// message with more than one fence rebuilds every fence in the same reconcile
// tick. A naive single-slot record lets an earlier fence clobber the slot
// before a later, still-growing fence reads it, so the grower never matches
// its OWN prior code and (mis)fires an immediate re-tokenize every tick — the
// exact O(n²) the debounce collapses. The gate must match a growing fence
// against its own recent snapshot regardless of siblings scheduled between.
test("a second streaming fence beside a settled one still debounces (no per-tick re-tokenize)", async () => {
const asked: string[] = [];
mock.module("../markdown/highlighter", () => ({
...realHighlighter,
highlightToHtml: (code: string) => {
asked.push(code);
return new Promise<string | null>(() => {});
},
}));
// F1 is settled; F2 streams. Count only F2's asks (its "GROW" marker) so
// F1's own per-tick reconstruction (harmless — it hits the R1 cache in
// production) does not pollute the assertion.
const F1 = "```ts\nconst settled = 1;\n```";
const grows = () => asked.filter((c) => c.includes("GROW")).length;
const [text, setText] = createSignal(`${F1}\n\n\`\`\`ts\nGROW\n\`\`\``);
render(() => <MarkdownText text={text()} byHandle={byHandle()} />);
flush();
// F2's leading edge: asked once.
expect(grows()).toBe(1);
// Grow ONLY F2, within the window. Under a single-slot gate F1 clobbers the
// slot each tick, so F2 reads as a leading edge and re-asks every tick
// (grows() climbs). The window-list gate matches F2 against its own prior
// snapshot → growth tick → debounced → no new ask.
setText(`${F1}\n\n\`\`\`ts\nGROW\nMORE\n\`\`\``);
flush();
setText(`${F1}\n\n\`\`\`ts\nGROW\nMORE\nEVEN\n\`\`\``);
flush();
expect(grows()).toBe(1);
});
});

describe("MarkdownText — link safety", () => {
Expand Down
28 changes: 28 additions & 0 deletions apps/ui/src/components/MarkdownText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import type { Account } from "../comms-stub";
import {
getCachedHighlight,
isLeadingEdgeHighlight,
setCachedHighlight,
} from "../markdown/highlight-cache";
import { highlightToHtml } from "../markdown/highlighter";
Expand Down Expand Up @@ -137,9 +138,36 @@ function BlockCode(props: { code: string; codeClass?: string }) {
// setTimeout/cleanup cycle in the (untracked) apply phase so it only re-runs
// when a tracked source changes. Solid 2's two-arg createEffect returns its
// cleanup from the apply phase.
//
// Leading-edge first, trailing-debounce the rest (RIG-1422): a settled
// (non-streaming) block — every historical message in a channel — must
// colorize on first paint, not sit on the plain `<pre>` fallback for
// HIGHLIGHT_DEBOUNCE_MS while a timer it never needed elapses. So a FRESH
// block pushes `[code, lang]` into `settled` immediately (no timer); only a
// streaming GROWTH tick arms the trailing timer, collapsing a burst into one
// pass exactly as before.
//
// Why the leading-edge test lives OUTSIDE this instance: under
// `renderingStrategy="reconcile"` solid-markdown REBUILDS the fenced subtree
// on every growth tick, so `BlockCode` is reconstructed each tick (a fresh
// closure — verified: the component body re-runs per tick). A per-instance
// `firstRun` flag would therefore read true on every tick and fire an
// immediate highlight for each, reintroducing the O(n²) re-tokenize the
// debounce exists to collapse. `isLeadingEdgeHighlight` is module-level so a
// reconstructed instance can tell a fresh block (immediate) from a growth tick
// of a stream still inside its debounce window (keep debouncing).
const immediate = isLeadingEdgeHighlight(
lang(),
props.code,
HIGHLIGHT_DEBOUNCE_MS,
);
createEffect(
() => [props.code, lang()] as const,
([nextCode, nextLang]) => {
if (immediate) {
setSettled([nextCode, nextLang] as const);
return;
}
const t = setTimeout(
() => setSettled([nextCode, nextLang] as const),
HIGHLIGHT_DEBOUNCE_MS,
Expand Down
72 changes: 72 additions & 0 deletions apps/ui/src/markdown/highlight-cache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { afterEach, describe, expect, test } from "bun:test";
import { clearHighlightCache, isLeadingEdgeHighlight } from "./highlight-cache";

// A window large enough that no snapshot is ever time-pruned within a single
// test — so these cases isolate the classification logic (lang match, strict
// extension, newest-first scan, size cap) from wall-clock timing. The `windowMs
// = 0` case below deliberately exercises the time-window boundary instead.
const WINDOW = 100_000;

describe("isLeadingEdgeHighlight", () => {
afterEach(() => {
clearHighlightCache();
});

test("a fresh (lang, code) highlights on the leading edge", () => {
expect(isLeadingEdgeHighlight("ts", "const a", WINDOW)).toBe(true);
});

test("re-scheduling the identical (lang, code) is a leading edge, not a growth tick", () => {
expect(isLeadingEdgeHighlight("ts", "const a", WINDOW)).toBe(true);
// identical code is not a *strict* extension of its own prior snapshot.
expect(isLeadingEdgeHighlight("ts", "const a", WINDOW)).toBe(true);
});

test("code that strictly extends a recent same-lang snapshot is a debounced growth tick", () => {
expect(isLeadingEdgeHighlight("ts", "const a", WINDOW)).toBe(true);
expect(isLeadingEdgeHighlight("ts", "const a = 1", WINDOW)).toBe(false);
});

test("a strict extension in a different language is a leading edge (lang must match)", () => {
expect(isLeadingEdgeHighlight("ts", "const a", WINDOW)).toBe(true);
expect(isLeadingEdgeHighlight("py", "const a = 1", WINDOW)).toBe(true);
});

test("an interleaved sibling fence does not steal a stream's growth classification", () => {
// Two fences scheduled in one reconcile tick (document order), then both
// grow in the next — the exact multi-fence case the single-slot gate got
// wrong. Each grower must match its OWN prior snapshot despite the sibling
// snapshot appended in between.
expect(isLeadingEdgeHighlight("ts", "x", WINDOW)).toBe(true); // F1 v1
expect(isLeadingEdgeHighlight("ts", "a", WINDOW)).toBe(true); // F2 v1
expect(isLeadingEdgeHighlight("ts", "xy", WINDOW)).toBe(false); // F1 v2 — extends "x"
expect(isLeadingEdgeHighlight("ts", "ab", WINDOW)).toBe(false); // F2 v2 — extends "a"
});

test("windowMs = 0 disables growth detection — every tick is a leading edge", () => {
expect(isLeadingEdgeHighlight("ts", "const a", 0)).toBe(true);
// A strict extension, but a zero-width window admits no recent snapshot.
expect(isLeadingEdgeHighlight("ts", "const a = 1", 0)).toBe(true);
});

test("clearHighlightCache resets the snapshots so a later extension reads as fresh", () => {
expect(isLeadingEdgeHighlight("ts", "const a", WINDOW)).toBe(true);
clearHighlightCache();
// Without the reset this would classify as a growth tick (false).
expect(isLeadingEdgeHighlight("ts", "const a = 1", WINDOW)).toBe(true);
});

test("the snapshot list is size-capped — extending a snapshot evicted by the cap reads as a leading edge", () => {
// Push MAX_RECENT + 1 distinct, non-prefix-related snapshots inside one
// window (trailing "." keeps e.g. "f1." from being a prefix of "f10.").
// The size cap keeps the newest 64, so the oldest ("f0.") is evicted.
const CAP = 64;
for (let i = 0; i <= CAP; i++) {
expect(isLeadingEdgeHighlight("ts", `f${i}.`, WINDOW)).toBe(true);
}
// Extending the evicted oldest: no surviving snapshot matches → leading edge.
expect(isLeadingEdgeHighlight("ts", "f0.x", WINDOW)).toBe(true);
// Extending a snapshot still within the cap: a growth tick.
expect(isLeadingEdgeHighlight("ts", `f${CAP}.x`, WINDOW)).toBe(false);
});
});
81 changes: 77 additions & 4 deletions apps/ui/src/markdown/highlight-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,83 @@ export function setCachedHighlight(
cache.set(key(lang, code), html);
}

/** Empty the cache. The cache is module-level and persists across component
* instances by design (that persistence is what R1 buys); a test that asserts
* on the highlight path must clear it between cases so a prior render's entry
* does not seed a later one. */
/** The recent `(lang, code)` snapshots blocks have scheduled a highlight for, in
* schedule order (oldest first) — the cross-instance thread the leading-edge
* gate needs (RIG-1422).
*
* `BlockCode` is reconstructed on every streaming growth tick (solid-markdown
* rebuilds the fenced subtree under `renderingStrategy="reconcile"`), so a
* per-instance "first run" flag reads true every tick and cannot tell a fresh
* settled block from a growth tick. A module-level record can — but a SINGLE
* slot is wrong: a message with more than one fence rebuilds every fence in the
* same reconcile tick, in document order, so an earlier fence clobbers the slot
* before a later, still-growing fence reads it; the grower never sees its own
* prior code and (mis)fires an immediate re-tokenize every tick — the exact
* O(n²) the debounce exists to collapse. A short list, matched against ANY
* recent same-lang snapshot, lets each fence find its own prior code regardless
* of siblings scheduled in between. Bounded by the debounce window (stale
* entries pruned) with a coarse size cap as a backstop. */
const recentScheduled: { lang: string; code: string; at: number }[] = [];
const MAX_RECENT = 64;

/** Whether `(lang, code)` should highlight on the LEADING edge — immediately,
* no debounce — vs. be debounced as a streaming growth tick. Leading edge for a
* fresh block and for a batch of distinct settled blocks (a history load, where
* no two are a prefix of each other); debounced only when this code strictly
* extends a recent same-lang snapshot within `windowMs` (an active stream).
*
* Records `(lang, code)` as a recent snapshot as a side effect, so later calls
* see it. `windowMs` guards against a stale snapshot: two blocks far apart in
* time that happen to be prefix-related are independent, not a stream. A
* settled block that coincidentally extends a sibling settled block rendered in
* the same paint is misread as a growth tick (debounced ~one window, then
* colorized) — a rare one-time timing shift, never a wrong render. Uses
* `performance.now()` (monotonic) so a wall-clock adjustment can't reorder
* snapshots. */
export function isLeadingEdgeHighlight(
lang: string,
code: string,
windowMs: number,
): boolean {
const now = performance.now();
// Scan newest-first: an active stream matches its own latest prior snapshot
// even when sibling fences were scheduled in between. Snapshots are appended
// in (monotonic) time order, so once one is older than the window every
// earlier one is too — stop there.
let isGrowthTick = false;
for (let i = recentScheduled.length - 1; i >= 0; i--) {
const prev = recentScheduled[i];
if (now - prev.at >= windowMs) break;
if (
prev.lang === lang &&
code !== prev.code &&
code.startsWith(prev.code)
) {
isGrowthTick = true;
break;
}
}
recentScheduled.push({ lang, code, at: now });
// Prune snapshots older than the window (a prefix of the array, by time
// order); cap the length as a coarse backstop against a dense burst.
const cutoff = now - windowMs;
let stale = 0;
while (stale < recentScheduled.length && recentScheduled[stale].at <= cutoff)
stale++;
if (stale > 0) recentScheduled.splice(0, stale);
if (recentScheduled.length > MAX_RECENT)
recentScheduled.splice(0, recentScheduled.length - MAX_RECENT);
return !isGrowthTick;
}

/** Empty the cache AND clear the leading-edge snapshots. The cache is
* module-level and persists across component instances by design (that
* persistence is what R1 buys); a test that asserts on the highlight path must
* clear it between cases so a prior render's entry does not seed a later one —
* and must reset the leading-edge snapshots (RIG-1422) too, so a prior test's
* recently-scheduled code cannot make a later test's fresh block read as a
* growth tick. */
export function clearHighlightCache(): void {
cache.clear();
recentScheduled.length = 0;
}
Loading