From d1c3c120a60e2db1ba964f24492ad9571f006fde Mon Sep 17 00:00:00 2001 From: ivandebot Date: Fri, 24 Jul 2026 00:16:34 +0800 Subject: [PATCH] fix(propose_takes): memoize empty extractions so zero-claim pages don't re-spend every cycle (#2514) The idempotency row is only written inside `for (const p of proposals)`, so a page that extracts ZERO gradeable claims never records an idempotency tuple and is re-sent to the LLM on every cycle forever. The docstring's "unchanged page never re-spends tokens" contract only holds for pages that produce >=1 claim; a page that legitimately has no gradeable claims (or any machine- generated page) is a perpetual cache miss and re-spends tokens indefinitely. Fix: when `proposals.length === 0`, write one tombstone row keyed by the same (source_id, page_slug, content_hash, prompt_version) tuple, with status='rejected' so it never surfaces in a pending-review query (the pending index filters status='pending'). Content changes (new content_hash) or a PROPOSE_TAKES_PROMPT_VERSION bump still miss the tombstone and re-extract. The extractor-throw path `continue`s before the tombstone, so failed pages are retried rather than cached. Guard against a subtle regression: `parseExtractorOutput` returns [] for BOTH a genuine empty extraction AND malformed/prose/truncated model output, so naively tombstoning every [] would permanently suppress a page that has claims but hit a transient parse failure. `defaultExtractor` now throws when the output is empty-but-not-a-clean-`[]` (new `isWellFormedEmptyExtraction` predicate), routing transient failures into the existing retry path; only a cleanly-parsed empty array is memoized. Adds a `tombstones_written` counter for observability. Tests: tombstone written on genuine empty extraction; two-cycle idempotency (no repeat LLM call on an unchanged zero-claim page); extractor error writes no tombstone; isWellFormedEmptyExtraction discriminates clean-[] from malformed/prose/non-empty output. propose-takes suite: 36 pass / 0 fail. Co-authored-by: ivandebot <187176982+ivandebot@users.noreply.github.com> --- src/core/cycle/propose-takes.ts | 88 +++++++++++++++++++++++- test/propose-takes.test.ts | 117 +++++++++++++++++++++++++++++++- 2 files changed, 202 insertions(+), 3 deletions(-) diff --git a/src/core/cycle/propose-takes.ts b/src/core/cycle/propose-takes.ts index 4fe1f1b2a..135c4e7d2 100644 --- a/src/core/cycle/propose-takes.ts +++ b/src/core/cycle/propose-takes.ts @@ -55,6 +55,17 @@ import type { PhaseStatus, CyclePhase } from '../cycle.ts'; */ export const PROPOSE_TAKES_PROMPT_VERSION = 'v0.36.1.0-tuned-cat15'; +/** + * Sentinel claim_text for the tombstone row written when a page extracts + * ZERO gradeable claims. Without a tombstone the idempotency tuple is never + * recorded, so every cycle re-spends an LLM call on unchanged zero-claim + * prose — the "unchanged page never re-spends tokens" contract only held + * for pages that produced >=1 claim. The tombstone is inserted with + * status='rejected' so no pending-review query surfaces it as a live + * proposal; its only job is to make the next cycle a cache hit. + */ +export const EMPTY_EXTRACTION_TOMBSTONE_TEXT = '(no gradeable claims)'; + /** * Tuned extractor prompt, validated against the hand-labeled synthetic * corpus at test/fixtures/calibration/. Measured F1 on first live run @@ -154,6 +165,8 @@ export interface ProposeTakesResult { cache_hits: number; cache_misses: number; proposals_inserted: number; + /** Idempotency rows written for pages that extracted zero claims. */ + tombstones_written: number; budget_exhausted: boolean; /** True when the phase deadline fired before the page loop completed (partial result). */ deadline_hit?: boolean; @@ -287,7 +300,43 @@ export async function defaultExtractor( }); // ChatResult.text is already the concatenated text content. - return parseExtractorOutput(result.text); + const takes = parseExtractorOutput(result.text); + // A parse-level `[]` is AMBIGUOUS: it means either "the model genuinely + // found no gradeable claims" OR "the model returned malformed/prose/ + // truncated output we couldn't parse." The caller memoizes empty + // extractions with a tombstone, so a transient parse failure would + // PERMANENTLY suppress a page that actually has claims. Only a cleanly + // parsed empty array is a real "no claims" result worth memoizing; treat + // anything else as a transient error and throw, so the phase's catch + // retries the page next cycle (writing no tombstone). + if (takes.length === 0 && !isWellFormedEmptyExtraction(result.text)) { + throw new Error('propose_takes extractor: no parseable takes JSON (transient — retry)'); + } + return takes; +} + +/** + * True only when `raw` is a cleanly-parseable EMPTY JSON array — the + * well-behaved "no gradeable claims" response (the prompt instructs the model + * to return `[]`). Distinguishes a genuine empty extraction (safe to memoize + * via a tombstone) from malformed / prose / truncated output (transient — + * must be retried, never tombstoned). Mirrors parseExtractorOutput's + * fence-strip + first-array handling so both agree on what "the model + * returned []" means. + */ +export function isWellFormedEmptyExtraction(raw: string): boolean { + if (!raw || raw.trim().length === 0) return false; + let text = raw.trim(); + const fenced = text.match(/^```(?:json)?\s*\n?([\s\S]*?)\n?```$/); + if (fenced) text = (fenced[1] ?? '').trim(); + const arrStart = text.indexOf('['); + if (arrStart === -1) return false; + try { + const parsed = JSON.parse(text.slice(arrStart)); + return Array.isArray(parsed) && parsed.length === 0; + } catch { + return false; + } } /** @@ -405,6 +454,7 @@ class ProposeTakesPhase extends BaseCyclePhase { cache_hits: 0, cache_misses: 0, proposals_inserted: 0, + tombstones_written: 0, budget_exhausted: false, warnings: [], }; @@ -511,6 +561,40 @@ class ProposeTakesPhase extends BaseCyclePhase { ); result.proposals_inserted += 1; } + + // Memoize the empty case too. A page that extracted zero claims gets + // NO row from the loop above, so without this its idempotency tuple is + // never recorded and the next cycle re-spends an LLM call on unchanged + // prose (the idle-cost bug). Write one tombstone row keyed by the same + // (source, slug, content_hash, prompt_version) tuple. status='rejected' + // keeps it out of any pending-review query; its sole purpose is to make + // the next cycle a cache hit. Only reached on a SUCCESSFUL empty extract + // — the extractor-throw path `continue`s above, so failed pages are + // retried rather than tombstoned. + if (proposals.length === 0) { + await engine.executeRaw( + `INSERT INTO take_proposals + (source_id, page_slug, content_hash, prompt_version, proposal_run_id, + claim_text, kind, holder, weight, domain, dedup_against_fence_rows, model_id, status) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, 'rejected') + ON CONFLICT (source_id, page_slug, content_hash, prompt_version) DO NOTHING`, + [ + sourceId, + page.slug, + ch, + promptVersion, + proposalRunId, + EMPTY_EXTRACTION_TOMBSTONE_TEXT, + 'fact', + 'brain', + 0, + null, + JSON.stringify(existingTakes), + opts.model ?? 'claude-sonnet-4-6', + ], + ); + result.tombstones_written += 1; + } } if (opts.reporter) opts.reporter.finish(); @@ -547,7 +631,7 @@ class ProposeTakesPhase extends BaseCyclePhase { }); return { - summary: `propose_takes: scanned ${result.pages_scanned} pages, ${result.cache_hits} cached, ${result.proposals_inserted} new proposals (run ${proposalRunId})`, + summary: `propose_takes: scanned ${result.pages_scanned} pages, ${result.cache_hits} cached, ${result.proposals_inserted} new proposals, ${result.tombstones_written} empty (run ${proposalRunId})`, details: { ...result, proposal_run_id: proposalRunId, prompt_version: promptVersion }, status: result.budget_exhausted || result.deadline_hit ? 'warn' : 'ok', }; diff --git a/test/propose-takes.test.ts b/test/propose-takes.test.ts index fbe8deb26..681730ecb 100644 --- a/test/propose-takes.test.ts +++ b/test/propose-takes.test.ts @@ -22,7 +22,9 @@ import { contentHash, hasCompleteFence, extractExistingTakesForDedup, + isWellFormedEmptyExtraction, PROPOSE_TAKES_PROMPT_VERSION, + EMPTY_EXTRACTION_TOMBSTONE_TEXT, type ProposeTakesExtractor, type ProposedTake, } from '../src/core/cycle/propose-takes.ts'; @@ -68,7 +70,15 @@ function buildMockEngine(opts: { if (existing.has(key)) return [{ id: 1 } as unknown as T]; return []; } - // INSERT — return nothing + // INSERT into take_proposals — persist the idempotency key so a + // subsequent cycle observes a cache hit, mirroring the real unique + // index on (source_id, page_slug, content_hash, prompt_version). + if (sql.includes('INSERT INTO take_proposals')) { + const [sourceId, slug, ch, pv] = params ?? []; + existing.add(`${sourceId}|${slug}|${ch}|${pv}`); + return []; + } + // Other writes — return nothing. return []; }, } as unknown as BrainEngine; @@ -171,6 +181,52 @@ describe('parseExtractorOutput', () => { }); }); +// ─── isWellFormedEmptyExtraction ──────────────────────────────────── +// Guards the tombstone against permanently memoizing a transient parse +// failure as "no claims". Only a cleanly-parsed empty array counts as a +// genuine empty extraction; malformed/prose/truncated output must not. + +describe('isWellFormedEmptyExtraction', () => { + test('true for a clean empty array (the well-behaved "no claims" response)', () => { + expect(isWellFormedEmptyExtraction('[]')).toBe(true); + expect(isWellFormedEmptyExtraction(' [] ')).toBe(true); + expect(isWellFormedEmptyExtraction('[ ]')).toBe(true); + }); + + test('true for a fenced empty array', () => { + expect(isWellFormedEmptyExtraction('```json\n[]\n```')).toBe(true); + }); + + test('true for leading prose then an empty array', () => { + expect(isWellFormedEmptyExtraction('No gradeable claims.\n\n[]')).toBe(true); + }); + + test('false for empty / whitespace output (transient, must retry)', () => { + expect(isWellFormedEmptyExtraction('')).toBe(false); + expect(isWellFormedEmptyExtraction(' \n ')).toBe(false); + }); + + test('false for prose-only / non-JSON output (transient, must retry)', () => { + expect(isWellFormedEmptyExtraction('There are no gradeable claims here.')).toBe(false); + expect(isWellFormedEmptyExtraction('null')).toBe(false); + }); + + test('false for malformed / truncated JSON (transient, must retry)', () => { + expect(isWellFormedEmptyExtraction('[')).toBe(false); + expect(isWellFormedEmptyExtraction('[{"claim_text":"x"')).toBe(false); + }); + + test('false for a NON-empty array (has content — not an empty extraction)', () => { + expect(isWellFormedEmptyExtraction('[{"claim_text":"x","kind":"take","holder":"brain","weight":0.5}]')).toBe(false); + // Parseable but claim-less array is ambiguous garbage → not a genuine empty. + expect(isWellFormedEmptyExtraction('[{"foo":"bar"}]')).toBe(false); + }); + + test('false for an empty object (model ignored the array-format instruction)', () => { + expect(isWellFormedEmptyExtraction('{}')).toBe(false); + }); +}); + // ─── contentHash ──────────────────────────────────────────────────── describe('contentHash', () => { @@ -554,3 +610,62 @@ New prose appended here.`; expect(pageSelect!.params[0]).toEqual(['team-a', 'team-b']); }); }); + +// ─── Empty-extraction memoization (idle-cost fix) ─────────────────── +// A page that yields zero gradeable claims must still record an +// idempotency row, or every cycle re-spends an LLM call on unchanged +// prose. Regression guard for the "empty result never memoized" bug. + +describe('runPhaseProposeTakes — empty extraction memoization', () => { + test('zero-claim page writes a tombstone row (proposals_inserted stays 0)', async () => { + const pages = [buildPage({ slug: 'test/embed-probe', body: '# probe\njust a test, nothing to grade.' })]; + const { engine, captured } = buildMockEngine({ pages }); + const extractor: ProposeTakesExtractor = async () => []; + const result = await runPhaseProposeTakes(buildCtx(engine), { extractor }); + + const details = result.details as Record; + expect(details.cache_misses).toBe(1); + expect(details.proposals_inserted).toBe(0); + expect(details.tombstones_written).toBe(1); + + const inserts = captured.filter(c => c.sql.includes('INSERT INTO take_proposals')); + expect(inserts).toHaveLength(1); + // Tombstone carries the sentinel claim_text and an out-of-queue status. + expect(inserts[0]!.params[5]).toBe(EMPTY_EXTRACTION_TOMBSTONE_TEXT); // claim_text + expect(inserts[0]!.sql).toContain("'rejected'"); + }); + + test('unchanged zero-claim page is a cache hit next cycle (no repeat LLM call)', async () => { + const pages = [buildPage({ slug: 'test/embed-probe', body: '# probe\njust a test, nothing to grade.' })]; + const { engine } = buildMockEngine({ pages }); + let extractorCalls = 0; + const extractor: ProposeTakesExtractor = async () => { + extractorCalls++; + return []; + }; + + // Cycle 1: cache miss → LLM call → tombstone written. + const r1 = await runPhaseProposeTakes(buildCtx(engine), { extractor }); + expect(extractorCalls).toBe(1); + expect((r1.details as Record).cache_misses).toBe(1); + expect((r1.details as Record).tombstones_written).toBe(1); + + // Cycle 2: same unchanged page → cache hit → extractor NOT called again. + const r2 = await runPhaseProposeTakes(buildCtx(engine), { extractor }); + expect(extractorCalls).toBe(1); // the whole point: no re-spend + expect((r2.details as Record).cache_hits).toBe(1); + expect((r2.details as Record).cache_misses).toBe(0); + }); + + test('extractor error does NOT write a tombstone (page retried next cycle)', async () => { + const pages = [buildPage({ slug: 'wiki/x', body: 'some prose' })]; + const { engine, captured } = buildMockEngine({ pages }); + const extractor: ProposeTakesExtractor = async () => { + throw new Error('LLM timeout'); + }; + const result = await runPhaseProposeTakes(buildCtx(engine), { extractor }); + + expect((result.details as Record).tombstones_written).toBe(0); + expect(captured.filter(c => c.sql.includes('INSERT INTO take_proposals'))).toHaveLength(0); + }); +});