mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 06:28:59 +02:00
fix(memory-ingest): stop two silent transcript-ingest failures
Two independent bugs made transcript pages silently fail to reach the brain.
1. Frontmatter fence gluing. buildTranscriptPage() built the closing "---"
with no trailing newline, and session bodies always start with "## ", so
the rendered page ended "...---## User". gbrain's frontmatter matcher
(/^---\r?\n([\s\S]*?)\r?\n---(\r?\n|$)/ in src/core/markdown.ts) requires
the closing "---" to end its own line, so it skipped the glued fence,
latched onto the next standalone "---" in the transcript body, parsed the
prose between as YAML, and dropped the page with "Invalid YAML frontmatter".
Transcripts with no later "---" fell back to body-only, silently losing
their frontmatter. Fix: emit the fence on its own line with a blank
separator, matching renderPageBody()'s artifact branch.
2. Slug collisions. Two source files can map to one path-derived slug (a
session resumed under the same id on one day, or two ids sharing a 12-char
prefix). writeStaged() names each file "${slug}.md", so the second
overwrote the first; gbrain collected N-1 of N staged files and the
reconciliation guard failed the whole batch every run. Fix:
disambiguateSlugs() keeps the first occurrence and gives each later collider
a stable "-<sha8(source_path)>" suffix (deterministic, and slug + page_slug
move together so writeStaged, the failure mapping, and state recording agree).
Exports buildTranscriptPage, renderPageBody, and disambiguateSlugs for tests.
Adds regression tests for both failures.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wave-amended: contributor's local-workaround docblock note removed; issue refs retargeted #2653 (closed by its author) -> #2724 (the live 887-staged-to-0-ingested report)
This commit is contained in:
committed by
Garry Tan
co-authored by
Claude Opus 4.8
parent
9f4e8eef48
commit
96ac9bbfef
@@ -744,7 +744,7 @@ function dateOnly(ts: string | undefined): string {
|
||||
}
|
||||
}
|
||||
|
||||
function buildTranscriptPage(path: string, session: ParsedSession): PageRecord {
|
||||
export function buildTranscriptPage(path: string, session: ParsedSession): PageRecord {
|
||||
const remote = resolveGitRemote(session.cwd);
|
||||
const slug_repo = repoSlug(remote);
|
||||
const date = dateOnly(session.start_time);
|
||||
@@ -762,7 +762,7 @@ function buildTranscriptPage(path: string, session: ParsedSession): PageRecord {
|
||||
const stats = statSync(path);
|
||||
const sha = fileSha256(path);
|
||||
|
||||
const frontmatter = [
|
||||
const fmLines = [
|
||||
"---",
|
||||
`agent: ${session.agent}`,
|
||||
`session_id: ${session.session_id}`,
|
||||
@@ -773,10 +773,21 @@ function buildTranscriptPage(path: string, session: ParsedSession): PageRecord {
|
||||
`message_count: ${session.message_count}`,
|
||||
`tool_calls: ${session.tool_calls}`,
|
||||
`source_path: ${path}`,
|
||||
session.partial ? "partial: true" : "",
|
||||
"---",
|
||||
"",
|
||||
].filter((l) => l !== "").join("\n");
|
||||
];
|
||||
if (session.partial) fmLines.push("partial: true");
|
||||
fmLines.push("---");
|
||||
// The closing `---` fence MUST terminate its own line. session.body always
|
||||
// starts with "## " (never a newline), so without the trailing "\n" the fence
|
||||
// renders as `---## User`, which gray-matter/gbrain reject as a closer (the
|
||||
// fence regex in gbrain markdown.ts requires `\n---(\r?\n|$)`). gbrain then
|
||||
// scans to the next standalone `---` in the transcript, parses the prose
|
||||
// between as YAML, and drops the whole page with "Invalid YAML frontmatter".
|
||||
// A prior `.filter((l) => l !== "")` — added to drop the empty non-partial
|
||||
// line — also stripped the blank that used to terminate the fence line, so
|
||||
// every transcript whose body carries a later `---` horizontal rule silently
|
||||
// failed to ingest. The explicit `+ "\n\n"` restores the fence newline plus a
|
||||
// blank separator, matching the artifact-page branch in renderPageBody().
|
||||
const frontmatter = fmLines.join("\n") + "\n\n";
|
||||
|
||||
return {
|
||||
slug,
|
||||
@@ -890,7 +901,7 @@ function gbrainAvailable(): boolean {
|
||||
* We do NOT set `slug:` in frontmatter — the staging-dir filename is the
|
||||
* source of truth and gbrain rejects mismatches.
|
||||
*/
|
||||
function renderPageBody(page: PageRecord): string {
|
||||
export function renderPageBody(page: PageRecord): string {
|
||||
let body = page.body;
|
||||
if (body.startsWith("---\n")) {
|
||||
const end = body.indexOf("\n---", 4);
|
||||
@@ -1314,6 +1325,43 @@ async function probeMode(args: CliArgs): Promise<ProbeReport> {
|
||||
* redundant defense-in-depth and made it opt-in via `--scan-secrets`
|
||||
* for users who want belt-and-suspenders.
|
||||
*/
|
||||
/**
|
||||
* Disambiguate colliding page slugs before staging (#2724).
|
||||
*
|
||||
* Two distinct source files can map to one transcript slug
|
||||
* (transcripts/<agent>/<repo>/<date>-<session_id[:12]>): a session resumed
|
||||
* under the same session_id on one day, or two session_ids sharing a 12-char
|
||||
* prefix. writeStaged() names each file `${slug}.md`, so the second OVERWRITES
|
||||
* the first — `written` counts both but only one lands on disk, gbrain collects
|
||||
* N-1 of N, and the staged-vs-collected reconciliation guard (correctly) fails
|
||||
* the whole batch. It repeats every run until the inputs age out of the window.
|
||||
*
|
||||
* Fix: keep the first occurrence's slug; give each later collider a stable
|
||||
* `-<sha8(source_path)>` suffix. Deterministic (same source path → same slug
|
||||
* across runs, so incremental dedup and gbrain's session_id dedup still line
|
||||
* up) and it mutates slug + page_slug together so every downstream consumer
|
||||
* (writeStaged, readNewFailures mapping, state recording) computes the same key.
|
||||
*/
|
||||
export function disambiguateSlugs(pages: PreparedPage[]): void {
|
||||
const seen = new Set<string>();
|
||||
for (const p of pages) {
|
||||
if (!seen.has(p.slug)) {
|
||||
seen.add(p.slug);
|
||||
continue;
|
||||
}
|
||||
const suffix = createHash("sha256").update(p.source_path).digest("hex").slice(0, 8);
|
||||
let candidate = `${p.slug}-${suffix}`;
|
||||
// Guarantee uniqueness even if a prior page already took the suffixed slug
|
||||
// (two colliders sharing a source_path-hash prefix is astronomically
|
||||
// unlikely, but a stuck source is not the place to trust luck).
|
||||
let n = 1;
|
||||
while (seen.has(candidate)) candidate = `${p.slug}-${suffix}-${n++}`;
|
||||
seen.add(candidate);
|
||||
p.slug = candidate;
|
||||
p.page_slug = candidate;
|
||||
}
|
||||
}
|
||||
|
||||
function preparePages(
|
||||
args: CliArgs,
|
||||
ctx: WalkContext,
|
||||
@@ -1477,6 +1525,11 @@ function preparePages(
|
||||
finalPrepared = finalPrepared.slice(0, args.limit);
|
||||
}
|
||||
|
||||
// Colliding path-derived slugs would overwrite in the staging dir, so two
|
||||
// source files land as one page and the staged-vs-collected guard fails the
|
||||
// whole batch every run (#2724: 887 staged → 0 ingested). Disambiguate before staging.
|
||||
disambiguateSlugs(finalPrepared);
|
||||
|
||||
// Derived from the FINAL set: partial counts must describe pages that are
|
||||
// actually eligible and within the limit, not the whole scanned corpus.
|
||||
partialPages = finalPrepared.filter((p) => p.partial).length;
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
/**
|
||||
* Regression: transcript frontmatter fence must terminate its own line.
|
||||
*
|
||||
* buildTranscriptPage() emitted a closing `---` with no trailing newline, and
|
||||
* session bodies always start with "## ", so the rendered page ended
|
||||
* `...---## User`. gbrain's frontmatter matcher
|
||||
* (`/^---\r?\n([\s\S]*?)\r?\n---(\r?\n|$)/`, src/core/markdown.ts) requires the
|
||||
* closing `---` to end its own line, so it skipped the glued fence, latched onto
|
||||
* the next standalone `---` in the transcript body, parsed the prose between as
|
||||
* YAML, and dropped the whole page with "Invalid YAML frontmatter". Transcripts
|
||||
* with no later `---` fell back to body-only (frontmatter silently lost).
|
||||
*/
|
||||
import { describe, it, expect } from "bun:test";
|
||||
import { mkdtempSync, writeFileSync, rmSync } from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import {
|
||||
parseTranscriptJsonl,
|
||||
buildTranscriptPage,
|
||||
renderPageBody,
|
||||
} from "../bin/gstack-memory-ingest";
|
||||
|
||||
// The exact fence matcher gbrain uses (src/core/markdown.ts). Kept here so the
|
||||
// test fails if the rendered fence ever regresses.
|
||||
const GBRAIN_FENCE_RE = /^---\r?\n([\s\S]*?)\r?\n---(\r?\n|$)/;
|
||||
|
||||
describe("regression: transcript frontmatter fence terminates its own line", () => {
|
||||
it("keeps a body-embedded `---` out of the frontmatter block", () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "gstack-fence-"));
|
||||
const file = join(dir, "sess.jsonl");
|
||||
// User content carries a markdown horizontal rule (`---`) followed by a
|
||||
// colon-bearing line — exactly what the old glued fence swept into YAML.
|
||||
const content =
|
||||
`{"type":"user","message":{"role":"user","content":"before rule\\n\\n---\\n\\nnot: valid: yaml: here"},` +
|
||||
`"timestamp":"2026-05-01T00:00:00Z","cwd":"${dir}"}\n` +
|
||||
`{"type":"assistant","message":{"role":"assistant","content":"ok"},"timestamp":"2026-05-01T00:00:01Z"}\n`;
|
||||
writeFileSync(file, content, "utf-8");
|
||||
|
||||
const session = parseTranscriptJsonl(file);
|
||||
expect(session).not.toBeNull();
|
||||
const page = buildTranscriptPage(file, session!);
|
||||
const staged = renderPageBody(page);
|
||||
|
||||
// The fence is never glued onto the body.
|
||||
expect(staged).not.toContain("---##");
|
||||
|
||||
// gbrain's matcher closes the frontmatter at the real fence, not at the
|
||||
// horizontal rule deep in the transcript body.
|
||||
const m = staged.match(GBRAIN_FENCE_RE);
|
||||
expect(m).not.toBeNull();
|
||||
const frontmatter = m![1];
|
||||
expect(frontmatter).toContain("session_id:");
|
||||
expect(frontmatter).toContain("title:");
|
||||
// The body (headings, the HR, the colon-trap line) must NOT bleed into YAML.
|
||||
expect(frontmatter).not.toContain("## User");
|
||||
expect(frontmatter).not.toContain("not: valid: yaml: here");
|
||||
|
||||
// And the parsed body (after the fence's trailing blank line) is the
|
||||
// session content, not YAML-absorbed prose.
|
||||
const body = staged.slice(m![0].length);
|
||||
expect(body.trimStart().startsWith("## User")).toBe(true);
|
||||
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,65 @@
|
||||
/**
|
||||
* Regression: disambiguateSlugs resolves colliding staged slugs.
|
||||
*
|
||||
* Two source files can map to one path-derived transcript slug (a session
|
||||
* resumed under the same id on one day, or two session ids sharing a 12-char
|
||||
* prefix). writeStaged() names each file `${slug}.md`, so the second overwrote
|
||||
* the first; gbrain then collected N-1 of N staged files and the
|
||||
* staged-vs-collected reconciliation guard failed the whole batch every run
|
||||
* ("accounted for N-1 of N staged ... Refusing to advance state").
|
||||
*/
|
||||
import { describe, it, expect } from "bun:test";
|
||||
import { disambiguateSlugs } from "../bin/gstack-memory-ingest";
|
||||
|
||||
const mk = (slug: string, source_path: string) => ({
|
||||
slug,
|
||||
source_path,
|
||||
rendered_body: "---\ntitle: x\n---\n\nbody",
|
||||
page_slug: slug,
|
||||
partial: false,
|
||||
type: "transcript" as const,
|
||||
git_remote: undefined,
|
||||
});
|
||||
|
||||
describe("regression: disambiguateSlugs resolves colliding staged slugs", () => {
|
||||
it("keeps the first occurrence and suffixes later colliders deterministically", () => {
|
||||
const slug = "transcripts/claude-code/repo/2026-08-25-abc123def456";
|
||||
const run = () => {
|
||||
const pages = [mk(slug, "/a.jsonl"), mk(slug, "/b.jsonl")];
|
||||
disambiguateSlugs(pages);
|
||||
return pages;
|
||||
};
|
||||
const pages = run();
|
||||
// First keeps the clean slug; second is disambiguated.
|
||||
expect(pages[0].slug).toBe(slug);
|
||||
expect(pages[1].slug).not.toBe(slug);
|
||||
expect(pages[1].slug.startsWith(slug + "-")).toBe(true);
|
||||
// slug and page_slug move together (downstream consumers must agree).
|
||||
expect(pages[1].page_slug).toBe(pages[1].slug);
|
||||
// Deterministic across runs (same source path → same suffix).
|
||||
expect(run()[1].slug).toBe(pages[1].slug);
|
||||
});
|
||||
|
||||
it("gives every member of a 3-way collision a distinct slug", () => {
|
||||
const slug = "transcripts/codex/repo/2026-08-25-deadbeefcafe";
|
||||
const pages = [
|
||||
mk(slug, "/one.jsonl"),
|
||||
mk(slug, "/two.jsonl"),
|
||||
mk(slug, "/three.jsonl"),
|
||||
];
|
||||
disambiguateSlugs(pages);
|
||||
const slugs = new Set(pages.map((p) => p.slug));
|
||||
expect(slugs.size).toBe(3);
|
||||
expect(pages[0].slug).toBe(slug);
|
||||
});
|
||||
|
||||
it("leaves non-colliding slugs untouched", () => {
|
||||
const pages = [
|
||||
mk("transcripts/a/repo/2026-08-25-1111", "/x.jsonl"),
|
||||
mk("transcripts/b/repo/2026-08-25-2222", "/y.jsonl"),
|
||||
];
|
||||
const before = pages.map((p) => p.slug);
|
||||
disambiguateSlugs(pages);
|
||||
expect(pages.map((p) => p.slug)).toEqual(before);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user