diff --git a/src/commands/review-show.ts b/src/commands/review-show.ts index 7574bb0..69ee65f 100644 --- a/src/commands/review-show.ts +++ b/src/commands/review-show.ts @@ -30,4 +30,12 @@ export default async function reviewShowCommand(id: string): Promise { output.status("!", output.warn(`[${v.severity}] ${v.message}`)); } } + + if (candidate.provenanceViolations && candidate.provenanceViolations.length > 0) { + console.log(); + output.header("Provenance violations"); + for (const v of candidate.provenanceViolations) { + output.status("!", output.warn(`[${v.severity}] ${v.message}`)); + } + } } diff --git a/src/compiler/candidates.ts b/src/compiler/candidates.ts index 2c8d621..eb0d2d1 100644 --- a/src/compiler/candidates.ts +++ b/src/compiler/candidates.ts @@ -49,6 +49,13 @@ interface CandidateDraft { * Omit (or pass `undefined`) when the candidate body is clean. */ schemaViolations?: LintResult[]; + /** + * Provenance lint violations for the candidate body — malformed claim + * citations, out-of-bounds spans, or missing source files. Surfaced + * alongside schema violations so reviewers see citation issues before + * approving. + */ + provenanceViolations?: LintResult[]; } /** Build a deterministic-but-unique id from a slug and a short random suffix. */ @@ -88,6 +95,7 @@ export async function writeCandidate( generatedAt: new Date().toISOString(), ...(draft.sourceStates ? { sourceStates: draft.sourceStates } : {}), ...(draft.schemaViolations ? { schemaViolations: draft.schemaViolations } : {}), + ...(draft.provenanceViolations ? { provenanceViolations: draft.provenanceViolations } : {}), }; await atomicWrite(candidatePath(root, candidate.id), JSON.stringify(candidate, null, 2)); diff --git a/src/compiler/index.ts b/src/compiler/index.ts index b5b5c44..9e7bc89 100644 --- a/src/compiler/index.ts +++ b/src/compiler/index.ts @@ -48,7 +48,12 @@ import { buildBudgetedCombinedContent, type SourceSlice } from "./prompt-budget. import { addObsidianMeta, generateMOC } from "./obsidian.js"; import { updateEmbeddings } from "../utils/embeddings.js"; import { writeCandidate } from "./candidates.js"; -import { checkPageCrossLinks } from "../linter/rules.js"; +import { + checkPageBrokenCitations, + checkPageCrossLinks, + checkPageMalformedCitations, +} from "../linter/rules.js"; +import type { LintResult } from "../linter/types.js"; import { renderMergedPageContent } from "./page-renderer.js"; import * as output from "../utils/output.js"; import { @@ -554,11 +559,18 @@ async function persistReviewCandidate( sourceStates: SourceStateMap, schema: SchemaConfig, ): Promise { - // Run schema-aware lint against the candidate body so violations are visible - // in `review show` before a reviewer approves the page. The virtual file path - // uses the slug so diagnostics are identifiable without a real disk path. + // Run schema-aware AND provenance-aware lint against the candidate body so + // both classes of violation are visible in `review show` before a reviewer + // approves the page. The virtual file path uses the slug so diagnostics + // are identifiable without a real disk path. Provenance lint covers the + // citation rules that previously only ran on the post-promotion compile. const virtualPath = `wiki/concepts/${entry.slug}.md`; - const violations = checkPageCrossLinks(fullPage, virtualPath, schema); + const schemaViolations = checkPageCrossLinks(fullPage, virtualPath, schema); + const provenanceViolations = await collectCandidateProvenanceViolations( + root, + fullPage, + virtualPath, + ); const candidate: ReviewCandidate = await writeCandidate(root, { title: entry.concept.concept, @@ -567,12 +579,33 @@ async function persistReviewCandidate( sources: entry.sourceFiles, body: fullPage, sourceStates: pickStatesForSources(sourceStates, entry.sourceFiles), - schemaViolations: violations.length > 0 ? violations : undefined, + schemaViolations: schemaViolations.length > 0 ? schemaViolations : undefined, + provenanceViolations: + provenanceViolations.length > 0 ? provenanceViolations : undefined, }); output.status("?", output.info(`Candidate ready: ${candidate.id} (${entry.slug})`)); return { candidateId: candidate.id }; } +/** + * Run the in-memory provenance lint rules against a candidate body: + * malformed claim citations + broken-source / out-of-bounds line spans. + * Returns the combined diagnostics so writeCandidate can persist them. + */ +async function collectCandidateProvenanceViolations( + root: string, + fullPage: string, + virtualPath: string, +): Promise { + const malformed = checkPageMalformedCitations(fullPage, virtualPath); + const broken = await checkPageBrokenCitations( + fullPage, + virtualPath, + path.join(root, SOURCES_DIR), + ); + return [...malformed, ...broken]; +} + /** * Materialise schema-declared seed pages (overview, comparison, entity). * Each seed page is written under wiki/concepts/ next to concept pages so diff --git a/src/linter/rules.ts b/src/linter/rules.ts index ecc2b0e..a9ab848 100644 --- a/src/linter/rules.ts +++ b/src/linter/rules.ts @@ -458,18 +458,47 @@ export async function checkBrokenCitations(root: string): Promise const pages = await collectAllPages(root); const sourcesDir = path.join(root, SOURCES_DIR); const results: LintResult[] = []; - /** Cache of source filename → line count to avoid repeated reads. */ const lineCountCache = new Map(); for (const page of pages) { - for (const { captured, line } of findMatchesInContent(page.content, CITATION_PATTERN)) { - await collectBrokenForMarker(captured, line, page.filePath, sourcesDir, lineCountCache, results); - } + const pageFindings = await checkPageBrokenCitations( + page.content, + page.filePath, + sourcesDir, + lineCountCache, + ); + results.push(...pageFindings); } return results; } +/** + * Pure-body variant of {@link checkBrokenCitations} that inspects a single + * page's content against an in-memory or on-disk sources directory. Used + * by the on-disk lint walker above, and by the in-memory candidate-lint + * path so `compile --review` surfaces broken-source-file and out-of-bounds + * span findings before a reviewer approves the candidate. + * + * @param content - Full page markdown including frontmatter. + * @param filePath - Logical path embedded in diagnostics (may be virtual). + * @param sourcesDir - Absolute path to the project's sources/ directory. + * @param lineCountCache - Optional cross-page cache; provide one when + * linting many pages so source file line counts aren't re-read. + */ +export async function checkPageBrokenCitations( + content: string, + filePath: string, + sourcesDir: string, + lineCountCache: Map = new Map(), +): Promise { + const results: LintResult[] = []; + for (const { captured, line } of findMatchesInContent(content, CITATION_PATTERN)) { + await collectBrokenForMarker(captured, line, filePath, sourcesDir, lineCountCache, results); + } + return results; +} + /** Append broken-citation diagnostics for every entry inside a single ^[...] marker. */ async function collectBrokenForMarker( captured: string, @@ -530,21 +559,31 @@ async function resolveLineCount( export async function checkMalformedClaimCitations(root: string): Promise { const pages = await collectAllPages(root); const results: LintResult[] = []; - for (const page of pages) { - for (const { captured, line } of findMatchesInContent(page.content, CITATION_PATTERN)) { - for (const part of captured.split(",")) { - if (!isMalformedCitationEntry(part)) continue; - results.push({ - rule: "malformed-claim-citation", - severity: "error", - file: page.filePath, - message: `Malformed claim citation ^[${captured}] — expected file.md, file.md:N-N, or file.md#LN-LN`, - line, - }); - } - } + results.push(...checkPageMalformedCitations(page.content, page.filePath)); } + return results; +} +/** + * Pure-body variant of {@link checkMalformedClaimCitations} that inspects + * a single page's content. Used by both the on-disk lint walker above and + * the in-memory candidate-lint path so `compile --review` surfaces + * malformed claim citations before a reviewer approves the candidate. + */ +export function checkPageMalformedCitations(content: string, filePath: string): LintResult[] { + const results: LintResult[] = []; + for (const { captured, line } of findMatchesInContent(content, CITATION_PATTERN)) { + for (const part of captured.split(",")) { + if (!isMalformedCitationEntry(part)) continue; + results.push({ + rule: "malformed-claim-citation", + severity: "error", + file: filePath, + message: `Malformed claim citation ^[${captured}] — expected file.md, file.md:N-N, or file.md#LN-LN`, + line, + }); + } + } return results; } diff --git a/src/utils/types.ts b/src/utils/types.ts index 4029dde..a9e83e3 100644 --- a/src/utils/types.ts +++ b/src/utils/types.ts @@ -156,6 +156,16 @@ export interface ReviewCandidate { * `review show` surfaces these so reviewers see failures before approving. */ schemaViolations?: import("../linter/types.js").LintResult[]; + /** + * Provenance lint violations detected at candidate-generation time. + * + * Covers malformed claim citations (`^[file.md:abc]`), out-of-bounds + * line spans, and citations referencing source files that don't exist. + * Surfaced in `review show` next to schema violations so reviewers + * catch citation issues before approving — these used to only show up + * on the next normal `compile` after the page was already promoted. + */ + provenanceViolations?: import("../linter/types.js").LintResult[]; } /** A single chunk citation surfaced as part of a query result. */ diff --git a/test/fixtures/review-show-helpers.ts b/test/fixtures/review-show-helpers.ts new file mode 100644 index 0000000..80d1490 --- /dev/null +++ b/test/fixtures/review-show-helpers.ts @@ -0,0 +1,21 @@ +/** + * Shared helpers for tests that exercise `llmwiki review show`. + * + * Centralises the console.log spy + invocation pattern so the schema and + * provenance violation suites share one implementation. fallow's CI mode + * flags the duplicate boilerplate as a clone group otherwise. + */ + +import { vi } from "vitest"; +import reviewShowCommand from "../../src/commands/review-show.js"; + +/** + * Run `reviewShowCommand` for a single candidate id and return all the + * console.log output it produced as a single newline-joined string. + * Tests assert against substrings of the returned text. + */ +export async function captureShowOutput(candidateId: string): Promise { + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + await reviewShowCommand(candidateId); + return logSpy.mock.calls.map((args) => args.join(" ")).join("\n"); +} diff --git a/test/provenance-violations.test.ts b/test/provenance-violations.test.ts new file mode 100644 index 0000000..8cf8e65 --- /dev/null +++ b/test/provenance-violations.test.ts @@ -0,0 +1,186 @@ +/** + * Tests for provenance violation attachment to review candidates + * (codex audit follow-up to the schema-overlap cluster review). + * + * `compile --review` previously only attached schema-cross-link + * violations to candidates. Citation-level lint (malformed claim + * citations, broken-source / out-of-bounds spans) only fired on the + * post-promotion compile, so reviewers approved candidates without + * seeing these issues. The fix runs both lint passes in + * persistReviewCandidate and persists the citation findings under + * `provenanceViolations` for `review show` to surface. + * + * Coverage: + * - per-page lint helpers detect malformed claim citations and + * broken / out-of-bounds spans on in-memory bodies + * - writeCandidate persists / omits provenanceViolations correctly + * - readCandidate round-trips provenanceViolations + * - reviewShowCommand prints the new block when populated + */ + +import { describe, it, expect } from "vitest"; +import path from "path"; +import { writeFile, mkdir } from "fs/promises"; +import { + checkPageMalformedCitations, + checkPageBrokenCitations, +} from "../src/linter/rules.js"; +import { writeCandidate, readCandidate } from "../src/compiler/candidates.js"; +import { useTempRoot } from "./fixtures/temp-root.js"; +import { captureShowOutput } from "./fixtures/review-show-helpers.js"; +import type { LintResult } from "../src/linter/types.js"; + +const root = useTempRoot(["sources"]); + +const VALID_BODY = [ + "---", + "title: Sample", + 'summary: "A sample."', + "sources: []", + 'createdAt: "2026-01-01T00:00:00.000Z"', + 'updatedAt: "2026-01-01T00:00:00.000Z"', + "---", + "", + "Body.", +].join("\n"); + +const SAMPLE_PROVENANCE_VIOLATION: LintResult = { + rule: "malformed-claim-citation", + severity: "error", + file: "wiki/concepts/sample.md", + message: "Malformed claim citation ^[file.md:abc] — expected file.md, file.md:N-N, or file.md#LN-LN", +}; + +describe("checkPageMalformedCitations — pure body lint", () => { + it("flags entries that don't parse as paragraph or claim grammar", () => { + const body = + "Para one. ^[source.md:abc]\n\nPara two. ^[ok.md]\n\nPara three. ^[multi.md, broken.md#X]"; + const findings = checkPageMalformedCitations(body, "wiki/concepts/test.md"); + // Two malformed entries: `:abc` and `#X` (not `#L`). + expect(findings).toHaveLength(2); + expect(findings[0].rule).toBe("malformed-claim-citation"); + expect(findings.every((f) => f.severity === "error")).toBe(true); + }); + + it("returns empty for clean paragraph and claim citations", () => { + const body = + "Para one. ^[source.md]\n\nPara two. ^[other.md:1-3]\n\nPara three. ^[third.md#L10-L20]"; + expect(checkPageMalformedCitations(body, "wiki/concepts/test.md")).toEqual([]); + }); +}); + +describe("checkPageBrokenCitations — pure body lint", () => { + it("flags citations referencing source files that don't exist", async () => { + const sourcesDir = path.join(root.dir, "sources"); + await mkdir(sourcesDir, { recursive: true }); + await writeFile(path.join(sourcesDir, "exists.md"), "line1\nline2\nline3\n", "utf-8"); + + const body = "Para one. ^[exists.md]\n\nPara two. ^[missing.md]"; + const findings = await checkPageBrokenCitations( + body, + "wiki/concepts/test.md", + sourcesDir, + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe("broken-citation"); + expect(findings[0].message).toContain("missing.md"); + }); + + it("flags claim spans that exceed the source file's line count", async () => { + const sourcesDir = path.join(root.dir, "sources"); + await mkdir(sourcesDir, { recursive: true }); + await writeFile(path.join(sourcesDir, "short.md"), "line1\nline2\nline3\n", "utf-8"); + + const body = "Para. ^[short.md:10-12]"; + const findings = await checkPageBrokenCitations( + body, + "wiki/concepts/test.md", + sourcesDir, + ); + expect(findings).toHaveLength(1); + expect(findings[0].message.toLowerCase()).toContain("out of bounds"); + }); + + it("returns empty when all citations resolve and stay in-range", async () => { + const sourcesDir = path.join(root.dir, "sources"); + await mkdir(sourcesDir, { recursive: true }); + await writeFile(path.join(sourcesDir, "ok.md"), "a\nb\nc\nd\ne\n", "utf-8"); + + const body = "Para. ^[ok.md:1-3]"; + expect(await checkPageBrokenCitations(body, "wiki/concepts/test.md", sourcesDir)).toEqual( + [], + ); + }); +}); + +describe("candidate provenance violations — persistence", () => { + it("writeCandidate stores provenanceViolations when provided", async () => { + const candidate = await writeCandidate(root.dir, { + title: "Sample", + slug: "sample", + summary: "A sample.", + sources: ["source.md"], + body: VALID_BODY, + provenanceViolations: [SAMPLE_PROVENANCE_VIOLATION], + }); + + expect(candidate.provenanceViolations).toHaveLength(1); + expect(candidate.provenanceViolations![0].rule).toBe("malformed-claim-citation"); + }); + + it("readCandidate round-trips provenanceViolations from disk", async () => { + const written = await writeCandidate(root.dir, { + title: "Sample", + slug: "sample", + summary: "A sample.", + sources: ["source.md"], + body: VALID_BODY, + provenanceViolations: [SAMPLE_PROVENANCE_VIOLATION], + }); + + const loaded = await readCandidate(root.dir, written.id); + expect(loaded?.provenanceViolations).toEqual([SAMPLE_PROVENANCE_VIOLATION]); + }); + + it("writeCandidate omits provenanceViolations when not provided", async () => { + const candidate = await writeCandidate(root.dir, { + title: "Sample", + slug: "sample", + summary: "A sample.", + sources: ["source.md"], + body: VALID_BODY, + }); + + expect(candidate.provenanceViolations).toBeUndefined(); + }); +}); + +describe("review show — provenance violations display", () => { + it("prints provenance block when the candidate has provenanceViolations", async () => { + const candidate = await writeCandidate(root.dir, { + title: "Sample", + slug: "sample", + summary: "A sample.", + sources: ["source.md"], + body: VALID_BODY, + provenanceViolations: [SAMPLE_PROVENANCE_VIOLATION], + }); + + const allOutput = await captureShowOutput(candidate.id); + expect(allOutput).toContain("Provenance violations"); + expect(allOutput).toContain("Malformed claim citation"); + }); + + it("does not print provenance block when the candidate has no provenanceViolations", async () => { + const candidate = await writeCandidate(root.dir, { + title: "Clean", + slug: "clean", + summary: "No violations.", + sources: ["source.md"], + body: VALID_BODY, + }); + + const allOutput = await captureShowOutput(candidate.id); + expect(allOutput).not.toContain("Provenance violations"); + }); +}); diff --git a/test/review-provenance-integration.test.ts b/test/review-provenance-integration.test.ts new file mode 100644 index 0000000..16784af --- /dev/null +++ b/test/review-provenance-integration.test.ts @@ -0,0 +1,114 @@ +/** + * CLI integration test: `compile --review` runs provenance lint against + * the generated candidate body and persists the findings on the + * candidate JSON record. + * + * Reproduces what would happen if the LLM produces a body with a + * malformed claim citation: aimock returns the canned body, the compile + * subprocess writes the candidate, and the saved JSON should now carry + * `provenanceViolations` in addition to the existing `schemaViolations` + * surface. Without the fix, `provenanceViolations` would be absent and + * the reviewer would only see the issue on a normal compile after + * approval. + */ + +import { describe, it, expect } from "vitest"; +import { readdir, readFile } from "fs/promises"; +import path from "path"; +import { + mockClaudeEnv, + useAimockLifecycle, + type MockClaudeHandle, +} from "./fixtures/aimock-helper.js"; +import { runCLI, expectCLIExit } from "./fixtures/run-cli.js"; + +const aimock = useAimockLifecycle("review-provenance"); + +const CONCEPT = "Provenance Lint Test"; +const CONCEPT_SLUG = "provenance-lint-test"; + +/** Stub the extraction tool call to produce a single new concept. */ +function stubExtraction(handle: MockClaudeHandle): void { + handle.mock.onToolCall("extract_concepts", { + toolCalls: [ + { + name: "extract_concepts", + arguments: { + concepts: [ + { + concept: CONCEPT, + summary: "Concept used to test review-mode provenance lint.", + is_new: true, + tags: ["test"], + confidence: 0.9, + }, + ], + }, + }, + ], + }); +} + +/** Read the single candidate JSON written by `compile --review`. */ +async function readOnlyCandidate(cwd: string): Promise<{ + body: string; + schemaViolations?: unknown[]; + provenanceViolations?: unknown[]; +}> { + const dir = path.join(cwd, ".llmwiki", "candidates"); + const files = (await readdir(dir)).filter((f) => f.endsWith(".json")); + expect(files).toHaveLength(1); + return JSON.parse(await readFile(path.join(dir, files[0]), "utf-8")); +} + +/** + * Stand up aimock with the canned extraction + a stubbed page body, run + * `compile --review` through the CLI subprocess, and return the parsed + * single candidate. Centralised so the per-test bodies focus on the + * assertion that distinguishes them. + */ +async function compileReviewWithStubbedBody(stubBody: string): Promise<{ + body: string; + schemaViolations?: unknown[]; + provenanceViolations?: unknown[]; +}> { + const handle = await aimock.start(); + stubExtraction(handle); + handle.mock.onMessage(/.*/, { content: stubBody }); + const cwd = await aimock.makeWorkspace("# Source\n\nA short source for the review test.\n"); + const result = await runCLI(["compile", "--review"], cwd, mockClaudeEnv(handle)); + expectCLIExit(result, 0); + return readOnlyCandidate(cwd); +} + +describe("compile --review provenance lint integration", () => { + it("attaches provenanceViolations when the candidate body has malformed claim citations", async () => { + // Body ships TWO malformed claim citations (`:abc` and `#X`) — both surface. + const candidate = await compileReviewWithStubbedBody( + "First paragraph drawing from the source. ^[source.md:abc]\n\n" + + "Second paragraph with a hash-form malformed span. ^[source.md#X]\n", + ); + expect(candidate.provenanceViolations).toBeDefined(); + expect(candidate.provenanceViolations!.length).toBeGreaterThanOrEqual(2); + const firstRule = (candidate.provenanceViolations![0] as { rule?: unknown }).rule; + expect(firstRule).toBe("malformed-claim-citation"); + }, 30_000); + + it("attaches provenanceViolations when the candidate body cites a missing source file", async () => { + const candidate = await compileReviewWithStubbedBody( + "Body with an inline citation to a non-existent source. ^[does-not-exist.md]\n", + ); + expect(candidate.provenanceViolations).toBeDefined(); + const rules = (candidate.provenanceViolations as Array<{ rule: string }>).map((v) => v.rule); + expect(rules).toContain("broken-citation"); + }, 30_000); + + it("omits provenanceViolations when the candidate body has clean citations", async () => { + const cleanBody = "Body without any citation markers — clean.\n"; + const candidate = await compileReviewWithStubbedBody(cleanBody); + expect(candidate.provenanceViolations).toBeUndefined(); + // Real assertion replacing the prior no-op: the stubbed body content + // must round-trip through the candidate write. + expect(candidate.body).toContain("Body without any citation markers"); + }, 30_000); +}); diff --git a/test/schema-violations.test.ts b/test/schema-violations.test.ts index ed55e69..e9e3240 100644 --- a/test/schema-violations.test.ts +++ b/test/schema-violations.test.ts @@ -12,10 +12,10 @@ * - reviewShowCommand prints nothing extra when no violations */ -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect } from "vitest"; import { writeCandidate, readCandidate } from "../src/compiler/candidates.js"; -import reviewShowCommand from "../src/commands/review-show.js"; import { useTempRoot } from "./fixtures/temp-root.js"; +import { captureShowOutput } from "./fixtures/review-show-helpers.js"; import type { LintResult } from "../src/linter/types.js"; const root = useTempRoot(["sources"]); @@ -83,13 +83,6 @@ describe("candidate schema violations — persistence", () => { }); }); -/** Run reviewShowCommand for a candidate and return all console.log output. */ -async function captureShowOutput(candidateId: string): Promise { - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - await reviewShowCommand(candidateId); - return logSpy.mock.calls.map((args) => args.join(" ")).join("\n"); -} - describe("review show — schema violations display", () => { it("prints violations block when the candidate has schemaViolations", async () => { const candidate = await writeCandidate(root.dir, {