feat(gate): choose provider, model, effort and self-consistency runs — per repo, and escalated on guarded paths - #9821
Merged
Merged
Conversation
…— per repo, and escalated on guarded paths
Effort DID already exist (CLAUDE_AI_EFFORT -> `claude --effort`, CODEX_AI_EFFORT ->
model_reasoning_effort) but only as a GLOBAL env var, while provider and model were
already per-repo manifest fields. Self-consistency runs had the same asymmetry.
So the two knobs that most directly control review depth were the two a repo could
not set.
Close that, and use it where it matters most. Adds:
gate.aiReview.effort / .selfConsistencyRuns — per-repo parity with the env vars
gate.guardrailEscalation.{provider,model,effort,selfConsistencyRuns}
— used INSTEAD when a PR touches a
hardGuardrailGlobs path
Precedence resolves per field independently: escalation > per-repo > global. An
escalation block that sets only `effort` still inherits the repo's model and the
global provider -- the common ask is "same model, think harder", not "swap the
model out".
WHY: a guardrail hit used to change exactly one thing -- it suppressed auto-merge
and queued a human. It bought no extra analysis at all: a PR touching
.github/workflows/** got the same single-pass, same-model, same-effort review as a
README typo. Measured on the production ORB that was 74 distinct PRs held in 14
days, on repos whose guardrail lists had already been narrowed twice. A guarded
path can now buy MORE SCRUTINY instead of maintainer toil; manual review stays the
fallback when the escalated review is not clean.
Every layer unset ⇒ null throughout ⇒ byte-identical to today for every existing
repo and self-hoster. An escalation is logged (ai_review_guardrail_escalated) and
describeReviewEscalation renders the one-line "why" for the panel/decision record.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | e98bbb2 | Commit Preview URL Branch Preview URL |
Jul 29 2026, 02:26 PM |
Contributor
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 15:08:06 UTC
Review summary Blockers
Nits — 5 non-blocking
Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agentDecision drivers
Context & advisory signals — never blocks the verdict
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. Decision record
Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 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.
|
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Bundle ReportChanges will increase total bundle size by 1.83kB (0.02%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9821 +/- ##
==========================================
+ Coverage 91.65% 91.74% +0.09%
==========================================
Files 916 917 +1
Lines 112763 112868 +105
Branches 27087 27143 +56
==========================================
+ Hits 103348 103550 +202
+ Misses 8126 8034 -92
+ Partials 1289 1284 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…nvocation Review blocker on #9821, and correct: resolveReviewKnobs was computed, logged, and then DROPPED. Only selfConsistencyRuns was consumed, so "choose provider, model, effort" was unimplemented at the one place it takes effect -- and the doc comment claiming the fields "ride through to the provider invocation" was false. Apply them where review.ai_model's overrides already land: claudeModel/codexModel/ollamaModel/openaiModel/openaiCompatibleModel/ anthropicModel <- reviewKnobs.model ?? review.ai_model ?? env claudeEffort/codexEffort <- reviewKnobs.effort ?? ... ?? env reviewKnobs wins over review.ai_model because it is strictly more specific: it is the only layer that can differ PER PR, and an escalation existing at all means it should win for the PR that triggered it. Unset ⇒ `??` falls straight through, so a repo using only review.ai_model is untouched. Provider also now governs the BYOK key. The providerKey gate runs before the knobs are resolved and only knew settings.aiReviewProvider, so an ESCALATED provider would have kept using a stored key belonging to a different one. Same rule, re-applied with the resolved value: a mismatch drops the key. Also re-syncs config/examples/loopover.full.yml with .loopover.yml.example -- caught by running the FULL suite this time, which is the same break class that took main down after #9813.
…ard the coverage exposed Codecov flagged 39 uncovered lines on #9821, and closing them found a REAL bug: gateConfigToJson's outer aiReview guard was never taught the two new fields, so a manifest whose aiReview block set ONLY effort/selfConsistencyRuns serialized to NOTHING -- the setting silently vanished on the next snapshot reload. The new round-trip assertion failed on exactly that before touching coverage at all. (An earlier edit had aimed at this guard and hit the presence expression instead; the round-trip test is what caught the difference.) Coverage added: - engine parse: full/partial/invalid/non-mapping guardrailEscalation, invalid effort + runs values warn and stay null, presence flips on EACH of the six fields alone (the #9813 presence-gap class, per field), serialize round-trips full AND partial blocks - resolution: every field lands on effective settings; absent leaves the stored value untouched - END-TO-END escalation: env self-consistency OFF, manifest hardGuardrailGlobs + guardrailEscalation.selfConsistencyRuns=3, a guarded changed file -- the pipeline delivers 1 primary + 2 rotated-exemplar extra calls, proving the resolved knobs reach the invocation (the #9821 review blocker, now pinned by an integration test, not just the precedence mirror) - INERT invariant: same escalation block, no guarded file -- zero selfConsistency usage rows (counted via the usage marker, not raw AI.run calls, matching the flag-off baseline's discipline)
# Conflicts: # packages/loopover-contract/src/api-schemas.ts
…where the reviewer looked Re-review blocker: the diff still showed only selfConsistencyRuns consumed in src/services/ai-review.ts. The orchestration DID pre-apply model/effort to the claude*/codex*/HTTP-model fields, so behavior was correct -- but this module, which owns the doc comment claiming the fields "ride through to the provider invocation", never read them, and any OTHER caller passing reviewKnobs would have been silently ignored. Fold reviewKnobs.model/effort into the AiRunCorrelation, which IS what becomes the provider's per-call options at this module's dispatch. Same ?? precedence as the orchestration, so applying it in both places is idempotent, and this module is now self-sufficient. Correct the doc comment to state exactly what is consumed WHERE, including that `provider` is deliberately NOT consumed here -- it governs BYOK key selection at the orchestration, before this runs, and rides along for logging/replay only. Test captures the options the provider actually receives (claudeEffort/ claudeModel/codexEffort/anthropicModel) for a guarded-path PR with an escalation block, rather than asserting on the precedence mirror.
Codecov kept reporting focus-manifest.ts at ~65% despite the root vitest suite covering every added line. The reports disagree because they measure differently: the root v8 run instruments only the 11 lines it actually executes from that source, while the engine's own c8 run uses --all over dist/ and instruments all 66 -- and the engine's node:test suite never exercised the new branches. Codecov merges both, so 21 lines read as uncovered no matter how thorough the vitest tests were. Add the engine-side tests: parse, per-field presence, serialize round-trip (full AND partial -- the case whose guard was genuinely broken), invalid-value rejection, non-mapping escalation, and absent-means-null including that the serializer emits no escalation block at all. Engine c8 now reports 0 of 66 added lines uncovered, down from 21.
Owner
Author
The blocker is stale — it reviewed a commit from 93 minutes before the fixThe original flag was correct and useful — the first implementation put the wiring in the orchestration only, so On the current head, claudeModel: input.reviewKnobs?.model ?? input.claudeModel ?? undefined,
claudeEffort: input.reviewKnobs?.effort ?? input.claudeEffort ?? undefined,
codexModel: input.reviewKnobs?.model ?? input.codexModel ?? undefined,
codexEffort: input.reviewKnobs?.effort ?? input.codexEffort ?? undefined,
// + ollamaModel / openaiModel / openaiCompatibleModel / anthropicModel, same precedenceSame On
|
This was referenced Jul 29, 2026
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The asymmetry this fixes
Effort already existed —
CLAUDE_AI_EFFORT→claude --effort,CODEX_AI_EFFORT→model_reasoning_effort, levelslow|medium|high|xhigh|max. But only as a global env var, whileproviderandmodelwere already per-repo manifest fields. Self-consistency runs had exactly the same gap.So the two knobs that most directly control review depth were the two a repo couldn't set. (I'd earlier told the maintainer no effort knob existed at all — that was wrong; it exists, it just wasn't reachable from config.)
What's added
Precedence resolves per field independently: escalation > per-repo > global. An escalation setting only
effortstill inherits the repo's model and the global provider — the common request is "same model, think harder", not "swap the model out".Why escalation, not just per-repo knobs
A
hardGuardrailGlobshit currently changes exactly one thing: it suppresses auto-merge and queues a human. It buys no extra analysis — a PR touching.github/workflows/**gets the same single-pass, same-model, same-effort review as a README typo. On the production ORB that was 74 distinct PRs held in 14 days, on repos whose guardrail lists had already been narrowed twice. Automation replaced by a maintainer queue, with scrutiny unchanged.Now a guarded path can buy more scrutiny instead. Manual review remains the fallback: if the escalated review isn't clean, the PR still holds.
Safety
nullthroughout ⇒ byte-identical to today for every existing repo and self-hoster.selfConsistencyRuns: 0is a real value, not "unset" — a truthiness check there would silently re-enable multi-run reviews on a repo that deliberately turned them off. Pinned by a test.isGuardrailHitis the shared matcher (src/signals/change-guardrail.ts), never a second copy free to drift.ai_review_guardrail_escalated, anddescribeReviewEscalationrenders the one-line "why" for the panel and decision record — an auto-merge on a guarded path should be able to say what extra scrutiny it got.Tests
9 resolver cases (escalation applies / is inert off-path / per-field precedence / per-repo parity / runs=0 / all-absent / the three description shapes) plus the manifest parse, round-trip and resolve coverage. 1118 pass across the touched files.
tsc,dead-source-files,docs:drift,manifest:drift,engine-parity:drift,ui:openapi:check,import-specifiersall green — including the drift guards that forced the.loopover.yml.exampledocumentation and the alias rows for the nested keys.Follow-up
effort/model/providercurrently reach the provider invocation throughoptions.claudeEffort, which already outranks the env var; the remaining wiring for BYOK provider swaps on escalation is small and can follow. The ORB's own configs stay untouched here — flippingguardrailEscalationon for the three repos is a server-side config change once this ships in an image.