mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix(memory-ingest): --probe counts post-attribution, through the same gate --bulk uses
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
f9f0e84add
commit
0762fab809
@@ -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<string, string>();
|
||||
|
||||
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<ProbeReport> {
|
||||
const state = loadState();
|
||||
const ctx = makeWalkContext(args, state);
|
||||
@@ -1066,8 +1134,18 @@ async function probeMode(args: CliArgs): Promise<ProbeReport> {
|
||||
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<ProbeReport> {
|
||||
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) {
|
||||
|
||||
@@ -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 });
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user