From 0762fab80989adacf8be82249c09a797f35db301 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 17 Aug 2026 10:15:53 -0700 Subject: [PATCH] fix(memory-ingest): --probe counts post-attribution, through the same gate --bulk uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit probeMode previously stat'd every walked file, so setup-gbrain gated its silent bulk ingest on pre-filter counts that the write path would never ingest (#2394). The attribution decision now lives in ONE shared gate (sessionIsAttributable — cheap-parse: cwd extraction + memoized resolveGitRemote, never a full page build) used by BOTH probeMode and preparePages, so the two stages' post-attribution counts are structurally identical. ProbeReport gains skipped_unattributed; the probe prints what it excluded and --include-unattributed restores raw counts. The parity is pinned at the prepare stage (probe post-attribution == transcripts reaching import), deliberately NOT == final written. Re-derived from PR #2612 under the generated-file screening rule; the shared-gate design and the remote memo are additions from the plan review. Fixes #2394. Contributed by @Lockyer228 Co-Authored-By: Claude Fable 5 --- bin/gstack-memory-ingest.ts | 92 +++++++++++++++++++++++++++++-- test/gstack-memory-ingest.test.ts | 86 +++++++++++++++++++++++++++-- 2 files changed, 169 insertions(+), 9 deletions(-) diff --git a/bin/gstack-memory-ingest.ts b/bin/gstack-memory-ingest.ts index 5cbb535ba..8654d71ea 100644 --- a/bin/gstack-memory-ingest.ts +++ b/bin/gstack-memory-ingest.ts @@ -141,6 +141,7 @@ interface ProbeReport { new_count: number; updated_count: number; unchanged_count: number; + skipped_unattributed: number; estimate_minutes: number; } @@ -677,8 +678,21 @@ function extractContentText(rec: any): string { return ""; } +// Memo: probe and prepare both resolve remotes per-transcript, and transcripts +// share a small set of cwds — without this an 11.7K-file probe would spawn git +// 11.7K times instead of once per distinct cwd. +const REMOTE_MEMO = new Map(); + function resolveGitRemote(cwd: string): string { if (!cwd) return ""; + const memo = REMOTE_MEMO.get(cwd); + if (memo !== undefined) return memo; + const resolved = resolveGitRemoteUncached(cwd); + REMOTE_MEMO.set(cwd, resolved); + return resolved; +} + +function resolveGitRemoteUncached(cwd: string): string { try { // execFileSync (no shell) so `cwd` cannot trigger command substitution. // Transcript JSONL records are an untrusted surface (a poisoned `.cwd` @@ -1046,6 +1060,60 @@ export function readNewFailures( // ── Main ingest passes ───────────────────────────────────────────────────── +/** + * Lightweight attribution check: does a transcript have a resolvable git + * remote for its cwd? Extracts the cwd from the first JSONL line that has + * one (mirroring the logic in parseTranscriptJsonl) and calls + * resolveGitRemote. Avoids the full parse (body rendering, message counting) + * because probe only needs the yes/no answer. + * + * Non-transcript types (artifacts) always pass — the attribution filter in + * preparePages only applies to transcripts (#2394). + */ + +/** + * The ONE attribution gate (#2394): a transcript is attributable iff its cwd + * resolves to a git remote. Both probeMode (via transcriptIsAttributable) and + * preparePages route through THIS function, so the two stages' post-attribution + * counts are structurally identical — the parity the probe report promises. + */ +function sessionIsAttributable(cwd: string | undefined | null): boolean { + if (!cwd) return false; + return resolveGitRemote(cwd) !== ""; +} + +function transcriptIsAttributable(path: string): boolean { + let raw: string; + try { + raw = readFileSync(path, "utf-8"); + } catch { + return false; + } + const lines = raw.split("\n").filter((l) => l.trim().length > 0); + if (lines.length === 0) return false; + + // Detect format: Codex first line has type=session_meta, Claude Code + // has cwd on a user/assistant record. + let cwd = ""; + for (const line of lines) { + try { + const rec = JSON.parse(line); + if (rec?.type === "session_meta") { + cwd = rec.payload?.cwd || rec.cwd || ""; + break; + } + if (rec?.cwd) { + cwd = rec.cwd; + break; + } + } catch { + continue; + } + } + if (!cwd) return false; + return sessionIsAttributable(cwd); +} + async function probeMode(args: CliArgs): Promise { const state = loadState(); const ctx = makeWalkContext(args, state); @@ -1066,8 +1134,18 @@ async function probeMode(args: CliArgs): Promise { let newCount = 0; let updatedCount = 0; let unchangedCount = 0; + let skippedUnattributed = 0; for (const { path, type } of walkAllSources(ctx)) { + // Apply the same attribution filter preparePages uses (#2394): + // skip transcripts with no resolvable git remote unless --include-unattributed. + if (type === "transcript" && !args.includeUnattributed) { + if (!transcriptIsAttributable(path)) { + skippedUnattributed++; + continue; + } + } + totalFiles++; let size = 0; try { @@ -1096,6 +1174,7 @@ async function probeMode(args: CliArgs): Promise { new_count: newCount, updated_count: updatedCount, unchanged_count: unchangedCount, + skipped_unattributed: skippedUnattributed, estimate_minutes: estimateMinutes, }; } @@ -1176,15 +1255,15 @@ function preparePages( parseFailed++; continue; } - if (!args.includeUnattributed && !session.cwd) { + // The SAME gate probeMode uses (#2394) — routing both through + // sessionIsAttributable is what makes probe counts trustworthy. + // (Semantically identical to the old two-step check: no cwd, or a cwd + // whose remote resolves empty, both rendered git_remote "_unattributed".) + if (!args.includeUnattributed && !sessionIsAttributable(session.cwd)) { skippedUnattributed++; continue; } page = buildTranscriptPage(path, session); - if (!args.includeUnattributed && page.git_remote === "_unattributed") { - skippedUnattributed++; - continue; - } if (page.partial) partialPages++; } else { page = buildArtifactPage(path, type); @@ -2042,6 +2121,9 @@ function printProbeReport(r: ProbeReport, json: boolean): void { console.log(`New (never ingested): ${r.new_count}`); console.log(`Updated (mtime/hash): ${r.updated_count}`); console.log(`Unchanged: ${r.unchanged_count}`); + if (r.skipped_unattributed > 0) { + console.log(`Skipped (unattributed): ${r.skipped_unattributed} (no git remote; use --include-unattributed to include)`); + } console.log("By type:"); for (const [t, v] of Object.entries(r.by_type)) { if (v.count > 0) { diff --git a/test/gstack-memory-ingest.test.ts b/test/gstack-memory-ingest.test.ts index b2d0a7b42..e0e95e25f 100644 --- a/test/gstack-memory-ingest.test.ts +++ b/test/gstack-memory-ingest.test.ts @@ -93,7 +93,7 @@ describe("gstack-memory-ingest CLI", () => { const session = `{"type":"user","message":{"role":"user","content":"hello"},"timestamp":"${new Date().toISOString()}","cwd":"/tmp/x"}\n{"type":"assistant","message":{"role":"assistant","content":"hi"},"timestamp":"${new Date().toISOString()}"}\n`; writeClaudeCodeSession(home, "tmp-x", "abc123", session); - const r = runScript(["--probe"], { HOME: home, GSTACK_HOME: gstackHome }); + const r = runScript(["--probe", "--include-unattributed"], { HOME: home, GSTACK_HOME: gstackHome }); expect(r.exitCode).toBe(0); expect(r.stdout).toContain("Total files in window: 1"); expect(r.stdout).toContain("transcript"); @@ -109,7 +109,7 @@ describe("gstack-memory-ingest CLI", () => { const session = `{"type":"session_meta","payload":{"id":"sess-xyz","cwd":"/tmp/x","git":{"repository_url":"https://github.com/foo/bar"}},"timestamp":"${today.toISOString()}"}\n`; writeCodexSession(home, ymd, session); - const r = runScript(["--probe"], { HOME: home, GSTACK_HOME: gstackHome }); + const r = runScript(["--probe", "--include-unattributed"], { HOME: home, GSTACK_HOME: gstackHome }); expect(r.exitCode).toBe(0); expect(r.stdout).toContain("Total files in window: 1"); rmSync(home, { recursive: true, force: true }); @@ -269,7 +269,7 @@ describe("internal: parseTranscriptJsonl + buildTranscriptPage shape", () => { mkdirSync(projDir, { recursive: true }); writeFileSync(join(projDir, "abc123.jsonl"), content, "utf-8"); - const r = runScript(["--probe"], { HOME: home, GSTACK_HOME: join(home, ".gstack") }); + const r = runScript(["--probe", "--include-unattributed"], { HOME: home, GSTACK_HOME: join(home, ".gstack") }); expect(r.exitCode).toBe(0); expect(r.stdout).toContain("Total files in window: 1"); @@ -288,7 +288,7 @@ describe("internal: parseTranscriptJsonl + buildTranscriptPage shape", () => { `{"type":"assistant","message":{"role":"assistant","content":"this is truncat`; // no closing brace + no newline writeFileSync(join(projDir, "trunc.jsonl"), content, "utf-8"); - const r = runScript(["--probe"], { HOME: home, GSTACK_HOME: join(home, ".gstack") }); + const r = runScript(["--probe", "--include-unattributed"], { HOME: home, GSTACK_HOME: join(home, ".gstack") }); // Should not crash; should report 1 transcript expect(r.exitCode).toBe(0); expect(r.stdout).toContain("Total files in window: 1"); @@ -861,3 +861,81 @@ describe("#2105 codex response_item rollout shape", () => { rmSync(dir, { recursive: true, force: true }); }); }); + +// ── #2394: --probe counts post-attribution, matching what --bulk would write ─ + +describe("#2394: probe applies the same attribution gate as prepare", () => { + function makeAttributableCwd(home: string): string { + const repo = join(home, "work", "attributable-repo"); + mkdirSync(repo, { recursive: true }); + spawnSync("git", ["-C", repo, "init", "-q"], { encoding: "utf-8" }); + spawnSync("git", ["-C", repo, "remote", "add", "origin", "https://github.com/foo/bar.git"], { encoding: "utf-8" }); + return repo; + } + + function writeMixedCorpus(home: string): void { + const attributableCwd = makeAttributableCwd(home); + const ts = new Date().toISOString(); + writeClaudeCodeSession( + home, "work-attributable", "attr1", + `{"type":"user","message":{"role":"user","content":"hello"},"timestamp":"${ts}","cwd":"${attributableCwd.replace(/\\/g, "\\\\")}"}\n`, + ); + writeClaudeCodeSession( + home, "tmp-nowhere", "unattr1", + `{"type":"user","message":{"role":"user","content":"hello"},"timestamp":"${ts}","cwd":"${join(home, "not-a-repo").replace(/\\/g, "\\\\")}"}\n`, + ); + mkdirSync(join(home, "not-a-repo"), { recursive: true }); + } + + it("probe reports post-attribution counts and names what it skipped", () => { + const home = makeTestHome(); + const gstackHome = join(home, ".gstack"); + mkdirSync(gstackHome, { recursive: true }); + writeMixedCorpus(home); + + const r = runScript(["--probe"], { HOME: home, GSTACK_HOME: gstackHome }); + expect(r.exitCode).toBe(0); + // Post-attribution: only the transcript whose cwd resolves to a remote. + expect(r.stdout).toContain("Total files in window: 1"); + // The excluded remainder is visible, never silent. + expect(r.stdout).toContain("Skipped (unattributed): 1"); + rmSync(home, { recursive: true, force: true }); + }); + + it("--include-unattributed restores raw counts", () => { + const home = makeTestHome(); + const gstackHome = join(home, ".gstack"); + mkdirSync(gstackHome, { recursive: true }); + writeMixedCorpus(home); + + const r = runScript(["--probe", "--include-unattributed"], { HOME: home, GSTACK_HOME: gstackHome }); + expect(r.exitCode).toBe(0); + expect(r.stdout).toContain("Total files in window: 2"); + rmSync(home, { recursive: true, force: true }); + }); + + it("parity: probe post-attribution count equals what prepare actually processes", () => { + const home = makeTestHome(); + const gstackHome = join(home, ".gstack"); + mkdirSync(gstackHome, { recursive: true }); + writeMixedCorpus(home); + + const probe = runScript(["--probe"], { HOME: home, GSTACK_HOME: gstackHome }); + expect(probe.exitCode).toBe(0); + const probeNew = Number((probe.stdout.match(/New \(never ingested\):\s+(\d+)/) || [])[1]); + expect(probeNew).toBe(1); + + // Same stage on the ingest side: the transcripts that reach the import + // step (written + failed) are exactly the ones that passed the shared + // attribution gate in preparePages. No gbrain is configured in this + // hermetic env, so the attributable transcript FAILS at import — that is + // fine: parity is a prepare-stage invariant (probe post-attribution == + // prepare post-attribution), deliberately NOT == final written (#2394). + const inc = runScript(["--incremental", "--quiet"], { HOME: home, GSTACK_HOME: gstackHome }); + const m = inc.stderr.match(/(\d+) written, (\d+) failed/) || inc.stdout.match(/(\d+) written, (\d+) failed/); + expect(m).not.toBeNull(); + const reachedImport = Number(m![1]) + Number(m![2]); + expect(reachedImport).toBe(probeNew); + rmSync(home, { recursive: true, force: true }); + }); +});