diff --git a/package-lock.json b/package-lock.json index c7d6c33..d74354f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "vectorlint", - "version": "2.1.1", + "version": "2.3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "vectorlint", - "version": "2.1.1", + "version": "2.3.0", "license": "Apache-2.0", "dependencies": { "@anthropic-ai/sdk": "^0.30.1", diff --git a/src/cli/issue-deduplication.ts b/src/cli/issue-deduplication.ts new file mode 100644 index 0000000..ccc6b90 --- /dev/null +++ b/src/cli/issue-deduplication.ts @@ -0,0 +1,76 @@ +import { RawIssue } from './types'; +import { Severity } from '../evaluators/types'; + +// Map severities to their relative rank for tie-breaking. +const SEVERITY_RANK: Record = { + [Severity.ERROR]: 3, + [Severity.WARNING]: 2, +}; + +/** + * Filter and deduplicate overlapping issues to reduce noise. + * Groups by exact (file, line, match) and picks the best issue heuristically. + * + * Deduplication Heuristic: + * 1. Prefer rules with a `suggestion` (to make the error actionable). + * 2. Tie breaker 1: Pick the one with the longest `summary` (to provide maximum context). + * 3. Tie breaker 2: Prefer higher severity (`ERROR` > `WARNING`). + * 4. Tie breaker 3: Default to the first one evaluated. + * + * Note: Issues with an empty `match` text are explicitly preserved and not deduplicated + * against each other, as their exact overlap cannot be verified. + */ +export function filterDuplicateIssues(issues: RawIssue[]): RawIssue[] { + const grouped = new Map(); + + for (const issue of issues) { + const matchText = issue.match || ''; + const key = `${issue.file}:${issue.line}:${matchText}`; + + const group = grouped.get(key) || []; + group.push(issue); + grouped.set(key, group); + } + + const filtered: RawIssue[] = []; + + for (const group of grouped.values()) { + const first = group[0]; + if (!first) continue; + + const matchText = first.match || ''; + if (matchText === '' && group.length > 1) { + filtered.push(...group); + continue; + } + + if (group.length === 1) { + filtered.push(first); + continue; + } + + const best = group.reduce((prev, curr) => { + // 1. Prefer suggestion + const prevHas = !!prev.suggestion; + const currHas = !!curr.suggestion; + if (currHas !== prevHas) return currHas ? curr : prev; + + // 2. Tie breaker 1: longest summary + const prevLen = prev.summary?.length || 0; + const currLen = curr.summary?.length || 0; + if (currLen !== prevLen) return currLen > prevLen ? curr : prev; + + // 3. Tie breaker 2: higher severity + const prevRank = SEVERITY_RANK[prev.severity] || 0; + const currRank = SEVERITY_RANK[curr.severity] || 0; + if (currRank !== prevRank) return currRank > prevRank ? curr : prev; + + // 4. Tie breaker 3: First evaluated (prev is always earlier in array) + return prev; + }); + + filtered.push(best); + } + + return filtered; +} diff --git a/src/cli/orchestrator.ts b/src/cli/orchestrator.ts index 69d4a0b..390018a 100644 --- a/src/cli/orchestrator.ts +++ b/src/cli/orchestrator.ts @@ -18,13 +18,15 @@ import type { ReportIssueParams, ProcessViolationsParams, ProcessCriterionParams, ProcessCriterionResult, ValidationParams, ProcessPromptResultParams, RunPromptEvaluationParams, RunPromptEvaluationResult, EvaluateFileParams, EvaluateFileResult, - RunPromptEvaluationResultSuccess + RunPromptEvaluationResultSuccess, + RawIssue } from './types'; import { calculateCost, TokenUsageStats } from '../providers/token-usage'; import { locateQuotedText } from "../output/location"; +import { filterDuplicateIssues } from './issue-deduplication'; /* @@ -150,6 +152,7 @@ function reportIssue(params: ReportIssueParams): void { */ function locateAndReportViolations(params: ProcessViolationsParams): { hadOperationalErrors: boolean; + issues: RawIssue[]; } { const { violations, @@ -227,6 +230,8 @@ function locateAndReportViolations(params: ProcessViolationsParams): { } } + const issues: RawIssue[] = []; + // Report only verified, unique violations for (const { v, @@ -235,7 +240,7 @@ function locateAndReportViolations(params: ProcessViolationsParams): { matchedText, rowSummary, } of verifiedViolations) { - reportIssue({ + issues.push({ file: relFile, line, column, @@ -251,7 +256,7 @@ function locateAndReportViolations(params: ProcessViolationsParams): { }); } - return { hadOperationalErrors }; + return { hadOperationalErrors, issues }; } /* @@ -276,6 +281,7 @@ function extractAndReportCriterion( } = params; let hadOperationalErrors = false; let hadSeverityErrors = false; + const issues: RawIssue[] = []; const nameKey = String(exp.name || exp.id || ""); const criterionId = exp.id @@ -307,7 +313,7 @@ function extractAndReportCriterion( expTargetSpec?.suggestion || metaTargetSpec?.suggestion || "Add the required target section."; - reportIssue({ + issues.push({ file: relFile, line: 1, column: 1, @@ -327,6 +333,7 @@ function extractAndReportCriterion( maxScore, hadOperationalErrors, hadSeverityErrors, + issues, scoreEntry: { id: ruleName, scoreText: "0.0/10", score: 0.0 }, scoreComponent: { criterion: nameKey, @@ -351,6 +358,7 @@ function extractAndReportCriterion( maxScore, hadOperationalErrors, hadSeverityErrors, + issues: [], scoreEntry: { id: ruleName, scoreText: "-", score: 0.0 }, scoreComponent: { criterion: nameKey, @@ -419,6 +427,7 @@ function extractAndReportCriterion( }); hadOperationalErrors = hadOperationalErrors || violationResult.hadOperationalErrors; + issues.push(...violationResult.issues); } else if (score <= 2) { // No violations but low score - report with summary severity = score <= 1 ? Severity.ERROR : Severity.WARNING; @@ -433,7 +442,7 @@ function extractAndReportCriterion( const words = sum.split(/\s+/).filter(Boolean); const limited = words.slice(0, 15).join(" "); const summaryText = limited || "No findings"; - reportIssue({ + issues.push({ file: relFile, line: 1, column: 1, @@ -464,6 +473,7 @@ function extractAndReportCriterion( normalizedScore: normalizedScore, normalizedMaxScore: 10, }, + issues, }; } @@ -584,6 +594,7 @@ function routePromptResult( // Report violations grouped by criterion let totalErrors = 0; let totalWarnings = 0; + const issues: RawIssue[] = []; for (const [criterionName, violations] of violationsByCriterion) { // Find criterion ID from meta @@ -608,6 +619,7 @@ function routePromptResult( verbose: !!verbose, }); hadOperationalErrors = hadOperationalErrors || violationResult.hadOperationalErrors; + issues.push(...violationResult.issues); if (severity === Severity.ERROR) { totalErrors += violations.length; @@ -620,7 +632,7 @@ function routePromptResult( // If no violations but we have a message (JSON output), report it if (violationCount === 0 && (outputFormat === OutputFormat.Json || outputFormat === OutputFormat.ValeJson) && result.message) { const ruleName = buildRuleName(promptFile.pack, promptId, undefined); - reportIssue({ + issues.push({ file: relFile, line: 1, column: 1, @@ -646,6 +658,7 @@ function routePromptResult( hadOperationalErrors, hadSeverityErrors: severity === Severity.ERROR, scoreEntries: [scoreEntry], + issues, }; } @@ -661,6 +674,7 @@ function routePromptResult( promptWarnings = 0; const criterionScores: EvaluationSummary[] = []; const scoreComponents: ScoreComponent[] = []; + const issues: RawIssue[] = []; // Iterate through each criterion for (const exp of meta.criteria || []) { @@ -688,6 +702,9 @@ function routePromptResult( if (criterionResult.scoreComponent) { scoreComponents.push(criterionResult.scoreComponent); } + if (criterionResult.issues) { + issues.push(...criterionResult.issues); + } } if (outputFormat === OutputFormat.Json && scoreComponents.length > 0) { @@ -706,6 +723,7 @@ function routePromptResult( hadOperationalErrors, hadSeverityErrors, scoreEntries: criterionScores, + issues, }; } @@ -781,6 +799,8 @@ async function evaluateFile( let totalInputTokens = 0; let totalOutputTokens = 0; + const allIssues: RawIssue[] = []; + const allScores = new Map(); const content = readFileSync(file, "utf-8"); @@ -897,8 +917,6 @@ async function evaluateFile( jsonFormatter, verbose, }); - totalErrors += promptResult.errors; - totalWarnings += promptResult.warnings; hadOperationalErrors = hadOperationalErrors || promptResult.hadOperationalErrors; hadSeverityErrors = hadSeverityErrors || promptResult.hadSeverityErrors; @@ -907,6 +925,23 @@ async function evaluateFile( const ruleName = (p.meta.id || p.filename).toString(); allScores.set(ruleName, promptResult.scoreEntries); } + if (promptResult.issues) { + allIssues.push(...promptResult.issues); + } + } + + // Deduplicate issues + const deduplicatedIssues = filterDuplicateIssues(allIssues); + + // Recompute counts from deduplicated issues so they match what is reported + for (const issue of deduplicatedIssues) { + if (issue.severity === Severity.ERROR) totalErrors += 1; + else totalWarnings += 1; + } + + // Group and format output appropriately + for (const issue of deduplicatedIssues) { + reportIssue(issue); } const tokenUsageStats: TokenUsageStats = { diff --git a/src/cli/types.ts b/src/cli/types.ts index d4653ee..01dd7f1 100644 --- a/src/cli/types.ts +++ b/src/cli/types.ts @@ -47,6 +47,7 @@ export interface ErrorTrackingResult { hadOperationalErrors: boolean; hadSeverityErrors: boolean; scoreEntries?: EvaluationSummary[]; + issues?: RawIssue[]; } export interface EvaluationContext { @@ -57,7 +58,7 @@ export interface EvaluationContext { verbose?: boolean; } -export interface ReportIssueParams { +export interface RawIssue { file: string; line: number; column: number; @@ -72,6 +73,11 @@ export interface ReportIssueParams { match?: string; } +/** + * @deprecated Use `RawIssue` instead. This type alias is retained for backward compatibility. + */ +export type ReportIssueParams = RawIssue; + export interface ProcessViolationsParams extends EvaluationContext { violations: Array<{ line?: number; diff --git a/tests/issue-deduplication.test.ts b/tests/issue-deduplication.test.ts new file mode 100644 index 0000000..5c8b47c --- /dev/null +++ b/tests/issue-deduplication.test.ts @@ -0,0 +1,85 @@ +import { describe, it, expect } from 'vitest'; +import { filterDuplicateIssues } from '../src/cli/issue-deduplication'; +import { RawIssue, OutputFormat } from '../src/cli/types'; +import { Severity } from '../src/evaluators/types'; +import { JsonFormatter } from '../src/output/json-formatter'; + +const BASE_ISSUE: RawIssue = { + file: 'test.md', + line: 10, + column: 5, + severity: Severity.WARNING, + summary: 'A test issue', + ruleName: 'Test.Rule', + outputFormat: OutputFormat.Json, + jsonFormatter: new JsonFormatter(), + match: 'test match text', +}; + +describe('filterDuplicateIssues', () => { + it('returns a single issue as-is', () => { + const issues = [BASE_ISSUE]; + expect(filterDuplicateIssues(issues)).toEqual(issues); + }); + + it('deduplicates based on exact file, line, and match', () => { + const i1 = { ...BASE_ISSUE, summary: 'First summary' }; + const i2 = { ...BASE_ISSUE, summary: 'Second summary', severity: Severity.ERROR }; + const i3 = { ...BASE_ISSUE, summary: 'Third summary len' }; + + // i3 wins because it has the longest summary length (17 > 14 > 13). + const filtered = filterDuplicateIssues([i1, i2, i3]); + expect(filtered).toHaveLength(1); + expect(filtered[0]).toBe(i3); + }); + + it('prefers rule with a suggestion', () => { + const i1 = { ...BASE_ISSUE, summary: 'Longest summary by far but no suggestion', severity: Severity.ERROR }; + const i2 = { ...BASE_ISSUE, summary: 'Short', suggestion: 'Fix this' }; + + // i2 wins because it provides a suggestion, superseding summary length and severity. + const filtered = filterDuplicateIssues([i1, i2]); + expect(filtered).toHaveLength(1); + expect(filtered[0]).toBe(i2); + }); + + it('uses longer summary as tie breaker 1 (if no suggestion)', () => { + const i1 = { ...BASE_ISSUE, summary: 'Short sum', severity: Severity.ERROR }; + const i2 = { ...BASE_ISSUE, summary: 'A much longer summary text', severity: Severity.ERROR }; + + // i2 wins because it has a longer summary. + const filtered = filterDuplicateIssues([i1, i2]); + expect(filtered).toHaveLength(1); + expect(filtered[0]).toBe(i2); + }); + + it('uses higher severity as tie breaker 2', () => { + const i1 = { ...BASE_ISSUE, summary: 'Same len', severity: Severity.WARNING }; + const i2 = { ...BASE_ISSUE, summary: 'Same len', severity: Severity.ERROR }; + + // i2 wins because ERROR takes precedence over WARNING. + const filtered = filterDuplicateIssues([i1, i2]); + expect(filtered).toHaveLength(1); + expect(filtered[0]).toBe(i2); + }); + + it('keeps multiple issues on same line if match property is empty', () => { + const i1 = { ...BASE_ISSUE, match: '', summary: 'One issue' }; + const i2 = { ...BASE_ISSUE, match: '', summary: 'Another issue' }; + + // Both are kept since duplicate overlap cannot be explicitly verified without match text. + const filtered = filterDuplicateIssues([i1, i2]); + expect(filtered).toHaveLength(2); + expect(filtered).toContain(i1); + expect(filtered).toContain(i2); + }); + + it('does not deduplicate issues with different lines or different match text', () => { + const i1 = { ...BASE_ISSUE, match: 'match A' }; + const i2 = { ...BASE_ISSUE, match: 'match B' }; + const i3 = { ...BASE_ISSUE, line: 11, match: 'match A' }; + + const filtered = filterDuplicateIssues([i1, i2, i3]); + expect(filtered).toHaveLength(3); + }); +});