mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-29 19:01:39 +00:00
fix(migrate,doctor): self-heal idx_timeline_dedup drift (#2038)
A migration renumbered during a merge (v102) could be recorded-as-applied without its DDL running, leaving the 3-column index so every timeline write failed the 4-column ON CONFLICT. runMigrations now always runs a shape-keyed drift repair (dedupe-then-rebuild) even when no migration is pending, and doctor surfaces the drift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
79a2983e0b
commit
c31be2a223
@@ -510,6 +510,33 @@ export async function doctorReportRemote(engine: BrainEngine): Promise<DoctorRep
|
||||
checks.push({ name: 'schema_version', status: 'warn', message: 'Could not check schema version' });
|
||||
}
|
||||
|
||||
// 2b. #2038: idx_timeline_dedup shape. A renumbered-during-merge migration
|
||||
// (v102) can be recorded-as-applied without its DDL running, leaving the
|
||||
// 3-column index in place — every timeline write then fails the 4-column
|
||||
// ON CONFLICT. The version counter can't see this, so check the index SHAPE.
|
||||
try {
|
||||
const { checkTimelineDedupIndex } = await import('../core/timeline-dedup-repair.ts');
|
||||
const idx = await checkTimelineDedupIndex(engine);
|
||||
if (!idx.tablePresent || !idx.needsRepair) {
|
||||
checks.push({
|
||||
name: 'timeline_dedup_index',
|
||||
status: 'ok',
|
||||
message: idx.tablePresent ? 'idx_timeline_dedup has the 4-column shape' : 'no timeline_entries table yet',
|
||||
});
|
||||
} else {
|
||||
checks.push({
|
||||
name: 'timeline_dedup_index',
|
||||
status: 'fail',
|
||||
message:
|
||||
`idx_timeline_dedup is ${idx.indexPresent ? `(${idx.columns.join(', ')})` : 'absent'}, ` +
|
||||
`expected (page_id, date, summary, source) — timeline writes are failing (#2038). ` +
|
||||
`Run \`gbrain apply-migrations --force-schema\` to heal it.`,
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
checks.push({ name: 'timeline_dedup_index', status: 'warn', message: 'Could not check idx_timeline_dedup shape' });
|
||||
}
|
||||
|
||||
// 3. Brain score
|
||||
try {
|
||||
const health = await engine.getHealth();
|
||||
|
||||
@@ -173,6 +173,7 @@ export const META_CHECK_NAMES: ReadonlySet<string> = new Set([
|
||||
'schema_pack_source_drift',
|
||||
'schema_version',
|
||||
'slug_fallback_audit',
|
||||
'timeline_dedup_index',
|
||||
'upgrade_errors',
|
||||
]);
|
||||
|
||||
|
||||
@@ -5524,6 +5524,26 @@ export async function runMigrations(engine: BrainEngine): Promise<{ applied: num
|
||||
const sorted = [...MIGRATIONS].sort((a, b) => a.version - b.version);
|
||||
|
||||
const pending = sorted.filter(m => m.version > current);
|
||||
|
||||
// #2038: schema-drift self-heal. A migration renumbered during a master
|
||||
// merge (v102 timeline dedup, originally v99) can be recorded-as-applied
|
||||
// without its DDL ever running — the version counter can't see it. Repair
|
||||
// the known drift on EVERY pass, including when nothing is pending (the
|
||||
// affected brains are stamped AHEAD of the missing migration, so they never
|
||||
// reach the loop below). Best-effort + idempotent: a no-op on a healthy
|
||||
// index; `doctor` surfaces it independently if this ever fails.
|
||||
try {
|
||||
const { repairTimelineDedupIndex } = await import('./timeline-dedup-repair.ts');
|
||||
const r = await repairTimelineDedupIndex(engine);
|
||||
if (r.repaired) {
|
||||
console.error(
|
||||
`[migrate] healed idx_timeline_dedup drift (#2038): ${r.before.join(',') || '(absent)'} ` +
|
||||
`→ page_id,date,summary,source` +
|
||||
(r.collapsedDuplicates > 0 ? ` (collapsed ${r.collapsedDuplicates} duplicate row(s))` : ''),
|
||||
);
|
||||
}
|
||||
} catch { /* best-effort; doctor reports the drift if this couldn't run */ }
|
||||
|
||||
if (pending.length === 0) {
|
||||
return { applied: 0, current };
|
||||
}
|
||||
|
||||
@@ -0,0 +1,117 @@
|
||||
/**
|
||||
* #2038 — idx_timeline_dedup schema-drift self-heal.
|
||||
*
|
||||
* Migration v102 (`timeline_entries_source_in_dedup`) widens the dedup index
|
||||
* from (page_id, date, summary) to (page_id, date, summary, source). It was
|
||||
* renumbered from v99 during a master merge, so a brain that ran the OLD v99
|
||||
* variant has its version counter stamped PAST v102 while the index stayed
|
||||
* 3-column. `runMigrations` then can't see the drift (it early-returns when no
|
||||
* version is pending), and every `addTimelineEntry(esBatch)` fails with
|
||||
* "no unique or exclusion constraint matching the ON CONFLICT specification"
|
||||
* because both insert sites infer on the 4-column tuple — timeline writes
|
||||
* silently break brain-wide.
|
||||
*
|
||||
* The version counter can't detect this, so the repair is keyed off the actual
|
||||
* index SHAPE and runs on every migrate pass (including the no-pending path).
|
||||
* Idempotent: a no-op when the index is already 4-column.
|
||||
*/
|
||||
|
||||
import type { BrainEngine } from './engine.ts';
|
||||
|
||||
const INDEX_NAME = 'idx_timeline_dedup';
|
||||
const EXPECTED_COLUMNS = ['page_id', 'date', 'summary', 'source'];
|
||||
|
||||
export interface TimelineDedupStatus {
|
||||
/** The timeline_entries table exists (nothing to repair if not). */
|
||||
tablePresent: boolean;
|
||||
/** The index exists. */
|
||||
indexPresent: boolean;
|
||||
/** Indexed columns in order (empty when the index is absent). */
|
||||
columns: string[];
|
||||
/** Index exists in the wrong (pre-v102) shape — needs a rebuild. */
|
||||
needsRepair: boolean;
|
||||
}
|
||||
|
||||
/** Parse the column list out of a pg_indexes `indexdef` string. */
|
||||
function parseIndexColumns(indexdef: string): string[] {
|
||||
const open = indexdef.lastIndexOf('(');
|
||||
const close = indexdef.lastIndexOf(')');
|
||||
if (open < 0 || close < 0 || close < open) return [];
|
||||
return indexdef
|
||||
.slice(open + 1, close)
|
||||
.split(',')
|
||||
.map(c => c.trim().split(/\s+/)[0]) // drop any "col DESC"/opclass suffix
|
||||
.filter(Boolean);
|
||||
}
|
||||
|
||||
export async function checkTimelineDedupIndex(engine: BrainEngine): Promise<TimelineDedupStatus> {
|
||||
const tbl = await engine.executeRaw<{ reg: string | null }>(
|
||||
`SELECT to_regclass('timeline_entries')::text AS reg`,
|
||||
);
|
||||
const tablePresent = !!tbl[0]?.reg;
|
||||
if (!tablePresent) {
|
||||
return { tablePresent: false, indexPresent: false, columns: [], needsRepair: false };
|
||||
}
|
||||
const rows = await engine.executeRaw<{ indexdef: string }>(
|
||||
`SELECT indexdef FROM pg_indexes WHERE indexname = $1`,
|
||||
[INDEX_NAME],
|
||||
);
|
||||
const indexPresent = rows.length > 0;
|
||||
const columns = indexPresent ? parseIndexColumns(rows[0].indexdef) : [];
|
||||
const correct =
|
||||
columns.length === EXPECTED_COLUMNS.length &&
|
||||
EXPECTED_COLUMNS.every((c, i) => columns[i] === c);
|
||||
// An ABSENT index is also "needs repair" — the migration that creates it was
|
||||
// skipped. (A fresh brain always has it, created by the migration chain.)
|
||||
return { tablePresent, indexPresent, columns, needsRepair: !correct };
|
||||
}
|
||||
|
||||
export interface TimelineDedupRepairResult {
|
||||
repaired: boolean;
|
||||
before: string[];
|
||||
collapsedDuplicates: number;
|
||||
reason: 'already_correct' | 'no_table' | 'rebuilt';
|
||||
}
|
||||
|
||||
/**
|
||||
* Heal the index if it's missing the v102 4-column shape. Dedupes FIRST —
|
||||
* the loose 3-column index let rows differing only by `source` coexist, and
|
||||
* `CREATE UNIQUE INDEX` would throw on those collisions otherwise. Keeps the
|
||||
* earliest row (min ctid) of each 4-tuple group.
|
||||
*/
|
||||
export async function repairTimelineDedupIndex(engine: BrainEngine): Promise<TimelineDedupRepairResult> {
|
||||
const status = await checkTimelineDedupIndex(engine);
|
||||
if (!status.tablePresent) {
|
||||
return { repaired: false, before: [], collapsedDuplicates: 0, reason: 'no_table' };
|
||||
}
|
||||
if (!status.needsRepair) {
|
||||
return { repaired: false, before: status.columns, collapsedDuplicates: 0, reason: 'already_correct' };
|
||||
}
|
||||
|
||||
const del = await engine.executeRaw<{ n: string }>(
|
||||
`WITH d AS (
|
||||
DELETE FROM timeline_entries t
|
||||
USING (
|
||||
SELECT page_id, date, summary, source, MIN(ctid) AS keep
|
||||
FROM timeline_entries
|
||||
GROUP BY page_id, date, summary, source
|
||||
HAVING COUNT(*) > 1
|
||||
) dup
|
||||
WHERE t.page_id = dup.page_id
|
||||
AND t.date = dup.date
|
||||
AND t.summary = dup.summary
|
||||
AND t.source IS NOT DISTINCT FROM dup.source
|
||||
AND t.ctid <> dup.keep
|
||||
RETURNING 1
|
||||
)
|
||||
SELECT COUNT(*)::text AS n FROM d`,
|
||||
);
|
||||
const collapsedDuplicates = parseInt(del[0]?.n ?? '0', 10);
|
||||
|
||||
await engine.executeRaw(`DROP INDEX IF EXISTS ${INDEX_NAME}`);
|
||||
await engine.executeRaw(
|
||||
`CREATE UNIQUE INDEX IF NOT EXISTS ${INDEX_NAME}
|
||||
ON timeline_entries(page_id, date, summary, source)`,
|
||||
);
|
||||
return { repaired: true, before: status.columns, collapsedDuplicates, reason: 'rebuilt' };
|
||||
}
|
||||
@@ -0,0 +1,119 @@
|
||||
/**
|
||||
* #2038 — idx_timeline_dedup schema-drift self-heal.
|
||||
*
|
||||
* A brain that ran the pre-renumber v99 variant of the dedup migration is
|
||||
* stamped past v102 with the OLD 3-column index. `runMigrations` early-returns
|
||||
* (nothing pending) so a migration verify-hook can't fix it. The repair is
|
||||
* keyed off the index SHAPE and runs regardless. These tests simulate the
|
||||
* drifted states directly and pin: detection, rebuild, dedupe-before-rebuild
|
||||
* (only possible when the index was absent), and idempotency.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import {
|
||||
checkTimelineDedupIndex,
|
||||
repairTimelineDedupIndex,
|
||||
} from '../src/core/timeline-dedup-repair.ts';
|
||||
import { importFromContent } from '../src/core/import-file.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
let pageId: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
await importFromContent(engine, 'people/alice-example', `---\ntitle: Alice\ntype: note\n---\n\n# Alice\n`, {
|
||||
noEmbed: true,
|
||||
sourceId: 'default',
|
||||
sourcePath: 'people/alice-example.md',
|
||||
});
|
||||
const pid = await engine.executeRaw<{ id: string }>(
|
||||
`SELECT id::text AS id FROM pages WHERE slug = 'people/alice-example' AND source_id = 'default'`,
|
||||
);
|
||||
pageId = pid[0].id;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
});
|
||||
|
||||
/** Force the index back to the broken pre-v102 3-column shape. */
|
||||
async function regressTo3Col() {
|
||||
await engine.executeRaw(`DELETE FROM timeline_entries`);
|
||||
await engine.executeRaw(`DROP INDEX IF EXISTS idx_timeline_dedup`);
|
||||
await engine.executeRaw(
|
||||
`CREATE UNIQUE INDEX idx_timeline_dedup ON timeline_entries(page_id, date, summary)`,
|
||||
);
|
||||
}
|
||||
|
||||
/** The other drift shape: the index was dropped entirely, letting true
|
||||
* 4-tuple duplicates accumulate that would block a naive CREATE UNIQUE INDEX. */
|
||||
async function regressToAbsentWithDupes() {
|
||||
await engine.executeRaw(`DELETE FROM timeline_entries`);
|
||||
await engine.executeRaw(`DROP INDEX IF EXISTS idx_timeline_dedup`);
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO timeline_entries (page_id, date, summary, source, detail)
|
||||
VALUES ($1, '2026-04-03', 'met alice', 'meeting', ''),
|
||||
($1, '2026-04-03', 'met alice', 'meeting', ''),
|
||||
($1, '2026-04-03', 'met alice', 'cli:extract', '')`,
|
||||
[pageId],
|
||||
);
|
||||
}
|
||||
|
||||
describe('#2038 idx_timeline_dedup drift repair', () => {
|
||||
test('detects the 3-column drift', async () => {
|
||||
await regressTo3Col();
|
||||
const status = await checkTimelineDedupIndex(engine);
|
||||
expect(status.tablePresent).toBe(true);
|
||||
expect(status.indexPresent).toBe(true);
|
||||
expect(status.columns).toEqual(['page_id', 'date', 'summary']);
|
||||
expect(status.needsRepair).toBe(true);
|
||||
});
|
||||
|
||||
test('rebuilds the 3-column index to 4 columns (no dupes to collapse)', async () => {
|
||||
await regressTo3Col();
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO timeline_entries (page_id, date, summary, source, detail)
|
||||
VALUES ($1, '2026-04-03', 'met alice', 'meeting', '')`,
|
||||
[pageId],
|
||||
);
|
||||
|
||||
const res = await repairTimelineDedupIndex(engine);
|
||||
expect(res.repaired).toBe(true);
|
||||
expect(res.reason).toBe('rebuilt');
|
||||
expect(res.collapsedDuplicates).toBe(0);
|
||||
|
||||
const after = await checkTimelineDedupIndex(engine);
|
||||
expect(after.columns).toEqual(['page_id', 'date', 'summary', 'source']);
|
||||
expect(after.needsRepair).toBe(false);
|
||||
});
|
||||
|
||||
test('dedupes true 4-tuple duplicates before building the unique index', async () => {
|
||||
await regressToAbsentWithDupes(); // index absent + a real (meeting) dup
|
||||
|
||||
const before = await checkTimelineDedupIndex(engine);
|
||||
expect(before.indexPresent).toBe(false);
|
||||
expect(before.needsRepair).toBe(true);
|
||||
|
||||
const res = await repairTimelineDedupIndex(engine);
|
||||
expect(res.repaired).toBe(true);
|
||||
expect(res.collapsedDuplicates).toBe(1); // one of the two 'meeting' rows removed
|
||||
|
||||
const after = await checkTimelineDedupIndex(engine);
|
||||
expect(after.columns).toEqual(['page_id', 'date', 'summary', 'source']);
|
||||
const rows = await engine.executeRaw<{ n: string }>(
|
||||
`SELECT COUNT(*)::text AS n FROM timeline_entries`,
|
||||
);
|
||||
expect(parseInt(rows[0].n, 10)).toBe(2); // meeting (deduped) + cli:extract
|
||||
});
|
||||
|
||||
test('idempotent — a second repair is a no-op', async () => {
|
||||
await regressTo3Col();
|
||||
await repairTimelineDedupIndex(engine);
|
||||
const second = await repairTimelineDedupIndex(engine);
|
||||
expect(second.repaired).toBe(false);
|
||||
expect(second.reason).toBe('already_correct');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user