internal/reposcan at 91d433696a, reviewed by claude-code, verified by codex.
Rank() reports Signal "churn-x-size" while the churn factor is uniformly 1 whenever root is a subdirectory of a git work tree, because fileChurn compares repo-root-relative `git log --name-only` paths against cwd-relative candidate paths — so a disclosed ranking rule silently degrades to size-only and --top selects the wrong files.
#!/bin/sh
set -e
cat > internal/reposcan/zz_review_rank_test.go <<'GOEOF'
package reposcan
import (
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
)
func zzGit(t *testing.T, dir string, args ...string) {
t.Helper()
full := append([]string{"-c", "commit.gpgsign=false"}, args...)
cmd := exec.Command("git", full...)
cmd.Dir = dir
cmd.Env = append(cmd.Environ(),
"GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@e",
"GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@e")
if out, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("git %v failed; this reproduction needs a working git: %v\n%s", args, err, out)
}
}
func TestZZRankChurnInertWhenRootIsSubdirectory(t *testing.T) {
root := t.TempDir()
sub := filepath.Join(root, "sub")
if err := os.MkdirAll(sub, 0o755); err != nil {
t.Fatal(err)
}
write := func(name string, pad int) {
if err := os.WriteFile(filepath.Join(sub, name), []byte("package p\n"+string(make([]byte, pad))), 0o644); err != nil {
t.Fatal(err)
}
}
// hot.go is SMALL but heavily churned; cold.go is LARGE and touched once.
write("hot.go", 200)
write("cold.go", 1000)
zzGit(t, root, "init", "-q")
zzGit(t, root, "add", ".")
zzGit(t, root, "commit", "-q", "-m", "c0", "--no-gpg-sign")
for i := 0; i < 9; i++ {
f, err := os.OpenFile(filepath.Join(sub, "hot.go"), os.O_APPEND|os.O_WRONLY, 0o600)
if err != nil {
t.Fatal(err)
}
if _, err := f.WriteString("//x\n"); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
zzGit(t, root, "add", "sub/hot.go")
zzGit(t, root, "commit", "-q", "-m", "c", "--no-gpg-sign")
}
hi, err1 := os.Stat(filepath.Join(sub, "hot.go"))
ci, err2 := os.Stat(filepath.Join(sub, "cold.go"))
if err1 != nil || err2 != nil {
t.Fatalf("fixture stat: %v %v", err1, err2)
}
t.Logf("sizes: hot.go=%d bytes (10 commits), cold.go=%d bytes (1 commit)", hi.Size(), ci.Size())
t.Logf("correct churn x size: hot=%d cold=%d -> hot.go must rank first", 10*hi.Size(), 1*ci.Size())
// THE EVIDENCE: what fileChurn's own command prints when run from the
// subdirectory. These are the keys `churn` is built from.
lg := exec.Command("git", "log", "--format=", "--name-only", "-z")
lg.Dir = sub
out, lerr := lg.Output()
if lerr != nil {
t.Fatalf("git log in the subdirectory: %v", lerr)
}
keys := map[string]bool{}
for _, p := range strings.Split(string(out), "\x00") {
if p = strings.Trim(p, "\n"); p != "" {
keys[p] = true
}
}
t.Logf("churn map keys produced with root=<repo>/sub: %v", keys)
t.Logf("candidate paths Enumerate would produce with root=<repo>/sub: [hot.go cold.go]")
if keys["hot.go"] {
t.Fatalf("git log printed a cwd-relative path; the premise of this reproduction does not hold on this git")
}
// CONTROL: at the repository top level the churn signal genuinely works.
ctl, ctlInfo := Rank(root, []Candidate{
{Path: "sub/cold.go", TestPath: "sub/cold_test.go", Lang: "go"},
{Path: "sub/hot.go", TestPath: "sub/hot_test.go", Lang: "go"},
})
t.Logf("CONTROL Rank(<repo>) -> signal=%q order=[%s %s]", ctlInfo.Signal, ctl[0].Path, ctl[1].Path)
if ctl[0].Path != "sub/hot.go" {
t.Fatalf("control failed: churn is not working even at the repo root, so this test proves nothing")
}
// THE DEFECT: same tree, root one level down.
got, info := Rank(sub, []Candidate{
{Path: "cold.go", TestPath: "cold_test.go", Lang: "go"},
{Path: "hot.go", TestPath: "hot_test.go", Lang: "go"},
})
t.Logf("DEFECT Rank(<repo>/sub) -> signal=%q note=%q order=[%s %s]", info.Signal, info.Note, got[0].Path, got[1].Path)
if info.Signal != "churn-x-size" {
t.Fatalf("Signal = %q; the defect requires Rank to still CLAIM churn-x-size", info.Signal)
}
if info.Note != "" {
t.Fatalf("Note = %q; the defect requires the degradation to be UNdisclosed", info.Note)
}
if got[0].Path != "cold.go" {
t.Fatalf("ranked %q first; churn appears to have been applied, defect NOT demonstrated", got[0].Path)
}
t.Logf("DEMONSTRATED: Rank claims signal %q with no note, yet ranked the 1-commit large file ahead of the 10-commit one — every candidate scored churn=1 because the churn map is keyed \"sub/hot.go\" while the candidate is \"hot.go\".", info.Signal)
}
GOEOF
go test -count=1 -vet=off -timeout 120s -run TestZZRankChurnInertWhenRootIsSubdirectory -v ./internal/reposcan=== RUN TestZZRankChurnInertWhenRootIsSubdirectory
zz_review_rank_test.go:61: sizes: hot.go=246 bytes (10 commits), cold.go=1010 bytes (1 commit)
zz_review_rank_test.go:62: correct churn x size: hot=2460 cold=1010 -> hot.go must rank first
zz_review_rank_test.go:78: churn map keys produced with root=<repo>/sub: map[sub/cold.go:true sub/hot.go:true]
zz_review_rank_test.go:79: candidate paths Enumerate would produce with root=<repo>/sub: [hot.go cold.go]
zz_review_rank_test.go:89: CONTROL Rank(<repo>) -> signal="churn-x-size" order=[sub/hot.go sub/cold.go]
zz_review_rank_test.go:99: DEFECT Rank(<repo>/sub) -> signal="churn-x-size" note="" order=[cold.go hot.go]
zz_review_rank_test.go:110: DEMONSTRATED: Rank claims signal "churn-x-size" with no note, yet ranked the 1-commit large file ahead of the 10-commit one — every candidate scored churn=1 because the churn map is keyed "sub/hot.go" while the candidate is "hot.go".
--- PASS: TestZZRankChurnInertWhenRootIsSubdirectory (0.07s)
PASS
ok github.com/pdbethke/corralai/internal/reposcan 0.084sI checked for path normalization and disclosed fallback handling. internal/reposcan/rank.go:97 runs git log without making its output relative to the scan root; the resulting paths become churn keys unchanged. Rank looks up churn[c.Path] and substitutes 1 for missing entries. Successful, nonempty history still returns churn-x-size without a note. The supplied subdirectory fixture matches this mismatch.
ref e920c2249182c0e3618175fb0bc719eeea04348435012e16615dc3f07955e859#R1
BuildLanguageProfile attributes an exclusion by enumerating four reasons, so a source file relabelled by WidenCandidacyByEvidence to uncovered / import-only / no-executable-code is counted in NO bucket at all — LanguageStat.Total(), documented as "every file this language accounts for", silently shrinks on exactly the scans that collect coverage evidence.
#!/bin/sh
set -e
cat > internal/reposcan/zz_review_profile_test.go <<'GOEOF'
package reposcan
import "testing"
func zzPy(t *testing.T, stats []LanguageStat) LanguageStat {
t.Helper()
for _, s := range stats {
if s.Lang == "python" {
return s
}
}
return LanguageStat{}
}
func TestZZLanguageProfileDropsEvidenceRelabelledFiles(t *testing.T) {
// Three python source files: one paired, two unpaired. Nothing else.
cands := []Candidate{{Path: "src/pkg/live.py", TestPath: "tests/test_live.py", Lang: "python"}}
excl := []Exclusion{
{Path: "src/pkg/dead.py", Reason: ReasonNoPairedTest},
{Path: "src/pkg/other.py", Reason: ReasonNoPairedTest},
}
before := zzPy(t, BuildLanguageProfile(cands, excl))
t.Logf("BEFORE evidence: python %+v Total()=%d", before, before.Total())
if before.Total() != 3 {
t.Fatalf("fixture wrong: pairing-only profile Total()=%d, want 3", before.Total())
}
// One instrumented run that MEASURED dead.py and found zero covering
// tests, no import-time coverage, and real statements: the textbook
// ReasonUncovered case.
idx := EvidenceIndex{files: map[string]evidenceFileEntry{
"src/pkg/dead.py": {
coveringTests: 0,
mostCovering: "",
coveringFiles: map[string]bool{},
hasStatic: false,
hasStatements: true,
},
}}
c2, e2, promoted := WidenCandidacyByEvidence(cands, excl, idx, true)
if promoted != 0 {
t.Fatalf("fixture wrong: promoted=%d, want 0", promoted)
}
if len(e2) != 2 {
t.Fatalf("fixture wrong: %d exclusions after widening, want 2 (both files still ACCOUNTED)", len(e2))
}
relabelled := false
for _, e := range e2 {
t.Logf(" exclusion after widening: %s -> %q", e.Path, e.Reason)
if e.Path == "src/pkg/dead.py" && e.Reason == ReasonUncovered {
relabelled = true
}
}
if !relabelled {
t.Fatalf("fixture wrong: dead.py was not relabelled to ReasonUncovered")
}
after := zzPy(t, BuildLanguageProfile(c2, e2))
t.Logf("AFTER evidence: python %+v Total()=%d", after, after.Total())
if after.Total() != before.Total()-1 {
t.Fatalf("Total() went %d -> %d; the defect requires it to LOSE the relabelled file", before.Total(), after.Total())
}
if after.NoPairedTest != 1 || after.Auditable != 1 || after.Ambiguous != 0 || after.TestFiles != 0 {
t.Fatalf("unexpected post-widening shape %+v", after)
}
t.Logf("DEMONSTRATED: the same three python files are still fully enumerated (1 candidate + 2 exclusions), but the language profile now accounts for only %d of them. src/pkg/dead.py is in no bucket: not Auditable, not NoPairedTest, not Ambiguous, not TestFiles.", after.Total())
}
GOEOF
go test -count=1 -vet=off -timeout 120s -run TestZZLanguageProfileDropsEvidenceRelabelledFiles -v ./internal/reposcan=== RUN TestZZLanguageProfileDropsEvidenceRelabelledFiles
zz_review_profile_test.go:24: BEFORE evidence: python {Lang:python Auditable:1 NoPairedTest:2 Ambiguous:0 TestFiles:0} Total()=3
zz_review_profile_test.go:50: exclusion after widening: src/pkg/dead.py -> "uncovered — no test executes this file"
zz_review_profile_test.go:50: exclusion after widening: src/pkg/other.py -> "no-paired-test"
zz_review_profile_test.go:60: AFTER evidence: python {Lang:python Auditable:1 NoPairedTest:1 Ambiguous:0 TestFiles:0} Total()=2
zz_review_profile_test.go:68: DEMONSTRATED: the same three python files are still fully enumerated (1 candidate + 2 exclusions), but the language profile now accounts for only 2 of them. src/pkg/dead.py is in no bucket: not Auditable, not NoPairedTest, not Ambiguous, not TestFiles.
--- PASS: TestZZLanguageProfileDropsEvidenceRelabelledFiles (0.00s)
PASS
ok github.com/pdbethke/corralai/internal/reposcan 0.009sI checked whether the excluded files remain counted elsewhere or the profile is built before widening. internal/reposcan/profile.go:82 attributes only no-paired-test, ambiguous-test, and is-test exclusions. WidenCandidacyByEvidence replaces the original exclusion for the three evidence outcomes without adding those files as candidates. cmd/corral/certify_repo.go:820 performs widening before the profile call at line 1062. The supplied src/pkg fixture also satisfies the library-code restriction, so that restriction does not refute the loss.
ref e920c2249182c0e3618175fb0bc719eeea04348435012e16615dc3f07955e859#R2
EmitJobs, on the FileScopedTests path, digests c.TestPath with no emptiness guard, so an evidence-only candidate (TestPath == "" by construction in WidenCandidacyByEvidence) aborts the ENTIRE scan with an error naming "." instead of the candidate; the only thing preventing this today is gradesFileScoped in a different package.
#!/bin/sh
set -e
cat > internal/reposcan/zz_review_emitjobs_test.go <<'GOEOF'
package reposcan
import (
"os"
"path/filepath"
"strings"
"testing"
)
type zzGoals struct{}
func (zzGoals) GoalFor(c Candidate) (Goal, bool, error) {
return Goal{Text: "g", Provenance: GoalFromFile}, true, nil
}
func TestZZEmitJobsFileScopedAbortsOnEvidenceOnlyCandidate(t *testing.T) {
root := t.TempDir()
if err := os.MkdirAll(filepath.Join(root, "pkg"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(root, "pkg", "a.py"), []byte("x = 1\n"), 0o644); err != nil {
t.Fatal(err)
}
// EXACTLY the shape WidenCandidacyByEvidence produces: a promoted,
// evidence-only candidate has TestPath == "" by construction.
cands := []Candidate{{Path: "pkg/a.py", TestPath: "", Lang: "python", CoveringTestPath: "tests/test_x.py"}}
// Control: the whole-suite path handles the same candidate fine.
jobs, excl, err := EmitJobs(EmitConfig{Owner: "o", Repo: "r", Commit: "c", Root: root, FileScopedTests: false}, cands, zzGoals{})
if err != nil {
t.Fatalf("control (FileScopedTests=false) failed: %v", err)
}
t.Logf("CONTROL FileScopedTests=false -> %d job(s), %d exclusion(s), err=nil", len(jobs), len(excl))
// The defect: same candidate, file-scoped.
jobs2, excl2, err2 := EmitJobs(EmitConfig{Owner: "o", Repo: "r", Commit: "c", Root: root, FileScopedTests: true}, cands, zzGoals{})
t.Logf("DEFECT FileScopedTests=true -> %d job(s), %d exclusion(s), err=%v", len(jobs2), len(excl2), err2)
if err2 == nil {
t.Fatalf("EmitJobs returned no error; defect NOT demonstrated")
}
if jobs2 != nil || excl2 != nil {
t.Fatalf("expected the whole call to abort with nil results, got %d jobs / %d exclusions", len(jobs2), len(excl2))
}
if !strings.Contains(err2.Error(), "is not a regular file") {
t.Fatalf("unexpected error shape %q", err2.Error())
}
if strings.Contains(err2.Error(), "pkg/a.py") {
t.Fatalf("the error names the candidate after all; the misattribution half is not demonstrated: %q", err2.Error())
}
if !strings.Contains(err2.Error(), "reposcan: . ") {
t.Fatalf("the error does not name %q as the unreadable path: %q", ".", err2.Error())
}
t.Logf("DEMONSTRATED: one candidate with an empty TestPath aborts EmitJobs for EVERY candidate, and the operator-facing error is %q — it names the repository root directory, not the file that has no paired test.", err2.Error())
}
GOEOF
go test -count=1 -vet=off -timeout 120s -run TestZZEmitJobsFileScopedAbortsOnEvidenceOnlyCandidate -v ./internal/reposcan=== RUN TestZZEmitJobsFileScopedAbortsOnEvidenceOnlyCandidate
zz_review_emitjobs_test.go:34: CONTROL FileScopedTests=false -> 1 job(s), 0 exclusion(s), err=nil
zz_review_emitjobs_test.go:38: DEFECT FileScopedTests=true -> 0 job(s), 0 exclusion(s), err=reposcan: . is not a regular file (d---------)
zz_review_emitjobs_test.go:55: DEMONSTRATED: one candidate with an empty TestPath aborts EmitJobs for EVERY candidate, and the operator-facing error is "reposcan: . is not a regular file (d---------)" — it names the repository root directory, not the file that has no paired test.
--- PASS: TestZZEmitJobsFileScopedAbortsOnEvidenceOnlyCandidate (0.00s)
PASS
ok github.com/pdbethke/corralai/internal/reposcan 0.009sI checked EmitConfig's contract and the caller guard. FileScopedTests describes grading against an actual paired test, and cmd/corral/certify_repo.go:1939 rejects an empty TestPath when determining that mode. However, the finding explicitly acknowledges this protection. EmitJobs itself still passes the empty TestPath to DigestFile at internal/reposcan/job.go:195 and returns nil results on error. The demonstrated package-level failure is not refuted by the guarded CLI path.
ref e920c2249182c0e3618175fb0bc719eeea04348435012e16615dc3f07955e859#R3
EvidenceIndex answers "which tests cover this file" two different ways from the same parse: coveringTests is len(fc.Tests) (every listed test id) while coveringFiles admits only ids with lines > 0, so an evidence document carrying a zero-line test id yields CoveringTests > 0 and a mostCovering hint while CoveredByAny — the oracle a --diff-base scan uses to decide which sources a changed test defends — reports false for every test file.
I checked whether the real evidence parser rejects or removes zero-line test IDs. internal/lang/python.go:1453 initializes every listed test ID to zero before adding recorded line counts, and parsePySelectionEvidence at line 1312 does not require a positive count. internal/reposcan/evidence_index.go counts all those IDs and considers them for mostCovering, while adding only positive-count entries to coveringFiles. Thus the named evidence input can produce the inconsistent answers.
ref e920c2249182c0e3618175fb0bc719eeea04348435012e16615dc3f07955e859#R4
Preflight has no guard against an instrumented run that exits non-zero and prints nothing — it uses the plain commandRunner (no exit code, no stderr) and hands empty stdout straight to ParseCoverage, unlike CollectSelectionEvidence which was hardened for exactly that case; only each plugin's own ParseCoverage happening to reject empty input keeps Preflight from returning Ran=true with an empty Executed map, i.e. a repo-wide "your suite touches nothing".
I checked the runner interface, error handling, and coverage-parser contract. internal/reposcan/preflight.go:74 receives stdout and an error but no exit status, then passes empty stdout to ParseCoverage if the runner returns no error. Parser rejection is an explicit CoverageReporter contract, and shipped parsers provide protection; therefore this is not evidence of a current repo-wide false accusation. Nevertheless, the finding already acknowledges parser protection, and its narrower claim that Preflight lacks an independent empty-output/exit-status safeguard remains correct.
ref e920c2249182c0e3618175fb0bc719eeea04348435012e16615dc3f07955e859#R5
RepoReport documents "UncoveredFiles is a SUBSET of SelectedFiles", but Aggregate increments UncoveredFiles from r.Verdict.Uncovered without regard to whether TestSelection.Method was set, so a verdict carrying Uncovered on the whole-suite path would make UncoveredFiles exceed SelectedFiles and break the stated partition a printer relies on.
I traced verdict construction to try to exclude the proposed input. internal/advpool/aggregate.go:58 requires a nonempty selection method when setting Uncovered, preventing this combination through that constructor. However, the finding is explicitly conditional about Aggregate receiving such a verdict. internal/reposcan/report.go:515–522 counts selection and uncovered status independently, so the supplied combination violates the documented subset relationship. internal/reposcan/report_selection_test.go:59 already constructs that combination, although it asserts only the kill-rate behavior. Production reachability remains unestablished.
ref e920c2249182c0e3618175fb0bc719eeea04348435012e16615dc3f07955e859#R6
Recorded so that "found nothing here" can be told apart from "never looked here".
internal/reposcan is unusually careful prose-wise — nearly every degradation is named, and the failure posture (absence of evidence is never evidence of absence) is genuinely enforced in the places I probed hardest: CollectSelectionEvidence refuses to call an empty or unparseable instrumented run 'Ran', WidenCandidacyByEvidence refuses to relabel a file the evidence never measured, and Aggregate refuses to average an uncovered or zero-denominator file into the headline. Where it fails is exactly where the doc comments stop looking: at the SECOND door. Finding 1 is the one I would fix first. Rank() disclosres 'churn-x-size' as the ranking rule, and the package already fixed one path-shape bug in that exact comparison (the -z/core.quotePath fix cited at rank.go:93). It fixed the quoting and left the ROOT-RELATIVITY. gitIgnored and TreeDigest both call `git -C root ls-files`, which is cwd-relative and agrees with Enumerate's candidate paths; fileChurn calls `git log --name-only`, which is REPO-ROOT-relative and does not. Point --repo at a subdirectory of a repository — a monorepo service, which is the ordinary way to scan one — and every churn lookup misses, every candidate scores churn=1, the ranking silently collapses to size-only, and RankInfo.Signal still says 'churn-x-size'. --top then selects the wrong files, and the report discloses a rule that did not run. Every Rank test uses the repo top-level as root, so nothing reaches this. Finding 2 is a smaller version of the same shape one layer up: BuildLanguageProfile's switch enumerates the exclusion reasons it attributes, and the evidence-widening pass invented three new ones (uncovered / import-only / no-executable-code) that replace ReasonNoPairedTest in place. Those files then belong to no bucket at all — LanguageStat.Total(), documented as 'every file this language accounts for', shrinks on precisely the scans that measure most. profile_test.go never mentions any of the three reasons. Finding 3 is the latent-crash version: EmitJobs' file-scoped branch digests c.TestPath unconditionally, and an evidence-only candidate has TestPath == "" by construction. cmd/corral's gradesFileScoped happens to return false whenever any selected candidate has an empty TestPath, so the live path is guarded — at one door, in a different package, by a function whose purpose is something else. The package itself has no guard, and the failure it produces aborts the entire scan with an error naming '.' rather than the candidate. What I could not break: the cache-key injectivity work (CanonicalKV escaping, the length-prefixed digests), the empty/unparseable-evidence refusals, and the uncovered/import-only/no-executable-code taxonomy itself, which is correct on every input I could construct.
R1–R4 survive code inspection. R5 survives as a missing independent safeguard, not a demonstrated false result with shipped parsers. R6 survives as the stated conditional Aggregate inconsistency, although the production verdict constructor prevents that input. No tests or repository code were executed.