mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-30 03:12:32 +00:00
fix(cycle): scope the extract_facts guard to its source and fix the drain advice (#2646, #3526) (#3528)
Defect A (#3526): the empty-fence guard COUNT had no source_id predicate, so one pending legacy row in any mounted source jammed extract_facts for every source in the brain. The count now binds f.source_id = $1 to the run's sourceId (source-isolation invariant). Defect B: the guard advised `gbrain apply-migrations --yes`, which is a proven no-op once the v0.32.2 ledger entry is complete (the runner classifies it as already-applied and Phase B never re-runs). The warning (and cycle.ts's phase hint) now gives the drain path verified to work end-to-end on a real brain: `apply-migrations --force-retry 0.32.2` then `apply-migrations --yes` (Phase B is idempotent), or forget_fact per row. New test proves a pending legacy row in source A does not jam extraction for source B (fails on master, passes with the fix). Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Garry Tan
Claude Opus 5
parent
bd049d2969
commit
91464564cd
+3
-1
@@ -1179,7 +1179,9 @@ async function runPhaseExtractFacts(
|
||||
summary: `extract_facts skipped: ${result.legacyRowsPending} legacy v0.31 facts pending fence backfill`,
|
||||
details: {
|
||||
legacyRowsPending: result.legacyRowsPending,
|
||||
hint: 'gbrain apply-migrations --yes',
|
||||
// A bare `apply-migrations --yes` no-ops once the v0.32.2 ledger
|
||||
// entry is complete; the retry marker is what re-runs Phase B.
|
||||
hint: 'gbrain apply-migrations --force-retry 0.32.2 && gbrain apply-migrations --yes',
|
||||
warnings: result.warnings,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -26,13 +26,17 @@
|
||||
*
|
||||
* Empty-fence guard (Codex R2-#7; #2484; #2646): 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) AND the row is not soft-expired
|
||||
* (`expired_at IS NULL`). 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.
|
||||
* rows still exist — in THIS run's source only (`source_id = sourceId`;
|
||||
* a pending row in source A must not jam extraction for source B — the
|
||||
* source-isolation invariant) — `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) AND the row is not soft-expired
|
||||
* (`expired_at IS NULL`). Status returns `warn` with a hint to re-run
|
||||
* the v0.32.2 fence backfill (`apply-migrations --force-retry 0.32.2`
|
||||
* then `--yes` — a bare `--yes` is a no-op once the ledger says
|
||||
* complete). 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`
|
||||
@@ -225,10 +229,17 @@ export async function runExtractFacts(
|
||||
// soft-expires legacy rows rather than deleting them, so counting
|
||||
// expired rows would leave the guard permanently stuck with no
|
||||
// supported way to drain the backlog.
|
||||
//
|
||||
// Source isolation (#3526): the count is scoped to THIS run's
|
||||
// sourceId. The pre-fix query counted brain-wide, so a single pending
|
||||
// legacy row in any mounted source jammed extract_facts for every
|
||||
// source — a cross-source leak of one source's migration state into
|
||||
// another's cycle (CLAUDE.md source-isolation invariant).
|
||||
const legacy = await engine.executeRaw<{ n: string }>(
|
||||
`SELECT COUNT(*) AS n
|
||||
FROM facts f
|
||||
WHERE f.row_num IS NULL
|
||||
WHERE f.source_id = $1
|
||||
AND f.row_num IS NULL
|
||||
AND f.entity_slug IS NOT NULL
|
||||
AND f.expired_at IS NULL
|
||||
AND EXISTS (
|
||||
@@ -237,15 +248,25 @@ export async function runExtractFacts(
|
||||
AND p.slug = f.entity_slug
|
||||
AND p.deleted_at IS NULL
|
||||
)`,
|
||||
[sourceId],
|
||||
);
|
||||
const legacyCount = parseInt(legacy[0]?.n ?? '0', 10);
|
||||
result.legacyRowsPending = legacyCount;
|
||||
if (legacyCount > 0) {
|
||||
result.guardTriggered = true;
|
||||
// Drain advice must actually work: a bare `apply-migrations --yes`
|
||||
// is a no-op once the v0.32.2 ledger entry says complete (the
|
||||
// runner classifies it as already-applied), so the sanctioned
|
||||
// re-run path is the explicit retry marker first. Phase B is
|
||||
// idempotent — it only touches `row_num IS NULL` rows and de-dupes
|
||||
// against the existing fence — so the re-run is safe. Individual
|
||||
// rows can instead be drained through `forget_fact` (soft-expired
|
||||
// rows stop counting).
|
||||
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 in source "${sourceId}" ` +
|
||||
`(entity page present, not yet fenced) pending fence backfill. Re-run the v0.32.2 ` +
|
||||
`fence backfill: \`gbrain apply-migrations --force-retry 0.32.2\` then ` +
|
||||
`\`gbrain apply-migrations --yes\`. Or drain individual rows via \`forget_fact\`.`,
|
||||
);
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -672,6 +672,51 @@ describe('runExtractFacts — empty-fence guard (Codex R2-#7)', () => {
|
||||
});
|
||||
|
||||
describe('runExtractFacts — multi-source isolation', () => {
|
||||
test('a pending legacy row in source A does NOT jam extraction for source B (#2646 source-scope)', async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
await (engine as any).db.query(
|
||||
`INSERT INTO sources (id, name, config) VALUES ('work', 'work', '{}'::jsonb)
|
||||
ON CONFLICT (id) DO NOTHING`,
|
||||
);
|
||||
|
||||
// Source "work": a genuine pending legacy row (row_num NULL, active,
|
||||
// live backing page) — the exact shape that must gate work's cycle.
|
||||
await engine.putPage('people/alice', {
|
||||
title: 'people/alice', type: 'person',
|
||||
compiled_truth: FACT_FENCE(`| 1 | work fence fact | fact | 1.0 | world | high | 2026-01-01 | | s | |`),
|
||||
frontmatter: {}, timeline: '',
|
||||
}, { sourceId: 'work' });
|
||||
// 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 ('work', 'people/alice', 'work legacy claim', 'fact', 'private', 'medium',
|
||||
now(), 'mcp:put_page', 1.0)`,
|
||||
);
|
||||
|
||||
// Source "default": clean — no legacy rows, one fenced page.
|
||||
await putPage('people/bob', FACT_FENCE(
|
||||
`| 1 | default fact | fact | 1.0 | world | high | 2026-01-01 | | s | |`,
|
||||
));
|
||||
|
||||
// default's run must NOT be jammed by work's pending backlog.
|
||||
const rDefault = await runExtractFacts(engine, { slugs: ['people/bob'], sourceId: 'default' });
|
||||
expect(rDefault.guardTriggered).toBe(false);
|
||||
expect(rDefault.legacyRowsPending).toBe(0);
|
||||
expect(rDefault.factsInserted).toBe(1);
|
||||
|
||||
// work's own run still gates (discriminator stays sharp).
|
||||
const rWork = await runExtractFacts(engine, { slugs: ['people/alice'], sourceId: 'work' });
|
||||
expect(rWork.guardTriggered).toBe(true);
|
||||
expect(rWork.legacyRowsPending).toBe(1);
|
||||
expect(rWork.factsInserted).toBe(0);
|
||||
// The drain advice must be one that actually re-runs Phase B — a bare
|
||||
// `apply-migrations --yes` no-ops once the ledger says complete.
|
||||
expect(rWork.warnings.some(w => w.includes('--force-retry 0.32.2'))).toBe(true);
|
||||
expect(rWork.warnings.some(w => w.includes('forget_fact'))).toBe(true);
|
||||
expect(rWork.warnings.some(w => w.includes('source "work"'))).toBe(true);
|
||||
});
|
||||
|
||||
test('deleteFactsForPage scoping does not affect other sources', async () => {
|
||||
// Seed sources work + home.
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
|
||||
Reference in New Issue
Block a user