fix: generated file detection, check summary reconciliation, glob matching (#181)

Three CKB review infrastructure fixes:

1. Generated file marker detection was defined but never called —
   detectGeneratedFile now reads the first 10 lines of files and checks
   for GeneratedMarkers (DO NOT EDIT, @generated, etc.)

2. Add dist/*.js and dist/*.css to default generated patterns so bundled
   output (webpack, ncc, etc.) is automatically excluded from review.

3. Fix matchGlob ** suffix matching — was only checking filepath.Base(),
   now tries all path tail segments so patterns like **/dist/*.js work.

4. After HoldTheLine and dismissal filtering, reconcile check summaries
   with surviving findings. A check that reported "5 new bug patterns"
   but had all findings dropped (on unchanged lines) is now downgraded
   to pass with a note.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: SimplyLiz

internal/query/review.go

@@ -1,8 +1,10 @@
 package query
 
 import (
+	"bufio"
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -176,6 +178,7 @@ func DefaultReviewPolicy() *ReviewPolicy {
 			"*.swagger.json", "*.openapi.json",
 			"*_generated.go", "*_gen.go",
 			"*.min.js", "*.min.css",
+			"**/dist/*.js", "**/dist/*.css",
 		},
 		GeneratedMarkers: []string{
 			"DO NOT EDIT", "Generated by", "AUTO-GENERATED", "This file is generated",
@@ -284,7 +287,7 @@ func (e *Engine) ReviewPR(ctx context.Context, opts ReviewPROptions) (*ReviewPRR
 	generatedSet := make(map[string]bool)
 	var generatedFiles []GeneratedFileInfo
 	for _, df := range diffStats {
-		if info, ok := detectGeneratedFile(df.FilePath, opts.Policy); ok {
+		if info, ok := detectGeneratedFile(e.repoRoot, df.FilePath, opts.Policy); ok {
 			generatedSet[df.FilePath] = true
 			generatedFiles = append(generatedFiles, info)
 		}
@@ -551,6 +554,12 @@ func (e *Engine) ReviewPR(ctx context.Context, opts ReviewPROptions) (*ReviewPRR
 		findings, _ = dismissals.FilterDismissed(findings)
 	}
 
+	// Reconcile check summaries with post-filtered findings. Checks set
+	// their summary before HoldTheLine/dismissal filtering, so a check
+	// may report "5 new bug patterns" while all 5 findings were dropped
+	// because they're on unchanged lines. Update summaries to match.
+	reconcileCheckSummaries(checks, findings)
+
 	// Sort checks by severity (fail first, then warn, then pass)
 	sortChecks(checks)
 
@@ -1197,7 +1206,8 @@ func determineVerdict(checks []ReviewCheck, policy *ReviewPolicy) string {
 }
 
 // detectGeneratedFile checks if a file is generated based on policy patterns and markers.
-func detectGeneratedFile(filePath string, policy *ReviewPolicy) (GeneratedFileInfo, bool) {
+// Checks (in order): glob patterns, flex/yacc mappings, marker strings in first 10 lines.
+func detectGeneratedFile(repoRoot, filePath string, policy *ReviewPolicy) (GeneratedFileInfo, bool) {
 	// Check glob patterns
 	for _, pattern := range policy.GeneratedPatterns {
 		matched, _ := matchGlob(pattern, filePath)
@@ -1227,9 +1237,40 @@ func detectGeneratedFile(filePath string, policy *ReviewPolicy) (GeneratedFileIn
 		}, true
 	}
 
+	// Check marker strings in the first 10 lines of the file
+	if len(policy.GeneratedMarkers) > 0 && repoRoot != "" {
+		if marker := checkFileForMarkers(filepath.Join(repoRoot, filePath), policy.GeneratedMarkers); marker != "" {
+			return GeneratedFileInfo{
+				File:   filePath,
+				Reason: fmt.Sprintf("Contains marker: %s", marker),
+			}, true
+		}
+	}
+
 	return GeneratedFileInfo{}, false
 }
 
+// checkFileForMarkers reads the first 10 lines of a file and returns the first
+// matching marker string, or empty if none match.
+func checkFileForMarkers(absPath string, markers []string) string {
+	f, err := os.Open(absPath)
+	if err != nil {
+		return ""
+	}
+	defer f.Close()
+
+	scanner := bufio.NewScanner(f)
+	for i := 0; i < 10 && scanner.Scan(); i++ {
+		line := scanner.Text()
+		for _, marker := range markers {
+			if strings.Contains(line, marker) {
+				return marker
+			}
+		}
+	}
+	return ""
+}
+
 // matchGlob performs simple glob matching (supports ** and *).
 func matchGlob(pattern, path string) (bool, error) {
 	// Use filepath.Match for patterns without **
@@ -1270,8 +1311,17 @@ func matchGlob(pattern, path string) (bool, error) {
 		return false, nil
 	}
 
-	// Simple suffix: check if it matches the file name or path tail
-	return matchSimpleGlob(suffix, filepath.Base(path)), nil
+	// Simple suffix: try matching against every possible tail of the path.
+	// e.g., for suffix "dist/*.js" and path "a/b/dist/index.js",
+	// try "a/b/dist/index.js", "b/dist/index.js", "dist/index.js".
+	parts := strings.Split(remaining, "/")
+	for i := range parts {
+		candidate := strings.Join(parts[i:], "/")
+		if matchSimpleGlob(suffix, candidate) {
+			return true, nil
+		}
+	}
+	return false, nil
 }
 
 // matchSimpleGlob matches a pattern with * wildcards against a string.
@@ -1597,3 +1647,23 @@ func filterByChangedLines(findings []ReviewFinding, changedLines map[string]map[
 	}
 	return filtered
 }
+
+// reconcileCheckSummaries updates check summaries to reflect post-filtered
+// findings. Without this, a check may report "5 new bug patterns" while all
+// 5 were dropped by HoldTheLine because they're on unchanged lines.
+func reconcileCheckSummaries(checks []ReviewCheck, findings []ReviewFinding) {
+	// Count surviving findings per check
+	countByCheck := make(map[string]int)
+	for _, f := range findings {
+		countByCheck[f.Check]++
+	}
+
+	for i, c := range checks {
+		remaining := countByCheck[c.Name]
+		if c.Status == "warn" && remaining == 0 {
+			// All findings were filtered — downgrade to pass and note it
+			checks[i].Status = "pass"
+			checks[i].Summary = c.Summary + " (all on unchanged lines)"
+		}
+	}
+}

internal/query/review_test.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -444,14 +445,102 @@ func TestDetectGeneratedFile(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.path, func(t *testing.T) {
-			_, detected := detectGeneratedFile(tt.path, policy)
+			_, detected := detectGeneratedFile("", tt.path, policy)
 			if detected != tt.expected {
 				t.Errorf("detectGeneratedFile(%q) = %v, want %v", tt.path, detected, tt.expected)
 			}
 		})
 	}
 }
 
+func TestDetectGeneratedFile_DistPattern(t *testing.T) {
+	t.Parallel()
+	policy := DefaultReviewPolicy()
+
+	tests := []struct {
+		path     string
+		expected bool
+	}{
+		{".github/actions/pr-analysis/dist/index.js", true},
+		{"frontend/dist/bundle.js", true},
+		{"frontend/dist/styles.css", true},
+		{"src/dist.go", false},         // not a dist/ directory
+		{"dist/README.md", false},      // not JS/CSS
+		{"src/components/app.js", false}, // not in dist/
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.path, func(t *testing.T) {
+			_, detected := detectGeneratedFile("", tt.path, policy)
+			if detected != tt.expected {
+				t.Errorf("detectGeneratedFile(%q) = %v, want %v", tt.path, detected, tt.expected)
+			}
+		})
+	}
+}
+
+func TestDetectGeneratedFile_MarkerInFile(t *testing.T) {
+	t.Parallel()
+	dir := t.TempDir()
+	policy := DefaultReviewPolicy()
+
+	// File with a generated marker in the first 10 lines
+	genFile := filepath.Join(dir, "gen.go")
+	if err := os.WriteFile(genFile, []byte("// Code generated by protoc-gen-go. DO NOT EDIT.\npackage pb\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	// File without any marker
+	normalFile := filepath.Join(dir, "normal.go")
+	if err := os.WriteFile(normalFile, []byte("package main\n\nfunc main() {}\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	info, detected := detectGeneratedFile(dir, "gen.go", policy)
+	if !detected {
+		t.Error("expected gen.go to be detected as generated via marker")
+	}
+	if !strings.Contains(info.Reason, "marker") {
+		t.Errorf("reason should mention marker, got %q", info.Reason)
+	}
+
+	_, detected = detectGeneratedFile(dir, "normal.go", policy)
+	if detected {
+		t.Error("normal.go should not be detected as generated")
+	}
+}
+
+func TestReconcileCheckSummaries(t *testing.T) {
+	t.Parallel()
+
+	checks := []ReviewCheck{
+		{Name: "bug-patterns", Status: "warn", Summary: "5 new bug pattern(s)"},
+		{Name: "secrets", Status: "pass", Summary: "No secrets detected"},
+		{Name: "coupling", Status: "warn", Summary: "3 missing co-change files"},
+	}
+	// Only coupling has surviving findings
+	findings := []ReviewFinding{
+		{Check: "coupling", Message: "Missing co-change: foo.go"},
+	}
+
+	reconcileCheckSummaries(checks, findings)
+
+	// bug-patterns had warn but 0 surviving findings → should be downgraded
+	if checks[0].Status != "pass" {
+		t.Errorf("bug-patterns status = %q, want pass", checks[0].Status)
+	}
+	if !strings.Contains(checks[0].Summary, "unchanged lines") {
+		t.Errorf("bug-patterns summary should note unchanged lines, got %q", checks[0].Summary)
+	}
+	// secrets was already pass → unchanged
+	if checks[1].Status != "pass" {
+		t.Errorf("secrets status = %q, want pass", checks[1].Status)
+	}
+	// coupling has surviving findings → stays warn
+	if checks[2].Status != "warn" {
+		t.Errorf("coupling status = %q, want warn", checks[2].Status)
+	}
+}
+
 func TestMatchGlob(t *testing.T) {
 	t.Parallel()