fix(ai): make prompt drift a cache miss, bound the Ollama context, and attribute votes to the producing model (#9477, #9478) - #9513
Conversation
…d attribute votes to the producing model (#9477, #9478) getCachedAiReview reuses a cacheable row with NO maxAgeMs and replays its findings verbatim, including a severity:critical ai_consensus_defect. AI_REVIEW_CACHE_INPUT_VERSION was hand-bumped and last moved for #8364, but #8789, #8791, #8833, #8845, #8961, #9035, #9074, #9087, #9114, #9145 and #9445 all changed prompt text or verdict logic without one -- so a PR closed or held under the pre-#9074 false-consensus rule re-gated at the same head to the SAME stale finding and the corrected logic never ran. The fingerprint now folds in REVIEW_PROMPT_VERSION and a digest of the canonical judge prompt, both pure and available before the call, so drift is an automatic miss rather than something a human must remember. Bumped to v7 to kill rows written under the incomplete fingerprint. Ollama silently left-truncates past num_ctx (typically 4k-8k by default) and then answers confidently over whatever survived -- the TAIL of the prompt, i.e. the context sections rather than the diff. The review path sends up to 120k chars of diff plus a 240k-char context budget, so on the fallback provider a review could come from a fraction of the change while carrying the same blocker authority and confidence semantics as the primary. An explicit num_ctx is now sent for the ollama provider only, since other OpenAI-compatible servers may reject an unknown options key, and an explicit caller providerOptions still wins. runWorkersOpinion iterates [primary, fallback] internally and its outcome carried no model identity, so a fallback-produced review was recorded as a PRIMARY vote -- poisoning the reviewer_vote audit events, #8229's routing track records, and scoreJudgmentAgreement's contribution to decision-record confidence. The doc's claim that slot-to-model is unambiguous by construction holds for the tie-break swap, not for in-slot fallback.
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
Logic backtestReplayed 0 historical case(s) for Backtest comparison:
|
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 26e6ca5 | Commit Preview URL Branch Preview URL |
Jul 28 2026, 05:11 AM |
Bundle ReportChanges will increase total bundle size by 44 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
…r log, and cap the third advisory (#9489, #9491) (#9514) * fix(ledger,drift): retry an unanchored tip, stop truncating the anchor log, and cap the third advisory (#9489, #9491) loadLastLedgerAnchorAttempt deliberately returns the newest attempt regardless of status so the scheduler advances to newer tips rather than hammering a stale checkpoint -- but that made a FAILURE at a QUIET tip unrecoverable. Rekor 429s at seq N, the ledger goes quiet, and every hourly tick then sees tipUnchanged and returns "unchanged", so the tip carries no valid external anchor indefinitely: exactly the unanchored window the feature exists to bound. It was also backend-blind, since the newest row won regardless of which backend wrote it, so git succeeding masked rekor failing at the same seq. The scheduler now asks per backend whether THIS rowHash has a successful anchor, and retries on the hourly tick when it does not. loadDecisionLedgerTip read COUNT(*) and the tip row as two statements under Promise.all. The seq/totalCount pair is precisely how a verifier detects truncation-and-rechaining, so a concurrent append between them produced totalCount = seq + 1 beside the OLD rowHash -- an internally inconsistent checkpoint that anchoring then signs and publishes to Rekor, unretractably, as a FALSE tamper signal about the maintainer's own ledger. One statement now reads both from a single snapshot. For a file between 1 MB and 100 MB the Contents API returns content:"" with encoding:"none", and because typeof "" === "string" the old code accepted that as the file's contents and rewrote the whole anchor log to a single line while reporting ok. At ~300 bytes per line hourly that lands 4-5 months out; git history keeps the commits but the file a skeptic is told to read shrinks, which is indistinguishable from tampering. It now refuses and records a failed attempt, leaving the log untouched and the gap visible on the public attempt log. The linked-issue satisfaction advisory was the one paid LLM call in its family with no per-PR commit cap -- ai_slop and ai_review both stop past auto_pause_after_reviewed_commits -- so a long-lived PR kept paying for a fresh assessment on every push after its siblings had stopped. * fix(ci): unpin a hardcoded cache-input version that broke main, and cover the new ledger paths queue-4 asserted /^ai-review-input:v6:/ against a literal, so the v6 -> v7 bump in #9513 broke it on main. It now asserts against AI_REVIEW_CACHE_INPUT_VERSION itself: the property that matters is that the fingerprint carries the CURRENT version, not which version that happens to be, so a future bump cannot break it again. Adds per-backend anchor-lookup coverage (a failed attempt is not anchored; one backend's success does not mask another's failure; a success at a different rowHash does not anchor this tip), the three Contents-API body shapes, and the commit-threshold cap regression. Two fail-open catches are annotated: an unreadable counter must not SUPPRESS the advisory, and an unreadable anchors table degrades to the pre-#9489 scheduling behaviour rather than forcing an anchor every tick.
Summary
Three ways the AI review pipeline produced or preserved a verdict that did not reflect the current code, the current policy, or the model that actually decided.
Closes #9477
Closes #9478
#9477 — superseded verdicts replayed forever
getCachedAiReviewreuses acacheable=1row with nomaxAgeMs, replaying its findings verbatim — including aseverity: "critical"ai_consensus_defect.AI_REVIEW_CACHE_INPUT_VERSIONis hand-bumped, and last moved for #8364. Since then #8789, #8791, #8833, #8845, #8961, #9035, #9074, #9087, #9114, #9145 and #9445 all changed prompt text or verdict logic without a bump.REVIEW_PROMPT_VERSIONis stillv1despite its own doc instructing a bump on any verdict-surface change, which also silently no-ops #8222's counterfactual replay gate.Concretely: a PR closed or held under the pre-#9074 "false consensus" rule, re-gated at the same head, hits an identical fingerprint and replays the old critical finding and its close rationale. The corrected logic never runs for any already-reviewed head.
The fingerprint now folds in
REVIEW_PROMPT_VERSIONandsha256(buildCanonicalJudgePrompt()). Both are pure and available before the call, so drift becomes an automatic miss rather than something a human has to remember — the mechanism that has already failed eleven times. Bumped tov7to invalidate rows written under the incomplete fingerprint.#9478a — the fallback got a prompt it could not fit
Ollama silently left-truncates anything past
num_ctx(typically 4k–8k by default) and then answers confidently over whatever survived — which is the tail of the prompt, i.e. the context sections rather than the diff. The review path sends up to 120k chars of diff plus a 240k-char aggregate context budget.So during a Claude outage a review could be produced from a fraction of the change while carrying the same blocker authority and the same
confidencesemantics againstaiReviewCloseConfidenceas the primary, with nothing in the verdict surfacing which provider decided.An explicit
num_ctxis now sent — for theollamaprovider only, since other OpenAI-compatible servers may reject an unknownoptionskey and only Ollama has this truncation behaviour. An explicit callerproviderOptionsstill wins, so the vision path's ownnum_ctxis untouched. Overridable viaOLLAMA_NUM_CTX, because the window trades GPU memory against how much of a large diff the model can see.#9478b — votes attributed to the wrong model
runWorkersOpinioniterates[primary, fallback]internally, andReviewerOpinionOutcomecarried no model identity — so a fallback-produced opinion was recorded as a primary vote. Those votes becomereviewer_voteaudit events and feed #8229's evidence-weighted routing track records andscoreJudgmentAgreement's contribution to decision-record confidence, so the calibration data was quietly wrong whenever the primary failed over.The existing doc claimed slot↔model is "unambiguous by construction". That holds for the tie-break slot swap; it does not hold for in-slot fallback. The outcome now carries
producedBy.Scope
Not closing #9478's "tier the fallback's authority" half. Routing a fallback-served lone blocker to
hold_for_reviewinstead of a one-shot close is a policy change to the gate's disposition, not a correctness fix, and it deserves its own decision — especially now that the context bound removes the truncation that made the fallback's verdict untrustworthy in the first place. Flagged on the issue rather than bundled in silently.Validation
npx tsc --noEmit -p tsconfig.json— cleanai-review-cache-input,ai-review,selfhost-aiRegressions:
REVIEW_PROMPT_VERSIONbump also invalidates.num_ctxis sent forollama.Invariants:
openai/openai-compatible/unknown providers, and a caller's ownproviderOptionsalways wins.OLLAMA_NUM_CTXfalls back above the truncation-prone stock defaults.