mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
Revert "fix(propose_takes): memoize empty extractions so zero-claim pages don't re-spend every cycle (#2514)"
This reverts commit 1233051a20.
This commit is contained in:
Garry Tan
@@ -55,17 +55,6 @@ 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
|
||||
@@ -163,8 +152,6 @@ 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[];
|
||||
}
|
||||
@@ -247,43 +234,7 @@ export async function defaultExtractor(
|
||||
});
|
||||
|
||||
// ChatResult.text is already the concatenated text content.
|
||||
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;
|
||||
}
|
||||
return parseExtractorOutput(result.text);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -363,7 +314,6 @@ class ProposeTakesPhase extends BaseCyclePhase {
|
||||
cache_hits: 0,
|
||||
cache_misses: 0,
|
||||
proposals_inserted: 0,
|
||||
tombstones_written: 0,
|
||||
budget_exhausted: false,
|
||||
warnings: [],
|
||||
};
|
||||
@@ -465,40 +415,6 @@ 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();
|
||||
@@ -532,7 +448,7 @@ class ProposeTakesPhase extends BaseCyclePhase {
|
||||
});
|
||||
|
||||
return {
|
||||
summary: `propose_takes: scanned ${result.pages_scanned} pages, ${result.cache_hits} cached, ${result.proposals_inserted} new proposals, ${result.tombstones_written} empty (run ${proposalRunId})`,
|
||||
summary: `propose_takes: scanned ${result.pages_scanned} pages, ${result.cache_hits} cached, ${result.proposals_inserted} new proposals (run ${proposalRunId})`,
|
||||
details: { ...result, proposal_run_id: proposalRunId, prompt_version: promptVersion },
|
||||
status: result.budget_exhausted ? 'warn' : 'ok',
|
||||
};
|
||||
|
||||
+1
-116
@@ -21,9 +21,7 @@ import {
|
||||
contentHash,
|
||||
hasCompleteFence,
|
||||
extractExistingTakesForDedup,
|
||||
isWellFormedEmptyExtraction,
|
||||
PROPOSE_TAKES_PROMPT_VERSION,
|
||||
EMPTY_EXTRACTION_TOMBSTONE_TEXT,
|
||||
type ProposeTakesExtractor,
|
||||
type ProposedTake,
|
||||
} from '../src/core/cycle/propose-takes.ts';
|
||||
@@ -61,15 +59,7 @@ function buildMockEngine(opts: {
|
||||
if (existing.has(key)) return [{ id: 1 } as unknown as T];
|
||||
return [];
|
||||
}
|
||||
// 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.
|
||||
// INSERT — return nothing
|
||||
return [];
|
||||
},
|
||||
} as unknown as BrainEngine;
|
||||
@@ -172,52 +162,6 @@ 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', () => {
|
||||
@@ -492,62 +436,3 @@ 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