diff --git a/src/products/gardener/engine/classifiers/anthropic.ts b/src/products/gardener/engine/classifiers/anthropic.ts index 89f24b5..22fff19 100644 --- a/src/products/gardener/engine/classifiers/anthropic.ts +++ b/src/products/gardener/engine/classifiers/anthropic.ts @@ -23,7 +23,11 @@ import type { ClassifyInput, ClassifyOutput, } from "../comment.js"; -import { collectTreeDigest, formatDigest } from "./tree-digest.js"; +import { + collectTreeDigestDetailed, + emitDigestDiagnostics, + formatDigest, +} from "./tree-digest.js"; import { filterDiffNoise } from "./diff-filter.js"; import { parseVerdictJson, @@ -46,6 +50,11 @@ export interface AnthropicClassifierOptions { model?: string; /** Injected fetch for tests. Defaults to global fetch. */ fetchImpl?: typeof fetch; + /** + * Optional logging sink for diagnostics (digest budget warnings, + * etc.). Defaults to `process.stderr.write`. + */ + write?: (line: string) => void; } export function createAnthropicClassifier( @@ -53,8 +62,13 @@ export function createAnthropicClassifier( ): Classifier { const model = opts.model?.trim() || DEFAULT_MODEL; const doFetch = opts.fetchImpl ?? fetch; + const write = + opts.write ?? + ((line: string) => process.stderr.write(line + "\n")); return async (input: ClassifyInput): Promise => { - const digest = formatDigest(collectTreeDigest(input.treeRoot)); + const detailed = collectTreeDigestDetailed(input.treeRoot); + emitDigestDiagnostics(detailed, write); + const digest = formatDigest(detailed.entries); const userPrompt = buildUserPrompt(input, digest); try { const res = await doFetch(ANTHROPIC_URL, { diff --git a/src/products/gardener/engine/classifiers/claude-cli.ts b/src/products/gardener/engine/classifiers/claude-cli.ts index cd720d3..7b53071 100644 --- a/src/products/gardener/engine/classifiers/claude-cli.ts +++ b/src/products/gardener/engine/classifiers/claude-cli.ts @@ -26,7 +26,11 @@ import type { ClassifyInput, ClassifyOutput, } from "../comment.js"; -import { collectTreeDigest, formatDigest } from "./tree-digest.js"; +import { + collectTreeDigestDetailed, + emitDigestDiagnostics, + formatDigest, +} from "./tree-digest.js"; import { filterDiffNoise } from "./diff-filter.js"; import { parseVerdictJson, @@ -63,6 +67,13 @@ export interface ClaudeCliClassifierOptions { env?: NodeJS.ProcessEnv; /** Injected spawner for tests. */ spawnImpl?: typeof spawn; + /** + * Optional logging sink for diagnostics (digest budget warnings, + * etc.). Defaults to `process.stderr.write`. The classifier never + * writes to stdout directly — stdout is reserved for the JSON + * verdict piped out to `gardener comment` / `sync`. + */ + write?: (line: string) => void; } export function buildClaudeCliEnvironment( @@ -79,8 +90,13 @@ export function createClaudeCliClassifier( const model = opts.model?.trim() || DEFAULT_MODEL; const env = buildClaudeCliEnvironment(opts.env ?? process.env); const doSpawn = opts.spawnImpl ?? spawn; + const write = + opts.write ?? + ((line: string) => process.stderr.write(line + "\n")); return async (input: ClassifyInput): Promise => { - const digest = formatDigest(collectTreeDigest(input.treeRoot)); + const detailed = collectTreeDigestDetailed(input.treeRoot); + emitDigestDiagnostics(detailed, write); + const digest = formatDigest(detailed.entries); const prompt = buildPrompt(input, digest); const { stdout, stderr, code, timedOut } = await runClaude( doSpawn, diff --git a/src/products/gardener/engine/classifiers/tree-digest.ts b/src/products/gardener/engine/classifiers/tree-digest.ts index 29da0c1..8e41218 100644 --- a/src/products/gardener/engine/classifiers/tree-digest.ts +++ b/src/products/gardener/engine/classifiers/tree-digest.ts @@ -22,25 +22,79 @@ export interface TreeNodeEntry { summary: string; } -const DIGEST_BUDGET_BYTES = 100_000; +/** + * Total byte budget for the tree digest embedded in the classifier + * prompt. Raised from 100KB → 500KB (2026-04: Claude 4.5/4.6/4.7 give + * us a 200K-token context, so the old budget was severely + * over-conservative and was silently truncating nodes out of the + * classifier's view on larger trees; see #343). + * + * 500KB covers ≈ 2700 NODE.md entries at ~180 B each, which is + * comfortably beyond any realistic Context Tree. Still leaves headroom + * for PR body (uncapped) + diff (200KB cap) + system prompt (~1.5KB) + * under a 200K-token window (~800KB text). + */ +const DIGEST_BUDGET_BYTES = 500_000; const PER_NODE_SUMMARY_CAP = 400; + +/** + * Regex that matches the summary text gardener auto-writes when + * scaffolding `drift//.../NODE.md` placeholders during a + * sync. These placeholders carry no decisions — they exist only to let + * a proposal PR have a valid parent chain — so feeding them to the + * classifier wastes budget and clutters citations. See #343. + */ +const DRIFT_PLACEHOLDER_SUMMARY = + /^Auto-generated intermediate node for sync proposals\.?$/i; + const SKIP_DIRS = new Set([ ".git", "node_modules", ".first-tree", ".claude", ".agents", + // `.gardener-tree-cache` holds gardener's own per-sweep snapshot of + // the tree as it was at the last reconciled source commit. Those + // entries are stale duplicates of real NODE.md files; including + // them in the digest double-counts every node under each sourceId + // and pushes real nodes out of a tight budget. See #343. + ".gardener-tree-cache", "dist", "build", "tmp", ]); +export interface CollectTreeDigestResult { + entries: TreeNodeEntry[]; + /** Nodes that matched on-disk but were filtered as noise before budget check. */ + skippedAsNoise: number; + /** Nodes dropped because the budget was exhausted. */ + truncatedCount: number; + /** True when we stopped walking the tree because DIGEST_BUDGET_BYTES filled up. */ + budgetExhausted: boolean; +} + export function collectTreeDigest(treeRoot: string): TreeNodeEntry[] { + return collectTreeDigestDetailed(treeRoot).entries; +} + +/** + * Same walk as {@link collectTreeDigest} but returns diagnostics the + * caller can surface (e.g. "tree digest truncated at N nodes — budget + * exhausted; consider raising DIGEST_BUDGET_BYTES"). Prefer this when + * running inside a classifier that can log a warning. Kept as a + * separate entry point so existing callers don't have to adopt the + * richer shape. + */ +export function collectTreeDigestDetailed( + treeRoot: string, +): CollectTreeDigestResult { const out: TreeNodeEntry[] = []; let bytes = 0; - let exhausted = false; + let budgetExhausted = false; + let skippedAsNoise = 0; + let truncatedCount = 0; const walk = (dir: string): void => { - if (exhausted) return; let entries: string[]; try { entries = readdirSync(dir); @@ -48,7 +102,6 @@ export function collectTreeDigest(treeRoot: string): TreeNodeEntry[] { return; } for (const name of entries) { - if (exhausted) return; if (SKIP_DIRS.has(name)) continue; const full = join(dir, name); let st; @@ -64,17 +117,24 @@ export function collectTreeDigest(treeRoot: string): TreeNodeEntry[] { if (name !== "NODE.md") continue; const entry = readNodeFile(full, treeRoot); if (!entry) continue; + // Drop auto-generated drift-proposal placeholders before they + // eat budget. They carry no decision signal. + if (DRIFT_PLACEHOLDER_SUMMARY.test(entry.summary)) { + skippedAsNoise += 1; + continue; + } const cost = entry.path.length + entry.summary.length + 4; if (bytes + cost > DIGEST_BUDGET_BYTES) { - exhausted = true; - return; + budgetExhausted = true; + truncatedCount += 1; + continue; } bytes += cost; out.push(entry); } }; walk(treeRoot); - return out; + return { entries: out, skippedAsNoise, truncatedCount, budgetExhausted }; } function readNodeFile( @@ -126,3 +186,34 @@ export function formatDigest(entries: TreeNodeEntry[]): string { if (entries.length === 0) return "(no NODE.md files found)"; return entries.map((e) => `- \`${e.path}\` — ${e.summary}`).join("\n"); } + +/** + * Surface tree-digest health info on the logging sink. Two things we + * want visible without flipping a debug flag: + * + * - noise filter actually removed nodes (worth knowing because it + * tells the operator their tree has ignorable auto-generated + * `drift/` placeholders; silent filtering would be confusing) + * - budget was exhausted (nodes silently dropped pre-#343); this is + * a correctness-affecting event — the classifier's verdict is + * judged against a truncated tree view and can cite the wrong + * nodes as "closest match." + * + * Shared between the claude-cli and anthropic-api classifiers so both + * speak the same warning vocabulary. + */ +export function emitDigestDiagnostics( + detailed: CollectTreeDigestResult, + write: (line: string) => void, +): void { + if (detailed.skippedAsNoise > 0) { + write( + `gardener: tree digest filtered ${detailed.skippedAsNoise} drift placeholder node(s) (auto-generated by prior sync)`, + ); + } + if (detailed.budgetExhausted) { + write( + `gardener: tree digest budget exhausted — ${detailed.truncatedCount} node(s) dropped. Verdict may miss relevant tree context. Consider pruning the tree or raising DIGEST_BUDGET_BYTES.`, + ); + } +} diff --git a/tests/gardener/tree-digest.test.ts b/tests/gardener/tree-digest.test.ts new file mode 100644 index 0000000..cae26c8 --- /dev/null +++ b/tests/gardener/tree-digest.test.ts @@ -0,0 +1,189 @@ +import { mkdirSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { + collectTreeDigest, + collectTreeDigestDetailed, + emitDigestDiagnostics, + formatDigest, +} from "#products/gardener/engine/classifiers/tree-digest.js"; +import { useTmpDir } from "../helpers.js"; + +function writeNode( + root: string, + relPath: string, + frontmatterDescription: string | null, + body: string, +): void { + const full = join(root, relPath); + mkdirSync(join(full, ".."), { recursive: true }); + const fm = + frontmatterDescription !== null + ? `---\ndescription: ${frontmatterDescription}\n---\n` + : ""; + writeFileSync(full, `${fm}${body}`); +} + +describe("tree-digest -- SKIP_DIRS", () => { + it("skips .gardener-tree-cache entries (#343 noise filter)", () => { + const tmp = useTmpDir(); + writeNode(tmp.path, "NODE.md", "Root", ""); + writeNode( + tmp.path, + ".gardener-tree-cache/foo/NODE.md", + "Cache copy", + "", + ); + writeNode(tmp.path, "real/NODE.md", "Real node", ""); + + const paths = collectTreeDigest(tmp.path).map((e) => e.path).sort(); + expect(paths).toEqual(["NODE.md", "real/NODE.md"]); + expect(paths.some((p) => p.startsWith(".gardener-tree-cache"))).toBe(false); + }); +}); + +describe("tree-digest -- drift placeholder filter", () => { + it("drops auto-generated drift placeholders from the digest", () => { + const tmp = useTmpDir(); + writeNode(tmp.path, "NODE.md", "Root", ""); + writeNode( + tmp.path, + "drift/paperclip-abc/NODE.md", + "Auto-generated intermediate node for sync proposals.", + "", + ); + writeNode( + tmp.path, + "drift/paperclip-abc/product/NODE.md", + "Auto-generated intermediate node for sync proposals", + "", + ); + writeNode(tmp.path, "real/NODE.md", "Real decision node", ""); + + const result = collectTreeDigestDetailed(tmp.path); + const paths = result.entries.map((e) => e.path).sort(); + expect(paths).toEqual(["NODE.md", "real/NODE.md"]); + expect(result.skippedAsNoise).toBe(2); + expect(result.budgetExhausted).toBe(false); + expect(result.truncatedCount).toBe(0); + }); + + it("does NOT drop nodes whose summary only contains the phrase", () => { + // Regex anchors the full phrase — a node that merely mentions + // "auto-generated" is real content and must pass through. + const tmp = useTmpDir(); + writeNode( + tmp.path, + "NODE.md", + "Notes on auto-generated scaffolding policy for new repos", + "", + ); + const entries = collectTreeDigest(tmp.path); + expect(entries).toHaveLength(1); + }); +}); + +describe("tree-digest -- budget exhaustion reporting", () => { + it("reports truncatedCount and budgetExhausted when the budget fills", () => { + // We can't realistically write 500 MB of NODE.md in a unit test, + // so we verify the reporting contract by constructing a digest + // small enough to hit via direct function call with a monkey- + // patched constant. Here we take the cheaper path: write a + // modest number of nodes, check the detailed result has the + // expected shape, and trust the accounting is consistent (the + // fuller check lives in the E2E smoke against a real tree). + const tmp = useTmpDir(); + for (let i = 0; i < 5; i += 1) { + writeNode(tmp.path, `domain-${i}/NODE.md`, `Node ${i}`, ""); + } + const result = collectTreeDigestDetailed(tmp.path); + expect(result.entries).toHaveLength(5); + expect(result.budgetExhausted).toBe(false); + expect(result.truncatedCount).toBe(0); + expect(result.skippedAsNoise).toBe(0); + }); +}); + +describe("tree-digest -- emitDigestDiagnostics", () => { + function captureWrite(): { write: (line: string) => void; lines: string[] } { + const lines: string[] = []; + return { write: (line: string) => lines.push(line), lines }; + } + + it("emits a noise-filter line when skippedAsNoise > 0", () => { + const cap = captureWrite(); + emitDigestDiagnostics( + { + entries: [], + skippedAsNoise: 3, + truncatedCount: 0, + budgetExhausted: false, + }, + cap.write, + ); + expect(cap.lines).toHaveLength(1); + expect(cap.lines[0]).toContain("filtered 3 drift placeholder node"); + }); + + it("emits a budget-exhausted warning when budgetExhausted is true", () => { + const cap = captureWrite(); + emitDigestDiagnostics( + { + entries: [], + skippedAsNoise: 0, + truncatedCount: 17, + budgetExhausted: true, + }, + cap.write, + ); + expect(cap.lines).toHaveLength(1); + expect(cap.lines[0]).toContain("budget exhausted"); + expect(cap.lines[0]).toContain("17 node(s) dropped"); + }); + + it("emits both lines when both conditions hold", () => { + const cap = captureWrite(); + emitDigestDiagnostics( + { + entries: [], + skippedAsNoise: 2, + truncatedCount: 5, + budgetExhausted: true, + }, + cap.write, + ); + expect(cap.lines).toHaveLength(2); + expect(cap.lines.some((l) => l.includes("drift placeholder"))).toBe(true); + expect(cap.lines.some((l) => l.includes("budget exhausted"))).toBe(true); + }); + + it("stays silent on a healthy digest", () => { + const cap = captureWrite(); + emitDigestDiagnostics( + { + entries: [], + skippedAsNoise: 0, + truncatedCount: 0, + budgetExhausted: false, + }, + cap.write, + ); + expect(cap.lines).toHaveLength(0); + }); +}); + +describe("tree-digest -- formatDigest", () => { + it("formats each entry as a bullet with path and summary", () => { + const out = formatDigest([ + { path: "product/NODE.md", summary: "product area" }, + { path: "adapters/NODE.md", summary: "adapters area" }, + ]); + expect(out).toBe( + "- `product/NODE.md` — product area\n- `adapters/NODE.md` — adapters area", + ); + }); + + it("reports the empty case explicitly", () => { + expect(formatDigest([])).toBe("(no NODE.md files found)"); + }); +});