internal/prior at 5ea687f383, reviewed by gemini-3.6-flash, verified by claude-code.
Render truncates priors over MaxRendered edits using the unsorted parameter slice instead of the sorted slice, producing wrong line range summaries for undisclosed edits.
cat << 'EOF' > internal/prior/render_bug_test.go
package prior
import (
"strings"
"testing"
"github.com/pdbethke/corralai/internal/lang"
)
func TestRenderCutUsesUnsortedSlice(t *testing.T) {
var tried []Tried
for i := 47; i >= 1; i-- {
tried = append(tried, Tried{
Span: lang.LineRange{Start: i, End: i},
Shape: "other",
Outcome: "killed",
})
}
para := Render(tried)
want := "between lines 41 and 47"
if !strings.Contains(para, want) {
t.Fatalf("BUG DEMONSTRATED: Render cut summary reported wrong line range.\nExpected substring: %q\nGot paragraph:\n%s", want, para)
}
}
EOF
output=$(go test ./internal/prior -run TestRenderCutUsesUnsortedSlice 2>&1)
status=$?
rm -f internal/prior/render_bug_test.go
if [ $status -ne 0 ]; then
echo "$output"
exit 0
else
echo "Expected test failure demonstrating bug, but test passed."
exit 1
fi--- FAIL: TestRenderCutUsesUnsortedSlice (0.00s)
render_bug_test.go:22: BUG DEMONSTRATED: Render cut summary reported wrong line range.
Expected substring: "between lines 41 and 47"
Got paragraph:
ALREADY TRIED on this exact version of the file (47 edit(s) from earlier runs). Do NOT repeat these edits or their shape at the same place; plant DIFFERENT faults — other decision points, other kinds of change:
- line 1, other — KILLED (the suite watches this)
- line 2, other — KILLED (the suite watches this)
- line 3, other — KILLED (the suite watches this)
- line 4, other — KILLED (the suite watches this)
- line 5, other — KILLED (the suite watches this)
- line 6, other — KILLED (the suite watches this)
- line 7, other — KILLED (the suite watches this)
- line 8, other — KILLED (the suite watches this)
- line 9, other — KILLED (the suite watches this)
- line 10, other — KILLED (the suite watches this)
- line 11, other — KILLED (the suite watches this)
- line 12, other — KILLED (the suite watches this)
- line 13, other — KILLED (the suite watches this)
- line 14, other — KILLED (the suite watches this)
- line 15, other — KILLED (the suite watches this)
- line 16, other — KILLED (the suite watches this)
- line 17, other — KILLED (the suite watches this)
- line 18, other — KILLED (the suite watches this)
- line 19, other — KILLED (the suite watches this)
- line 20, other — KILLED (the suite watches this)
- line 21, other — KILLED (the suite watches this)
- line 22, other — KILLED (the suite watches this)
- line 23, other — KILLED (the suite watches this)
- line 24, other — KILLED (the suite watches this)
- line 25, other — KILLED (the suite watches this)
- line 26, other — KILLED (the suite watches this)
- line 27, other — KILLED (the suite watches this)
- line 28, other — KILLED (the suite watches this)
- line 29, other — KILLED (the suite watches this)
- line 30, other — KILLED (the suite watches this)
- line 31, other — KILLED (the suite watches this)
- line 32, other — KILLED (the suite watches this)
- line 33, other — KILLED (the suite watches this)
- line 34, other — KILLED (the suite watches this)
- line 35, other — KILLED (the suite watches this)
- line 36, other — KILLED (the suite watches this)
- line 37, other — KILLED (the suite watches this)
- line 38, other — KILLED (the suite watches this)
- line 39, other — KILLED (the suite watches this)
- line 40, other — KILLED (the suite watches this)
… and 7 more, between lines 1 and 7 — not listed here, but tried; plant elsewhere in that stretch too.
FAIL
FAIL github.com/pdbethke/corralai/internal/prior 0.005s
FAILThe claim is literally true at prior.go:350: the loop consumes `sorted(tried)` but `rest := tried[MaxRendered:]` re-slices the unsorted parameter, so spanOf reports the line range of whichever edits happened to sit at indices 40+ in the caller's ordering rather than the tail the list actually dropped. I attempted to refute on three grounds and failed on each: the correct `len(rest)` does not rescue the range (the finding is about the range); the two production callers happening to pass already-sorted For output narrows the blast radius but does not make the claimed behaviour false, and the package comment at prior.go:226-227 states order independence as a design property of Render, so a caller passing arbitrary order is within contract; and the header's line citation (233) being off from the real site (350) is a bookkeeping slip, not a defect in the claim. The reviewer's own reproduction — 47 descending-order edits yielding "between lines 1 and 7" for edits that are in fact lines 41-47 — is exactly the behaviour the claim describes.
confirmed by pdbethke — real, reproduced, and mine: the cut line names tried[MaxRendered:] of the caller's order after Render began sorting a copy — the range it names is not the tail it dropped
ref dcd0874fecb8c7f10ccbd8197581d112fb89b7abf266d7eded8a27bca12bde6a#R1
Recorded so that "found nothing here" can be told apart from "never looked here".
The prior package is generally well-designed around its core requirements (order-independent merging, same-bytes SHA256 matching, and digest calculation). However, a defect was identified in `Render` (finding 1) where truncating a prior with more than 40 edits calculates the line range of dropped edits using the unsorted parameter slice instead of the sorted slice, outputting incorrect line ranges to the prompt generator.
Finding 1 is the only claim, and it is correct as stated. internal/prior/prior.go:339-353: Render iterates `sorted(tried)` (line 345) but computes the truncation remainder as `rest := tried[MaxRendered:]` (line 350) — a slice of the caller's ORIGINAL, unsorted parameter — and hands that to spanOf. The comment immediately above it ("The edits are sorted by line, so the cut always drops the FILE'S TAIL") asserts exactly the invariant the code does not hold, and the package's own `sorted` helper is documented (prior.go:226-227) as existing so that "Render and Digest read the same edits in the same order whatever order a caller holds them in" — so this is a stated property the function fails, not an unstated one. The reviewer's script reproduces it end-to-end: 47 edits supplied in descending line order render lines 1..40 in the list, then summarise the 7 undisclosed edits as "between lines 1 and 7" when the actually-undisclosed ones are lines 41-47. I tried three refutation angles and none worked. (a) Claiming the count is what matters and the range is decoration: no, the finding is specifically about the line range, and len(rest) being coincidentally right does not save spanOf. (b) Claiming Render is unreachable with unsorted input: the two production callers (cmd/corral/certify_local.go:1338 and cmd/corral/certify_repo.go:5419) do pass Prior.For output, which mergeTried sorted per path and For filters order-preservingly — so the defect is arguably latent on today's call graph and "medium" severity is generous — but that narrows the impact, not the claim, and Render is an exported function whose contract explicitly promises order independence. (c) Claiming the cited line 233 is wrong: line 233 is the close of `sorted`, and the defect is at line 350; that is a citation slip in the finding's header, not an error in the claim body, which names the mechanism ("unsorted parameter slice instead of the sorted slice") precisely. I could not refute it.