fix(gate,queue,ai): defer the gate on a renderer outage, re-drive merge-train waiters, debounce force-push storms, and book the real retry budget (#9464, #9483, #9479) - #9529
Conversation
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-28 06:18:24 UTC
Review summary Nits — 6 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Logic backtestReplayed 0 historical case(s) for Backtest comparison:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9529 +/- ##
==========================================
- Coverage 89.56% 88.66% -0.90%
==========================================
Files 843 843
Lines 110190 110201 +11
Branches 26227 26230 +3
==========================================
- Hits 98690 97714 -976
- Misses 10236 11515 +1279
+ Partials 1264 972 -292
Flags with carried forward coverage won't be shown. Click here to find out more.
|
loopover-orb
Bot
added
the
gittensor:bug
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
1 participant
Summary
Four defects that each let the pipeline act on an answer it did not actually have, or spend on work it did not need.
Closes #9464
Closes #9483
Closes #9479
#9464 — a browserless outage could auto-close legitimate visual PRs
#9030 and #9207 taught the screenshot-table gate to defer its one-shot close when the bot's own capture pipeline blips — but only for failures that report
previewPendingor throw. A browserless failure does neither:captureShotswallows its own renderer errors per shot and returns a null PNG, sobuildCapturecame back normally,previewPending: false, nothing thrown, no real pair.That is byte-identical to a legitimately evidence-free PR right up until the gate closes it. The gate is
action: closeon all three repos, and closes are one-shot — the contributor's only remedy is opening a fresh PR. Last time this gate misfired it closed five of them.The signal is deliberately "a render threw", not "zero pairs were produced". The latter is also true when the author genuinely supplied no evidence, which is the case the gate exists for — inferring the blip from an empty result neuters the gate entirely.
captureShotnow reportsrenderFailedfrom itscatchonly; an SSRF refusal, an auth wall, a redirect block, and an unconfigured binding all stay definite answers.#9483 — a merge-train waiter had no path back to evaluation
The merge-train denial does
audit("denied"); continuewith no re-enqueue. The sibling wake covers the blocker merging — and nothing else. A blocker that is closed unmerged (that wake returns early on!mergedAt), ages pastMERGE_TRAIN_MAX_WAIT_MS, or gains the manual-review label leaves the waiter open indefinitely with no signal, because each of those fires a webhook about the blocker and the age/label checks are consulted only at the waiter's own evaluation time — which nothing schedules. Under one-shot review that reads to the contributor as a silent rejection.Nothing else picked it up either: the sweep is permanently ineligible (
#never-endless-reregate), surface repair requiresmergeableState !== "clean"and a merge-ready waiter is clean, and the wedge alert needs 5 denials/hour against one blocker while a stranded waiter produces exactly one, ever.Fixed by option (b) from the issue: the merge-train wait joins
STALE_RECHECK_DENIAL_DETAIL_PATTERN. It was excluded there as "durable, externally-actioned" — a classification that only holds for the merge case. It is in fact the purest instance of the gap that mechanism exists for. Bounded by construction: it inherits the existing 5-attempts-per-PR repair budget, whose lookback window is rolling and the same 24h as the train's own cap, so a waiter regains looks exactly when the age-out makes them useful.#9479 (3) — force-push storms were undebounced
Every dedup layer was keyed on the head SHA — this coalesce key, and the AI-review lock's
...@${headSha}:${mode}— which is exactly the wrong key for a force-push storm, because each push mints a new SHA. Five amend-and-repush cycles in a minute looked like five unrelated events and bought five full prologues (file list, up to 96k chars of grounding fetch, RAG + impact-map embeddings, an enrichment POST) plus five LLM calls, for four heads that no longer exist by the time their reviews land.skipStaleReviewOutputsuppresses the stale comment, but only after the spend.A push now keys on the PR alone and carries a 45s trailing quiet window; the queue's coalesce keeps the newest payload while extending
run_after, so a burst converges to one review of the surviving head.Deliberately a separate key from
pr-refreshrather than dropping the SHA there. A shared PR-scoped key would let a push overwrite a still-pendingopened/ready_for_reviewpayload, trading a spend bug for a lost-lifecycle-event bug.#9479 (4) — the neuron budget under-booked by up to 6×
The reservation booked one call per opinion slot, but
runWorkersOpinionretries each model 3× and then falls through to that slot's fallback with its own full budget — so a dual-model block review can make 12 calls where 2 were booked. The daily budget is a runaway-loop backstop; booking the best case made it 6× looser than it reads, the one direction a backstop must never be wrong in. The tie-break judge andai-slop.tsalready reserve worst case; the main review path was the outlier.Consequence worth stating plainly: the recorded per-review estimate rises with the reservation, and
sumAiEstimatedNeuronsSincedebits the same number, so a deployment with an explicitly configuredAI_DAILY_NEURON_BUDGETwill see effective daily review throughput fall correspondingly. That is the intended meaning of the cap. The fail-safe default is the clamp maximum, so an unconfigured deployment is unaffected.Also fixed in passing
test/unit/github-webhook-coalesce.test.ts's burst assertion coveredreopened + synchronize + ready_for_reviewcollapsing to one key. With the push split that is no longer true by design, so it now asserts the non-push burst still collapses and that a push does not join it — the property that actually protects lifecycle events.Validation
npx tsc --noEmit,db:migrations:check(contiguous 0001..0197),selfhost:env-reference:check,git diff --check— all cleanRegressions, each verified to fail against the unfixed code (by reverting the fix and re-running): a resolved capture whose renderer failed defers the close and schedules the bounded retry; a merge-train wait now surfaces in the repair-priority set; five rapid pushes collapse to one job carrying the final head; a budget covering only the best case is refused.
Invariants: a healthy renderer that found no evidence still closes — the #4110 guard, and the test that an earlier "infer the blip from zero pairs" attempt broke; an auth wall never sets
renderFailed; one failed shot among healthy ones still defers, since a partial outage can still hide evidence; a merge-train detail without the executor's exact suffix, or with a non-numeric blocker, does not match; pushes to different PRs never coalesce; a push never shares a key with a lifecycle event; the quiet window extends rather than pulls in, so the debounce is not limited to the tail; every non-push action keeps zero delay; and the reservation tracks the real fallback structure, so a no-distinct-fallback pair books strictly less rather than being flatly inflated.