Skip to content

fix(dedup): Group findings by semantic bug identity #536

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
gricha merged 2 commits into from
Sep 28, 2026
Merged

fix(dedup): Group findings by semantic bug identity #536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95f5ed1
fix(dedup): Group findings by semantic bug identity
sentry-junior[bot] Sep 28, 2026
30ef763
ref(dedup): Trim redundant regression coverage
sentry-junior[bot] Sep 28, 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
48 changes: 34 additions & 14 deletions packages/warden/src/output/dedup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
} from './dedup.js';
import type { Finding } from '../types/index.js';
import type { ExistingComment } from './dedup.js';
import * as runtimes from '../sdk/runtimes/index.js';

describe('generateContentHash', () => {
it('generates consistent 8-char hex hash', () => {
Expand Down Expand Up @@ -803,7 +804,7 @@ describe('consolidateBatchFindings', () => {
expect(result.removedCount).toBe(0);
});

it('does not group findings more than 5 lines apart', async () => {
it('keeps findings more than 5 lines apart in hashOnly mode', async () => {
const finding1: Finding = {
id: 'f1',
severity: 'high',
Expand All @@ -826,7 +827,7 @@ describe('consolidateBatchFindings', () => {
expect(result.removedCount).toBe(0);
});

it('does not group findings in different files', async () => {
it('keeps findings in different files in hashOnly mode', async () => {
const finding1: Finding = {
id: 'f1',
severity: 'high',
Expand All @@ -848,32 +849,51 @@ describe('consolidateBatchFindings', () => {
expect(result.removedCount).toBe(0);
});

it('skips LLM when no proximity clusters exist', async () => {
it('merges semantic duplicates across distant lines and files but retains unrelated findings', async () => {
const finding1: Finding = {
id: 'f1',
severity: 'high',
title: 'Bug A',
description: 'Description A',
title: 'Outputs bypass the privacy gate',
description: 'Both wrappers serialize answers even when output collection is disabled.',
location: { path: 'src/file.ts', startLine: 10 },
};

const finding2: Finding = {
id: 'f2',
severity: 'medium',
title: 'Bug B',
description: 'Description B',
title: 'Output collection opt-out is ignored',
description: 'Check the output setting before writing answers in both wrappers.',
location: { path: 'src/file.ts', startLine: 100 },
};

// Even with an API key, no LLM call should be made since findings are far apart
const result = await consolidateBatchFindings([finding1, finding2], { apiKey: 'test-key' });
expect(result.findings).toHaveLength(2);
expect(result.removedCount).toBe(0);
expect(result.usage).toBeUndefined();
const finding3: Finding = {
...finding2, id: 'f3', title: 'Evaluation answers recorded without consent',
location: { path: 'tests/file.test.ts', startLine: 200 },
};
const unrelated: Finding = {
...finding1, id: 'f4', title: 'Score probabilities and legend can misalign',
description: 'Independent dictionary iteration can pair probabilities with the wrong labels.',
};
const locationless: Finding = { ...finding1, id: 'f5', location: undefined };
const runAuxiliary = vi.fn().mockResolvedValue({
success: true, data: [[1, 2, 3]],
usage: { inputTokens: 100, outputTokens: 10, costUSD: 0.001 },
});
vi.spyOn(runtimes, 'getRuntime').mockReturnValue({
name: 'pi', runAuxiliary,
} as unknown as ReturnType<typeof runtimes.getRuntime>);

const result = await consolidateBatchFindings([locationless, finding1, finding2, finding3, unrelated], { runtime: 'pi' });
expect(result.findings).toEqual([
locationless,
{ ...finding1, additionalLocations: [finding2.location, finding3.location] },
unrelated,
]);
expect(result.removedCount).toBe(2);
expect(result.removedFindings).toEqual([finding2, finding3]);
});

it('groups findings within 5 lines of each other for proximity check', async () => {
// This tests the proximity grouping logic (without LLM since no API key)
it('keeps nearby findings when runtime authentication is unavailable', async () => {
const finding1: Finding = {
id: 'f1',
severity: 'high',
Expand Down
81 changes: 14 additions & 67 deletions packages/warden/src/output/dedup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,8 +521,9 @@ ${findingsList}
</new_findings>`,
`<deduplication_rules>
Return a JSON array of objects identifying which findings are DUPLICATES of which existing comments.
Only mark as duplicate if they describe the SAME issue at the SAME location (within a few lines).
Different issues at the same location are NOT duplicates.
Mark as duplicate when the finding describes the SAME underlying bug as an existing comment, even if wording, evidence, or the reported file and line differ.
A report anchored in a caller or test can duplicate one anchored in the implementation. Compare the actual defect and affected behavior, not just the location or issue category.
Independent bugs requiring separate fixes are NOT duplicates, even at the same location. Keep uncertain matches and findings with additional independent bugs separate.
</deduplication_rules>`,
buildJsonOutputSection(`[{"findingIndex": 1, "existingIndex": 2}]
where findingIndex is the 1-based index of the new finding and existingIndex is the 1-based index of the matching existing comment.
Expand Down Expand Up @@ -694,8 +695,6 @@ export function findingToExistingComment(finding: Finding, skill?: string): Exis
// Intra-batch consolidation
// -----------------------------------------------------------------------------

const PROXIMITY_THRESHOLD = 5;

/**
* Result from consolidating findings within a single batch.
*/
Expand All @@ -711,62 +710,12 @@ const ConsolidationGroupsSchema = z.array(
z.array(z.number().int())
);

/**
* Group findings by file path, then identify clusters where findings are within
* PROXIMITY_THRESHOLD lines of each other. Returns only clusters with 2+ findings.
*/
function findProximityClusters(findings: Finding[]): Finding[][] {
// Group by file path
const byPath = new Map<string, Finding[]>();
for (const f of findings) {
const path = f.location?.path ?? '';
const existing = byPath.get(path);
if (existing) {
existing.push(f);
} else {
byPath.set(path, [f]);
}
}

const clusters: Finding[][] = [];

for (const group of byPath.values()) {
if (group.length < 2) continue;

// Sort by line number
const sorted = [...group].sort((a, b) => findingLine(a) - findingLine(b));

// Single-linkage clustering: consecutive findings within PROXIMITY_THRESHOLD
// lines of each other are grouped together.
const first = sorted[0];
if (!first) continue;
let current: Finding[] = [first];

for (let i = 1; i < sorted.length; i++) {
const prev = sorted[i - 1];
const curr = sorted[i];
if (!prev || !curr) continue;

if (findingLine(curr) - findingLine(prev) <= PROXIMITY_THRESHOLD) {
current.push(curr);
} else {
if (current.length >= 2) clusters.push(current);
current = [curr];
}
}
if (current.length >= 2) clusters.push(current);
}

return clusters;
}

/**
* Consolidate findings within a single batch to remove duplicates that describe
* the same root cause. Three-phase approach:
* the same root cause, regardless of wording or location.
*
* 1. Hash dedup: remove exact duplicates (same path:line:contentHash)
* 2. Proximity grouping: identify clusters of findings within 5 lines of each other
* 3. LLM consolidation: ask the auxiliary runtime to group findings by root cause (only when proximity matches exist)
* 2. LLM consolidation: group remaining findings by semantic issue identity
*
* For each group, keeps the highest-severity finding.
*/
Expand Down Expand Up @@ -803,19 +752,16 @@ export async function consolidateBatchFindings(
console.log(`Consolidate: ${hashRemovedCount} exact duplicate findings removed within batch`);
}

// Phase 2: Proximity grouping
const clusters = findProximityClusters(hashDeduped);

// If no proximity clusters, hash-only mode, or no runtime auth, return hash-deduped results.
if (clusters.length === 0 || options.hashOnly || !canUseRuntimeAuth(options)) {
// A locationless winner would suppress an otherwise postable inline comment.
const withLocations = hashDeduped.filter((finding) => finding.location);
if (withLocations.length <= 1 || options.hashOnly || !canUseRuntimeAuth(options)) {
return { findings: hashDeduped, removedCount: hashRemovedCount, removedFindings: hashRemovedFindings };
}

// Phase 3: LLM consolidation for proximity clusters
// Only send clustered findings to the LLM (deduplicated across clusters)
const clusteredList = [...new Set(clusters.flat())];
const findingsList = formatIndexedFindingsForPrompt(clusteredList, {
// Compare across files: the same defect may be anchored in distant hunks or tests.
const findingsList = formatIndexedFindingsForPrompt(withLocations, {
includeSeverity: true,
includeVerification: true,
});

const prompt = joinPromptSections([
Expand All @@ -827,7 +773,8 @@ ${findingsList}
</findings>`,
`<deduplication_rules>
Return a JSON array of arrays, where each inner array contains the 1-based indices of findings that describe the same root cause.
Only group findings that are truly about the same underlying issue. Findings about different issues should NOT be grouped even if they're nearby.
Group by semantic meaning, not exact wording of titles, descriptions, or evidence. Reports of the same defect may point to different lines, files, callers, or tests.
Sharing an issue category is not enough: independent bugs requiring separate fixes are separate findings, even if nearby. Keep uncertain matches and findings with additional independent bugs separate.
Singletons (findings with no duplicates) should not appear in any group.
</deduplication_rules>`,
buildJsonOutputSection('Return the JSON array. Return [] if no findings share a root cause.'),
Expand All @@ -850,7 +797,7 @@ Singletons (findings with no duplicates) should not appear in any group.
return { findings: hashDeduped, removedCount: hashRemovedCount, removedFindings: hashRemovedFindings, usage: result.usage };
}

const { absorbed, replacements } = applyMergeGroups(clusteredList, result.data);
const { absorbed, replacements } = applyMergeGroups(withLocations, result.data);

if (absorbed.size === 0) {
return { findings: hashDeduped, removedCount: hashRemovedCount, removedFindings: hashRemovedFindings, usage: result.usage };
Expand Down
66 changes: 7 additions & 59 deletions packages/warden/src/sdk/extract.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,18 +206,22 @@ describe('mergeCrossLocationFindings', () => {
}
});

it('merges two findings into one with additionalLocations', async () => {
it('merges paraphrased findings and preserves their locations', async () => {
const findings = [
makeFinding({
id: 'f1',
severity: 'high',
title: 'Missing null check',
title: 'Outputs bypass the collection gate',
description: 'Both wrappers record answers when output collection is disabled.',
verification: 'Neither output write checks gen_ai.outputs.',
location: { path: 'src/a.ts', startLine: 3 },
}),
makeFinding({
id: 'f2',
severity: 'medium',
title: 'Missing null check',
title: 'Output opt-out is ignored',
description: 'Gate answer serialization in the sync and async wrappers.',
verification: 'The test records outputs without opting in.',
location: { path: 'src/b.ts', startLine: 2 },
}),
];
Expand Down Expand Up @@ -250,62 +254,6 @@ describe('mergeCrossLocationFindings', () => {
}));
});

it('preserves independent authorization bugs even when synthesis groups them together', async () => {
// Sanitized from the Sentry corpus: one broad authorization category, three fixes.
const findings = [
makeFinding({
id: 'thresholds',
title: 'Empty project filter exposes release thresholds',
description: 'An empty project selection falls back to thresholds from every tenant.',
verification: 'The empty branch omits the project predicate.',
location: { path: 'src/thresholds.py', startLine: 40 },
}),
makeFinding({
id: 'plugins',
title: 'Plugin endpoint skips project access',
description: 'A caller-supplied project ID is loaded without checking membership.',
verification: 'The plugin handler never checks access to the loaded project.',
location: { path: 'src/plugins.py', startLine: 25 },
}),
makeFinding({
id: 'detectors',
title: 'Detector endpoint skips project access',
description: 'Detector lookup accepts project IDs outside the caller organization.',
verification: 'The detector query does not intersect the accessible projects.',
location: { path: 'src/detectors.py', startLine: 30 },
}),
];
mockCallHaiku.mockResolvedValue({
success: true, data: [[1, 2, 3]],
usage: { inputTokens: 100, outputTokens: 10, costUSD: 0.001 },
});
const onFindingProcessing = vi.fn();

const result = await mergeCrossLocationFindings(findings, { apiKey: 'test-key', repoPath: tempDir, onFindingProcessing });

expect(result.findings).toEqual(findings);
expect(result.mergedCount).toBe(0);
expect(onFindingProcessing).not.toHaveBeenCalled();
});

it('keeps distinct evidence while merging identical claims within an overbroad group', async () => {
const first = makeFinding({ id: 'first', verification: 'The create handler omits the tenant guard.', location: { path: 'src/a.ts', startLine: 1 } });
const duplicate = { ...first, id: 'duplicate', location: { path: 'src/b.ts', startLine: 2 } };
const distinct = { ...first, id: 'distinct', verification: 'The delete handler checks the wrong tenant.', location: { path: 'src/a.ts', startLine: 5 } };
mockCallHaiku.mockResolvedValue({
success: true, data: [[1, 2, 3]],
usage: { inputTokens: 100, outputTokens: 10, costUSD: 0.001 },
});

const result = await mergeCrossLocationFindings([first, duplicate, distinct], { apiKey: 'test-key', repoPath: tempDir });

expect(result.findings).toEqual([
{ ...first, additionalLocations: [duplicate.location] },
distinct,
]);
expect(result.mergedCount).toBe(1);
});

it('still attributes the absorbed finding to its winner when both share the exact same location', async () => {
// Regression: mergeGroupLocations seeds `seen` with the winner's own
// location, so a loser at that same location is filtered out of
Expand Down
30 changes: 7 additions & 23 deletions packages/warden/src/sdk/extract.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,27 +405,12 @@ interface ApplyGroupsResult {
absorbedToWinner: Map<Finding, Finding>;
}

function splitMergeGroupByClaim(findings: Finding[], indices: number[]): number[][] {
const claims = new Map<string, number[]>();
for (const index of indices) {
const finding = findings[index - 1];
if (!finding || !finding.title.trim() || !finding.description.trim()) continue;
// Location-only merging cannot preserve a different title, claim, or
// evidence trace. Keep those reports independent even if synthesis groups them.
const key = JSON.stringify([finding.title.trim(), finding.description.trim(), finding.verification?.trim() ?? '']);
const group = claims.get(key) ?? [];
group.push(index);
claims.set(key, group);
}
return [...claims.values()];
}

/**
* Apply LLM-returned merge groups to a list of findings.
*
* For each group, the highest-priority finding becomes the winner, and all
* other findings' locations are folded into its additionalLocations only when
* their titles, descriptions, and evidence agree. Distinct claims survive.
* other findings' locations are folded into its additionalLocations. The model
* decides semantic equivalence; paraphrased titles or evidence do not veto a group.
* Handles overlapping groups by substituting prior replacements and tracking
* absorbed findings by their original identity.
*
Expand All @@ -440,7 +425,7 @@ export function applyMergeGroups(
const replacements = new Map<Finding, Finding>();
const absorbedToWinner = new Map<Finding, Finding>();

for (const group of groups.flatMap((indices) => splitMergeGroupByClaim(indexedFindings, indices))) {
for (const group of groups) {
const uniqueIndices = [...new Set(group)];
if (uniqueIndices.length < 2) continue;

Expand Down Expand Up @@ -538,10 +523,8 @@ function readSnippet(repoPath: string, filePath: string, startLine: number, cont
* Merge findings that describe the same issue across different code locations.
*
* Uses the configured auxiliary runtime to identify groups of findings about
* the same root cause at different locations. Only findings with identical
* trimmed titles, descriptions, and verification text are merged: the
* the same root cause at different locations, regardless of wording. The
* highest-priority finding keeps the other locations in `additionalLocations`.
* Differing claims or evidence remain independent.
*
* Skips entirely (no LLM call) when:
* - Fewer than 2 findings have locations
Expand Down Expand Up @@ -574,8 +557,9 @@ export async function mergeCrossLocationFindings(

const prompt = joinPromptSections([
`<task>
Identify which of these code review findings describe the SAME underlying issue appearing at different locations. Group them by shared root cause.
Sharing a category such as missing authorization is not enough: independent endpoints requiring separate fixes are separate findings. Keep uncertain matches separate.
Identify which of these code review findings describe the SAME underlying bug. Group by semantic meaning, not exact wording of titles, descriptions, or evidence.
Reports of the same defect may point to different lines, files, callers, or tests. For example, repeated reports that both sync and async wrappers omit the same output-collection guard are one finding, even when anchored in different tests.
Sharing a category such as missing authorization is not enough: independent endpoints requiring separate fixes are separate findings. Keep uncertain matches separate, and keep a broader finding separate if it contains an additional independent bug.
</task>`,
`<findings>
${findingDescriptions}
Expand Down
Loading