Skip to content

Commit 1bcabae

Browse files
committed
fix(incremental): round-2 review — terminator mention gate, defense restoration, checkpoint slimming
Second adversarial pass (4 finder angles, ~40 probes) over the previous fix commit. One shipped-input correctness finding, rest defense-in-depth: - terminator label mentions: the injection terminator is a synthetic DEFINITION, so a tail-side `[__aimd_injection_terminator__]` mention (shortcut/full/image form) resolved against it while the full parse renders literal text (probe-confirmed by two independent reviewers, persisting into the settled document). Structural fix over a doc caveat: frames whose pre-injection tail contains the label take the full path (tailMentionsTerminator gate); prefix mentions need no check (unresolved => taint-pinned into the tail; resolved => the real def wins first-def-wins over the terminator). Arbiter fixtures for all three reference forms. - resume-validity now gates the sticky-uninjectable short-circuit too (a regressed cache boundary must degrade to a fresh walk, never a stale verdict), and a position-less top-level mdast child makes the plan uncacheable + restarts fresh (a resumed walk would re-visit it and duplicate its cached events — unreachable with the shipped chain, closed structurally). - G4's straddle scan and the prefix cut give back their round-1 early breaks: the defensive gate must not share the ordered-children assumption it exists to catch (the O(top-level) cost is noise); the injection walk keeps its breaks (event ORDER already assumes document order there). - alignPrefixCut's leading-run branch tightened to the probe-verified model exactly: excess leading text is only accepted with ZERO stripped slots (hoist merges into a slot whenever one exists). - MarkdownContent wraps the engine in a throw fence: the scan checkpoint is advanced in place BEFORE the tail parse, so a mid-frame throw (an engine bug, or a plugin choking on the synthetic tail shape) used to leave the ref holding a state whose checkpoint described newer content — with a tail-repair preprocessor's non-append shapes that could eventually freeze an unsafe prefix. Now: clear the ref, full-parse the frame, dev-mode console.error. - SSR routes through the plain full-parse branch: the engine's scan only exists to seed the NEXT frame's checkpoint, and a per-request render has no next frame (was a dead O(document) line-lex per SSR'd message). - FreezeScanCheckpoint drops its lines array — it retained a full copy of the document (~2-3x doc size per mounted instance) to answer a question only ever asked about ONE line: blocker 4's next-line settle. Candidates now settle eagerly (tri-state on the candidate; only the newest can be pending), same verdicts, resume≡fresh property intact. - incrementalParseEnabled JSDoc updated to the v2 gate list (the two v1-only bullets — cross-chunk and [^ — were the IDE-visible surface); stale 'caller bypasses' test comment fixed. Cleared by probe in this round (no action): StrictMode double-invoke / concurrent abandonment / checkpoint resume idempotence; rolling-line and countNewlines bound fenceposts vs reference implementations; plan-cache monotonicity under splice/full alternation and suffix churn; every terminator/leading-hoist attack shape (setext, lazy continuation, defList after link-def-last, 1-3-space indents, double tables, growing hoist, comment+table starts, phantom-suffix interplay); all round-1 deletions verified consumer-free with equivalent replacement coverage. 810 tests green (unit + browser); typecheck/lint/prettier/build clean.
1 parent 3419941 commit 1bcabae

7 files changed

Lines changed: 175 additions & 63 deletions

File tree

packages/core/src/components/MarkdownContent.tsx

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,13 @@ const BlockMemoizedRenderer = memo(
463463
// plus the replay-regenerated footer (hast). When the flag is off, the
464464
// state is CLEARED — a later eligible frame must never splice against
465465
// trees parsed under different conditions.
466-
if (!config.incrementalParseEnabled) {
466+
//
467+
// SSR takes this branch too: the engine's first-frame scan exists to
468+
// seed the NEXT frame's checkpoint, and a per-request server render
469+
// has no next frame — routing through the engine would pay a dead
470+
// O(document) line-lex per request. Hydration is unaffected (the
471+
// client's first frame rebuilds from null either way).
472+
if (!config.incrementalParseEnabled || typeof window === 'undefined') {
467473
incrementalStateRef.current = null;
468474
// Dev-only stage telemetry (`ai-markdown:stage:*` performance
469475
// measures; no-op in production). Wraps only the stage calls — the
@@ -480,23 +486,47 @@ const BlockMemoizedRenderer = memo(
480486
return { mdast: parsed.mdast, hast: hastRoot };
481487
}
482488

483-
const result = advanceIncrementalParse(incrementalStateRef.current, content ?? '', {
484-
remarkPlugins,
485-
rehypePlugins,
486-
remarkRehypeOptions: mergedRemarkRehypeOptions,
487-
// Identity tuple over every parse input beyond the content itself.
488-
// Deliberately covers MORE than the G3 flush's 12 fields (handlers /
489-
// preserveForBodyHarvest / documentId can change without touching
490-
// any G3 field — e.g. a `preserveOrphanReferences` flip). The
491-
// phantom label sets are deliberately NOT here: their churn tracks
492-
// the suffix (always re-parsed with the tail), never the prefix.
493-
depsKey: [remarkPlugins, rehypePlugins, remarkRehypeOptions, handlers, preserveForBodyHarvest, documentId],
494-
defListEnabled: config.extraSyntaxSupported.includes(AIMarkdownRenderExtraSyntax.DEFINITION_LIST),
495-
phantomSuffix,
496-
measure: measureHere,
497-
});
498-
incrementalStateRef.current = result.nextState;
499-
return { mdast: result.mdast, hast: result.hast };
489+
try {
490+
const result = advanceIncrementalParse(incrementalStateRef.current, content ?? '', {
491+
remarkPlugins,
492+
rehypePlugins,
493+
remarkRehypeOptions: mergedRemarkRehypeOptions,
494+
// Identity tuple over every parse input beyond the content itself.
495+
// Deliberately covers MORE than the G3 flush's 12 fields (handlers /
496+
// preserveForBodyHarvest / documentId can change without touching
497+
// any G3 field — e.g. a `preserveOrphanReferences` flip). The
498+
// phantom label sets are deliberately NOT here: their churn tracks
499+
// the suffix (always re-parsed with the tail), never the prefix.
500+
depsKey: [remarkPlugins, rehypePlugins, remarkRehypeOptions, handlers, preserveForBodyHarvest, documentId],
501+
defListEnabled: config.extraSyntaxSupported.includes(AIMarkdownRenderExtraSyntax.DEFINITION_LIST),
502+
phantomSuffix,
503+
measure: measureHere,
504+
});
505+
incrementalStateRef.current = result.nextState;
506+
return { mdast: result.mdast, hast: result.hast };
507+
} catch (error) {
508+
// The engine mutates prev's scan checkpoint IN PLACE before the tail
509+
// parse/splice — a throw mid-frame (an engine bug, or a plugin
510+
// choking on the synthetic tail source) leaves the retained state's
511+
// checkpoint describing content the state's trees do not. Clearing
512+
// the ref restores the "state is CLEARED when unusable" discipline;
513+
// the frame then renders via the ordinary full pipeline so one bad
514+
// frame cannot take the surface down.
515+
incrementalStateRef.current = null;
516+
if (process.env.NODE_ENV !== 'production') {
517+
console.error('[ai-react-markdown] incremental parse failed — full parse fallback for this frame:', error);
518+
}
519+
const parsed = measureHere('parse', () =>
520+
parseStage({
521+
children: augmented,
522+
remarkPlugins,
523+
rehypePlugins,
524+
remarkRehypeOptions: mergedRemarkRehypeOptions,
525+
})
526+
);
527+
const hastRoot = measureHere('transform', () => transformStage(parsed));
528+
return { mdast: parsed.mdast, hast: hastRoot };
529+
}
500530
}, [
501531
content,
502532
targetPhantoms,

packages/core/src/components/incrementalParse/advanceIncrementalParse.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,13 @@ import type { Root as MdastRoot } from 'mdast';
4949

5050
import { parseStage, transformStage, type Options as MarkdownOptions } from '../markdown';
5151
import { computeFreezeBoundary, type FreezeScanCheckpoint } from './computeFreezeBoundary';
52-
import { buildInjectionPrefix, collectPrefixInjection, spliceTrees, type CachedInjectionPlan } from './spliceParse';
52+
import {
53+
buildInjectionPrefix,
54+
collectPrefixInjection,
55+
spliceTrees,
56+
tailMentionsTerminator,
57+
type CachedInjectionPlan,
58+
} from './spliceParse';
5359

5460
export interface IncrementalParseState {
5561
/** The CHUNK's own text — excludes the phantom suffix. */
@@ -195,24 +201,34 @@ export function advanceIncrementalParse(
195201
// G3
196202
const boundary = Math.min(freshBoundary, prev!.stableBoundary);
197203
if (boundary <= 0) return fullPath();
198-
// G4 (defensive) — children are position-ordered, so the walk ends at the
199-
// first child starting at/past the boundary.
204+
// G4 (defensive) — deliberately a FULL scan, no ordered-children early
205+
// break: this is the gate that catches ordering/straddle violations, so
206+
// it must not share the assumption it defends against (round-2 review).
207+
// O(top-level children) per frame is noise next to the tail parse.
200208
for (const child of prev!.mdast.children) {
201209
const start = child.position?.start?.offset;
202210
const end = child.position?.end?.offset;
203-
if (start !== undefined && start >= boundary) break;
204-
if (start !== undefined && end !== undefined && end > boundary) {
211+
if (start !== undefined && end !== undefined && start < boundary && end > boundary) {
205212
return fullPath();
206213
}
207214
}
208215

216+
// The one input class where the injection's synthetic terminator label
217+
// could change how the tail parses: the tail literally mentioning it.
218+
// Checked on the PRE-injection tail (the injection itself contains the
219+
// label). Pathological by construction — correctness over splice rate.
220+
const tailAndSuffix = content.slice(boundary) + phantomSuffix;
221+
if (tailMentionsTerminator(tailAndSuffix)) return fullPath();
222+
209223
const plan = collectPrefixInjection(prev!.mdast, prev!.content, boundary, injectionPlan);
210-
injectionPlan = { boundary, events: plan.events, uninjectable: plan.uninjectable };
224+
if (plan.cacheable !== false) {
225+
injectionPlan = { boundary, events: plan.events, uninjectable: plan.uninjectable };
226+
}
211227
// A nested definition/footnote-def that cannot be re-injected verbatim —
212228
// take the full path this frame rather than splice without it (A3).
213229
if (plan.uninjectable) return fullPath();
214230
const injection = buildInjectionPrefix(plan.events);
215-
const tailSource = injection.text + content.slice(boundary) + phantomSuffix;
231+
const tailSource = injection.text + tailAndSuffix;
216232
const tail = runPipeline(tailSource, options);
217233
const spliced = spliceTrees({
218234
prevMdast: prev!.mdast,

packages/core/src/components/incrementalParse/computeFreezeBoundary.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ describe('computeFreezeBoundary — continuation blockers', () => {
112112
expect(computeFreezeBoundary(terminated, OFF)).toBe(terminated.indexOf('tail'));
113113
});
114114

115-
test('footnote definition context blocks (defensive; caller bypasses [^ anyway)', () => {
115+
test('footnote definition context blocks (LOAD-BEARING since v2: defs splice via replay)', () => {
116116
expect(computeFreezeBoundary('[^n]: body\n\n', OFF)).toBe(0);
117117
});
118118
});

packages/core/src/components/incrementalParse/computeFreezeBoundary.ts

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,12 @@ interface Candidate {
115115
htmlBalanced: boolean;
116116
/** Rolling continuation-hazard verdict at emission (blocker 3). */
117117
hazard: boolean;
118-
/** Index into the checkpoint's confirmed-lines array. */
119-
lineIndex: number;
118+
/** Blocker-4 settle verdict, decided by the NEXT confirmed line (`null`
119+
* while that line hasn't confirmed — only the newest candidate can be
120+
* pending). Storing the verdict instead of the line lets the checkpoint
121+
* drop its lines array, which retained a full copy of the document
122+
* (round-2 review: ~2-3× doc size per mounted instance). */
123+
defListSettled: boolean | null;
120124
}
121125

122126
interface UnresolvedRef {
@@ -131,8 +135,6 @@ export interface FreezeScanCheckpoint {
131135
defListEnabled: boolean;
132136
/** Start offset of the first line NOT yet baked into this checkpoint. */
133137
confirmedOffset: number;
134-
/** 1-based count guard: text.length the checkpoint was last advanced on. */
135-
lines: LineRec[];
136138
candidates: Candidate[];
137139
defs: Map<string, number>; // normalized label → def line end offset
138140
footnoteDefs: Map<string, number>;
@@ -279,7 +281,6 @@ function freshCheckpoint(defListEnabled: boolean): FreezeScanCheckpoint {
279281
return {
280282
defListEnabled,
281283
confirmedOffset: 0,
282-
lines: [],
283284
candidates: [],
284285
defs: new Map(),
285286
footnoteDefs: new Map(),
@@ -365,14 +366,12 @@ export function computeFreezeBoundary(
365366
let earliestUnresolved = Infinity;
366367
for (const ref of cp.unresolvedRefs) earliestUnresolved = Math.min(earliestUnresolved, ref.offset);
367368

368-
// ── blocker 4: defList settled check (looks FORWARD from a candidate) ──
369+
// ── blocker 4: defList settled check (decided by the NEXT line) ──
369370
const defListSettled = (c: Candidate): boolean => {
370371
if (!options.defListEnabled || c.blankRun >= 2) return true;
371-
for (let i = c.lineIndex + 1; i < cp.lines.length; i++) {
372-
const ln = cp.lines[i];
373-
if (ln.blank) return true; // a second blank makes the run ≥ 2 — the backward scan cannot cross it
374-
return !canBecomeDdLine(ln.text, true);
375-
}
372+
// Confirmed next lines settle candidates eagerly in processConfirmedLine;
373+
// only the newest candidate can still be pending here.
374+
if (c.defListSettled !== null) return c.defListSettled;
376375
if (tailLine) return !canBecomeDdLine(tailLine.text, false);
377376
return false; // no next line yet — a future `: desc` could still claim the block above
378377
};
@@ -393,7 +392,13 @@ export function computeFreezeBoundary(
393392

394393
/** Bake one confirmed line into the checkpoint. */
395394
function processConfirmedLine(cp: FreezeScanCheckpoint, ln: LineRec, text: string): void {
396-
cp.lines.push(ln);
395+
// Blocker-4 eager settle: this line is the "next confirmed line" of the
396+
// newest candidate. The verdict uses the RAW line exactly like the old
397+
// lines-array lookback did (fence/math state deliberately not consulted).
398+
const newest = cp.candidates[cp.candidates.length - 1];
399+
if (newest && newest.defListSettled === null) {
400+
newest.defListSettled = ln.blank ? true : !canBecomeDdLine(ln.text, true);
401+
}
397402
const isBlockStart = cp.prevLineBlank;
398403
const applyTag = (tag: string, closing: boolean): void => {
399404
if (closing) {
@@ -484,7 +489,7 @@ function processConfirmedLine(cp: FreezeScanCheckpoint, ln: LineRec, text: strin
484489
blankRun: cp.blankRun,
485490
htmlBalanced: cp.openTotal === 0 && !cp.commentOpen && !cp.piOpen && !cp.declOpen && !cp.cdataOpen,
486491
hazard: cp.hazardVerdict,
487-
lineIndex: cp.lines.length - 1,
492+
defListSettled: null,
488493
});
489494
cp.paragraphHasUnpairedRun = false;
490495
cp.prevLineBlank = true;

packages/core/src/components/incrementalParse/spliceEquivalence.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,23 @@ describe('splice equivalence — footnote injection replay', () => {
548548
}
549549
});
550550

551+
test('tail mentioning the terminator label falls back instead of mis-resolving (round-2)', () => {
552+
// The injection's terminator is a synthetic DEFINITION — a tail-side
553+
// `[label]` mention would resolve against it while the full parse
554+
// renders literal text. The engine must take the full path for such
555+
// frames (pathological input; correctness over splice rate). The
556+
// shortcut-, full-, and image-reference forms are all covered.
557+
const mentions = [
558+
'see [__aimd_injection_terminator__] maybe.\n',
559+
'see [text][__aimd_injection_terminator__] maybe.\n',
560+
'see ![alt][__aimd_injection_terminator__] maybe.\n',
561+
];
562+
for (const mention of mentions) {
563+
const frame0 = 'note[^q] first.\n\n[^q]: def body\n\nplain settles.\n\n';
564+
assertStreamEquivalence('terminator-mention', [frame0, frame0 + mention], BASELINE);
565+
}
566+
});
567+
551568
test('injection continuation leaks (final-review R1): defList `: desc` claim through the join', () => {
552569
// The original document separates with TWO blank lines (blankRun>=2 is
553570
// defList-claim-immune), but the injection joins to the tail with ONE —

packages/core/src/components/incrementalParse/spliceParse.ts

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ export interface PrefixInjectionPlan {
6363
/** True when an event exists that CANNOT be injected reliably — the caller
6464
* must fall back to a full parse for this frame (safe, one-frame cost). */
6565
uninjectable: boolean;
66+
/** False when the plan must NOT be cached for resume: a position-less
67+
* top-level child cannot be partitioned by the resume offset, so a
68+
* resumed walk would re-visit it and duplicate its events (round-2
69+
* review). Fresh walks stay correct — they just can't be incremental. */
70+
cacheable?: boolean;
6671
}
6772

6873
/** A plan cached in engine state: valid for any LATER boundary of the same
@@ -103,12 +108,18 @@ export function collectPrefixInjection(
103108
// frame to frame, so a cached plan only needs the children in
104109
// [resume.boundary, boundary) appended — without this the deep visit
105110
// re-walks the ENTIRE prefix every splice frame (the collectDefLabels
106-
// O(stream²) shape). `uninjectable` is sticky: the offending node stays
107-
// in the prefix for the lineage's lifetime.
108-
if (resume && resume.uninjectable) return { events: resume.events, uninjectable: true };
109-
const resumeAt = resume && resume.boundary <= boundary ? resume.boundary : 0;
111+
// O(stream²) shape). Boundary validity gates EVERYTHING resumed —
112+
// including the sticky-uninjectable short-circuit: the boundary is
113+
// monotone within an append lineage today, but a regressed cache must
114+
// degrade to a fresh walk, never to a stale verdict (round-2 review).
115+
const resumeValid = resume != null && resume.boundary <= boundary;
116+
// `uninjectable` is sticky: the offending node stays in the prefix for
117+
// the lineage's lifetime.
118+
if (resumeValid && resume!.uninjectable) return { events: resume!.events, uninjectable: true };
119+
const resumeAt = resumeValid ? resume!.boundary : 0;
110120
const events: InjectionEvent[] = resumeAt > 0 ? cloneEventsForAppend(resume!.events) : [];
111121
let uninjectable = false;
122+
let cacheable = true;
112123
const pushRef = (token: string): void => {
113124
const last = events[events.length - 1];
114125
if (last && last.kind === 'refs') last.tokens.push(token);
@@ -163,11 +174,22 @@ export function collectPrefixInjection(
163174
// boundary can contribute a prefix event (E5), and nothing before the
164175
// resume point needs re-visiting.
165176
const start = child.position?.start?.offset;
166-
if (start !== undefined && start >= boundary) break;
167-
if (start !== undefined && start < resumeAt) continue;
177+
if (start === undefined) {
178+
// A position-less top-level child cannot be partitioned by offset: a
179+
// resumed walk would re-visit it and DUPLICATE its cached events.
180+
// Restart fresh once (correct by construction) and mark the plan
181+
// uncacheable. Unreachable with the shipped chain (the parser
182+
// positions every top-level node); defense for plugin-shaped trees.
183+
if (resumeAt > 0) return collectPrefixInjection(mdast, content, boundary, null);
184+
cacheable = false;
185+
visit(child, false);
186+
continue;
187+
}
188+
if (start >= boundary) break;
189+
if (start < resumeAt) continue;
168190
visit(child, false);
169191
}
170-
return { events, uninjectable };
192+
return { events, uninjectable, cacheable };
171193
}
172194

173195
/** Cached plans are shared with the previous state — clone the array, and
@@ -200,10 +222,25 @@ export interface InjectionPrefix {
200222
* A column-0 link definition neutralizes both: it terminates a footnote
201223
* body (not >=4-indented), cannot be claimed as a <dt> (not a paragraph),
202224
* emits zero hast and zero wrap-separator slots, and is stripped with the
203-
* rest of the injected region. The sentinel label collides with real
204-
* content only if a document defines the exact same label — the same
205-
* accepted-risk class as the phantom sentinel URLs. */
206-
const INJECTION_TERMINATOR = '[__aimd_injection_terminator__]: __aimd_sentinel_link__';
225+
* rest of the injected region.
226+
*
227+
* Unlike the phantom sentinel URLs (values — they can never change parse
228+
* SHAPE), a definition LABEL is resolvable by any `[label]` mention, so a
229+
* document that literally writes the sentinel label would have its tail
230+
* mentions resolve against the terminator (round-2 review, probe-
231+
* confirmed). {@link tailMentionsTerminator} closes that structurally:
232+
* such frames take the full path instead of splicing. */
233+
const TERMINATOR_LABEL = '__aimd_injection_terminator__';
234+
const INJECTION_TERMINATOR = `[${TERMINATOR_LABEL}]: __aimd_sentinel_link__`;
235+
236+
/** True when the tail input could REFERENCE the terminator's label — the
237+
* one input class where the splice's synthetic definition would change how
238+
* the tail parses. Prefix mentions need no check: an unresolved mention is
239+
* reference-tainted (pinned into the tail), and a resolved one has a real
240+
* def that wins first-def-wins over the terminator. */
241+
export function tailMentionsTerminator(tailSource: string): boolean {
242+
return tailSource.includes(`[${TERMINATOR_LABEL}`);
243+
}
207244

208245
/** Build the injection text prepended to the tail source (with per-footnote-
209246
* def coordinate segments). Blocks are '\n\n'-joined: a def line directly
@@ -342,12 +379,13 @@ export function spliceTrees(input: SpliceInput): { mdast: MdastRoot; hast: HastR
342379
const lineDelta = prefixLines - injectedLines;
343380

344381
// --- prefix cuts (non-mutating reads of the previous frame's trees) ---
382+
// Filter semantics (no ordered-children early break): together with G4's
383+
// full straddle scan this keeps the cut correct-or-bailing even for
384+
// disordered trees a plugin-shaped mdast could present (round-2 review).
345385
const prefixMdast: MdastContent[] = [];
346386
for (const child of prevMdast.children) {
347-
// Position-ordered — the first child at/past the boundary ends the prefix.
348387
const start = child.position?.start?.offset;
349-
if (start !== undefined && start >= boundary) break;
350-
if (start !== undefined) prefixMdast.push(child);
388+
if (start !== undefined && start < boundary) prefixMdast.push(child);
351389
}
352390
const attrs = attributeHastChildren(prevMdast, prevHast, boundary);
353391
const cutRegion: HastContent[] = [];
@@ -550,11 +588,13 @@ function alignPrefixCut(
550588
}
551589
}
552590
if (nextIdx === -1) return null;
553-
// j slots belong to the j leading stripped children; the rest is hoist.
554-
// Only the no-stripped shape is probe-verified (hoist merges into the
555-
// last slot when slots exist) — mixed shapes beyond one hoist text are
556-
// out of model.
557-
if (sepBuffer.length - nextIdx > 1) return null;
591+
// Excess leading texts beyond the stripped-child slots can only be
592+
// hoist, and hoist exists as a SEPARATE node only when there is no
593+
// slot to merge into (nextIdx === 0, probe-verified). Any excess in
594+
// the presence of slots is out of model → bail (round-2 review
595+
// tightened this from `excess > 1`).
596+
const excess = sepBuffer.length - nextIdx;
597+
if (excess > (nextIdx === 0 ? 1 : 0)) return null;
558598
} else {
559599
const stripped = sawContent ? sepBuffer.length - 1 : sepBuffer.length;
560600
if (stripped < 0) return null;

0 commit comments

Comments
 (0)