Skip to content

feat: discovery-evidence dedupe + schema versioning #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
let-sunny merged 5 commits into from
Mar 25, 2026
Merged

feat: discovery-evidence dedupe + schema versioning #43

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6470656
feat: add dedupe + schema versioning to discovery-evidence
claude Mar 25, 2026
4c1ad19
fix: address CodeRabbit review feedback
claude Mar 25, 2026
d1d968d
fix: throw on unsupported schemaVersion to prevent silent data loss
claude Mar 25, 2026
b03abd3
fix: add try/catch to call sites + reorganize tests
claude Mar 25, 2026
a9d5969
fix: add fixture case-insensitive dedupe test + normalize CLI error o…
claude Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion data/discovery-evidence.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
[]
{
"schemaVersion": 1,
"entries": []
}
9 changes: 9 additions & 0 deletions src/agents/contracts/evidence.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export type CrossRunEvidence = Record<string, CrossRunEvidenceGroup>;

// --- Discovery evidence ---

export const DISCOVERY_EVIDENCE_SCHEMA_VERSION = 1;

export const DiscoveryEvidenceEntrySchema = z.object({
description: z.string(),
category: z.string(),
Expand All @@ -35,3 +37,10 @@ export const DiscoveryEvidenceEntrySchema = z.object({
});

export type DiscoveryEvidenceEntry = z.infer<typeof DiscoveryEvidenceEntrySchema>;

export const DiscoveryEvidenceFileSchema = z.object({
schemaVersion: z.literal(DISCOVERY_EVIDENCE_SCHEMA_VERSION),
entries: z.array(z.unknown()),
});
Comment on lines +41 to +44

@coderabbitai coderabbitai Bot Mar 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Constrain schemaVersion to the current format.

schemaVersion: number means a future { schemaVersion: 2, entries: [...] } file still satisfies this contract. readDiscoveryEvidence() will then load it as if it were v1, and the next append/prune can silently rewrite it back as version 1. Make this field accept only DISCOVERY_EVIDENCE_SCHEMA_VERSION so unsupported formats are rejected instead of downgraded. As per coding guidelines, "Validate all external inputs with Zod schemas".

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/agents/contracts/evidence.ts` around lines 41 - 44, Change the Zod schema
for DiscoveryEvidenceFileSchema so schemaVersion is constrained to the current
constant DISCOVERY_EVIDENCE_SCHEMA_VERSION instead of a free number: replace
z.number() with a literal/enum that matches DISCOVERY_EVIDENCE_SCHEMA_VERSION
(e.g. z.literal(DISCOVERY_EVIDENCE_SCHEMA_VERSION) or z.union if needed) so
readDiscoveryEvidence and any append/prune operations will reject unsupported
versions rather than silently downgrading; update DiscoveryEvidenceFileSchema in
src/agents/contracts/evidence.ts and ensure imports/reference to
DISCOVERY_EVIDENCE_SCHEMA_VERSION and DiscoveryEvidenceEntrySchema are present.


export type DiscoveryEvidenceFile = z.infer<typeof DiscoveryEvidenceFileSchema>;
227 changes: 206 additions & 21 deletions src/agents/evidence-collector.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
loadDiscoveryEvidence,
appendDiscoveryEvidence,
pruneDiscoveryEvidence,
DISCOVERY_EVIDENCE_SCHEMA_VERSION,
} from "./evidence-collector.js";
import type {
CalibrationEvidenceEntry,
Expand Down Expand Up @@ -187,7 +188,21 @@ describe("evidence-collector", () => {
expect(result).toEqual([]);
});

it("returns entries from file", () => {
it("loads entries from versioned format", () => {
const file = {
schemaVersion: DISCOVERY_EVIDENCE_SCHEMA_VERSION,
entries: [
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
],
};
writeFileSync(disPath, JSON.stringify(file), "utf-8");

const result = loadDiscoveryEvidence(disPath);
expect(result).toHaveLength(1);
expect(result[0]!.category).toBe("layout");
});

it("loads entries from legacy plain-array format (v0 fallback)", () => {
const entries: DiscoveryEvidenceEntry[] = [
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
];
Expand All @@ -197,59 +212,229 @@ describe("evidence-collector", () => {
expect(result).toHaveLength(1);
expect(result[0]!.category).toBe("layout");
});

it("handles malformed JSON gracefully", () => {
writeFileSync(disPath, "not json", "utf-8");
const result = loadDiscoveryEvidence(disPath);
expect(result).toEqual([]);
});

it("skips invalid entries in legacy array", () => {
writeFileSync(disPath, JSON.stringify([
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
{ bad: "entry" },
]), "utf-8");

const result = loadDiscoveryEvidence(disPath);
expect(result).toHaveLength(1);
});

it("skips invalid entries in versioned format (partial corruption)", () => {
const file = {
schemaVersion: DISCOVERY_EVIDENCE_SCHEMA_VERSION,
entries: [
{ description: "good", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
{ bad: "entry" },
{ description: "also good", category: "color", impact: "easy", fixture: "fx2", timestamp: "t2", source: "gap-analysis" },
],
};
writeFileSync(disPath, JSON.stringify(file), "utf-8");

const result = loadDiscoveryEvidence(disPath);
expect(result).toHaveLength(2);
expect(result[0]!.description).toBe("good");
expect(result[1]!.description).toBe("also good");
});

it("throws on unsupported schemaVersion to prevent silent overwrite", () => {
const file = {
schemaVersion: 999,
entries: [
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
],
};
writeFileSync(disPath, JSON.stringify(file), "utf-8");

expect(() => loadDiscoveryEvidence(disPath)).toThrow(/Unsupported discovery-evidence schemaVersion: 999/);
});

it("append is no-op when file has unsupported schemaVersion", () => {
const file = { schemaVersion: 999, entries: [] };
writeFileSync(disPath, JSON.stringify(file), "utf-8");
const before = readFileSync(disPath, "utf-8");

expect(() => appendDiscoveryEvidence([
{ description: "new", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
], disPath)).toThrow(/Unsupported discovery-evidence schemaVersion/);

// File must not be overwritten
expect(readFileSync(disPath, "utf-8")).toBe(before);
});

it("prune is no-op when file has unsupported schemaVersion", () => {
const file = { schemaVersion: 999, entries: [
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
]};
writeFileSync(disPath, JSON.stringify(file), "utf-8");
const before = readFileSync(disPath, "utf-8");

expect(() => pruneDiscoveryEvidence(["layout"], disPath)).toThrow(/Unsupported discovery-evidence schemaVersion/);

expect(readFileSync(disPath, "utf-8")).toBe(before);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
});

describe("appendDiscoveryEvidence", () => {
it("creates file and appends entries", () => {
it("creates file in versioned format", () => {
appendDiscoveryEvidence([
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "gap-analysis" },
], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as DiscoveryEvidenceEntry[];
expect(raw).toHaveLength(1);
expect(raw[0]!.source).toBe("gap-analysis");
const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { schemaVersion: number; entries: DiscoveryEvidenceEntry[] };
expect(raw.schemaVersion).toBe(DISCOVERY_EVIDENCE_SCHEMA_VERSION);
expect(raw.entries).toHaveLength(1);
expect(raw.entries[0]!.source).toBe("gap-analysis");
});

it("appends to existing entries", () => {
writeFileSync(disPath, JSON.stringify([
it("appends to existing entries (different keys)", () => {
appendDiscoveryEvidence([
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
]), "utf-8");
], disPath);

appendDiscoveryEvidence([
{ description: "gap2", category: "color", impact: "moderate", fixture: "fx2", timestamp: "t2", source: "gap-analysis" },
], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as DiscoveryEvidenceEntry[];
expect(raw).toHaveLength(2);
const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { entries: DiscoveryEvidenceEntry[] };
expect(raw.entries).toHaveLength(2);
});

it("deduplicates by (category + description + fixture), last-write-wins", () => {
appendDiscoveryEvidence([
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
], disPath);

// Same category+description+fixture, different impact/timestamp → replaces
appendDiscoveryEvidence([
{ description: "gap1", category: "layout", impact: "moderate", fixture: "fx1", timestamp: "t2", source: "evaluation" },
], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { entries: DiscoveryEvidenceEntry[] };
expect(raw.entries).toHaveLength(1);
expect(raw.entries[0]!.impact).toBe("moderate");
expect(raw.entries[0]!.timestamp).toBe("t2");
});

it("dedupe is case-insensitive for category and description", () => {
appendDiscoveryEvidence([
{ description: "Gap One", category: "Layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
], disPath);

appendDiscoveryEvidence([
{ description: "gap one", category: "layout", impact: "easy", fixture: "fx1", timestamp: "t2", source: "evaluation" },
], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { entries: DiscoveryEvidenceEntry[] };
expect(raw.entries).toHaveLength(1);
expect(raw.entries[0]!.impact).toBe("easy");
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it("dedupes within a single append call (last row wins)", () => {
appendDiscoveryEvidence([
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
{ description: "gap1", category: "layout", impact: "easy", fixture: "fx1", timestamp: "t2", source: "evaluation" },
], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { entries: DiscoveryEvidenceEntry[] };
expect(raw.entries).toHaveLength(1);
expect(raw.entries[0]!.impact).toBe("easy");
});

it("same description different fixture → kept as separate entries", () => {
appendDiscoveryEvidence([
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx2", timestamp: "t1", source: "evaluation" },
], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { entries: DiscoveryEvidenceEntry[] };
expect(raw.entries).toHaveLength(2);
});

it("migrates legacy array to versioned format on append", () => {
// Write legacy format
writeFileSync(disPath, JSON.stringify([
{ description: "old", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t0", source: "evaluation" },
]), "utf-8");

appendDiscoveryEvidence([
{ description: "new", category: "color", impact: "easy", fixture: "fx2", timestamp: "t1", source: "gap-analysis" },
], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { schemaVersion: number; entries: DiscoveryEvidenceEntry[] };
expect(raw.schemaVersion).toBe(DISCOVERY_EVIDENCE_SCHEMA_VERSION);
expect(raw.entries).toHaveLength(2);
});

it("does nothing for empty entries", () => {
appendDiscoveryEvidence([
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
], disPath);
const before = readFileSync(disPath, "utf-8");

appendDiscoveryEvidence([], disPath);

const after = readFileSync(disPath, "utf-8");
expect(after).toBe(before);
});
});

describe("pruneDiscoveryEvidence", () => {
it("removes entries for specified categories (case-insensitive)", () => {
const entries: DiscoveryEvidenceEntry[] = [
appendDiscoveryEvidence([
{ description: "gap1", category: "Layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
{ description: "gap2", category: "layout", impact: "hard", fixture: "fx2", timestamp: "t2", source: "gap-analysis" },
{ description: "gap3", category: "color", impact: "moderate", fixture: "fx1", timestamp: "t1", source: "evaluation" },
];
writeFileSync(disPath, JSON.stringify(entries), "utf-8");
], disPath);

pruneDiscoveryEvidence(["layout"], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as DiscoveryEvidenceEntry[];
expect(raw).toHaveLength(1);
expect(raw[0]!.category).toBe("color");
const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { entries: DiscoveryEvidenceEntry[] };
expect(raw.entries).toHaveLength(1);
expect(raw.entries[0]!.category).toBe("color");
});

it("writes versioned format after prune", () => {
appendDiscoveryEvidence([
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
], disPath);

pruneDiscoveryEvidence(["layout"], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { schemaVersion: number; entries: DiscoveryEvidenceEntry[] };
expect(raw.schemaVersion).toBe(DISCOVERY_EVIDENCE_SCHEMA_VERSION);
expect(raw.entries).toHaveLength(0);
});

it("does nothing for empty categories", () => {
const entries: DiscoveryEvidenceEntry[] = [
appendDiscoveryEvidence([
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
];
writeFileSync(disPath, JSON.stringify(entries), "utf-8");
], disPath);

pruneDiscoveryEvidence([], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as DiscoveryEvidenceEntry[];
expect(raw).toHaveLength(1);
const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { entries: DiscoveryEvidenceEntry[] };
expect(raw.entries).toHaveLength(1);
});

it("trims categories when matching", () => {
appendDiscoveryEvidence([
{ description: "gap1", category: "layout", impact: "hard", fixture: "fx1", timestamp: "t1", source: "evaluation" },
], disPath);

pruneDiscoveryEvidence([" layout "], disPath);

const raw = JSON.parse(readFileSync(disPath, "utf-8")) as { entries: DiscoveryEvidenceEntry[] };
expect(raw.entries).toHaveLength(0);
});
});
});
Loading
Loading