Skip to content

Commit a58cb08

Browse files
committed
Fix false-positive duplicate userMessage warning in validateFixtures
The dedup key now includes turnIndex, hasToolResult, and sequenceIndex alongside userMessage, so fixtures sharing a userMessage but differing on those fields no longer trigger a spurious shadowing warning.
1 parent 2af7d21 commit a58cb08

2 files changed

Lines changed: 53 additions & 3 deletions

File tree

src/__tests__/fixture-loader.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,53 @@ describe("validateFixtures", () => {
813813
);
814814
});
815815

816+
it("no warning: same userMessage but different turnIndex", () => {
817+
const fixtures = [
818+
makeFixture({ match: { userMessage: "hello", turnIndex: 0 } }),
819+
makeFixture({ match: { userMessage: "hello", turnIndex: 1 } }),
820+
];
821+
const results = validateFixtures(fixtures);
822+
const duplicateWarnings = results.filter(
823+
(r) => r.severity === "warning" && r.message.includes("duplicate"),
824+
);
825+
expect(duplicateWarnings).toHaveLength(0);
826+
});
827+
828+
it("no warning: same userMessage but different hasToolResult", () => {
829+
const fixtures = [
830+
makeFixture({ match: { userMessage: "hello", hasToolResult: false } }),
831+
makeFixture({ match: { userMessage: "hello", hasToolResult: true } }),
832+
];
833+
const results = validateFixtures(fixtures);
834+
const duplicateWarnings = results.filter(
835+
(r) => r.severity === "warning" && r.message.includes("duplicate"),
836+
);
837+
expect(duplicateWarnings).toHaveLength(0);
838+
});
839+
840+
it("no warning: same userMessage but different sequenceIndex", () => {
841+
const fixtures = [
842+
makeFixture({ match: { userMessage: "hello", sequenceIndex: 0 } }),
843+
makeFixture({ match: { userMessage: "hello", sequenceIndex: 1 } }),
844+
];
845+
const results = validateFixtures(fixtures);
846+
const duplicateWarnings = results.filter(
847+
(r) => r.severity === "warning" && r.message.includes("duplicate"),
848+
);
849+
expect(duplicateWarnings).toHaveLength(0);
850+
});
851+
852+
it("warning: same userMessage with identical turnIndex/hasToolResult/sequenceIndex", () => {
853+
const fixtures = [
854+
makeFixture({ match: { userMessage: "hello", turnIndex: 1, hasToolResult: true } }),
855+
makeFixture({ match: { userMessage: "hello", turnIndex: 1, hasToolResult: true } }),
856+
];
857+
const results = validateFixtures(fixtures);
858+
expect(results.some((r) => r.severity === "warning" && r.message.includes("duplicate"))).toBe(
859+
true,
860+
);
861+
});
862+
816863
it("warning: catch-all not in last position", () => {
817864
const fixtures = [makeFixture({ match: {} }), makeFixture({ match: { userMessage: "hello" } })];
818865
const results = validateFixtures(fixtures);

src/fixture-loader.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -533,18 +533,21 @@ export function validateFixtures(fixtures: Fixture[]): ValidationResult[] {
533533

534534
// --- Warning checks ---
535535

536-
// Duplicate userMessage shadowing
536+
// Duplicate userMessage shadowing — include turnIndex, hasToolResult, and
537+
// sequenceIndex in the dedup key so that fixtures which share a userMessage
538+
// but differ on those fields are NOT considered duplicates.
537539
const um = f.match.userMessage;
538540
if (typeof um === "string" && um) {
539-
const prev = seenUserMessages.get(um);
541+
const dedupKey = `${um}|${f.match.turnIndex}|${f.match.hasToolResult}|${f.match.sequenceIndex}`;
542+
const prev = seenUserMessages.get(dedupKey);
540543
if (prev !== undefined) {
541544
results.push({
542545
severity: "warning",
543546
fixtureIndex: i,
544547
message: `duplicate userMessage '${um}' — shadows fixture ${prev}`,
545548
});
546549
} else {
547-
seenUserMessages.set(um, i);
550+
seenUserMessages.set(dedupKey, i);
548551
}
549552
}
550553

0 commit comments

Comments
 (0)