From 4b6cf32c9fd4dc0c147e12d304be3e1edb5cd024 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 23 Jul 2026 11:01:29 -0700 Subject: [PATCH] Revert "fix(cycle): extract_facts guard requires live backing page, not just non-NULL entity_slug (#2497)" This reverts commit 53c9086945aeb7019b086b7bd4ab17223721445a. --- src/core/cycle/extract-facts.ts | 70 ++++++---------------- test/extract-facts-phase.test.ts | 100 ------------------------------- 2 files changed, 17 insertions(+), 153 deletions(-) diff --git a/src/core/cycle/extract-facts.ts b/src/core/cycle/extract-facts.ts index 04ee53cea..d805fdec4 100644 --- a/src/core/cycle/extract-facts.ts +++ b/src/core/cycle/extract-facts.ts @@ -23,24 +23,14 @@ * page coordinate only; legacy NULL-source_markdown_slug rows survive * because deleteFactsForPage targets source_markdown_slug = slug only. * - * Empty-fence guard (Codex R2-#7; #2484): the phase refuses to do its - * destructive reconciliation pass when genuinely-backfillable legacy - * rows still exist — `row_num IS NULL` (never fenced) AND `entity_slug` - * resolves to a live page in this source (so the v0_32_2 migration's - * Phase B could fence them). Status returns `warn` with a hint to run - * `gbrain apply-migrations --yes`. Without the guard, an interrupted - * upgrade where v0_32_2 hasn't run could leave the cycle silently - * misreporting "0 facts on people/alice" while legacy rows linger. - * - * The live-page requirement (#2484) is load-bearing: the inline facts - * writer keeps producing `row_num IS NULL, entity_slug IS NOT NULL` - * rows AFTER the migration completes, whenever a resolved slug has no - * fenceable page (slugify-floor / stub-guard-blocked unprefixed slugs). - * Those are structurally unfenceable — no page to fence onto, and the - * ledger-complete migration won't re-run — so they must NOT gate, or - * the phase jams forever (~16/day observed). Requiring a backing page - * keeps genuine pre-v0.32.2 rows (whose entity page exists) gating - * while excluding the inline-writer's permanent-unfenceable rows. + * Empty-fence guard (Codex R2-#7): the phase refuses to do its + * destructive reconciliation pass when legacy rows (row_num IS NULL, + * entity_slug IS NOT NULL) still exist in the brain — they're the + * v0.31 hot-memory facts pending the v0_32_2 backfill. Status returns + * `warn` with a hint to run `gbrain apply-migrations --yes`. Without + * the guard, an interrupted upgrade where v0_32_2 hasn't run could + * leave the cycle silently misreporting "0 facts on people/alice" + * while legacy rows linger in the DB. */ import type { BrainEngine } from '../engine.ts'; @@ -173,48 +163,22 @@ export async function runExtractFacts( phantomsMorePending: false, }; - // ── Empty-fence guard (Codex R2-#7; #2484) ───────────────────── - // Pre-check: if any genuinely-backfillable legacy fact rows exist, - // refuse to run the destructive reconciliation pass — the v0_32_2 - // orchestrator must fence them first. - // - // A row is a real backfill candidate only when `row_num IS NULL` - // (never fenced) AND its `entity_slug` resolves to a LIVE page in - // this source (the migration's Phase B only fences rows whose - // entity_slug maps to a writable page). #2484: the original - // predicate was just `row_num IS NULL AND entity_slug IS NOT NULL`, - // which ALSO matched structurally-unfenceable hot-memory rows the - // inline writer keeps producing post-migration: the legacy DB-only - // fallback (backstop.ts) writes `entity_slug` (a resolved slug, e.g. - // a slugify-floor or stub-guard-blocked unprefixed slug like - // `people-jane-doe`) with `row_num` NULL whenever the slug has no - // fenceable page. Those rows can never satisfy the migration's exit - // condition (no page to fence onto, and `apply-migrations` is a - // ledger-complete no-op for them), so they jammed the phase forever - // — ~16/day, mislabeled "v0.31 pending backfill." We now require a - // live backing page, which both genuine pre-v0.32.2 rows (their - // entity page exists) satisfy and inline-writer unfenceable rows do - // not. + // ── Empty-fence guard (Codex R2-#7) ──────────────────────────── + // Pre-check: if any legacy fact rows exist (row_num NULL but + // entity_slug NOT NULL), refuse to run the destructive + // reconciliation pass. The v0_32_2 orchestrator must complete + // first. const legacy = await engine.executeRaw<{ n: string }>( - `SELECT COUNT(*) AS n - FROM facts f - WHERE f.row_num IS NULL - AND f.entity_slug IS NOT NULL - AND EXISTS ( - SELECT 1 FROM pages p - WHERE p.source_id = f.source_id - AND p.slug = f.entity_slug - AND p.deleted_at IS NULL - )`, + `SELECT COUNT(*) AS n FROM facts WHERE row_num IS NULL AND entity_slug IS NOT NULL`, ); const legacyCount = parseInt(legacy[0]?.n ?? '0', 10); result.legacyRowsPending = legacyCount; if (legacyCount > 0) { result.guardTriggered = true; result.warnings.push( - `extract_facts: ${legacyCount} legacy v0.31 fact rows (entity page present, not yet ` + - `fenced) pending fence backfill. Run \`gbrain apply-migrations --yes\` to complete ` + - `v0_32_2 before this phase can safely reconcile fence → DB.`, + `extract_facts: ${legacyCount} legacy v0.31 fact rows pending fence backfill. ` + + `Run \`gbrain apply-migrations --yes\` to complete v0_32_2 before this phase ` + + `can safely reconcile fence → DB.`, ); return result; } diff --git a/test/extract-facts-phase.test.ts b/test/extract-facts-phase.test.ts index 5f046dfdf..1fdd24ef5 100644 --- a/test/extract-facts-phase.test.ts +++ b/test/extract-facts-phase.test.ts @@ -351,106 +351,6 @@ describe('runExtractFacts — empty-fence guard (Codex R2-#7)', () => { expect(r.guardTriggered).toBe(false); expect(r.factsInserted).toBe(1); }); - - // ── #2484: structurally-unfenceable hot-memory rows ─────────── - // The inline facts writer (backstop.ts) keeps producing - // `row_num IS NULL, entity_slug IS NOT NULL` rows AFTER the v0_32_2 - // migration completes: when a resolved slug has no fenceable page - // (slugify-floor / stub-guard-blocked unprefixed slugs like - // `wingman` or `people-jane-doe`), it falls through to a DB-only - // insert with row_num NULL. The OLD guard predicate - // (`row_num IS NULL AND entity_slug IS NOT NULL`) matched these and - // jammed the phase forever (~16/day) — they can never be fenced (no - // page to fence onto; the ledger-complete migration won't re-run). - // The fix requires a LIVE backing page, so these rows no longer gate. - test('#2484: unfenceable inline-writer rows (entity_slug set, NO backing page) do NOT trigger the guard', async () => { - // Two unfenceable rows whose entity_slug has no page row at all. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await (engine as any).db.query( - `INSERT INTO facts (source_id, entity_slug, fact, kind, visibility, notability, - valid_from, source, confidence) - VALUES - ('default', 'wingman', 'handoff note A', 'fact', 'private', 'medium', now(), 'mcp:extract_facts', 1.0), - ('default', 'people-jane-doe', 'handoff note B', 'fact', 'private', 'medium', now(), 'mcp:extract_facts', 1.0)`, - ); - - // A real page with a fence that SHOULD reconcile (proves the phase - // converges past the guard rather than early-returning). - await putPage('people/alice', FACT_FENCE( - `| 1 | real fenced fact | fact | 1.0 | world | high | 2026-01-01 | | s | |`, - )); - - const r = await runExtractFacts(engine, { slugs: ['people/alice'] }); - - // Guard must NOT trip — the unfenceable rows are permanent by - // construction, not a migration blocker. - expect(r.guardTriggered).toBe(false); - expect(r.legacyRowsPending).toBe(0); - // The phase ran its reconcile pass (did not early-return). - expect(r.factsInserted).toBe(1); - - // The unfenceable rows survive untouched (still row_num NULL). - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const survivors = await (engine as any).db.query( - `SELECT entity_slug FROM facts WHERE row_num IS NULL ORDER BY entity_slug`, - ); - expect(survivors.rows.map((x: { entity_slug: string }) => x.entity_slug)) - .toEqual(['people-jane-doe', 'wingman']); - }); - - test('#2484: a genuine legacy row WITH a backing page still triggers the guard (discriminator stays sharp)', async () => { - // Same shape as the unfenceable row above (row_num NULL, entity_slug - // set) — the ONLY difference is a live backing page exists, so the - // migration's Phase B could fence it. This MUST still gate. - await putPage('people/bob', FACT_FENCE( - `| 1 | fence fact | fact | 1.0 | world | high | 2026-01-01 | | s | |`, - )); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await (engine as any).db.query( - `INSERT INTO facts (source_id, entity_slug, fact, kind, visibility, notability, - valid_from, source, confidence) - VALUES ('default', 'people/bob', 'genuine legacy claim', 'fact', 'private', 'medium', - now(), 'mcp:put_page', 1.0)`, - ); - - const r = await runExtractFacts(engine, { slugs: ['people/bob'] }); - - expect(r.guardTriggered).toBe(true); - expect(r.legacyRowsPending).toBe(1); - expect(r.factsInserted).toBe(0); - expect(r.factsDeleted).toBe(0); - expect(r.warnings.some(w => w.includes('apply-migrations'))).toBe(true); - }); - - test('#2484: a soft-deleted backing page makes its legacy row unfenceable (does NOT gate)', async () => { - // Page exists then gets soft-deleted (deleted_at set). The migration - // can't fence onto a deleted page, so the row must not gate. - await putPage('people/carol', FACT_FENCE( - `| 1 | live fact | fact | 1.0 | world | high | 2026-01-01 | | s | |`, - )); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await (engine as any).db.query( - `INSERT INTO facts (source_id, entity_slug, fact, kind, visibility, notability, - valid_from, source, confidence) - VALUES ('default', 'people/carol', 'orphaned legacy claim', 'fact', 'private', 'medium', - now(), 'mcp:put_page', 1.0)`, - ); - // Soft-delete the page. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await (engine as any).db.query( - `UPDATE pages SET deleted_at = now() WHERE slug = 'people/carol' AND source_id = 'default'`, - ); - - // Reconcile a DIFFERENT live page so the phase has work to do. - await putPage('people/dave', FACT_FENCE( - `| 1 | dave fact | fact | 1.0 | world | high | 2026-01-01 | | s | |`, - )); - - const r = await runExtractFacts(engine, { slugs: ['people/dave'] }); - expect(r.guardTriggered).toBe(false); - expect(r.legacyRowsPending).toBe(0); - expect(r.factsInserted).toBe(1); - }); }); describe('runExtractFacts — multi-source isolation', () => {