perf(#306): build learnings dedupe index once per bulk import (#386)

learningsCopyFromProject called learningsWrite K times in one process,
and each call re-scanned the entire learnings store to dedupe — O(K*N).
Build the content_hash -> id index once at the start of the bulk import
and thread it through; single-write behavior and the return contract are
unchanged (no caller reads `id` on the created:false branch). O(K*N) ->
O(N+K). Adds a regression test asserting store scan count is independent
of import size.

The larger persistent on-disk index (atomic updates, corruption rebuild,
cross-process dedupe) is deferred — needs design decisions and is not
required to resolve the bulk-import scan this issue reports.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

.changeset/fix-306-learnings-dedupe-index.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 306
+---
+Build the learnings dedupe index once per bulk import instead of rescanning the whole store on every write (#306).

get-shit-done/bin/lib/learnings.cjs

@@ -106,7 +106,33 @@ function learningsWrite(entry, opts) {
 
   const hash = contentHash(entry.learning, entry.source_project);
 
-  // Check for duplicate by scanning existing files
+  // #306: In bulk-import paths, callers may supply a pre-built dedupeIndex
+  // (Map<content_hash, id>) to avoid the per-write O(N) store scan.
+  // When present, use it for the dedupe check instead of scanning the directory.
+  // After writing a new record, add it to the index so subsequent items in the
+  // same bulk operation dedupe against it.
+  // When absent (single-write path), fall back to the existing full-store scan.
+  if (opts && opts.dedupeIndex) {
+    const dedupeIndex = opts.dedupeIndex;
+    if (dedupeIndex.has(hash)) {
+      return { id: dedupeIndex.get(hash), created: false, content_hash: hash };
+    }
+    const id = generateId();
+    const record = {
+      id,
+      source_project: entry.source_project,
+      date: new Date().toISOString(),
+      context: entry.context || '',
+      learning: entry.learning,
+      tags: entry.tags || [],
+      content_hash: hash,
+    };
+    platformWriteSync(path.join(dir, `${id}.json`), JSON.stringify(record, null, 2));
+    dedupeIndex.set(hash, id);
+    return { id, created: true, content_hash: hash };
+  }
+
+  // Check for duplicate by scanning existing files (single-write path, unchanged)
   const files = fs.readdirSync(dir).filter(f => f.endsWith('.json'));
   for (const file of files) {
     const existing = readLearningFile(path.join(dir, file));
@@ -229,6 +255,22 @@ function learningsCopyFromProject(planningDir, opts) {
   const content = fs.readFileSync(learningsPath, 'utf-8');
   const sourceProject = (opts && opts.sourceProject) || path.basename(path.resolve(planningDir, '..'));
 
+  // #306: Build the content_hash -> id dedupe index once before the loop so
+  // that learningsWrite does not re-scan the entire store on every call —
+  // O(K*N) -> O(N+K).
+  const dir = getStoreDir(opts);
+  ensureStoreDir(dir);
+  const dedupeIndex = new Map();
+  for (const file of fs.readdirSync(dir).filter(f => f.endsWith('.json'))) {
+    const existing = readLearningFile(path.join(dir, file));
+    // First-seen-wins, matching the legacy scan path's first-match return so the
+    // dedupe-hit `id` is identical on both paths even if the store already holds
+    // duplicate content_hashes. (#306)
+    if (existing && existing.content_hash && !dedupeIndex.has(existing.content_hash)) {
+      dedupeIndex.set(existing.content_hash, existing.id);
+    }
+  }
+
   // Parse markdown: split on ## headings
   const sections = content.split(/^## /m).slice(1); // skip preamble before first ##
   let created = 0;
@@ -248,7 +290,7 @@ function learningsCopyFromProject(planningDir, opts) {
       learning: body,
       context: title,
       tags,
-    }, opts);
+    }, { ...opts, dedupeIndex });
 
     if (result.created) {
       created++;

tests/learnings.test.cjs

@@ -524,3 +524,108 @@ describe('CLI integration', () => {
     assert.ok(res.error.includes('Unknown learnings subcommand'));
   });
 });
+
+// ─── Dedupe Scaling (#306) ───────────────────────────────────────────────────
+
+/**
+ * Build a LEARNINGS.md string with k unique ## sections.
+ * Mirrors the ## heading format that learningsCopyFromProject parses.
+ */
+function makeLearningsMd(k) {
+  const sections = [];
+  for (let i = 1; i <= k; i++) {
+    sections.push(`## Title ${i}\n\nBody content for item ${i}, unique text.`);
+  }
+  return `# Project Learnings\n\n${sections.join('\n\n')}\n`;
+}
+
+describe('learnings dedupe scaling (#306)', () => {
+  test('store directory scan count is independent of imported item count', (t) => {
+    // Test A: assert readdirSync call count does NOT scale with K (regression guard for #306)
+    // BEFORE the fix: count scales 1:1 with K (one scan per learningsWrite call).
+    // AFTER the fix: count is constant (one index-build scan per learningsCopyFromProject call).
+
+    const storeDir2 = makeTempDir();
+    const projectDir2 = makeTempDir();
+    t.after(() => {
+      cleanupDir(storeDir2);
+      cleanupDir(projectDir2);
+    });
+
+    const storeDir6 = makeTempDir();
+    const projectDir6 = makeTempDir();
+    t.after(() => {
+      cleanupDir(storeDir6);
+      cleanupDir(projectDir6);
+    });
+
+    // K=2
+    fs.writeFileSync(path.join(projectDir2, 'LEARNINGS.md'), makeLearningsMd(2), 'utf-8');
+    const spy2 = t.mock.method(fs, 'readdirSync');
+    const before2 = spy2.mock.calls.length;
+    learningsCopyFromProject(projectDir2, { storeDir: storeDir2, sourceProject: 'proj-a' });
+    const c1 = spy2.mock.calls.length - before2;
+    spy2.mock.restore();
+
+    // K=6
+    fs.writeFileSync(path.join(projectDir6, 'LEARNINGS.md'), makeLearningsMd(6), 'utf-8');
+    const spy6 = t.mock.method(fs, 'readdirSync');
+    const before6 = spy6.mock.calls.length;
+    learningsCopyFromProject(projectDir6, { storeDir: storeDir6, sourceProject: 'proj-b' });
+    const c6 = spy6.mock.calls.length - before6;
+    spy6.mock.restore();
+
+    assert.strictEqual(c1, c6,
+      `store directory scan count must be independent of imported item count (#306) — got ${c1} for K=2 vs ${c6} for K=6`);
+  });
+
+  test('dedupe semantics preserved: duplicate entry in existing store is skipped', (t) => {
+    // Test B part 1: importing a section that duplicates an already-stored entry → skipped
+    const storeDir = makeTempDir();
+    const projectDir = makeTempDir();
+    t.after(() => {
+      cleanupDir(storeDir);
+      cleanupDir(projectDir);
+    });
+
+    // Pre-seed one entry that matches the first section of our LEARNINGS.md
+    learningsWrite({
+      source_project: 'my-proj',
+      learning: 'Body content for item 1, unique text.',
+      context: 'Title 1',
+      tags: ['title'],
+    }, { storeDir });
+
+    // LEARNINGS.md has 3 sections; section 1 duplicates the pre-seeded entry
+    fs.writeFileSync(path.join(projectDir, 'LEARNINGS.md'), makeLearningsMd(3), 'utf-8');
+    const result = learningsCopyFromProject(projectDir, { storeDir, sourceProject: 'my-proj' });
+
+    assert.strictEqual(result.created, 2, 'two new sections should be created');
+    assert.strictEqual(result.skipped, 1, 'one duplicate section should be skipped');
+
+    const all = learningsList({ storeDir });
+    assert.strictEqual(all.length, 3);
+  });
+
+  test('dedupe semantics preserved: two identical sections in same file → one created, one skipped', (t) => {
+    // Test B part 2: exercises the index.set during-loop dedup path
+    const storeDir = makeTempDir();
+    const projectDir = makeTempDir();
+    t.after(() => {
+      cleanupDir(storeDir);
+      cleanupDir(projectDir);
+    });
+
+    // Two identical ## sections in the same LEARNINGS.md
+    const md = `# Learnings\n\n## Duplicate Section\n\nExact same body text.\n\n## Duplicate Section\n\nExact same body text.\n`;
+    fs.writeFileSync(path.join(projectDir, 'LEARNINGS.md'), md, 'utf-8');
+
+    const result = learningsCopyFromProject(projectDir, { storeDir, sourceProject: 'my-proj' });
+
+    assert.strictEqual(result.created, 1, 'exactly one entry should be created');
+    assert.strictEqual(result.skipped, 1, 'the duplicate should be skipped');
+
+    const all = learningsList({ storeDir });
+    assert.strictEqual(all.length, 1);
+  });
+});