From f981f70a2f81d5674ffe200098a74ae3de413dcd Mon Sep 17 00:00:00 2001 From: joelwp Date: Thu, 16 Jul 2026 20:53:13 -0600 Subject: [PATCH] =?UTF-8?q?fix(extract):=20deterministic=20atom=20slug=20?= =?UTF-8?q?=E2=80=94=20stop=20cross-day=20+=20trailing-dash=20duplicate=20?= =?UTF-8?q?atoms=20(#2482)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extract_atoms minted duplicate atom pages two ways: A. Trailing-dash twins. The local slugger truncated the title at 60 chars with no re-strip, so a cut landing on a hyphen left a trailing dash (`…would-`). The FS-import normalizer (slugifySegment) strips it (`…would`), so the same atom persisted under two slugs and the dedup-by-slug check never collapsed them. B. Cross-day re-mint. The slug used the run date (todayDate()), while the idempotency guard keys on the whole-file source_hash. Append-only sources (chat/transcript exports) grow daily, so the file hash changes, the guard never matches, the source is re-extracted, and each re-mint lands under a new date prefix → a new slug → no upsert → a duplicate. Fix: make the atom slug a deterministic function of stable inputs — `atoms//-`: - source date is parsed from the source ref (transcript filename / page slug), not the run date, so re-extraction converges on the same slug and putPage upserts instead of duplicating; - the 6-char title hash keeps two atoms whose titles share the first 60 chars on distinct slugs (no silent clobber of a different atom); - the stem routes through the canonical slugifySegment and re-strips a trailing dash, so the two write paths can no longer disagree. The whole-file source_hash batch check is retained only as a cost fast-path (skip re-running the model on an unchanged source); correctness no longer depends on it. Adds a hermetic PGLite regression test (no DATABASE_URL) asserting the source-dated prefix, title-hash suffix, trailing-dash strip, and upsert on re-extraction of a grown append-only transcript. Co-authored-by: Claude Opus 4.8 (1M context) --- src/core/cycle/extract-atoms.ts | 86 ++++++++++++++++------- test/extract-atoms-page-discovery.test.ts | 37 +++++++++- 2 files changed, 96 insertions(+), 27 deletions(-) diff --git a/src/core/cycle/extract-atoms.ts b/src/core/cycle/extract-atoms.ts index 582a41044..4c73ea400 100644 --- a/src/core/cycle/extract-atoms.ts +++ b/src/core/cycle/extract-atoms.ts @@ -9,24 +9,27 @@ // 4. Write each atom via engine.putPage(slug, page, {sourceId}) // with sourceId threaded so federated brains route correctly. // -// Idempotency (D1 from /plan-eng-review): -// Each atom carries frontmatter.source_hash (16-char sha256 prefix). -// Before processing a transcript/page, query "any atom with this -// source_hash exists in this source?". If yes, skip. Closes both: -// - PR #1414's primary concern (page-side re-extraction) -// - Pre-existing v0.41.2.0 transcript-side date-stamp duplicate bug -// (atom slugs are `atoms/YYYY-MM-DD/`, so re-discovered -// transcripts on day N+1 used to write second atoms; now skipped). +// Idempotency (per-atom, via deterministic slug): +// Each atom's slug is `atoms/<source-date>/<stem>-<title-hash>` — built from +// the SOURCE date (the transcript's own date / the page slug), NOT the run +// date, plus a 6-char hash of the title. Re-extracting the same atom resolves +// to the SAME slug, so engine.putPage upserts in place instead of minting a +// duplicate. This closes three bugs in one scheme: +// - PR #1414's page-side re-extraction. +// - The cross-day transcript duplicate: append-only transcripts grow daily, +// so a run-date prefix (`atoms/<today>/…`) used to re-mint the same atom +// under a new date every day. A source-date prefix is stable, so it now +// upserts. +// - The "trailing-dash twin": the stem routes through slugifySegment (the +// FS-import normalizer) and re-strips a trailing dash after the 60-char +// truncation, so the two write paths can no longer disagree on `…would` +// vs `…would-` and persist the same atom twice. // -// Known limitation (D9 #2 — documented, not blocking): -// If extraction writes atom 1 of 3 then atom 2 throws, source_hash -// filter sees atom 1 exists and skips on next discovery. Atoms 2+3 -// stay missing until content_hash changes. Acceptable for v0.41.2.1: -// - Haiku call failure is rare; network/budget failures rarer. -// - Content edits trigger natural re-extract via new content_hash. -// - The original incident (duplicate atoms) is fully closed. -// Per-atom idempotency via deterministic slug is v0.42+ TODO -// (see TODOS.md). +// The source_hash batch check (atomsExistingForHashes) is retained ONLY as a +// cost fast-path — it skips re-running Haiku on a transcript whose whole-file +// hash is unchanged. On append-only sources that hash changes daily so the +// fast-path won't skip, but the deterministic slug makes the re-run upsert +// rather than duplicate, so correctness no longer depends on it. // // Config: // Reads dream.synthesize.session_corpus_dir + meeting_transcripts_dir @@ -51,6 +54,8 @@ import type { ProgressReporter } from '../progress.ts'; import { chat as gatewayChat } from '../ai/gateway.ts'; import { writeReceipt } from '../extract/receipt-writer.ts'; import { upsertExtractRollup } from '../extract/rollup-writer.ts'; +import { createHash } from 'crypto'; +import { slugifySegment } from '../sync.ts'; const DEFAULT_BUDGET_USD = 0.3; @@ -558,7 +563,8 @@ export async function runPhaseExtractAtoms( if (!opts.dryRun) { for (const atom of atoms) { - const slug = `atoms/${todayDate()}/${slugify(atom.title)}`; + const srcRef = item.kind === 'transcript' ? item.filePath : item.slug; + const slug = atomSlug(atom.title, srcRef); const originFrontmatter = item.kind === 'transcript' ? { source_path: item.filePath } @@ -732,12 +738,40 @@ function todayDate(): string { return new Date().toISOString().slice(0, 10); } -function slugify(s: string): string { - return s - .toLowerCase() - .replace(/[^a-z0-9\s-]/g, '') - .trim() - .replace(/\s+/g, '-') - .replace(/-+/g, '-') - .slice(0, 60); +/** + * Canonical slug stem for an atom title. Routes through slugifySegment (the + * same normalizer the FS-import path uses) and RE-STRIPS a trailing dash after + * the 60-char truncation — the cut can land on a hyphen and re-introduce one. + * Two writers disagreeing on that trailing dash (`…would` vs `…would-`) was the + * "trailing-dash twin" duplicate bug. + */ +function atomSlugStem(title: string): string { + return slugifySegment(title).slice(0, 60).replace(/-+$/g, '') || 'untitled'; +} + +/** + * Pull a YYYY-MM-DD date from a source reference — a transcript file path like + * `…/2026-06-11-telegram.md`, or a dated page slug. Checks the basename first + * to avoid matching a date in a parent directory. Falls back to the run date + * only when the source carries no date, so dated sources are fully deterministic. + */ +function sourceDate(ref: string): string { + const base = ref.split('/').pop() ?? ref; + const m = base.match(/(\d{4}-\d{2}-\d{2})/) ?? ref.match(/(\d{4}-\d{2}-\d{2})/); + return m ? m[1] : todayDate(); +} + +/** + * Deterministic per-atom slug: `atoms/<source-date>/<stem>-<title-hash>`. + * - Date comes from the SOURCE, not the run date, so re-extracting an + * append-only transcript on a later day yields the SAME slug → putPage + * upserts instead of minting a cross-day duplicate. + * - The 6-char title hash keeps two distinct atoms whose titles share the + * first 60 chars on separate slugs, so a deterministic slug never silently + * clobbers a *different* atom. Hash is over the title only (not body) so an + * LLM rewording the body on re-extraction still upserts rather than dupes. + */ +function atomSlug(title: string, srcRef: string): string { + const hash = createHash('sha256').update(title).digest('hex').slice(0, 6); + return `atoms/${sourceDate(srcRef)}/${atomSlugStem(title)}-${hash}`; } diff --git a/test/extract-atoms-page-discovery.test.ts b/test/extract-atoms-page-discovery.test.ts index 50919b21e..7289cc52e 100644 --- a/test/extract-atoms-page-discovery.test.ts +++ b/test/extract-atoms-page-discovery.test.ts @@ -50,7 +50,7 @@ function stubChat(text: string): (o: ChatOpts) => Promise<ChatResult> { /** * Stub that returns a unique-title atom on each call so atoms write to - * distinct slugs (`atoms/${date}/${slugify(title)}`) instead of upserting + * distinct slugs (`atoms/<source-date>/<stem>-<title-hash>`) instead of upserting * into one row. Needed for tests that count atoms after multiple work items. */ function stubChatUnique(): (o: ChatOpts) => Promise<ChatResult> { @@ -343,6 +343,41 @@ describe('v0.41.2.1: runPhaseExtractAtoms — dual-source merge + idempotency', expect(after[0].count).toBe(before[0].count); }); + test('deterministic slug: source-dated + title-hashed, trailing dash stripped, re-extract upserts (no cross-day twin)', async () => { + // 16 three-letter words → slugifySegment output truncates ON a hyphen at the + // 60-char cut, exercising the trailing-dash re-strip (Bug A). + const title = 'aaa bbb ccc ddd eee fff ggg hhh iii jjj kkk lll mmm nnn ooo ppp'; + const chat = stubChat(`[{"title":"${title}","atom_type":"insight","body":"b"}]`); + // Transcript filename carries a date DIFFERENT from the run date, so a + // source-dated slug is observably distinct from the old run-date one. + const filePath = '/srv/transcripts/2026-06-12-telegram.md'; + await runPhaseExtractAtoms(engine, { + _transcripts: [{ filePath, content: 'first', contentHash: 'aaaa1111bbbb2222' }], + _pages: [], + _chat: chat, + }); + // Same file, GROWN content (append-only) → different contentHash, so the + // source-hash fast-path does NOT skip and the atom is re-extracted. Pre-fix + // this minted a second atom under a new run-date prefix (Bug B); the + // source-dated, title-hashed slug must upsert into the same row instead. + await runPhaseExtractAtoms(engine, { + _transcripts: [{ filePath, content: 'first plus appended', contentHash: 'cccc3333dddd4444' }], + _pages: [], + _chat: chat, + }); + const rows = await engine.executeRaw<{ slug: string }>( + `SELECT slug FROM pages WHERE type = 'atom'`, + ); + expect(rows.length).toBe(1); // upsert, not a cross-day duplicate + const slug = rows[0].slug; + expect(slug.startsWith('atoms/2026-06-12/')).toBe(true); // SOURCE date, not run date + expect(slug).toMatch(/-[0-9a-f]{6}$/); // 6-char title-hash suffix + expect(slug).not.toContain('--'); // trailing dash stripped before -<hash> + const stem = slug.slice('atoms/2026-06-12/'.length).replace(/-[0-9a-f]{6}$/, ''); + expect(stem.endsWith('-')).toBe(false); + expect(stem.length).toBeLessThanOrEqual(60); + }); + test('PhaseResult.details has additive page fields populated', async () => { const chat = stubChat(`[{"title":"x","atom_type":"insight","body":"b"}]`); const result = await runPhaseExtractAtoms(engine, {