Repository navigation
feat: ai-slop category, benchmark overhaul, and prompt rework - #32
Merged
Merged
Conversation
Introduces state.Category as the validated category type. UnmarshalJSON is the single validation point — downstream code can rely on every Category value being either a known slug or the zero value. The old scattered validators (filterValidCategories, normaliseCategory, and the out-of-taxonomy log in validateAOIs) are removed. The deep-review parse now runs in two stages: parseDeepReviewRaw produces a rawDeepReviewResult with Category as a raw string, then convertRawToTyped promotes via state.ParseCategory after the corrector has had its chance. Between the two stages, verifyAndCorrectCategory loops up to 2 round trips on the same chat thread, asking the model to fix off-list categories or withdraw findings. RetryTransient wraps the corrector call so 5xx blips don't silently lose findings. rawDeepFinding embeds state.DeepFinding (shadowing Category as string) so adding fields to DeepFinding doesn't require updates in three places. ParseDeepReviewResult now returns []DroppedFinding so callers can see how many findings were lost to off-list categories instead of relying on log scraping. AreaOfInterest.Category is typed too; AOIScanResult.UnmarshalJSON parses AOIs one at a time so a single off-list category drops that AOI alone instead of failing the whole file's batch. New ai-slop category file covers bug-narration, commit-message-prose in code comments, vacuous comments, defensive noise, single-use abstractions, performative naming, and ceremonial error wrapping.
… corrector signatures - rawDeepReviewResult embeds state.DeepReviewResult instead of duplicating fields; convertRawToTyped copies the struct directly. - Add applyTypoFixes: fix edit-distance-1 category slugs without an LLM call (levenshtein + nearestSlug, cached via sync.OnceValue). - Bundle corrector params into correctorContext struct, simplifying verifyAndCorrectCategory and verifyAndCorrectEvidence signatures. - buildCategoryCorrectorMessage skips re-listing allowed categories on attempt 2+ to save tokens. - splitFindingsByCategory keys on state.Category directly instead of stringifying with an _uncategorized fallback. - parseEvidenceCorrections uses extractLastJSONValue + StripMarkdownFences.
The injection subcategory listed only client-side XSS shapes (innerHTML, v-html, template engines). Server-rendered XSS where user input is interpolated directly into the response body — fmt.Fprintf, ResponseWriter.Write, framework equivalents — wasn't documented, so models had no shape-match for that pattern.
…g, deep review parity Archive layout -------------- - Archives now write to ~/.config/prr/benchmarks/<name>-benchmark-<datetime>.json so AOI and review runs can coexist. SaveBenchmarkResults/LoadBenchmarkResults take a name; LoadBenchmarkResults merges only that name's archives. - internal/config/benchmark.json renamed to aoi-benchmark.json. Embedded defaults are keyed by name; benchmarks without a registered default get an empty result. Scoring ------- - Replace first-match-wins greedy matcher with independent per-GT assertion in both the AOI and deep-review tests. The old logic silently under-reported recall when ground-truth entries overlapped — e.g. a too-wide "User input from URL" GT (lines 14-26) was swallowing matches intended for the narrower "XSS" GT (25-26). - Drop the over-broad handler.go "User input" GT entry that triggered the misattribution. Config independence ------------------- - Both benchmarks stop reading tuning from ~/.config/prr/models.json. Tuning is now fixed by env-overridable benchmark constants (PRR_BENCH_* for AOI, PRR_DEEP_* for deep review), with a baseline + variants pattern so results don't drift across machines. - Stop auto-creating ~/.config/prr/models.json on first read; user customization is now opt-in. Deep review parity ------------------ - Cross-product fan-out (temperatures x thinking budgets) matching AOI. - Skip thinking-budget variants for github-copilot and claude-code — both decide thinking internally, so the explicit knob is a no-op. - Deep review now persists dated archives (previously only logged to test output). - Schema extended with optional severity_correct, severity_total, tool_calls fields (omitempty so AOI archives stay clean). Pricing ------- - Use provider-qualified model IDs for EstimateCost. Bare-ID lookup preferred priced entries, so subscription-billed claude-code runs were being charged at API rates in the benchmark report. Model defaults -------------- - gemini-3.1-flash-lite thinking_budget.fast: 2048 -> 0. Thinking did not improve recall in 48 observations and added ~1s latency. - gemini-3.5-flash thinking_budget.fast: 1536 -> 1024. Avoids the thinking=0 latency quirk; same recall. - Refresh aoi-benchmark.json flash-lite entry with post-matcher-fix numbers (recall 86.7 -> 94.9%, must-find 87.1 -> 100% from 3 reps). - Update DefaultFastModel comment to match.
Each benchmark archive now records the HEAD short SHA, whether the working tree was dirty, and an optional free-form tag from the PRR_BENCH_TAG environment variable. Makes it trivial to map archives back to the exact prompt/code state that produced them.
Adds an explicit inter-procedural feasibility check to both recheck prompts: walk the existing trace field from suspect to boundary and dismiss with rationale infeasible-path only when a specific hop can be shown to contradict the bug's required inputs. Inspired by LLM4PFA (arXiv:2506.10322). Bench on claude-opus-4-7 over 4 reps shows FP median 1->0.5 and SevAcc median 50%->69% (+19pp) at flat must-find recall, with one rep tying the all-time peak SevAcc of 77%.
For the six security-shaped categories (input-validation, external-io, authorization, authentication, cryptography, web-security), the AOI scanner now emits short identifier-shaped lists describing the taint shape of each AOI: where the suspect value enters (sources), where it lands (sinks), and which guards defuse it (sanitizers). An empty sanitizers list is the signal of a real bug. The reviewer prompts (individual + grouped) require evidence to trace at least one source-to-sink pair end-to-end and confirm whether a sanitizer covers the path. Inspired by IRIS (arXiv:2405.17238). Bench on Opus N=2 preserves Change 1's FP and SevAcc gains (0.5 FP, ~69% SevAcc) and pushes AOI must-find on flash-lite from 95% to 100% at +10% AOI latency, substantially better than the verification-questions approach which cost +52%.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five related threads on this branch:
1. New
ai-slopcategory (internal/ai/prompts/categories/ai-slop.md)Covers AI-generated code smells the existing categories don't catch cleanly: bug-narration comments, commit-message prose in code, vacuous comments, defensive noise, single-use abstractions, performative naming, ceremonial error wrapping. Category count goes 22 → 23.
2. Type-safe
state.CategoryboundaryIntroduces
state.Categoryas the validated category type.UnmarshalJSONis the single validation point — downstream code can rely on everyCategoryvalue being either a known slug or the zero value. The old scattered validators (filterValidCategories,normaliseCategory, the out-of-taxonomy log invalidateAOIs) are removed.Deep-review parsing splits into two stages:
parseDeepReviewRaw→rawDeepReviewResultwithCategoryas raw stringconvertRawToTyped→ promotes viastate.ParseCategoryafter the corrector runsBetween the two stages,
verifyAndCorrectCategoryloops up to 2 round trips on the same chat thread, asking the model to fix off-list categories or withdraw findings. A newapplyTypoFixespre-pass repairs edit-distance-1 slugs (e.g.correctnes→correctness) with zero LLM calls (levenshtein + cachednearestSlug).RetryTransientwraps the corrector call so 5xx blips don't silently lose findings.rawDeepFindingembedsstate.DeepFinding(shadowingCategoryas string) so adding fields toDeepFindingdoesn't require updates in three places.ParseDeepReviewResultnow returns[]DroppedFindingso callers can see how many findings were lost to off-list categories instead of relying on log scraping.AreaOfInterest.Categoryis typed too;AOIScanResult.UnmarshalJSONparses AOIs one at a time so a single off-list category drops that AOI alone instead of failing the whole file's batch.The follow-up refactor commit (
50c48fa) tightens the corrector internals: bundles corrector params into acorrectorContextstruct, embedsDeepReviewResultinstead of duplicating fields, and skips re-listing allowed categories on attempt 2+ to save tokens.3. Reflected-XSS prompt shape (
33339b2)The
input-validationcategory listed only client-side XSS shapes (innerHTML,v-html, template engines). Server-rendered XSS where user input is interpolated directly into the response body —fmt.Fprintf,ResponseWriter.Write, framework equivalents — wasn't documented, so models had no shape-match for that pattern. One-line addition.4. Benchmark infrastructure overhaul (
b26f846,7feab45)~/.config/prr/benchmarks/<name>-benchmark-<datetime>.jsonso AOI and deep-review runs coexist.SaveBenchmarkResults/LoadBenchmarkResultsare name-keyed.handler.goGT was swallowing matches for narrower XSS / SQLi GTs.~/.config/prr/models.json. Tuning is now fixed by env-overridable benchmark constants (PRR_BENCH_*for AOI,PRR_DEEP_*for deep review). Stop auto-creatingmodels.jsonon first read.github-copilot,claude-code); persist dated archives (previously only logged to test output); schema extended withseverity_correct,severity_total,tool_calls.EstimateCost. Bare-ID lookup preferred priced entries, so subscription-billedclaude-coderuns were being charged at API rates.gemini-3.1-flash-litefast thinking 2048 → 0 (no recall benefit, +1s latency);gemini-3.5-flashfast thinking 1536 → 1024 (avoids thethinking=0latency quirk; same recall). Refreshedaoi-benchmark.jsonwith post-matcher-fix numbers.tagfromPRR_BENCH_TAG. Makes archives unambiguously traceable to the prompt/code state that produced them.5. Prompt rework (
9e24b02,a1aff88)Two research-driven prompt changes shipped after per-change bench gating. Two more were planned, evaluated, and dropped (one reverted as net-regressive, one skipped on architectural grounds). Full per-change tables in
plans/benchmark-results-prompt-rework-2026-05-22.md.9e24b02). Inspired by LLM4PFA (arXiv:2506.10322). The adversarial recheck pass now walks the existingtracefield from suspect → boundary and dismisses with rationaleinfeasible-pathonly when a specific hop can be shown to contradict the bug's required inputs. Touchesrecheck.md+recheck_dismiss.md.a1aff88). Inspired by IRIS (arXiv:2405.17238). For the six security-shaped categories (input-validation,external-io,authorization,authentication,cryptography,web-security), the AOI scanner emits short identifier-shaped lists describing the taint shape. An emptysanitizerslist is the signal of a real bug. Reviewer prompts (individual + grouped) require evidence to trace at least one source→sink end-to-end. Scoped to security categories to avoid the +52% AOI latency hit a wider scope produced.Prompt-rework deltas
Benched on
gemini-3.1-flash-lite(AOI, 4 reps baseline / 2 reps per change) andclaude-opus-4-7via Claude Code (deep review, 2–4 reps per change). Medians:Two deep-review reps hit 0 FP — a first on this fixture. One rep tied the all-time SevAcc peak at 77%.
Why the other two changes didn't land
ParallelReviewsknob is concurrency, not redundancy. The existing deep review → adversarial recheck pipeline is already the heterogeneous-judge pattern Multi-Review prescribes, and Change 1 strengthened it. Documented for follow-up.Context
Before this branch, "category is one of the allowed slugs" was checked in at least three places with subtly different behaviour (drop vs. coerce vs. log). The corrector loop made that worse — the LLM was being asked to fix categories that downstream code had already rewritten or dropped. Centralizing validation in
Category.UnmarshalJSONis what makes the corrector well-defined: there is exactly one moment where "unknown slug" can be observed, and the corrector owns it. The benchmark overhaul gave us a sharp enough signal to land the prompt-rework changes with per-change gates instead of vibes — the FP/SevAcc deltas above would have been invisible under the old greedy matcher.