Skip to content

Commit d7cd585

Browse files
committed
fix: address Gemini review feedback
- Add null check in extractPatternIds to prevent Object.values(null) - Use Set for foundPatterns to optimize O(n) lookups to O(1) - Export toPascalCase from benchmark.js and remove duplicate in tests
1 parent 950dca6 commit d7cd585

2 files changed

Lines changed: 10 additions & 14 deletions

File tree

__tests__/enhance-pattern-benchmarks.test.js

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ const {
44
runPatternBenchmarks,
55
runFixBenchmarks,
66
generateReport,
7-
assertThresholds
7+
assertThresholds,
8+
toPascalCase
89
} = require('../lib/enhance/benchmark');
910
const { analyzeAgent } = require('../lib/enhance/agent-analyzer');
1011
const { analyzePrompt } = require('../lib/enhance/prompt-analyzer');
@@ -208,10 +209,3 @@ describe('Pattern Validation Benchmarks', () => {
208209
});
209210
});
210211
});
211-
212-
function toPascalCase(str) {
213-
return str
214-
.split('_')
215-
.map(part => part.charAt(0).toUpperCase() + part.slice(1))
216-
.join('');
217-
}

lib/enhance/benchmark.js

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,15 @@ function runPatternBenchmarks(manifestPath, analyzers) {
4545
}
4646

4747
// Extract pattern IDs from findings
48-
const foundPatterns = extractPatternIds(findings);
48+
const foundPatternsArray = extractPatternIds(findings);
49+
const foundPatterns = new Set(foundPatternsArray);
4950
const expectedPatterns = new Set(expectations.expectedPatterns || []);
5051
const mustNotTrigger = new Set(expectations.mustNotTrigger || []);
5152

5253
// Calculate metrics for this fixture
5354
const fixtureResult = {
5455
expected: Array.from(expectedPatterns),
55-
found: foundPatterns,
56+
found: foundPatternsArray,
5657
truePositives: [],
5758
falsePositives: [],
5859
falseNegatives: [],
@@ -61,7 +62,7 @@ function runPatternBenchmarks(manifestPath, analyzers) {
6162

6263
// True positives: expected AND found
6364
for (const pattern of expectedPatterns) {
64-
if (foundPatterns.includes(pattern)) {
65+
if (foundPatterns.has(pattern)) {
6566
fixtureResult.truePositives.push(pattern);
6667
results.summary.truePositives++;
6768
updatePatternStats(results.byPattern, pattern, 'tp');
@@ -83,7 +84,7 @@ function runPatternBenchmarks(manifestPath, analyzers) {
8384

8485
// Must-not-trigger violations
8586
for (const pattern of mustNotTrigger) {
86-
if (foundPatterns.includes(pattern)) {
87+
if (foundPatterns.has(pattern)) {
8788
fixtureResult.mustNotTriggerViolations.push(pattern);
8889
}
8990
}
@@ -313,7 +314,7 @@ function extractPatternIds(findings) {
313314
for (const f of findings) {
314315
if (f.patternId) ids.push(f.patternId);
315316
}
316-
} else if (typeof findings === 'object') {
317+
} else if (findings && typeof findings === 'object') {
317318
// Handle analyzer result objects with issue arrays
318319
for (const value of Object.values(findings)) {
319320
if (Array.isArray(value)) {
@@ -378,5 +379,6 @@ module.exports = {
378379
runFixBenchmarks,
379380
generateReport,
380381
assertThresholds,
381-
extractPatternIds
382+
extractPatternIds,
383+
toPascalCase
382384
};

0 commit comments

Comments
 (0)