From 66f4beffdeb12737a52bd61b7dc8023395225c51 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Mon, 6 Jul 2026 22:19:42 -0700 Subject: [PATCH 1/6] Show candidate body as a collapsible toggle in the Atlas review artifact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The approval artifact rendered each candidate as a checkbox showing only the canonical-key marker, title, and flag badge — never c.content, the distilled why/how prose. A reviewer could not read the text they were approving or rejecting. Render c.content as a collapsible toggle child (labeled "Content (why/how)") of both the approvable to_do and the unverified note, placed before the provenance callout and evidence bullets. The body flows through the existing richText() helper (2000-char run split, 100-run cap, surrogate safety) and is split across multiple paragraph children when it exceeds one paragraph's run budget, so nothing is dropped. Guarded on non-empty trimmed content so an empty candidate gets no dead toggle. --- .../atlas-artifact-notion-blocks.test.ts | 105 +++++++++++++++++- src/atlas/artifact/notion-blocks.ts | 60 +++++++++- 2 files changed, 158 insertions(+), 7 deletions(-) diff --git a/src/__tests__/atlas-artifact-notion-blocks.test.ts b/src/__tests__/atlas-artifact-notion-blocks.test.ts index 017519f..81d4a77 100644 --- a/src/__tests__/atlas-artifact-notion-blocks.test.ts +++ b/src/__tests__/atlas-artifact-notion-blocks.test.ts @@ -143,6 +143,27 @@ function childrenOf(block: unknown): unknown[] { return b[key]?.children ?? []; } +// Concatenate the rendered text of a block and ALL its descendants (any nesting +// depth) into one string — used to assert the distilled body appears SOMEWHERE +// beneath a candidate to_do, regardless of how many toggle/paragraph levels it +// is wrapped in. +function deepText(block: unknown): string { + const self = plainTextOf(block); + const kids = childrenOf(block).map(deepText); + return [self, ...kids].join("\n"); +} + +// The first descendant block (any depth) of `block` whose `type` matches, or +// undefined. Depth-first, self excluded. +function findDescendant(block: unknown, type: string): unknown { + for (const child of childrenOf(block)) { + if ((child as { type?: string }).type === type) return child; + const nested = findDescendant(child, type); + if (nested !== undefined) return nested; + } + return undefined; +} + const LONE_SURROGATE_RE = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?`. - return children[1]; + // The candidate's children are: content toggle (if any) → provenance + // callout → evidence bullets. Locate the evidence bullet by its `thread: ` + // text rather than a fixed index, so the leading content toggle doesn't + // shift it out from under these split-boundary assertions. + const bullet = childrenOf(candidateToDoBlock(c)).find((ch) => + plainTextOf(ch).startsWith("thread: "), + ); + return bullet; } it("emits a SINGLE run when the content is exactly at the 2000-char cap (no split)", () => { @@ -301,7 +326,12 @@ describe("notion-blocks — 100-run cap with explicit truncation marker", () => const c = makeCandidate({ evidence: [{ kind: "thread", body }], }); - return runsOf(childrenOf(candidateToDoBlock(c))[1]); + // Locate the evidence bullet by its `thread: ` text (a leading content + // toggle child would otherwise shift a fixed index). + const bullet = childrenOf(candidateToDoBlock(c)).find((ch) => + plainTextOf(ch).startsWith("thread: "), + ); + return runsOf(bullet); } it("caps a pathological body at 100 runs and marks the truncation", () => { @@ -538,6 +568,71 @@ describe("notion-blocks — candidate ⇄ block round-trip", () => { }); }); +// ── candidate body shown as a collapsible toggle (reviewer can read the text) ─── +// The reviewer approves/rejects each candidate; they can only make that call if +// they can SEE the distilled why/how prose (`c.content`). It is rendered as a +// collapsible `toggle` child of the item (before the provenance callout + +// evidence bullets), whose paragraph children carry the content via richText. + +describe("notion-blocks — candidate body rendered as a collapsible toggle", () => { + const MARKER = "UNIQUE-BODY-MARKER-7f3a"; + const CONTENT = `${MARKER}: guards POST /admin/:op behind the RBAC check`; + + it("renders the distilled content inside a toggle under an APPROVABLE to_do", () => { + const c = makeCandidate({ content: CONTENT }); + const block = candidateToDoBlock(c); + const toggle = findDescendant(block, "toggle"); + expect(toggle).toBeDefined(); + // The content marker + concrete token must be reachable within the toggle's + // descendant paragraphs (the reviewer can read the body). + expect(deepText(toggle)).toContain(MARKER); + expect(deepText(toggle)).toContain("POST /admin/:op"); + // The toggle carries paragraph children (not just a bare label). + const para = findDescendant(toggle, "paragraph"); + expect(para).toBeDefined(); + expect(plainTextOf(para)).toContain(MARKER); + }); + + it("renders the distilled content inside a toggle under an UNVERIFIED note", () => { + const c = makeCandidate({ approvable: false, content: CONTENT }); + const block = unverifiedNoteBlock(c); + const toggle = findDescendant(block, "toggle"); + expect(toggle).toBeDefined(); + expect(deepText(toggle)).toContain(MARKER); + expect(deepText(toggle)).toContain("POST /admin/:op"); + }); + + it("does NOT emit a toggle when content is empty/whitespace (guard)", () => { + for (const content of ["", " ", "\n\t "]) { + expect( + findDescendant( + candidateToDoBlock(makeCandidate({ content })), + "toggle", + ), + ).toBeUndefined(); + expect( + findDescendant( + unverifiedNoteBlock(makeCandidate({ content })), + "toggle", + ), + ).toBeUndefined(); + } + }); + + it("splits a long body across multiple paragraph runs without dropping content", () => { + // A body far past a single 2000-char run: the toggle's paragraph(s) must + // still reconstruct the full content (marker at the tail proves no drop). + const tail = `${MARKER}-END`; + const content = `${"b".repeat(RICH_TEXT_RUN_MAX + 500)}${tail}`; + const toggle = findDescendant( + candidateToDoBlock(makeCandidate({ content })), + "toggle", + ); + expect(toggle).toBeDefined(); + expect(deepText(toggle)).toContain(tail); + }); +}); + // ── grouped block list: order + non-checkable notes (structure round-trip) ────── describe("notion-blocks — buildCandidateBlocks structure", () => { diff --git a/src/atlas/artifact/notion-blocks.ts b/src/atlas/artifact/notion-blocks.ts index 32916e4..c799c8f 100644 --- a/src/atlas/artifact/notion-blocks.ts +++ b/src/atlas/artifact/notion-blocks.ts @@ -76,6 +76,17 @@ type ChildBlockRequest = NonNullable< Extract["callout"]["children"] >[number]; +// Notion permits children ONE level deeper still under a depth-1 block (a +// candidate's toggle sits at depth 1 as a child of the to_do/note, so ITS OWN +// children — the content paragraphs — are at depth 2). The SDK types that +// deepest permitted level as a distinct (non-exported) leaf union; we capture +// it structurally from the toggle child's own `children` field, exactly as +// ChildBlockRequest is captured from the callout's. Our grandchildren are only +// leaf paragraphs, which live within this depth. +type GrandchildBlockRequest = NonNullable< + Extract["toggle"]["children"] +>[number]; + // ── Markers ─────────────────────────────────────────────────────────────────── // A canonical_key is wrapped `⟦atlas:⟧` at the START of a candidate block's @@ -250,6 +261,45 @@ function evidenceLine(item: Candidate["evidence"][number]): string { // page). 95 + callout + tail = 97 keeps headroom under the cap. const MAX_EVIDENCE_BULLETS = 95; +// The distilled why/how prose (`c.content`) rendered as a COLLAPSIBLE toggle +// child of a candidate's to_do/note, so the reviewer can actually read the text +// they approve/reject (the checkbox line itself stays terse — marker + title + +// badge). The toggle's paragraph children carry the content through `richText`, +// which handles the ≤2000-char run split + 100-run cap + surrogate safety, so a +// long body spans multiple paragraph children losslessly (each rich_text array +// is capped, so a body needing >100 runs gets a second paragraph). Returns an +// empty array (no toggle) when the candidate has no content to show — an empty +// toggle would be a dead disclosure widget for the reviewer. +// +// Placed FIRST among a candidate's children (before the provenance callout + +// evidence bullets) so the body — the thing being approved — is the first thing +// the reviewer expands. +function contentToggleChildren(c: Candidate): ChildBlockRequest[] { + if (!c.content?.trim()) return []; + // `richText` caps its output at RICH_TEXT_MAX_RUNS runs; a pathological body + // longer than that budget would otherwise be truncated at the marker. Split + // the content across as many paragraph children as needed so nothing is + // dropped: each paragraph takes up to RICH_TEXT_MAX_RUNS × RICH_TEXT_RUN_MAX + // characters, and richText re-splits that chunk into ≤2000-char runs. + const paragraphs: GrandchildBlockRequest[] = []; + const chunkMax = RICH_TEXT_RUN_MAX * RICH_TEXT_MAX_RUNS; + for (let i = 0; i < c.content.length; i += chunkMax) { + paragraphs.push({ + type: "paragraph", + paragraph: { rich_text: richText(c.content.slice(i, i + chunkMax)) }, + }); + } + return [ + { + type: "toggle", + toggle: { + rich_text: richText("Content (why/how)"), + children: paragraphs, + }, + }, + ]; +} + // Provenance + evidence as child blocks of a candidate's to_do/note. The // provenance line (source + url + date) is a callout; each evidence item is a // bulleted_list_item. Child blocks keep the checkbox itself terse while still @@ -340,7 +390,10 @@ export function candidateToDoBlock(c: Candidate): BlockObjectRequest { to_do: { rich_text: richText(text), checked: false, - children: provenanceAndEvidenceChildren(c), + children: [ + ...contentToggleChildren(c), + ...provenanceAndEvidenceChildren(c), + ], }, }; } @@ -359,7 +412,10 @@ export function unverifiedNoteBlock(c: Candidate): BlockObjectRequest { callout: { rich_text: richText(text), icon: { type: "emoji", emoji: "⚠️" }, // ⚠️ - children: provenanceAndEvidenceChildren(c), + children: [ + ...contentToggleChildren(c), + ...provenanceAndEvidenceChildren(c), + ], }, }; } From 68298db581d405fa76e577803f79fdc7fb5e000a Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Mon, 6 Jul 2026 22:19:52 -0700 Subject: [PATCH 2/6] Count nested grandchildren in the Atlas artifact per-request block budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit blockCost counted a block plus only its DIRECT children, assuming children are one level deep. The new content toggle adds a second nesting level (to_do → toggle → paragraphs), whose grandchildren went uncounted — a large run could silently exceed Notion's ~1000-block-per-request cap and 400 the whole append. Make blockCost recurse over all descendants so the batcher budgets the true total block count at every depth. --- src/__tests__/atlas-artifact-generate.test.ts | 75 ++++++++++++++++++- src/atlas/artifact/generate.ts | 20 +++-- 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/src/__tests__/atlas-artifact-generate.test.ts b/src/__tests__/atlas-artifact-generate.test.ts index 2c127c3..fee79b2 100644 --- a/src/__tests__/atlas-artifact-generate.test.ts +++ b/src/__tests__/atlas-artifact-generate.test.ts @@ -230,6 +230,20 @@ function childCountOf(block: unknown): number { return (b[key]?.children ?? []).length; } +// A block's TOTAL block count including ALL descendants at any depth (self + a +// recursive walk of every `children` array). Candidate to_dos now nest a +// `toggle` whose own paragraph children are a second level (to_do → toggle → +// paragraph), so the per-request budget must count them — mirrors generate.ts's +// recursive `blockCost`. +function deepBlockCount(block: unknown): number { + const b = block as Record; + const key = (block as { type?: string }).type as string; + const children = b[key]?.children ?? []; + let count = 1; + for (const child of children) count += deepBlockCount(child); + return count; +} + describe("notion-blocks — build side (candidate → blocks)", () => { it("renders an APPROVABLE candidate as a to_do checkbox, unchecked by default", () => { const c = makeCandidate({ title: "Two-layer shim to the v2 engine" }); @@ -501,7 +515,10 @@ describe("notion-blocks — per-block children cap (Notion ~100-children limit)" kind: "fused_from" as const, ref: `fragment-ref-${i}`, })); - const c = makeCandidate({ evidence }); + // Empty content ⇒ no leading content-toggle child, so this test's indices + // address the provenance callout + evidence bullets directly (this case is + // about the EVIDENCE cap, not the body toggle). + const c = makeCandidate({ evidence, content: "" }); const block = candidateToDoBlock(c) as { to_do: { children?: unknown[] }; }; @@ -520,6 +537,7 @@ describe("notion-blocks — per-block children cap (Notion ~100-children limit)" it("leaves a small evidence list uncapped, with no tail bullet", () => { const c = makeCandidate({ + content: "", // no body toggle — this test counts provenance + evidence evidence: [ { kind: "fused_from", ref: "a" }, { kind: "fused_from", ref: "b" }, @@ -1484,6 +1502,61 @@ describe("generateApprovalArtifact", () => { } }); + it("budgets by DEEP block count (toggle→paragraph grandchildren counted), keeping each request ≤800 total", async () => { + // Each candidate carries a long distilled body, which renders as a `toggle` + // whose paragraph children are a SECOND nesting level under the to_do + // (to_do → toggle → paragraphs). Plus 90 evidence bullets. A batcher that + // counted only the to_do's DIRECT children would miss the toggle's own + // grandchildren and could pack a request past Notion's ~1000-block total. + // The DEEP per-request count (self + all descendants) must stay ≤800. + const evidence: EvidenceItem[] = Array.from({ length: 90 }, (_, i) => ({ + kind: "fused_from" as const, + ref: `ev-${i}`, + })); + const candidates = Array.from({ length: 40 }, (_, i) => + makeCandidate({ + subsystem: "cpk-runtime", + rankScore: 40 - i, + canonical_key: `github-pr:cpk-runtime:deep-${i}`, + title: `deep candidate ${i}`, + content: `distilled body ${i}: `.padEnd(6000, "x"), + evidence, + }), + ); + const { client, createCalls, appendCalls } = makeMockNotion(); + await generateApprovalArtifact({ + notion: client, + parentPageId: "parent", + runId: "run-deep", + candidates, + rules: DEFAULT_EXCLUSION_RULES, + }); + + const requests: unknown[][] = [ + (createCalls[0].children ?? []) as unknown[], + ...appendCalls.map((call) => (call.children ?? []) as unknown[]), + ]; + for (const batch of requests) { + expect(batch.length).toBeLessThanOrEqual(100); + const deepTotal = batch.reduce( + (sum: number, block) => sum + deepBlockCount(block), + 0, + ); + expect(deepTotal).toBeLessThanOrEqual(800); + } + + // Every candidate still rendered, in rank order (nothing dropped by the + // tighter budget). + const allChildren = requests.flat() as Array<{ type: string }>; + const todoTitles = allChildren + .filter((b) => b.type === "to_do") + .map((b) => plainTextOf(b)); + expect(todoTitles).toHaveLength(40); + for (let i = 0; i < 40; i++) { + expect(todoTitles[i]).toContain(`deep candidate ${i}`); + } + }); + it("does not call append when the page fits in a single create (≤100 blocks)", async () => { const { client, appendCalls } = makeMockNotion(); await generateApprovalArtifact({ diff --git a/src/atlas/artifact/generate.ts b/src/atlas/artifact/generate.ts index 7ed98bd..a7d7b6b 100644 --- a/src/atlas/artifact/generate.ts +++ b/src/atlas/artifact/generate.ts @@ -137,16 +137,26 @@ function resolveSeedRules( const NOTION_MAX_BLOCKS_PER_REQUEST = 100; const NOTION_MAX_TOTAL_BLOCKS_PER_REQUEST = 800; -// A block-request's total block cost: itself + its nested children. Children -// are one level deep here (notion-blocks.ts renders callouts/bullets only), so -// no recursion is needed. +// A block-request's total block cost: itself PLUS every nested descendant, at +// any depth. Notion budgets a request by its TOTAL block count across all +// nesting levels, so the cost must recurse: a candidate to_do now carries a +// `toggle` whose OWN paragraph children are a second nesting level (to_do → +// toggle → paragraphs), and counting only the to_do's direct children would +// undercount the request — letting a batch silently exceed the ~1000-block cap +// and 400 the whole append. function blockCost(block: BlockObjectRequest): number { const { type } = block as { type?: string }; if (!type) return 1; const body = ( - block as unknown as Record + block as unknown as Record< + string, + { children?: BlockObjectRequest[] } | undefined + > )[type]; - return 1 + (body?.children?.length ?? 0); + const children = body?.children ?? []; + let cost = 1; + for (const child of children) cost += blockCost(child); + return cost; } // Split the ordered block list into request batches obeying both Notion budgets From c93ce933c25f058f9af7b6e033970d7edad253da Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Mon, 6 Jul 2026 22:30:46 -0700 Subject: [PATCH 3/6] Chunk toggle body on surrogate-safe, capacity-correct boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The content-toggle chunker sliced c.content at a fixed 200000 code units, then fed each slice to richText. Two defects for astral-heavy bodies: - Surrogate split: a slice boundary could cut a surrogate PAIR in half; richText's toWellFormedUtf16 then sanitized each lone half to U+FFFD, corrupting the astral char across the chunk seam. - Tail-drop: 200000 = RICH_TEXT_RUN_MAX x RICH_TEXT_MAX_RUNS assumed richText renders a full 200k chunk losslessly, but its per-run surrogate backoff shortens runs, pushing the tail past the char cap and tripping the truncation branch — silently dropping ~2000 chars per chunk. Iterate c.content by code point (never splitting a pair) and cap each chunk at the guaranteed-lossless floor (RICH_TEXT_RUN_MAX-1) x RICH_TEXT_MAX_RUNS, which absorbs a worst-case one-unit backoff on every run. Added red-green tests: an astral char straddling the chunk boundary stays intact (no U+FFFD), and an astral-heavy body >1 chunk round-trips with every code point preserved. Also restore the console.warn spy after each test in the rule-bullet delimiter-tolerance suite (restoreMocks is not globally on, so the spy was leaking into later suites). --- .../atlas-artifact-notion-blocks.test.ts | 55 +++++++++++++++++++ src/atlas/artifact/notion-blocks.ts | 45 ++++++++++++--- 2 files changed, 92 insertions(+), 8 deletions(-) diff --git a/src/__tests__/atlas-artifact-notion-blocks.test.ts b/src/__tests__/atlas-artifact-notion-blocks.test.ts index 81d4a77..bf4e0b4 100644 --- a/src/__tests__/atlas-artifact-notion-blocks.test.ts +++ b/src/__tests__/atlas-artifact-notion-blocks.test.ts @@ -443,6 +443,10 @@ describe("notion-blocks — canonical-key marker (delimiter parse contract)", () // ── rule bullet: the `atlas-rule:` delimiter (case / whitespace tolerance) ────── describe("notion-blocks — rule-bullet delimiter tolerance", () => { + // The malformed-JSON case spies on console.warn; restore after each test so + // the spy does not leak into later suites (restoreMocks is not globally on). + afterEach(() => vi.restoreAllMocks()); + const flagRule: ExclusionRule = { kind: "flag", dimension: "sensitivity", @@ -631,6 +635,57 @@ describe("notion-blocks — candidate body rendered as a collapsible toggle", () expect(toggle).toBeDefined(); expect(deepText(toggle)).toContain(tail); }); + + // Recover the FULL plain-text of a toggle's body: concatenate every paragraph + // grandchild's runs in order (the paragraphs partition the body, each carrying + // a richText run split). This is the exact lossless round-trip Notion delivers + // (fetched plain_text = concatenation of runs), so it must equal the input. + function recoveredToggleBody(block: unknown): string { + const toggle = findDescendant(block, "toggle") as unknown; + const paras = childrenOf(toggle).filter( + (ch) => (ch as { type?: string }).type === "paragraph", + ); + return paras.map((p) => plainTextOf(p)).join(""); + } + + it("preserves an astral char that straddles the chunk boundary (no U+FFFD)", () => { + // The chunk splitter partitions c.content into paragraph children. A naive + // fixed-code-unit slice can cut an astral char's surrogate PAIR in half at a + // chunk boundary; richText then sanitizes each lone half to U+FFFD, so the + // recovered body loses the char (two replacement chars in its place). Place + // an astral char (𝕏 = U+1D54F, a surrogate pair) EXACTLY at the boundary of + // the effective per-paragraph capacity so a fixed-unit slice would sever it. + const CAP = RICH_TEXT_MAX_RUNS * RICH_TEXT_RUN_MAX; // naive boundary = 200000 + const astral = "\u{1D54F}"; // 𝕏 — one code point, two UTF-16 units + // High half lands at index CAP-1, low half at index CAP → the naive + // slice(0, CAP) keeps a lone HIGH surrogate; slice(CAP) starts on a lone LOW. + const content = `${"a".repeat(CAP - 1)}${astral}${"b".repeat(100)}`; + const block = candidateToDoBlock(makeCandidate({ content })); + const recovered = recoveredToggleBody(block); + expect(recovered).toContain(astral); // pair intact — reviewer sees 𝕏 + expect(recovered).not.toContain("�"); // no corruption + expect(recovered).toBe(content); // nothing dropped or mangled + }); + + it("does not tail-drop an astral-heavy body larger than one chunk", () => { + // richText's per-run surrogate backoff means a full 100-run render of an + // astral-heavy chunk carries FEWER than 100×2000 code units losslessly: + // the accumulated backoff pushes the tail past the char cap and trips the + // truncation branch, silently dropping ~2001 chars per chunk. A body of all + // astral chars (every pair a candidate backoff point) longer than one chunk + // must round-trip with EVERY code point preserved. + const astral = "\u{1D54F}"; // 𝕏 — two UTF-16 units each + // A single leading ASCII char makes every astral pair straddle an EVEN + // index, so each 2000-unit run boundary lands mid-pair and backs off — the + // accumulated shortfall is what trips richText's truncation branch. ~1.5 + // naive chunks worth of astral: 1 + 300000 code units of content. + const content = `a${astral.repeat(150000)}`; + const block = candidateToDoBlock(makeCandidate({ content })); + const recovered = recoveredToggleBody(block); + expect(recovered).not.toContain("�"); // no lone-surrogate corruption + expect(recovered.length).toBe(content.length); // no silent tail-drop + expect(recovered).toBe(content); // exact, in order + }); }); // ── grouped block list: order + non-checkable notes (structure round-trip) ────── diff --git a/src/atlas/artifact/notion-blocks.ts b/src/atlas/artifact/notion-blocks.ts index c799c8f..ea2d31d 100644 --- a/src/atlas/artifact/notion-blocks.ts +++ b/src/atlas/artifact/notion-blocks.ts @@ -276,19 +276,48 @@ const MAX_EVIDENCE_BULLETS = 95; // the reviewer expands. function contentToggleChildren(c: Candidate): ChildBlockRequest[] { if (!c.content?.trim()) return []; - // `richText` caps its output at RICH_TEXT_MAX_RUNS runs; a pathological body - // longer than that budget would otherwise be truncated at the marker. Split - // the content across as many paragraph children as needed so nothing is - // dropped: each paragraph takes up to RICH_TEXT_MAX_RUNS × RICH_TEXT_RUN_MAX - // characters, and richText re-splits that chunk into ≤2000-char runs. + // Split the content across as many paragraph children as needed so nothing is + // dropped, feeding each chunk to `richText` (which re-splits it into ≤2000-char + // runs). The chunking must be BOTH surrogate-safe AND capacity-correct: + // + // • Surrogate-safe: a chunk boundary must never fall between the halves of a + // surrogate pair. A lone half would ride into `richText`, whose entry + // `toWellFormedUtf16` sanitizes it to U+FFFD — corrupting an astral char + // (an emoji in thread evidence) into a replacement char across the chunk + // seam. Iterating by CODE POINT (not code unit) makes every boundary land + // between whole characters, so no pair is ever severed (same invariant + // clampTitle guards at the title clamp and richText guards at run edges). + // + // • Capacity-correct: `richText` renders at most RICH_TEXT_MAX_RUNS runs, and + // its per-run surrogate backoff can shorten EACH run by one unit (a pair + // straddling a 2000-unit run boundary). A naive chunk of + // RICH_TEXT_RUN_MAX × RICH_TEXT_MAX_RUNS units therefore does NOT fit: the + // accumulated backoff pushes the last run past the char cap and trips + // richText's truncation marker, silently dropping ~RICH_TEXT_RUN_MAX chars + // per chunk for astral-heavy content — the very data this loop exists to + // preserve. Sizing each chunk to the GUARANTEED-lossless floor, (2000−1) + // per run × 100 runs, absorbs a worst-case one-unit backoff on every run, + // so richText renders each chunk without ever truncating. const paragraphs: GrandchildBlockRequest[] = []; - const chunkMax = RICH_TEXT_RUN_MAX * RICH_TEXT_MAX_RUNS; - for (let i = 0; i < c.content.length; i += chunkMax) { + const CHUNK_MAX_UNITS = (RICH_TEXT_RUN_MAX - 1) * RICH_TEXT_MAX_RUNS; + let chunk = ""; + const flush = () => { + if (chunk === "") return; paragraphs.push({ type: "paragraph", - paragraph: { rich_text: richText(c.content.slice(i, i + chunkMax)) }, + paragraph: { rich_text: richText(chunk) }, }); + chunk = ""; + }; + // `for…of` on a string iterates by code point, so `cp` is a whole character + // (1 or 2 UTF-16 units) — an astral pair is never split at a chunk boundary. + // Flush BEFORE appending a code point that would carry the chunk over the safe + // capacity (never mid-character), so every chunk stays ≤ CHUNK_MAX_UNITS. + for (const cp of c.content) { + if (chunk.length + cp.length > CHUNK_MAX_UNITS) flush(); + chunk += cp; } + flush(); return [ { type: "toggle", From f2dab35f003af5bb7e4f38ee37da88802cb1915f Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Mon, 6 Jul 2026 22:30:46 -0700 Subject: [PATCH 4/6] Fail loud when a single block exceeds the Notion per-request budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update the stale batchBlocks comment (a candidate now nests a content toggle whose paragraph children add a second level, so a block's recursive cost is no longer a small constant), and throw a clear error when one block's blockCost alone exceeds NOTION_MAX_TOTAL_BLOCKS_PER_REQUEST — such a block can never fit any batch and would 400 the append, so surface it here rather than at the API. --- src/atlas/artifact/generate.ts | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/atlas/artifact/generate.ts b/src/atlas/artifact/generate.ts index a7d7b6b..57842aa 100644 --- a/src/atlas/artifact/generate.ts +++ b/src/atlas/artifact/generate.ts @@ -161,15 +161,29 @@ function blockCost(block: BlockObjectRequest): number { // Split the ordered block list into request batches obeying both Notion budgets // (≤100 top-level, ≤800 total incl. nested children). Order-preserving: a batch -// is flushed exactly when the NEXT block would exceed either budget. Every -// individual block fits a batch by itself (top-level cost 1, total cost ≤ ~98 -// under notion-blocks.ts's children cap), so no block can be dropped here. +// is flushed exactly when the NEXT block would exceed either budget. A candidate +// block now nests a content toggle (whose OWN paragraph children add a second +// level) alongside its provenance callout + ≤~97 evidence bullets, so a single +// block's recursive cost is no longer bounded by a small constant: a candidate +// with a very long body carries many toggle paragraphs. Almost every candidate +// still fits a batch alone (evidence caps the bullets; typical bodies are one or +// two paragraphs), but a pathological body whose recursive blockCost alone +// exceeds the total-block budget could never fit ANY batch — emitting it would +// build a batch Notion 400s. Fail LOUD on that block instead of deferring the +// failure to the API. function batchBlocks(children: BlockObjectRequest[]): BlockObjectRequest[][] { const batches: BlockObjectRequest[][] = []; let batch: BlockObjectRequest[] = []; let total = 0; for (const block of children) { const cost = blockCost(block); + if (cost > NOTION_MAX_TOTAL_BLOCKS_PER_REQUEST) { + throw new Error( + `[atlas] a single block's total cost (${cost} blocks incl. nested children) exceeds ` + + `the Notion per-request budget (${NOTION_MAX_TOTAL_BLOCKS_PER_REQUEST}); ` + + `it cannot fit any batch and would 400 the append`, + ); + } if ( batch.length > 0 && (batch.length + 1 > NOTION_MAX_BLOCKS_PER_REQUEST || From 8deeff1fe91bd37ddc7fc09acdeb0505d27b49e5 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Mon, 6 Jul 2026 22:46:39 -0700 Subject: [PATCH 5/6] Descend into the content toggle when re-checking approved candidates for exclusions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The S16 review artifact renders each candidate's distilled body inside a "Content (why/how)" toggle whose paragraph children hold the prose, but the sync-side extractor (fetchChildProse) read only the to_do's direct children. For the toggle that captured its static label, not the body — so the on-approval credential/exclusion re-check (§11) judged title + label, never the body a reviewer actually sees. A clean-titled candidate whose body leaked a credential was approved, and the constant "Content (why/how)" leaked into the payload. fetchChildProse now descends one level into a toggle child, folding its paragraph grandchildren (with the same canonical-key marker filter) and skipping the toggle's own label. Non-toggle children stay depth-1 as before. Adds a real build->fetch->reconstruct->applyExclusions round-trip test proving a body-only ghp_ token is caught and the toggle label never enters the payload. --- src/__tests__/atlas-artifact-sync.test.ts | 222 ++++++++++++++++++++++ src/atlas/artifact/sync.ts | 54 ++++-- 2 files changed, 264 insertions(+), 12 deletions(-) diff --git a/src/__tests__/atlas-artifact-sync.test.ts b/src/__tests__/atlas-artifact-sync.test.ts index a43be9c..4db5a9c 100644 --- a/src/__tests__/atlas-artifact-sync.test.ts +++ b/src/__tests__/atlas-artifact-sync.test.ts @@ -252,6 +252,89 @@ function candidateAsFetchedToDo( return toDoResponse(text, checked, opts); } +// A generic fetched-block response of any text-bearing type — every such block +// carries its rich_text under its own type key (paragraph, toggle, callout, …), +// which is exactly the shape `blockPlainText`/`fetchChildProse` read back. +function textBlockResponse( + type: string, + plainText: string, + opts: { id?: string; hasChildren?: boolean } = {}, +): BlockObjectResponse { + return { + type, + [type]: { + rich_text: [ + { + type: "text", + plain_text: plainText, + href: null, + annotations: { + bold: false, + italic: false, + strikethrough: false, + underline: false, + code: false, + color: "default", + }, + text: { content: plainText, link: null }, + }, + ], + color: "default", + }, + parent: { type: "page_id", page_id: "p" }, + object: "block", + id: opts.id ?? `${type}-${Math.random().toString(36).slice(2)}`, + created_time: "2026-06-08T00:00:00.000Z", + created_by: { object: "user", id: "u" }, + last_edited_time: "2026-06-08T00:00:00.000Z", + last_edited_by: { object: "user", id: "u" }, + has_children: opts.hasChildren ?? false, + in_trash: false, + archived: false, + } as unknown as BlockObjectResponse; +} + +// Convert the REAL request-block subtree S16 builds (`candidateToDoBlock`'s +// children — a `toggle` whose paragraph grandchildren hold the distilled body, +// plus the provenance callout / evidence bullets) into the fetched-back Notion +// response tree, wiring each block's own children into a `children` map keyed by +// a synthesized id. This mirrors EXACTLY how the page round-trips: build → +// (human edit) → fetch, so `fetchChildProse` sees the same toggle→paragraph +// indirection it must descend through. Returns the top-level response blocks and +// the id→children map to feed `makeMockNotion`. +function requestChildrenAsFetched( + requestChildren: unknown[], + idPrefix: string, +): { + blocks: BlockObjectResponse[]; + childrenMap: Record; +} { + const childrenMap: Record = {}; + let counter = 0; + const convert = (req: unknown): BlockObjectResponse => { + const b = req as Record & { type: string }; + const type = b.type; + const data = b[type] as { + rich_text?: Array<{ text?: { content?: string } }>; + children?: unknown[]; + }; + const plainText = (data.rich_text ?? []) + .map((r) => r.text?.content ?? "") + .join(""); + const id = `${idPrefix}-${type}-${counter++}`; + const grandchildren = data.children ?? []; + const response = textBlockResponse(type, plainText, { + id, + hasChildren: grandchildren.length > 0, + }); + if (grandchildren.length > 0) { + childrenMap[id] = grandchildren.map(convert); + } + return response; + }; + return { blocks: requestChildren.map(convert), childrenMap }; +} + // ── Mock Notion client whose blocks.children.list returns a fixed page ─────── interface MockNotion { @@ -593,6 +676,145 @@ describe("syncApprovalArtifact (S17)", () => { ); }); + it("descends into the 'Content (why/how)' toggle so a body-only credential is caught (real round-trip, §11)", async () => { + // The REAL round-trip: S16 renders the distilled body inside a `toggle` + // ("Content (why/how)") whose PARAGRAPH grandchildren carry the prose + // (candidateToDoBlock → toggle → paragraphs), NOT as a direct child of the + // to_do. The title here is clean, but the body paragraph carries a GitHub + // PAT-shaped token (ghp_ + 36 alnum) the deterministic credential floor must + // catch. `fetchChildProse` must DESCEND into the toggle to reach it — a + // depth-1-only read would capture the toggle's static "Content (why/how)" + // label and MISS the body, wrongly approving a leaked-credential row. + const bodyToken = `ghp_${"a1b2c3d4e5".repeat(4).slice(0, 36)}`; + expect(bodyToken).toMatch(/^ghp_[A-Za-z0-9]{36}$/); + const bodyDirty = makeCandidate({ + canonical_key: "github-pr:cpk-runtime:toggle-body-credential", + title: "Rotate the deploy pipeline settings", + content: `The deploy runbook pasted a live token: ${bodyToken} into the log stream.`, + }); + + // The SHIPPED credential english rule (fail-restrictive deterministic floor; + // no LLM needed for the credential shape) — mirrors DEFAULT_EXCLUSION_RULES. + const credentialRule: ExclusionRule = { + kind: "english", + text: "Exclude anything that contains or reveals credentials, secret API keys, access tokens, passwords, or other sensitive secret values.", + }; + + // Build the candidate's REAL child tree and echo it back as the fetched + // toggle→paragraph response tree, wired into the children map. + const todoId = "todo-toggle-body-credential"; + const requestBlock = candidateToDoBlock(bodyDirty) as unknown as Record< + string, + { children?: unknown[] } + >; + const requestChildren = requestBlock.to_do?.children ?? []; + const { blocks: childBlocks, childrenMap } = requestChildrenAsFetched( + requestChildren, + todoId, + ); + + const { client: notion } = makeMockNotion( + [ + bulletResponse(ruleToBulletText(credentialRule)), + candidateAsFetchedToDo(bodyDirty, true, { + id: todoId, + hasChildren: true, + }), + ], + { + children: { + [todoId]: childBlocks, + ...childrenMap, + }, + }, + ); + const { client, approve, reject } = makeMockHttpClient(); + + const result = await syncApprovalArtifact({ + notion, + pageId: "page-toggle-body-credential", + client, + actor: ACTOR, + llm, + }); + + // The body credential is caught → candidate EXCLUDED (never approved). + expect(result.excluded).toEqual([ + "github-pr:cpk-runtime:toggle-body-credential", + ]); + expect(result.approved).toEqual([]); + expect(approve).not.toHaveBeenCalled(); + expect(reject).toHaveBeenCalledWith( + expect.objectContaining({ + canonicalKey: "github-pr:cpk-runtime:toggle-body-credential", + }), + ACTOR, + ); + }); + + it("does NOT leak the 'Content (why/how)' toggle LABEL into the reconstructed content", async () => { + // The toggle's own rich_text is a static UI label — it must never enter the + // english-rule payload (it would pollute every judgment and could even + // accidentally match a rule). The extractor folds the toggle's PARAGRAPH + // grandchildren, skipping the toggle label itself. Observed via the aimock + // journal: the reconstructed content carries the body prose ("bodyzzz") but + // NOT the "Content (why/how)" label. + mock.clearRequests(); + const c = makeCandidate({ + canonical_key: "github-pr:cpk-runtime:toggle-label-leak", + title: "A clean claim about bodyzzz retention", + content: "why/how prose about bodyzzz that should ride, label should not", + }); + const keepRule: ExclusionRule = { + kind: "english", + text: "Exclude rows that reveal customer contract values.", + }; + + const todoId = "todo-toggle-label-leak"; + const requestBlock = candidateToDoBlock(c) as unknown as Record< + string, + { children?: unknown[] } + >; + const requestChildren = requestBlock.to_do?.children ?? []; + const { blocks: childBlocks, childrenMap } = requestChildrenAsFetched( + requestChildren, + todoId, + ); + + const { client: notion } = makeMockNotion( + [ + bulletResponse(ruleToBulletText(keepRule)), + candidateAsFetchedToDo(c, true, { id: todoId, hasChildren: true }), + ], + { children: { [todoId]: childBlocks, ...childrenMap } }, + ); + const { client } = makeMockHttpClient(); + + const result = await syncApprovalArtifact({ + notion, + pageId: "page-toggle-label-leak", + client, + actor: ACTOR, + llm, + }); + + expect(result.approved).toEqual([ + "github-pr:cpk-runtime:toggle-label-leak", + ]); + const entry = mock + .getRequests() + .find((r) => JSON.stringify(r.body ?? {}).includes("bodyzzz")); + expect(entry).toBeDefined(); + const userMessage = String( + entry!.body!.messages.find((m) => m.role === "user")?.content ?? "", + ); + const payload = JSON.parse(userMessage) as { + candidate: { content: string }; + }; + expect(payload.candidate.content).toContain("bodyzzz"); + expect(payload.candidate.content).not.toContain("Content (why/how)"); + }); + it("falls back to title-only content for a checked row with no child blocks", async () => { // A hand-typed checkbox has no children; its title is the only judgeable // text. A clean-titled childless row must survive the same english rule that diff --git a/src/atlas/artifact/sync.ts b/src/atlas/artifact/sync.ts index 248fd76..c6218b2 100644 --- a/src/atlas/artifact/sync.ts +++ b/src/atlas/artifact/sync.ts @@ -289,12 +289,24 @@ function blockPlainText(block: BlockObjectResponse): string { return richText.map((r) => r.plain_text ?? "").join(""); } -// The prose under a candidate's to_do — the provenance callout + evidence -// bullets `provenanceAndEvidenceChildren` rendered at generate time (plus -// anything the lead added by hand). This is the real candidate CONTENT an -// english rule must judge (§11): a clean-titled candidate whose body reveals -// e.g. a credential is only catchable here. Empty for a childless row (a -// hand-typed checkbox) — the caller then falls back to title-only content. +// The prose under a candidate's to_do — the distilled candidate BODY plus the +// provenance callout + evidence bullets `provenanceAndEvidenceChildren` rendered +// at generate time (plus anything the lead added by hand). This is the real +// candidate CONTENT an english rule must judge (§11): a clean-titled candidate +// whose body reveals e.g. a credential is only catchable here. Empty for a +// childless row (a hand-typed checkbox) — the caller then falls back to +// title-only content. +// +// The distilled body does NOT sit at depth-1: S16 renders it inside a `toggle` +// ("Content (why/how)") whose PARAGRAPH children carry the prose (the to_do's +// direct children are that toggle plus the provenance/evidence blocks — see +// candidateToDoBlock). So the extractor must DESCEND one level into a toggle to +// reach the body it re-checks; a depth-1-only read would capture the toggle's +// static "Content (why/how)" label and MISS the body — the very §11 gate bypass +// this exists to close. The toggle's own label is deliberately SKIPPED (never +// folded into the prose), so the constant never enters the english-rule / +// credential-floor payload. Non-toggle children (callouts, bullets) stay at +// depth-1 exactly as before. // // A marker-bearing child is NOT prose, whatever its block TYPE: a nested // marker to_do is a CANDIDATE in its own right (the recursive discovery walk @@ -303,18 +315,36 @@ function blockPlainText(block: BlockObjectResponse): string { // block) is a machine record. Folding either's text into the parent's content // would leak the `⟦atlas:…⟧` machine marker into the english-rule payload, so // the filter keys on the marker itself (extractCanonicalKey), not on -// to_do-ness. +// to_do-ness. The same marker filter is applied to a toggle's descended +// paragraph grandchildren. async function fetchChildProse( notion: Client, block: BlockObjectResponse, ): Promise { if (!block.has_children) return ""; const children = await readAllBlocks(notion, block.id); - return children - .filter((child) => extractCanonicalKey(blockPlainText(child)) === null) - .map((child) => blockPlainText(child).trim()) - .filter((text) => text !== "") - .join("\n"); + const parts: string[] = []; + for (const child of children) { + if (child.type === "toggle") { + // The distilled body lives in the toggle's PARAGRAPH grandchildren. Skip + // the toggle's own label ("Content (why/how)") — fold only its children, + // applying the same marker filter so a hand-nested marker block never + // leaks the `⟦atlas:…⟧` machine marker into the payload. A toggle with no + // children contributes nothing. + if (!child.has_children) continue; + const grandchildren = await readAllBlocks(notion, child.id); + for (const gc of grandchildren) { + if (extractCanonicalKey(blockPlainText(gc)) !== null) continue; + const text = blockPlainText(gc).trim(); + if (text !== "") parts.push(text); + } + continue; + } + if (extractCanonicalKey(blockPlainText(child)) !== null) continue; + const text = blockPlainText(child).trim(); + if (text !== "") parts.push(text); + } + return parts.join("\n"); } // A neutral classification used when a checkbox carries no parseable badge (e.g. From eddabfcbce802b7fe95d25a7f6fff22ac1790f67 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Mon, 6 Jul 2026 22:46:44 -0700 Subject: [PATCH 6/6] Retitle the direct-child budget test to match what it asserts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test titled "budgets by TOTAL block count (top-level + nested)" summed only self + DIRECT children (childCountOf), so it never verified grandchild counting — the deep/toggle-paragraph budget is covered by the sibling "budgets by DEEP block count" test. Retitle and reword to "top-level + DIRECT children" so the title is true and points at the sibling for the deep case. --- src/__tests__/atlas-artifact-generate.test.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/__tests__/atlas-artifact-generate.test.ts b/src/__tests__/atlas-artifact-generate.test.ts index fee79b2..aa59667 100644 --- a/src/__tests__/atlas-artifact-generate.test.ts +++ b/src/__tests__/atlas-artifact-generate.test.ts @@ -1442,12 +1442,15 @@ describe("generateApprovalArtifact", () => { } }); - it("budgets batches by TOTAL block count (top-level + nested children), order preserved", async () => { + it("budgets batches by block count (top-level + DIRECT children), order preserved", async () => { // 30 candidates each carrying 150 evidence items. After the per-block cap, - // each to_do still carries ~97 nested children, so a batcher that counts + // each to_do still carries ~97 direct children, so a batcher that counts // only top-level blocks would pack all 30 to_dos (~3000 total blocks) into // one request and blow Notion's ~1000-total-blocks-per-request limit. The - // batcher must budget by TOTAL block count and flush early. + // batcher must budget by block count (self + direct children) and flush + // early. (DEEP/grandchild counting — the toggle→paragraph second level — is + // asserted by the sibling `budgets by DEEP block count` test below; this one + // scopes to the top-level + direct-child arithmetic it actually checks.) const evidence: EvidenceItem[] = Array.from({ length: 150 }, (_, i) => ({ kind: "fused_from" as const, ref: `ev-${i}`, @@ -1482,7 +1485,7 @@ describe("generateApprovalArtifact", () => { for (const block of batch) { expect(childCountOf(block)).toBeLessThanOrEqual(100); } - // …and the request's TOTAL block count (top-level + nested) stays under + // …and the request's block count (self + DIRECT children) stays under // a conservative budget below Notion's ~1000-blocks-per-request cap. const total = batch.reduce( (sum: number, block) => sum + 1 + childCountOf(block),