mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
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>
This commit is contained in:
@@ -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
|
||||
@@ -152,6 +163,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;
|
||||
warnings: string[];
|
||||
}
|
||||
@@ -234,7 +247,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;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -314,6 +363,7 @@ class ProposeTakesPhase extends BaseCyclePhase {
|
||||
cache_hits: 0,
|
||||
cache_misses: 0,
|
||||
proposals_inserted: 0,
|
||||
tombstones_written: 0,
|
||||
budget_exhausted: false,
|
||||
warnings: [],
|
||||
};
|
||||
@@ -415,6 +465,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();
|
||||
@@ -448,7 +532,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 ? 'warn' : 'ok',
|
||||
};
|
||||
|
||||
+116
-1
@@ -21,7 +21,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';
|
||||
@@ -59,7 +61,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;
|
||||
@@ -162,6 +172,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', () => {
|
||||
@@ -436,3 +492,62 @@ New prose appended here.`;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 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<string, unknown>;
|
||||
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<string, unknown>).cache_misses).toBe(1);
|
||||
expect((r1.details as Record<string, unknown>).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<string, unknown>).cache_hits).toBe(1);
|
||||
expect((r2.details as Record<string, unknown>).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<string, unknown>).tombstones_written).toBe(0);
|
||||
expect(captured.filter(c => c.sql.includes('INSERT INTO take_proposals'))).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user