Split category Shapes from Review criteria; bump Opus 4.7 to 4.8

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)
 }

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)
+}