From 205a278b7b1a6e4173a207a67363aafd7d722c91 Mon Sep 17 00:00:00 2001 From: "Juan C. Andreu" Date: Fri, 29 May 2026 14:22:17 +0000 Subject: [PATCH 1/2] feat(categories): split Shapes from Review criteria MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split each category file under internal/ai/prompts/categories/ into a `## Shapes or common patterns` section (pattern lists for the recall- biased AOI scan) and a `## Review criteria` section (verdict guidance for the precision-biased deep review), and teach the loader to address sections and subcategories independently. This gives the upcoming Claude-Red coverage pass a clean home to fill in verdict guidance without bloating the cheap AOI tagger, and lets each pass pull only the content it needs. Loader (internal/ai/categories.go): - Add AllCategoryShapes, GetCategoryShapes, GetShape, GetReviewCriteria with a prefix-matching section parser and subcategory bold-block extraction. Unmigrated files (no Shapes heading) fall back to whole- body-as-Shapes so patterns never silently vanish. - Remove AllCategories / GetCategory / GetCategories (no compat shims; not deployed). Callers: - AOI scan (scanner.go) injects Shapes only. - Deep review (prompts.go) composes Evaluation Criteria scoped per AOI (category, subcategory): a flagged-pattern echo plus verdict guidance. Subcategory-less AOIs fall back to whole-category Shapes. Cache: fold the composed Evaluation Criteria content into the individual and grouped review cache keys, so editing a category file — or the scoping itself — invalidates stale entries. The static prompt-template hash did not cover this runtime-composed section. Migration of the 23 category files is pure restructure — bullets carried in verbatim, Review criteria starts empty (filled later, separate plan). Validated (gate #1, -count=1 reps): AOI and deep review both at parity with no false-positive regression, across gemini-3.1-flash-lite and claude-opus-4-7. --- internal/ai/categories.go | 187 +++++++++++++++--- internal/ai/categories_parse_test.go | 187 ++++++++++++++++++ internal/ai/categories_test.go | 24 ++- internal/ai/prompts/categories/ai-slop.md | 6 +- internal/ai/prompts/categories/api-design.md | 6 +- .../ai/prompts/categories/authentication.md | 6 +- .../ai/prompts/categories/authorization.md | 6 +- internal/ai/prompts/categories/concurrency.md | 6 +- .../ai/prompts/categories/configuration.md | 6 +- internal/ai/prompts/categories/correctness.md | 6 +- .../ai/prompts/categories/cross-cutting.md | 6 +- .../ai/prompts/categories/cryptography.md | 6 +- .../ai/prompts/categories/data-integrity.md | 6 +- internal/ai/prompts/categories/design.md | 6 +- .../ai/prompts/categories/error-handling.md | 6 +- internal/ai/prompts/categories/external-io.md | 6 +- internal/ai/prompts/categories/financial.md | 6 +- .../ai/prompts/categories/input-validation.md | 6 +- .../ai/prompts/categories/malicious-code.md | 6 +- .../ai/prompts/categories/observability.md | 6 +- internal/ai/prompts/categories/performance.md | 6 +- internal/ai/prompts/categories/readability.md | 6 +- .../prompts/categories/resource-management.md | 6 +- .../ai/prompts/categories/test-coverage.md | 6 +- internal/ai/prompts/categories/testing.md | 6 +- .../ai/prompts/categories/web-security.md | 6 +- internal/review/cache.go | 17 +- internal/review/cache_test.go | 25 +++ internal/review/prompts.go | 90 ++++++--- internal/review/prompts_test.go | 60 ++++++ internal/security/scanner.go | 6 +- internal/security/scanner_test.go | 8 + 32 files changed, 649 insertions(+), 93 deletions(-) create mode 100644 internal/ai/categories_parse_test.go diff --git a/internal/ai/categories.go b/internal/ai/categories.go index 48847a3..e0d143d 100644 --- a/internal/ai/categories.go +++ b/internal/ai/categories.go @@ -58,46 +58,181 @@ func init() { state.SetCategoryValidator(CategoryExists) } -// GetCategory returns the content of a single category by slug. -// Returns empty string if the slug doesn't exist. -func GetCategory(slug string) string { - return categories[slug] +// AllCategorySlugs returns the slugs of all available categories in +// alphabetical order. +func AllCategorySlugs() []string { + result := make([]string, len(categoryOrder)) + copy(result, categoryOrder) + return result +} + +// CategoryExists returns true if the given slug corresponds to a +// loaded category. +func CategoryExists(slug string) bool { + _, ok := categories[slug] + return ok +} + +// ── Shapes / Review-criteria split ── +// +// Each category file is split into a `## Shapes or common patterns` +// section (pattern lists consumed by the recall-biased AOI scan) and a +// `## Review criteria` section (verdict guidance consumed by the +// precision-biased deep review). The funcs below address each section, +// and individual subcategories within them, independently. +// +// As a defensive fallback, a file with no `## Shapes` heading is treated +// as Shapes-only (whole post-header body), so a future category that +// forgets the heading still surfaces in the AOI scan rather than +// vanishing silently. + +const ( + shapesHeadingPrefix = "## Shapes" + reviewHeadingPrefix = "## Review criteria" +) + +// splitSections returns the Shapes body and Review-criteria body for a +// category's raw content. The category header + description (everything +// before the first `## ` section heading) is dropped — callers that want +// it use categoryHeader. Prefix-match on headings so trailing prose +// ("or common patterns") can change without breaking extraction. +// +// Unmigrated files (no Shapes heading) return the full post-header body +// as shapes and "" for review. +func splitSections(content string) (shapes, review string, migrated bool) { + lines := strings.Split(content, "\n") + var cur *strings.Builder + var sb, rb strings.Builder + for _, line := range lines { + trimmed := strings.TrimSpace(line) + switch { + case strings.HasPrefix(trimmed, shapesHeadingPrefix): + cur = &sb + migrated = true + continue + case strings.HasPrefix(trimmed, reviewHeadingPrefix): + cur = &rb + migrated = true + continue + } + if cur != nil { + cur.WriteString(line) + cur.WriteString("\n") + } + } + if !migrated { + // Whole body (minus the `### CATEGORY` header line) is Shapes. + return stripCategoryHeader(content), "", false + } + return strings.TrimSpace(sb.String()), strings.TrimSpace(rb.String()), true +} + +// stripCategoryHeader drops the leading `### CATEGORY` line so unmigrated +// content matches the post-header body that splitSections produces for +// migrated files. +func stripCategoryHeader(content string) string { + lines := strings.Split(content, "\n") + if len(lines) > 0 && strings.HasPrefix(strings.TrimSpace(lines[0]), "### ") { + return strings.TrimSpace(strings.Join(lines[1:], "\n")) + } + return strings.TrimSpace(content) +} + +// categoryHeader returns the `### CATEGORY` line + description (everything +// before the first `## ` section), so the Shapes prompt keeps the category +// title and one-paragraph framing it has today. +func categoryHeader(content string) string { + lines := strings.Split(content, "\n") + var hb strings.Builder + for _, line := range lines { + if strings.HasPrefix(strings.TrimSpace(line), "## ") { + break + } + hb.WriteString(line) + hb.WriteString("\n") + } + return strings.TrimSpace(hb.String()) +} + +// extractSubcat returns the `**slug**` bold block within a section body, +// from its `**slug**` line up to the next `**` block or end. Returns "" +// if not found. +func extractSubcat(section, subcat string) string { + if section == "" || subcat == "" { + return "" + } + lines := strings.Split(section, "\n") + marker := "**" + subcat + "**" + start := -1 + for i, line := range lines { + if strings.HasPrefix(strings.TrimSpace(line), marker) { + start = i + break + } + } + if start == -1 { + return "" + } + var bb strings.Builder + for i := start; i < len(lines); i++ { + if i > start && strings.HasPrefix(strings.TrimSpace(lines[i]), "**") { + break + } + bb.WriteString(lines[i]) + bb.WriteString("\n") + } + return strings.TrimSpace(bb.String()) } -// GetCategories returns the content of multiple categories concatenated, -// separated by double newlines. Unknown slugs are skipped with a log -// line so misspelled --focus flags or out-of-taxonomy AOI categories -// don't quietly produce a truncated prompt. -func GetCategories(slugs []string) string { +// GetCategoryShapes returns the Shapes sections (header + Shapes body) for +// the listed categories, concatenated. Unknown slugs are skipped with a +// log line. +func GetCategoryShapes(slugs []string) string { var parts []string for _, slug := range slugs { content, ok := categories[slug] if !ok { - log.Printf("ai.GetCategories: unknown category slug %q — skipping (prompt will omit that section)", slug) + log.Printf("ai.GetCategoryShapes: unknown category slug %q — skipping", slug) + continue + } + shapes, _, migrated := splitSections(content) + if !migrated { + // Defensive: a file missing the Shapes heading passes through + // whole so its patterns still reach the AOI scan. + parts = append(parts, content) continue } - parts = append(parts, content) + header := categoryHeader(content) + parts = append(parts, strings.TrimSpace(header+"\n\n"+shapes)) } return strings.Join(parts, "\n\n") } -// AllCategories returns all category content concatenated in -// alphabetical order, separated by double newlines. -func AllCategories() string { - return GetCategories(categoryOrder) +// AllCategoryShapes returns the Shapes sections across all categories in +// alphabetical order. Used by the AOI scan. +func AllCategoryShapes() string { + return GetCategoryShapes(categoryOrder) } -// AllCategorySlugs returns the slugs of all available categories in -// alphabetical order. -func AllCategorySlugs() []string { - result := make([]string, len(categoryOrder)) - copy(result, categoryOrder) - return result +// GetShape returns one subcategory's block within a category's Shapes +// section. Returns "" if missing. +func GetShape(category, subcategory string) string { + content, ok := categories[category] + if !ok { + return "" + } + shapes, _, _ := splitSections(content) + return extractSubcat(shapes, subcategory) } -// CategoryExists returns true if the given slug corresponds to a -// loaded category. -func CategoryExists(slug string) bool { - _, ok := categories[slug] - return ok +// GetReviewCriteria returns one subcategory's block within a category's +// Review criteria section. Returns "" if missing (most subcategories +// during migration). +func GetReviewCriteria(category, subcategory string) string { + content, ok := categories[category] + if !ok { + return "" + } + _, review, _ := splitSections(content) + return extractSubcat(review, subcategory) } diff --git a/internal/ai/categories_parse_test.go b/internal/ai/categories_parse_test.go new file mode 100644 index 0000000..7834c77 --- /dev/null +++ b/internal/ai/categories_parse_test.go @@ -0,0 +1,187 @@ +package ai + +import ( + "strings" + "testing" +) + +// These tests drive the section/subcategory parser directly with fixture +// strings, independent of the embedded category files, so they pin the +// extraction contract regardless of how the real files evolve. + +const bothSectionsFixture = `### EXAMPLE (category: "example") + +One-paragraph description. + +## Shapes or common patterns + +**alpha** — first shape: +- shape bullet a1 +- shape bullet a2 + +**beta** — second shape: +- shape bullet b1 + +## Review criteria + +**alpha**: +- judge alpha +- defense for alpha + +**beta**: +- judge beta +` + +func TestSplitSectionsBoth(t *testing.T) { + shapes, review, migrated := splitSections(bothSectionsFixture) + if !migrated { + t.Fatal("expected migrated=true for a file with a Shapes heading") + } + if !contains(shapes, "shape bullet a1") || !contains(shapes, "shape bullet b1") { + t.Errorf("shapes missing bullets: %q", shapes) + } + if contains(shapes, "judge alpha") { + t.Error("shapes leaked Review criteria content") + } + if !contains(review, "judge alpha") || !contains(review, "judge beta") { + t.Errorf("review missing content: %q", review) + } + if contains(review, "shape bullet") { + t.Error("review leaked Shapes content") + } +} + +func TestSplitSectionsShapesOnly(t *testing.T) { + fixture := `### X (category: "x") + +desc + +## Shapes or common patterns + +**only** — sole shape: +- bullet +` + shapes, review, migrated := splitSections(fixture) + if !migrated { + t.Fatal("expected migrated=true") + } + if !contains(shapes, "bullet") { + t.Errorf("shapes missing: %q", shapes) + } + if review != "" { + t.Errorf("expected empty review, got %q", review) + } +} + +func TestSplitSectionsReviewOnly(t *testing.T) { + fixture := `### X (category: "x") + +desc + +## Review criteria + +**only**: +- judge it +` + shapes, review, migrated := splitSections(fixture) + if !migrated { + t.Fatal("a file with a Review heading but no Shapes heading is still migrated") + } + if shapes != "" { + t.Errorf("expected empty shapes, got %q", shapes) + } + if !contains(review, "judge it") { + t.Errorf("review missing: %q", review) + } +} + +func TestSplitSectionsUnmigratedFallback(t *testing.T) { + fixture := `### LEGACY (category: "legacy") + +desc + +#### Subcategories + +**foo** — legacy shape: +- legacy bullet +` + shapes, review, migrated := splitSections(fixture) + if migrated { + t.Fatal("a file with no Shapes heading must report migrated=false") + } + if !contains(shapes, "legacy bullet") || !contains(shapes, "#### Subcategories") { + t.Errorf("unmigrated fallback should pass the whole post-header body as shapes: %q", shapes) + } + if contains(shapes, "### LEGACY") { + t.Error("category header line should be stripped from fallback shapes") + } + if review != "" { + t.Errorf("expected empty review for unmigrated file, got %q", review) + } +} + +func TestHeadingPrefixTolerance(t *testing.T) { + // "## Shapes" (terse) and "## Review criteria or verdict guidance" + // (extended) must both be recognized by prefix match. + fixture := `### X (category: "x") + +desc + +## Shapes + +**a** — s: +- sb + +## Review criteria or verdict guidance + +**a**: +- rb +` + shapes, review, migrated := splitSections(fixture) + if !migrated || !contains(shapes, "sb") || !contains(review, "rb") { + t.Errorf("prefix-match failed: migrated=%v shapes=%q review=%q", migrated, shapes, review) + } +} + +func TestExtractSubcat(t *testing.T) { + shapes, review, _ := splitSections(bothSectionsFixture) + + alpha := extractSubcat(shapes, "alpha") + if !contains(alpha, "shape bullet a1") || !contains(alpha, "shape bullet a2") { + t.Errorf("alpha shape block wrong: %q", alpha) + } + if contains(alpha, "shape bullet b1") { + t.Error("alpha block bled into beta") + } + + if got := extractSubcat(shapes, "missing"); got != "" { + t.Errorf("missing subcat should return empty, got %q", got) + } + if got := extractSubcat("", "alpha"); got != "" { + t.Errorf("empty section should return empty, got %q", got) + } + + betaReview := extractSubcat(review, "beta") + if !contains(betaReview, "judge beta") { + t.Errorf("beta review block wrong: %q", betaReview) + } +} + +func TestGetShapeAndReviewCriteria(t *testing.T) { + // Against the real embedded files: input-validation has an injection + // subcategory in Shapes; Review criteria is empty post-migration. + shape := GetShape("input-validation", "injection") + if !contains(shape, "injection") { + t.Errorf("expected injection shape, got %q", shape) + } + if got := GetReviewCriteria("input-validation", "injection"); got != "" { + t.Errorf("expected empty review criteria during migration, got %q", got) + } + if got := GetShape("nonexistent", "x"); got != "" { + t.Errorf("unknown category should return empty, got %q", got) + } +} + +func contains(haystack, needle string) bool { + return strings.Contains(haystack, needle) +} diff --git a/internal/ai/categories_test.go b/internal/ai/categories_test.go index dfcc994..e47a54c 100644 --- a/internal/ai/categories_test.go +++ b/internal/ai/categories_test.go @@ -17,17 +17,17 @@ func TestCategoriesLoaded(t *testing.T) { if !CategoryExists(slug) { t.Errorf("category %q not found", slug) } - content := GetCategory(slug) + content := GetCategoryShapes([]string{slug}) if len(content) < 100 { - t.Errorf("category %q too short (%d bytes)", slug, len(content)) + t.Errorf("category %q shapes too short (%d bytes)", slug, len(content)) } } } -func TestGetCategories(t *testing.T) { - result := GetCategories([]string{"authentication", "correctness"}) +func TestGetCategoryShapes(t *testing.T) { + result := GetCategoryShapes([]string{"authentication", "correctness"}) if len(result) < 200 { - t.Errorf("combined categories too short: %d bytes", len(result)) + t.Errorf("combined category shapes too short: %d bytes", len(result)) } if !strings.Contains(result, "authentication") { t.Error("missing authentication content") @@ -37,8 +37,8 @@ func TestGetCategories(t *testing.T) { } } -func TestGetCategoriesSkipsUnknown(t *testing.T) { - result := GetCategories([]string{"authentication", "nonexistent", "correctness"}) +func TestGetCategoryShapesSkipsUnknown(t *testing.T) { + result := GetCategoryShapes([]string{"authentication", "nonexistent", "correctness"}) if strings.Contains(result, "nonexistent") { t.Error("should skip unknown slugs") } @@ -47,9 +47,13 @@ func TestGetCategoriesSkipsUnknown(t *testing.T) { } } -func TestAllCategories(t *testing.T) { - all := AllCategories() +func TestAllCategoryShapes(t *testing.T) { + all := AllCategoryShapes() if len(all) < 1000 { - t.Errorf("all categories too short: %d bytes", len(all)) + t.Errorf("all category shapes too short: %d bytes", len(all)) + } + // Shapes must not carry the deep-review-only Review criteria section. + if strings.Contains(all, "## Review criteria") { + t.Error("AllCategoryShapes should not include the Review criteria section") } } diff --git a/internal/ai/prompts/categories/ai-slop.md b/internal/ai/prompts/categories/ai-slop.md index 33a3834..b2e5be2 100644 --- a/internal/ai/prompts/categories/ai-slop.md +++ b/internal/ai/prompts/categories/ai-slop.md @@ -6,7 +6,7 @@ Severity for this category: report at **nit** by default, **low** only when the Distinct from `readability`: readability asks "can a reader understand what's here?" (wrong, stale, or confusing). ai-slop asks "why is this here at all?" — the code is correct and the comment is accurate; the problem is the noise itself. -#### Subcategories +## Shapes or common patterns **bug-narration** — Comments that retell the bug being fixed or describe the prior broken behavior: - "The bug we are fixing was using X unconditionally, which silently overwrote..." @@ -56,3 +56,7 @@ Distinct from `readability`: readability asks "can a reader understand what's he - Catch-and-rethrow chains that don't add a layer of context — just propagate the original error directly - Logging an error and then returning it (caller will log it too — duplicate logs in prod) - Wrapping a panic message in a function whose only purpose is to construct the panic message + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/api-design.md b/internal/ai/prompts/categories/api-design.md index b7833e3..6ba6655 100644 --- a/internal/ai/prompts/categories/api-design.md +++ b/internal/ai/prompts/categories/api-design.md @@ -2,7 +2,7 @@ Code that defines, implements, or consumes API contracts. -#### Subcategories +## Shapes or common patterns **contract-violations** — Breaking changes, missing validation, type mismatches: - Renamed or removed public functions, types, or fields without deprecation @@ -33,3 +33,7 @@ Code that defines, implements, or consumes API contracts. - Missing or incorrect Content-Type headers - Inconsistent null vs absent vs empty representations - Rate limit responses without Retry-After header + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/authentication.md b/internal/ai/prompts/categories/authentication.md index e92afa1..60f9a1e 100644 --- a/internal/ai/prompts/categories/authentication.md +++ b/internal/ai/prompts/categories/authentication.md @@ -2,7 +2,7 @@ Code that handles identity verification and session lifecycle. -#### Subcategories +## Shapes or common patterns **login-flow** — Login/logout handlers, multi-factor authentication: - Login handlers that don't rate-limit or lock after failed attempts @@ -34,3 +34,7 @@ Code that handles identity verification and session lifecycle. - Password reset flows that leak valid email addresses - Plaintext passwords logged or stored - Missing password complexity enforcement at the right boundary + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/authorization.md b/internal/ai/prompts/categories/authorization.md index 6841631..a7fb73f 100644 --- a/internal/ai/prompts/categories/authorization.md +++ b/internal/ai/prompts/categories/authorization.md @@ -2,7 +2,7 @@ Code that makes access control decisions after identity is established. -#### Subcategories +## Shapes or common patterns **access-control** — RBAC, permissions, ACLs, policy enforcement: - Missing permission checks on endpoints or operations @@ -28,3 +28,7 @@ Code that makes access control decisions after identity is established. - Service-to-service calls that implicitly trust the caller without verification - Impersonation features without proper audit logging - Vertical escalation (user → admin) or horizontal (user A → user B) + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/concurrency.md b/internal/ai/prompts/categories/concurrency.md index c966f01..eb365e2 100644 --- a/internal/ai/prompts/categories/concurrency.md +++ b/internal/ai/prompts/categories/concurrency.md @@ -2,7 +2,7 @@ Code that uses goroutines, threads, async operations, or shared mutable state. -#### Subcategories +## Shapes or common patterns **race-conditions** — Shared state without synchronization: - Read-modify-write on shared variables without locks or atomics @@ -38,3 +38,7 @@ Code that uses goroutines, threads, async operations, or shared mutable state. - Returning pointers to shared data without copying - Closures that capture loop variables in goroutines - WaitGroup Add/Done count mismatch + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/configuration.md b/internal/ai/prompts/categories/configuration.md index 1e9c936..0444537 100644 --- a/internal/ai/prompts/categories/configuration.md +++ b/internal/ai/prompts/categories/configuration.md @@ -2,7 +2,7 @@ Code that handles secrets, environment configuration, defaults, and feature flags. -#### Subcategories +## Shapes or common patterns **secrets-exposure** — Hardcoded credentials, logged secrets, leaked PII: - Hardcoded API keys, passwords, tokens, or connection strings in source @@ -41,3 +41,7 @@ Code that handles secrets, environment configuration, defaults, and feature flag - Rate limiting removed or weakened - Security middleware ordering changes (auth check moved after handler) - Feature flags that gate security controls (disabling auth via config) + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/correctness.md b/internal/ai/prompts/categories/correctness.md index 155ac36..97e3ff3 100644 --- a/internal/ai/prompts/categories/correctness.md +++ b/internal/ai/prompts/categories/correctness.md @@ -2,7 +2,7 @@ Code where the logic may compile and run without errors but produce wrong results, violate intent, or fail on valid inputs. The hardest class of bug — the code looks fine but is subtly wrong. -#### Subcategories +## Shapes or common patterns **semantic-errors** — Code that runs but produces wrong results: - Inverted boolean checks (condition is backwards, wrong branch taken) @@ -53,3 +53,7 @@ Code where the logic may compile and run without errors but produce wrong result - Range boundaries: "first N items" returning N+1 or N-1 - Array allocation one element too small or too large - Modular arithmetic wrapping at wrong boundary + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/cross-cutting.md b/internal/ai/prompts/categories/cross-cutting.md index 92928ce..8f6a1c3 100644 --- a/internal/ai/prompts/categories/cross-cutting.md +++ b/internal/ai/prompts/categories/cross-cutting.md @@ -2,7 +2,7 @@ Issues that span multiple files or require understanding the change as a whole — incomplete refactors, inconsistent patterns, missing cascading updates. -#### Subcategories +## Shapes or common patterns **incomplete-refactors** — Changes that are only partially applied: - Function/type renamed in definition but not in all callers @@ -26,3 +26,7 @@ Issues that span multiple files or require understanding the change as a whole - Environment variable additions without updating deployment configs or docs - New dependencies without updating build scripts, Docker images, or CI config - Permission/role changes without updating authorization checks + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/cryptography.md b/internal/ai/prompts/categories/cryptography.md index 989b635..2a19b40 100644 --- a/internal/ai/prompts/categories/cryptography.md +++ b/internal/ai/prompts/categories/cryptography.md @@ -2,7 +2,7 @@ Code that performs cryptographic operations or handles sensitive data. -#### Subcategories +## Shapes or common patterns **encryption** — Algorithm choice, mode, padding, key size: - Use of deprecated algorithms (DES, 3DES, RC4, Blowfish) @@ -35,3 +35,7 @@ Code that performs cryptographic operations or handles sensitive data. - Insufficient random bytes for tokens, IDs, or nonces - PRNG state exposed or recoverable from output - UUID v1/v4 misuse (v1 leaks MAC address and time; v4 may not be cryptographically random depending on implementation) + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/data-integrity.md b/internal/ai/prompts/categories/data-integrity.md index 0eca841..6606884 100644 --- a/internal/ai/prompts/categories/data-integrity.md +++ b/internal/ai/prompts/categories/data-integrity.md @@ -2,7 +2,7 @@ Code that manages state, enforces business rules, or maintains consistency. -#### Subcategories +## Shapes or common patterns **state-machines** — Invalid transitions, missing states, lifecycle management: - State transitions that skip required intermediate states @@ -60,3 +60,7 @@ or type encodes a meaning the value doesn't carry." Look for it wherever a typed value crosses a function boundary — the most common bugs hide one hop downstream from where the unit was declared. + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/design.md b/internal/ai/prompts/categories/design.md index 0830132..ed28c4a 100644 --- a/internal/ai/prompts/categories/design.md +++ b/internal/ai/prompts/categories/design.md @@ -2,7 +2,7 @@ Structural and organizational qualities of the code — how components relate, how responsibilities are divided, how the code fits into the existing codebase. -#### Subcategories +## Shapes or common patterns **abstraction-level** — Over-engineering vs under-abstraction: - Unnecessary interfaces with only one implementation (premature abstraction) @@ -40,3 +40,7 @@ Structural and organizational qualities of the code — how components relate, h - Inconsistent naming relative to adjacent functions in the same package - Missing or misleading documentation on public APIs - API that is surprising — behavior differs from what the name/signature suggests + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/error-handling.md b/internal/ai/prompts/categories/error-handling.md index 0d2e73d..2b32f10 100644 --- a/internal/ai/prompts/categories/error-handling.md +++ b/internal/ai/prompts/categories/error-handling.md @@ -2,7 +2,7 @@ Code that handles, propagates, or recovers from errors. -#### Subcategories +## Shapes or common patterns **swallowed-errors** — Ignored returns, empty catch blocks: - Error return values assigned to `_` or explicitly ignored when the error matters (write paths, business operations, anything with side effects) @@ -35,3 +35,7 @@ Code that handles, propagates, or recovers from errors. - Type assertions without comma-ok pattern (`x.(T)` instead of `x, ok := x.(T)`) - Stack overflow from unbounded recursion - HTTP handlers that can panic (unrecovered = connection reset) + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/external-io.md b/internal/ai/prompts/categories/external-io.md index 83f2e54..73cd280 100644 --- a/internal/ai/prompts/categories/external-io.md +++ b/internal/ai/prompts/categories/external-io.md @@ -2,7 +2,7 @@ Code that communicates with external systems — APIs, databases, file system, network. -#### Subcategories +## Shapes or common patterns **api-calls** — HTTP clients, retries, timeouts, idempotency: - HTTP requests without timeout configured (hangs indefinitely) @@ -40,3 +40,7 @@ Code that communicates with external systems — APIs, databases, file system, n - Socket connections without timeout - Proxy configuration from user input - Webhook URLs accepted without validation + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/financial.md b/internal/ai/prompts/categories/financial.md index 462af74..e59c637 100644 --- a/internal/ai/prompts/categories/financial.md +++ b/internal/ai/prompts/categories/financial.md @@ -2,7 +2,7 @@ Code that handles money, pricing, billing, or financial calculations. -#### Subcategories +## Shapes or common patterns **money-arithmetic** — Floating point, rounding, precision: - Monetary amounts stored or computed as floating point (float32/float64/double) @@ -30,3 +30,7 @@ Code that handles money, pricing, billing, or financial calculations. - Missing reconciliation between local records and processor state - Race conditions on payment status updates (double processing) - Currency mismatch between what user sees and what is charged + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/input-validation.md b/internal/ai/prompts/categories/input-validation.md index 0329515..3d707ae 100644 --- a/internal/ai/prompts/categories/input-validation.md +++ b/internal/ai/prompts/categories/input-validation.md @@ -2,7 +2,7 @@ Code that receives, parses, or processes data from external sources. -#### Subcategories +## Shapes or common patterns **injection** — SQL, command, XSS, LDAP, header injection: - String concatenation or interpolation in SQL queries @@ -50,3 +50,7 @@ Code that receives, parses, or processes data from external sources. - Array/slice index without bounds checking - Negative numbers where only positive expected - Unicode normalization issues (different byte representations of same character) + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/malicious-code.md b/internal/ai/prompts/categories/malicious-code.md index 8a6c9ae..4be69f0 100644 --- a/internal/ai/prompts/categories/malicious-code.md +++ b/internal/ai/prompts/categories/malicious-code.md @@ -4,7 +4,7 @@ Code that's hostile by design, not by accident. The other categories ask "does t Treat this category as adversarial review. Assume the contributor may not be acting in good faith. Read every suspicious construct twice. When evidence is ambiguous, surface the finding at lower severity rather than dismissing — a false positive costs a minute; a missed backdoor costs the project. -#### Subcategories +## Shapes or common patterns **install-time-execution** — Code that runs during `go install` / `npm install` / `make` / `pip install` / similar, executed before the user has read the source: - `go:generate` directives that download and run arbitrary remote code @@ -83,3 +83,7 @@ Treat this category as adversarial review. Assume the contributor may not be act - **low** — Pattern-match only, no clear path to harm. Worth noting so future review can spot a trend. When in doubt, prefer to **emit the finding and let the human decide** rather than dismiss. False positives here are cheap; false negatives are catastrophic. + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/observability.md b/internal/ai/prompts/categories/observability.md index 58a069c..2857c4c 100644 --- a/internal/ai/prompts/categories/observability.md +++ b/internal/ai/prompts/categories/observability.md @@ -2,7 +2,7 @@ Code that emits logs, metrics, traces, or alerts — whether operators can understand what happened in production. -#### Subcategories +## Shapes or common patterns **logging-quality** — Log level, structure, content: - Log level mismatched to severity (errors logged at INFO, debug noise at WARN) @@ -38,3 +38,7 @@ Code that emits logs, metrics, traces, or alerts — whether operators can under - Stack traces or large payloads logged on every error in a hot path - High-cardinality labels (user IDs, full URLs with params) on metrics - Audit logs that accumulate without rotation or archival (cross-ref `resource-management/unbounded-growth`) + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/performance.md b/internal/ai/prompts/categories/performance.md index 20f3e5f..4028d8d 100644 --- a/internal/ai/prompts/categories/performance.md +++ b/internal/ai/prompts/categories/performance.md @@ -2,7 +2,7 @@ Code that may work correctly at small scale but degrade under production load — algorithmic inefficiency, memory waste, I/O bottlenecks. -#### Subcategories +## Shapes or common patterns **algorithmic-complexity** — Inefficient algorithms on growing data: - O(n²) or worse in loops over collections that could grow (nested iterations over same data) @@ -41,3 +41,7 @@ Code that may work correctly at small scale but degrade under production load - Cache invalidation bugs (stale data served after mutation) - Cache key that doesn't include all relevant inputs (different users get same cached result) - Caching non-deterministic or time-sensitive data without awareness + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/readability.md b/internal/ai/prompts/categories/readability.md index f011d70..aaf0f49 100644 --- a/internal/ai/prompts/categories/readability.md +++ b/internal/ai/prompts/categories/readability.md @@ -2,7 +2,7 @@ Code clarity, naming, complexity — whether a new team member can understand this code in one reading. -#### Subcategories +## Shapes or common patterns **naming** — Names that convey (or obscure) intent: - Single-letter variables for non-obvious things in non-trivial scope. Note: short conventional names are idiomatic in Go and should NOT be flagged — receivers (`r *Repo`), `t *testing.T`, `r io.Reader`, `w io.Writer`, `b *bytes.Buffer`, `i`/`j` in loops, `err` for errors, `ok` for boolean returns. @@ -37,3 +37,7 @@ Code clarity, naming, complexity — whether a new team member can understand th - String literals used as identifiers or keys in multiple places - Threshold values without explanation of why that specific value - Repeated magic values that should be a single named constant + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/resource-management.md b/internal/ai/prompts/categories/resource-management.md index 82739a5..fc3e72c 100644 --- a/internal/ai/prompts/categories/resource-management.md +++ b/internal/ai/prompts/categories/resource-management.md @@ -2,7 +2,7 @@ Code that acquires, holds, or releases system resources. -#### Subcategories +## Shapes or common patterns **memory-leaks** — Unbounded caches, retained references, accumulation: - Caches without size limits or eviction policy (grows forever) @@ -35,3 +35,7 @@ Code that acquires, holds, or releases system resources. - Request/response bodies read fully into memory without size limit - History/audit logs that accumulate without archival - Retry queues that grow during outages without backpressure + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/test-coverage.md b/internal/ai/prompts/categories/test-coverage.md index acf5c7d..af4ee6b 100644 --- a/internal/ai/prompts/categories/test-coverage.md +++ b/internal/ai/prompts/categories/test-coverage.md @@ -2,7 +2,7 @@ Whether production code has adequate test coverage — flagged when reviewing non-test files. -#### Subcategories +## Shapes or common patterns **coverage-gaps** — Missing test cases, untested paths: - Happy path tested but error/edge cases missing @@ -14,3 +14,7 @@ Whether production code has adequate test coverage — flagged when reviewing no - Conditional branches where only one branch is exercised - Bug fixes without a regression test that would catch the bug recurring - Test assertions weakened to make tests pass (e.g., `assertEqual` relaxed to `assertNotNil`) + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/testing.md b/internal/ai/prompts/categories/testing.md index 71d58b4..84cebfa 100644 --- a/internal/ai/prompts/categories/testing.md +++ b/internal/ai/prompts/categories/testing.md @@ -2,7 +2,7 @@ Code that tests application logic — unit tests, integration tests, test helpers. -#### Subcategories +## Shapes or common patterns **assertion-quality** — Meaningful assertions vs tautological checks: - Assertions that are always true regardless of implementation behavior (tautologies). Examples: asserting a variable against itself, checking that a constructed object is truthy, verifying a list has items without checking which items. The test passes whether the code works or is completely broken. @@ -40,3 +40,7 @@ Code that tests application logic — unit tests, integration tests, test helper - Multiple unrelated behaviors asserted in the same test — split so each test fails for one reason - Missed property-based or fuzz testing on parsers, validators, encoders, ID generators, and other input-shape-sensitive code where examples can't cover the space - Helpers that hide what's being tested behind generic names (`setup()`, `assertOk()`) — the assertion or the surprising setup belongs visible in the test body + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/ai/prompts/categories/web-security.md b/internal/ai/prompts/categories/web-security.md index a0e625f..4e9bbbf 100644 --- a/internal/ai/prompts/categories/web-security.md +++ b/internal/ai/prompts/categories/web-security.md @@ -2,7 +2,7 @@ Browser-facing concerns that don't fit cleanly into injection, authentication, or authorization — CSRF, security headers, cookie attributes, CORS, clickjacking, parameter binding. -#### Subcategories +## Shapes or common patterns **csrf-protection** — Cross-Site Request Forgery defenses: - State-changing endpoints (POST/PUT/DELETE/PATCH) without CSRF token validation @@ -50,3 +50,7 @@ Browser-facing concerns that don't fit cleanly into injection, authentication, o - User-controlled URL passed to `Location` header without scheme/host allowlist (cross-ref `input-validation/boundary-validation`) - OAuth `redirect_uri` validated by substring match instead of exact host comparison - `next`/`return_to`/`continue` query params followed without validation after login + +## Review criteria + +[empty during migration — filled later via the Claude-Red coverage pass] diff --git a/internal/review/cache.go b/internal/review/cache.go index 2e052da..fd84428 100644 --- a/internal/review/cache.go +++ b/internal/review/cache.go @@ -12,7 +12,7 @@ import ( ) // IndividualCacheKey computes the cache key for an individual review call. -// Key = hash(code_context + aoi_serialized + sorted_focus_categories + sha256(prompt) + priorsHash) +// Key = hash(code_context + aoi_serialized + sorted_focus_categories + sha256(prompt) + sha256(criteria) + priorsHash) // // codeContext is the diff (PR mode) or source slice (audit mode) that // the prompt builder will inline. Folding it into the key means any @@ -44,12 +44,20 @@ func IndividualCacheKey(codeContext string, aoi security.AreaOfInterest, focusCa promptHash := sha256.Sum256([]byte(ai.ReviewIndividualPrompt)) h.Write(promptHash[:]) h.Write([]byte{0}) + // Evaluation Criteria content (Shapes echo + Review criteria, scoped + // to this AOI's (category, subcategory)). Folded in so that editing a + // category .md file — or the Shapes/Review-criteria scoping itself — + // invalidates stale entries. The static prompt hash above does not + // cover this composed-at-runtime section. + criteriaHash := sha256.Sum256([]byte(scopedCriteria([]security.AreaOfInterest{aoi}))) + h.Write(criteriaHash[:]) + h.Write([]byte{0}) h.Write([]byte(priorsHash)) return hex.EncodeToString(h.Sum(nil))[:32] // 32 hex chars = 128 bits, plenty } // GroupedCacheKey computes the cache key for a grouped review call. -// Key = hash(all_aoi_serialized + code_context + sorted_focus_categories + sha256(prompt) + priorsHash) +// Key = hash(all_aoi_serialized + code_context + sorted_focus_categories + sha256(prompt) + sha256(criteria) + priorsHash) // // codeContext is the per-file diff blob (PR mode) or per-AOI source // slice blob (audit mode) — see codeContextDigest for the format. @@ -73,6 +81,11 @@ func GroupedCacheKey(aois []security.AreaOfInterest, codeContext string, focusCa promptHash := sha256.Sum256([]byte(ai.ReviewGroupedPrompt)) h.Write(promptHash[:]) h.Write([]byte{0}) + // Evaluation Criteria content for the group's distinct (category, + // subcategory) pairs — see IndividualCacheKey for the rationale. + criteriaHash := sha256.Sum256([]byte(scopedCriteria(aois))) + h.Write(criteriaHash[:]) + h.Write([]byte{0}) h.Write([]byte(priorsHash)) return hex.EncodeToString(h.Sum(nil))[:32] } diff --git a/internal/review/cache_test.go b/internal/review/cache_test.go index a6fb8d5..a083e0c 100644 --- a/internal/review/cache_test.go +++ b/internal/review/cache_test.go @@ -144,6 +144,31 @@ func TestGroupedCacheKey_CodeContextChangeInvalidates(t *testing.T) { } } +// TestCacheKey_Deterministic guards the criteria component (folded into +// the key via scopedCriteria, which uses an internal map) against +// accidental nondeterminism — a flaky key would silently disable the +// review cache. +func TestCacheKey_Deterministic(t *testing.T) { + aoi := security.AreaOfInterest{ + File: "billing/charge.go", Line: 45, + Category: "financial", Subcategory: "money-arithmetic", + } + for i := 0; i < 5; i++ { + if got := IndividualCacheKey("body", aoi, nil, ""); got != IndividualCacheKey("body", aoi, nil, "") { + t.Fatalf("individual key not deterministic on iter %d: %s", i, got) + } + } + aois := []security.AreaOfInterest{ + {File: "a.go", Line: 1, Category: "financial", Subcategory: "money-arithmetic"}, + {File: "b.go", Line: 2, Category: "web-security", Subcategory: "csrf-protection"}, + } + for i := 0; i < 5; i++ { + if got := GroupedCacheKey(aois, "body", nil, ""); got != GroupedCacheKey(aois, "body", nil, "") { + t.Fatalf("grouped key not deterministic on iter %d: %s", i, got) + } + } +} + func TestComputeCacheKey_DiffChangeInvalidates(t *testing.T) { aoi := security.AreaOfInterest{File: "a.go", Line: 1, Category: "correctness"} callA := ReviewCall{ diff --git a/internal/review/prompts.go b/internal/review/prompts.go index abd1a2f..6493608 100644 --- a/internal/review/prompts.go +++ b/internal/review/prompts.go @@ -81,11 +81,11 @@ func BuildIndividualPrompt(mode Mode, projectContext, customInstructions, bugPri sb.WriteString(section) } - // Relevant category criteria - cats := relevantCategories(aoi) - if len(cats) > 0 { + // Relevant category criteria — scoped to the AOI's (category, + // subcategory): the flagged-pattern echo plus the verdict guidance. + if crit := scopedCriteria([]security.AreaOfInterest{aoi}); crit != "" { sb.WriteString("\n\n## Evaluation Criteria\n\n") - sb.WriteString(ai.GetCategories(cats)) + sb.WriteString(crit) } // Custom instructions @@ -163,11 +163,11 @@ func BuildGroupedPrompt(mode Mode, projectContext, customInstructions, bugPriors sb.WriteString(renderPRDiffsSection(call)) } - // Relevant category criteria — collect from all AOIs in the group - cats := relevantCategoriesFromGroup(call.AOIs) - if len(cats) > 0 { + // Relevant category criteria — scoped to the distinct (category, + // subcategory) pairs across the AOIs in this group. + if crit := scopedCriteria(call.AOIs); crit != "" { sb.WriteString("\n\n## Evaluation Criteria\n\n") - sb.WriteString(ai.GetCategories(cats)) + sb.WriteString(crit) } // Custom instructions @@ -344,34 +344,64 @@ func formatAOI(aoi security.AreaOfInterest) string { return sb.String() } -// relevantCategories returns the category slugs to include for a single AOI. -// One AOI = one category — see the AOI scan prompt's "One AOI = one -// category" rule. Returns the AOI's category as a single-element slice -// when it's in the canonical taxonomy, or nil otherwise. +// scopedCriteria composes the Evaluation Criteria section: per distinct +// (category, subcategory) pair across the AOIs, emit the Shapes block +// (flagged-pattern echo) plus the Review criteria block (verdict +// guidance). AOIs with no subcategory fall back to the whole-category +// Shapes section, no Review criteria. Returns "" if nothing resolves. // -// validateAOIs logs at scan time, but cached AOIs whose category was -// renamed or removed in the taxonomy land here at prompt-build time -// without re-validation — the log below surfaces that case. -func relevantCategories(aoi security.AreaOfInterest) []string { - if !aoi.Category.IsZero() { - return []string{aoi.Category.String()} - } - return nil -} - -// relevantCategoriesFromGroup collects all relevant categories across a group of AOIs. -func relevantCategoriesFromGroup(aois []security.AreaOfInterest) []string { +// One AOI = one category — see the AOI scan prompt's "One AOI = one +// category" rule. Category-less AOIs are skipped. +func scopedCriteria(aois []security.AreaOfInterest) string { seen := make(map[string]bool) - var result []string + var parts []string for _, aoi := range aois { - for _, c := range relevantCategories(aoi) { - if !seen[c] { - seen[c] = true - result = append(result, c) + if aoi.Category.IsZero() { + continue + } + cat := aoi.Category.String() + key := cat + "/" + aoi.Subcategory + if seen[key] { + continue + } + seen[key] = true + + if aoi.Subcategory == "" { + // Fallback: whole-category Shapes, no Review criteria. + if shapes := ai.GetCategoryShapes([]string{cat}); shapes != "" { + parts = append(parts, shapes) + } + continue + } + + var block strings.Builder + block.WriteString(fmt.Sprintf("### %s / %s\n\n", cat, aoi.Subcategory)) + wrote := false + if shape := ai.GetShape(cat, aoi.Subcategory); shape != "" { + block.WriteString("**Flagged pattern:**\n\n") + block.WriteString(shape) + wrote = true + } + if crit := ai.GetReviewCriteria(cat, aoi.Subcategory); crit != "" { + if wrote { + block.WriteString("\n\n") } + block.WriteString("**How to judge:**\n\n") + block.WriteString(crit) + wrote = true + } + if !wrote { + // Neither shape nor criteria resolved (e.g. a subcategory + // label that doesn't match any Shapes block): fall back to + // the whole-category Shapes. + if shapes := ai.GetCategoryShapes([]string{cat}); shapes != "" { + parts = append(parts, shapes) + } + continue } + parts = append(parts, strings.TrimSpace(block.String())) } - return result + return strings.Join(parts, "\n\n") } // appendProjectContext writes the project context section, avoiding a diff --git a/internal/review/prompts_test.go b/internal/review/prompts_test.go index 2dd2c94..c86fe3a 100644 --- a/internal/review/prompts_test.go +++ b/internal/review/prompts_test.go @@ -121,6 +121,66 @@ func TestBuildIndividualPrompt_ContainsAllSections(t *testing.T) { } } +// TestBuildIndividualPrompt_ScopedCriteria pins the Shapes/Review-criteria +// split contract on the deep-review side: the Evaluation Criteria section is +// scoped to the AOI's own (category, subcategory) and must NOT drag in +// sibling subcategories of the same category. +func TestBuildIndividualPrompt_ScopedCriteria(t *testing.T) { + aoi := security.AreaOfInterest{ + File: "billing/charge.go", + Line: 45, + Category: "financial", + Subcategory: "money-arithmetic", + ID: "charge-go-1", + Concern: "float currency", + } + prompt := BuildIndividualPrompt(ModeAudit, "", "", "", nil, individualCall(aoi)) + + if !strings.Contains(prompt, "## Evaluation Criteria") { + t.Fatal("missing Evaluation Criteria section") + } + if !strings.Contains(prompt, "**Flagged pattern:**") { + t.Error("scoped criteria should echo the flagged pattern (Shapes block)") + } + // In-scope subcategory content present. + if !strings.Contains(prompt, "Floating point, rounding, precision") { + t.Error("scoped criteria missing the money-arithmetic shape") + } + // Sibling subcategories of `financial` must be excluded by the scoping. + for _, sibling := range []string{"stacking exploits", "Payment processor interaction"} { + if strings.Contains(prompt, sibling) { + t.Errorf("scoped criteria leaked sibling-subcategory content: %q", sibling) + } + } +} + +// TestBuildIndividualPrompt_SubcatlessFallback verifies that an AOI with a +// category but no subcategory falls back to the whole-category Shapes (so +// criteria are never silently dropped), without emitting a scoped header. +func TestBuildIndividualPrompt_SubcatlessFallback(t *testing.T) { + aoi := security.AreaOfInterest{ + File: "x.go", + Line: 1, + Category: "financial", // no Subcategory + ID: "x-go-1", + Concern: "money thing", + } + prompt := BuildIndividualPrompt(ModeAudit, "", "", "", nil, individualCall(aoi)) + + if !strings.Contains(prompt, "## Evaluation Criteria") { + t.Fatal("subcatless AOI should still get an Evaluation Criteria section") + } + // Whole-category fallback: all subcategories present, no scoped header. + for _, want := range []string{"money-arithmetic", "billing-logic", "payment-integration"} { + if !strings.Contains(prompt, want) { + t.Errorf("fallback should include whole-category Shapes (missing %q)", want) + } + } + if strings.Contains(prompt, "**Flagged pattern:**") { + t.Error("subcatless fallback should not emit a per-subcategory Flagged pattern header") + } +} + func TestBuildIndividualPrompt_PRMode(t *testing.T) { aoi := security.AreaOfInterest{ File: "main.go", diff --git a/internal/security/scanner.go b/internal/security/scanner.go index 479ce20..5de8cbf 100644 --- a/internal/security/scanner.go +++ b/internal/security/scanner.go @@ -65,15 +65,17 @@ func buildAOIScanPrompt(auditMode bool) string { // buildAOIScanPromptWithCategories composes the AOI scan prompt with specific // categories. If cats is nil or empty, all categories are included. func buildAOIScanPromptWithCategories(auditMode bool, cats []string) string { + // AOI scan is the recall-biased pre-filter — it only needs the + // Shapes (pattern lists), not the deep reviewer's verdict guidance. var categoryContent string var slugs []string if len(cats) > 0 { - categoryContent = ai.GetCategories(cats) + categoryContent = ai.GetCategoryShapes(cats) slugs = make([]string, len(cats)) copy(slugs, cats) sort.Strings(slugs) } else { - categoryContent = ai.AllCategories() + categoryContent = ai.AllCategoryShapes() slugs = ai.AllCategorySlugs() } slugList := strings.Join(slugs, ", ") diff --git a/internal/security/scanner_test.go b/internal/security/scanner_test.go index 30e40f8..b1dfb93 100644 --- a/internal/security/scanner_test.go +++ b/internal/security/scanner_test.go @@ -421,6 +421,14 @@ func TestBuildAOIScanPromptWithCategories(t *testing.T) { t.Error("prompt with nil cats should contain CRYPTOGRAPHY") } + // Shapes/Review-criteria split: the AOI scan is the recall-biased + // pre-filter and must carry only the Shapes patterns, never the + // deep-review-only Review criteria section. (The category bullets + // themselves still appear; only the section heading is dropped.) + if containsSubstring(promptAll, "## Review criteria") { + t.Error("AOI scan prompt should not include the Review criteria section") + } + // Audit mode rules if !containsSubstring(prompt, "full-project audit") { t.Error("audit mode prompt should contain audit rules") From 4bdcae4d4d5300ad8fe66185a8e70c134e060706 Mon Sep 17 00:00:00 2001 From: "Juan C. Andreu" Date: Fri, 29 May 2026 17:05:42 +0000 Subject: [PATCH 2/2] feat(models): replace Opus 4.7 with Opus 4.8 Opus 4.8 was released. Swap the two claude-opus-4-7 registry entries (anthropic + claude-code) to claude-opus-4-8, rename the models.json tuning block, and refresh the example references in claudecode.go and token-bench. Default strong model bumps to claude-code/claude-opus-4-8. claude-code is keyless, so a fresh install keeps working without an API key and default-relying upgraders don't break on load. Users who prefer the metered Anthropic API can pick anthropic/claude-opus-4-8 in the picker. Existing configs are left untouched: model refs are stored as free-form "provider/model-id" strings and ParseModelRef doesn't validate against the registry, so a config still pointing at claude-opus-4-7 keeps running (it just loses pricing-table cost reporting and drops out of the picker). No auto-migration. --- internal/ai/claudecode.go | 2 +- internal/ai/claudecode_test.go | 6 +++--- internal/config/config.go | 8 ++++++-- internal/config/config_test.go | 2 +- internal/config/known_models.go | 4 ++-- internal/config/models.json | 2 +- scripts/token-bench/run.sh | 4 ++-- 7 files changed, 16 insertions(+), 12 deletions(-) diff --git a/internal/ai/claudecode.go b/internal/ai/claudecode.go index d4b934e..a5e62c6 100644 --- a/internal/ai/claudecode.go +++ b/internal/ai/claudecode.go @@ -136,7 +136,7 @@ const claudeCodeAppendSystemPrompt = `## Review rules // or ANTHROPIC_API_KEY) — prr does not need an API key for this provider. type ClaudeCodeProvider struct { // Model is a Claude Code model name or alias ("opus", "sonnet", - // "haiku", "claude-opus-4-7", etc.). + // "haiku", "claude-opus-4-8", etc.). Model string // BinaryPath optionally overrides the resolved claude binary path diff --git a/internal/ai/claudecode_test.go b/internal/ai/claudecode_test.go index 9fb61e3..254c97b 100644 --- a/internal/ai/claudecode_test.go +++ b/internal/ai/claudecode_test.go @@ -580,12 +580,12 @@ func TestClaudeCodeProvider_EffortDefaults(t *testing.T) { {"sonnet default → low", "claude-sonnet-4-6", "", "low"}, {"sonnet alias → low", "sonnet", "", "low"}, {"sonnet uppercase still triggers default", "Claude-Sonnet-4-6", "", "low"}, - {"opus default → xhigh", "claude-opus-4-7", "", "xhigh"}, + {"opus default → xhigh", "claude-opus-4-8", "", "xhigh"}, {"opus alias → xhigh", "opus", "", "xhigh"}, - {"opus uppercase still triggers default", "Claude-Opus-4-7", "", "xhigh"}, + {"opus uppercase still triggers default", "Claude-Opus-4-8", "", "xhigh"}, {"haiku → no default effort", "claude-haiku-4-5", "", ""}, {"env override beats sonnet default", "claude-sonnet-4-6", "high", "high"}, - {"env override beats opus default", "claude-opus-4-7", "max", "max"}, + {"env override beats opus default", "claude-opus-4-8", "max", "max"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index 48cb241..d076c5c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -38,8 +38,12 @@ func ParseModelRef(s string) (ModelRef, error) { } // DefaultStrongModel is the default strong model reference. -// Opus 4.6 via Copilot: 100% recall, 0 FP, 27 tool calls, 202s on deep review benchmark. -const DefaultStrongModel = "github-copilot/claude-opus-4.6" +// Opus 4.8 via the claude-code provider — latest strong model for deep +// review, re-review, and synthesis. claude-code is keyless (auth handled +// by the Claude Code CLI), so a fresh install works out of the box +// without an API key. Users who prefer the metered Anthropic API can +// pick anthropic/claude-opus-4-8 via the model picker. +const DefaultStrongModel = "claude-code/claude-opus-4-8" // DefaultFastModel is the default fast model reference. // Flash Lite via Gemini: 100% must-find recall, 1-2 FP, ~4.5s, $0.007 per scan diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 910962e..a9a9c74 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -689,7 +689,7 @@ func TestProviderConfig_UseCLI_RoundTrip(t *testing.T) { path := filepath.Join(home, ".config", "prr", "config.json") cfg := &Config{ - StrongModel: "claude-code/claude-opus-4-7", + StrongModel: "claude-code/claude-opus-4-8", FastModel: "claude-code/claude-haiku-4-5", Providers: map[string]ProviderConfig{ "claude-code": {UseCLI: true}, diff --git a/internal/config/known_models.go b/internal/config/known_models.go index 61a9936..146a43d 100644 --- a/internal/config/known_models.go +++ b/internal/config/known_models.go @@ -38,7 +38,7 @@ var knownModels = []KnownModel{ InputPricePer1M: 0.25, OutputPricePer1M: 1.50, Speed: "fast"}, // ── Anthropic (direct API) ───────────────────────────────────────── - {ID: "claude-opus-4-7", Label: "Claude Opus 4.7", Provider: "anthropic", Thinking: true, Review: true, AOI: true, + {ID: "claude-opus-4-8", Label: "Claude Opus 4.8", Provider: "anthropic", Thinking: true, Review: true, AOI: true, InputPricePer1M: 5.00, OutputPricePer1M: 25.00, Speed: "slow"}, {ID: "claude-sonnet-4-6", Label: "Claude Sonnet 4.6", Provider: "anthropic", Thinking: true, Review: true, AOI: true, InputPricePer1M: 3.00, OutputPricePer1M: 15.00, Speed: "medium"}, @@ -90,7 +90,7 @@ var knownModels = []KnownModel{ // public Anthropic rates. Keep these in sync with the anthropic/ // entries above. Haiku has no anthropic/ entry yet; rate sourced // from anthropic.com/pricing. - {ID: "claude-opus-4-7", Label: "Claude Opus 4.7 (Claude Code)", Provider: "claude-code", Thinking: true, Review: true, AOI: true, + {ID: "claude-opus-4-8", Label: "Claude Opus 4.8 (Claude Code)", Provider: "claude-code", Thinking: true, Review: true, AOI: true, InputPricePer1M: 5.00, OutputPricePer1M: 25.00, Speed: "slow"}, {ID: "claude-sonnet-4-6", Label: "Claude Sonnet 4.6 (Claude Code)", Provider: "claude-code", Thinking: true, Review: true, AOI: true, InputPricePer1M: 3.00, OutputPricePer1M: 15.00, Speed: "medium"}, diff --git a/internal/config/models.json b/internal/config/models.json index aa86f1f..0e0ff71 100644 --- a/internal/config/models.json +++ b/internal/config/models.json @@ -29,7 +29,7 @@ }, "aoi_context_lines": 5 }, - "claude-opus-4-7": { + "claude-opus-4-8": { "max_output_tokens": 32000, "temperature": 0.2, "thinking_budget": { diff --git a/scripts/token-bench/run.sh b/scripts/token-bench/run.sh index b414df9..5ce841f 100755 --- a/scripts/token-bench/run.sh +++ b/scripts/token-bench/run.sh @@ -7,7 +7,7 @@ set -euo pipefail # Env vars: # ITERATIONS number of runs per condition (default 5) # CONDITIONS space-separated list (default "none full distilled") -# MODEL claude model (default claude-opus-4-7) +# MODEL claude model (default claude-opus-4-8) BENCH_DIR="$(cd "$(dirname "$0")" && pwd)" FIXTURE="$BENCH_DIR/fixtures/audit-target" @@ -16,7 +16,7 @@ mkdir -p "$RESULTS/outputs" ITERATIONS="${ITERATIONS:-5}" CONDITIONS="${CONDITIONS:-none full distilled}" -MODEL="${MODEL:-claude-opus-4-7}" +MODEL="${MODEL:-claude-opus-4-8}" CSV="$RESULTS/runs.csv" if [[ ! -f "$CSV" ]]; then