fix(capture): reserve a cap slot for failed-then-fixed candidates

[marcusrbrown]

A live capture run confirmed the failed-then-fixed trigger detects candidates correctly (6 real candidates found against 105 merged PRs, all with fixing diffs, privacy gate clean) but never authored a proposal: a backlog of review-heavy candidates sorted ahead of the failed-then-fixed ones and filled the per-run cap.

This reserves at least one cap slot for failed-then-fixed candidates when any exist, then fills the rest of the budget in freshness order. The per-run total stays bounded by the existing cap, so the new trigger delivers when it fires without increasing review load.

A regression test reproduces the exact starvation scenario (review-heavy candidates sorted ahead of a failed-then-fixed one, cap smaller than the review-heavy count) and asserts the failed-then-fixed candidate is now selected, plus edge cases for order preservation, no-candidate passthrough, multi-slot reservation, and floor/cap bounds.

[fro-bot]

The fix is surgical and lands where the bug actually lives. A working trigger that detects candidates but never authors a proposal is the worst failure mode — silent competence — and reserving a cap slot is the right correction without inflating review load.

Verdict: PASS

Blocking issues

None.

Non-blocking concerns

• selectWithCiFixFloor keys its Set<Candidate> on object reference identity. That's correct here because ordered flows from the byMergeSha Map dedup, so every element is a unique reference. It's a quiet invariant, though — if a future refactor ever feeds this function a list with duplicated references (e.g. the same candidate object appearing twice), the reservation math drifts. A one-line note on the parameter ("expects reference-unique candidates") would chrome that contract.
• The fill loop checks chosen.size >= cap before add, then relies on the trailing .slice(0, cap) to bound overflow. It's correct, but the bound is enforced in two places rather than one. Minor indirection, not worth a respin.
• The floor now pulls a ci-fix candidate that a flat slice would have dropped. That candidate still passes through the same fail-closed privacy scan downstream (buildCandidateDigest scans post-cap), so no under-block hole opens — the safe failure direction (over-block) is preserved. Confirmed by reading the ordering, not assumed.

Missing tests

None. The regression test reproduces the exact production starvation (8 review-heavy ahead of 1 ci-fix, cap 5) and asserts inclusion, plus order preservation, no-candidate passthrough, floor=0, multi-slot reservation, floor>cap, and cap=0. Full file: 140 tests green, tsc --noEmit clean, no any/@ts-ignore/console/classes introduced.

Risk assessment: LOW

Pure, dependency-free function gated behind an existing cap. Per-run total stays bounded by MAX_LEARNINGS_PER_RUN, so the blast radius is "one freshness slot reallocated," not "more proposals." No workflow, permissions, or untrusted-input surface touched. The only thing that could rot is the reference-identity assumption, and the data flow currently guarantees it.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/.github
Run ID	28068535860
Cache	hit
Session	ses_108c8a9b0ffeRVTNd3ku16Cad9

[marcusrbrown]

Applied NBC #1: documented the reference-uniqueness contract on selectWithCiFixFloor's parameter, noting selection keys on object identity and that production feeds it the byMergeSha-deduped (reference-unique) list. NBC #2 (the two-place cap bound) and #3 (post-cap privacy scan preserved — thanks for confirming the over-block safe direction holds) are accepted as-is.

[fro-bot]

Re-review after bc4a5f5. The follow-up is docs-only: it inscribes the reference-uniqueness contract directly on selectWithCiFixFloor, naming the byMergeSha dedup as the source of distinct references and warning that duplicated references would skew the identity-keyed count. That's exactly the quiet invariant I flagged — now written into the wall where the next refactor will read it before it trips. The selection logic is byte-identical to the verified version.

NBC #2 and #3 accepted as-is, correctly: the two-place cap bound is harmless indirection, and the post-cap privacy scan still fail-closes the freshly-reserved ci-fix candidate, so the over-block safe direction holds.

Verdict: PASS

Blocking issues

None.

Non-blocking concerns

None outstanding. The reference-uniqueness contract is now documented; the remaining two are accepted and carry no risk.

Missing tests

None. Logic unchanged since the prior pass — 140 tests green, tsc --noEmit clean, no any/@ts-ignore/console/classes. The starvation regression plus floor/cap boundary cases remain covered.

Risk assessment: LOW

Docs-only delta over an already-validated, pure, cap-bounded function. No workflow, permissions, dependency, or untrusted-input surface touched. The one assumption that could have rotted is now load-bearing documentation instead of tribal knowledge. Ship it.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/.github
Run ID	28068673124
Cache	hit
Session	ses_108c8a9b0ffeRVTNd3ku16Cad9