Skip to content

fix(review): route refuter and validator through Pi's default model when unrouted - #2479

Merged
Alan-TheGentleman merged 1 commit into
mainfrom
fix/review-role-default-model
Oct 23, 2026
Merged

Alan-TheGentleman merged 1 commit into
mainfrom
fix/review-role-default-model

Conversation

@Alan-TheGentleman

@Alan-TheGentleman Alan-TheGentleman commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Linked Issue

Closes #2271

PR Type

  • Bug fix

Summary

  • review-refuter and review-validator are provider review roles, not installable agents: no agent file exists for them, nothing seeds their routing entries, and profiles never carry them. A fresh standalone install, or any profile pin or session binding, hit a typed reviewer-config-invalid refusal naming agents the package itself retired.
  • When the routing snapshot in effect (saved routing, pin, or session binding) has no model for one of these two keys, the host relay now uses Pi's persisted default model and effort (defaultProvider/defaultModel/defaultThinkingLevel), the default the routing panel already labels them with ("Pi persisted default model").
  • A configured entry still wins. Lens reviewers (review-risk, etc.) keep their fail-closed routing. With neither an entry nor a Pi default, the typed refusal is unchanged.
  • Copying the retired review-refuter.md/review-validator.md files into the agent home (the workaround reported in the issue) has no effect: reviewer routing never reads the agent home.

Changes

File Change
extensions/gentle-ai.ts reviewHostRelaySelection falls back to Pi's persisted default for the two provider review roles only
tests/review-host-relay-routing.test.ts Default used when unrouted, default used when a pin omits the role, configured entry wins, lens never borrows the default; the existing refusal test now runs against an isolated agent home with no default

docs/review-integration.md still says a missing entry is refused. That file is a vendored contract artifact pinned by hash to the Gentle AI runtime, so its wording has to change in Gentle AI.

Test Plan

  • node --test tests/review-host-relay-routing.test.ts: 53 pass, 0 fail (the new tests failed before the fix)
  • npx tsc --noEmit -p .: exit 0
  • npm test: 155 failures, the same set as on origin/main (pre-existing: editor/vim replay, config home, Pi baseline). None are in review routing.

Contributor Checklist

  • Conventional commit format
  • No Co-Authored-By trailers
  • Linked issue carries status:approved (it has no labels)

Summary by CodeRabbit

  • Bug Fixes
    • Review refuter and validator slots without a selected model can now use the saved default model and, when no effort is specified, the saved effort.
    • Explicit routing selections continue to take precedence. Other review slots do not inherit the saved default.

…hen unrouted

review-refuter and review-validator are provider review roles, not
installable agents: nothing seeds their routing entries and profiles never
carry them, so fresh installs and any profile pin or session binding hit a
typed reviewer-config-invalid refusal naming agents the package retired.
When the routing snapshot in effect has no model for one of these two keys,
the relay now uses Pi's persisted default model and effort, the default the
routing panel already labels them with. Lens reviewers keep fail-closed
routing.

Refs #2271
@Alan-TheGentleman Alan-TheGentleman added the type:bug Bug fix label Oct 23, 2026
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 0f74175b-a0d7-4a2c-a0cc-638a4f3b4608
📥 Commits

Reviewing files that changed from the base of the PR and between 05065e7 and 4a41c13.

📒 Files selected for processing: 2
  • extensions/gentle-ai.ts
  • tests/review-host-relay-routing.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Refuter and validator relay slots now use persisted orchestrator model settings when their routing entries have no model. Persisted effort is used only when the entry does not specify effort. Other routing keys do not use this fallback.

Changes: Relay model fallback

Layer / File(s) Summary
Relay selection fallback
extensions/gentle-ai.ts, tests/review-host-relay-routing.test.ts
Refuter and validator slots use persisted orchestrator settings when their routing entry has no model. An entry’s explicit model or effort takes precedence. Tests cover pinned profiles that omit the role and confirm that lens slots do not use the fallback.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

Possibly related PRs

Suggested reviewers: decode2, dnlrsls, barbatdev

Merge Risk: ⚪ Minimal · up to 4a41c

Unrouted refuter and validator roles can use Pi’s persisted default without changing lens routing. No merge-blocking issue was identified.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Docstring Coverage Inconclusive Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: unrouted review-refuter and review-validator roles now use Pi’s default model.
Linked Issues check Passed [#2271] The issue lists three acceptable outcomes. The PR uses the second: routing for the two provider review roles is derived automatically at review start. In extensions/gentle-ai.ts, `reviewHost…
Out of Scope Changes check Passed The whole-PR diff against the merge base changes two files. extensions/gentle-ai.ts adds the provider-role fallback inside reviewHostRelaySelection and a comment that cites #2271. `tests/review-ho…
Full details: Linked Issues check

Explanation

[#2271] The issue lists three acceptable outcomes. The PR uses the second: routing for the two provider review roles is derived automatically at review start. In extensions/gentle-ai.ts, reviewHostRelaySelection now falls back to Pi's persisted default model and thinking level (readOrchestratorSettings) when the effective entry for review-refuter or review-validator has no model. A configured entry still wins. The fallback is gated by isProviderReviewRole, so lens keys keep the fail-closed rule. The tests in tests/review-host-relay-routing.test.ts cover: the default used when no entry exists; the default used when a pin omits the role; a configured entry overriding the default; and a lens slot receiving no selection. Residual behavior: when no Pi default exists, the typed reviewer-config-invalid refusal still names the retired key. The issue does not require a different refusal text in that case, because the derivation outcome is satisfied. The docs/review-integration.md wording is not changed in this PR. The PR states that file is a vendored artifact and that the change belongs in Gentle AI. It is not a coding objective of #2271.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base changes two files. extensions/gentle-ai.ts adds the provider-role fallback inside reviewHostRelaySelection and a comment that cites #2271. tests/review-host-relay-routing.test.ts adds helpers (isolatedAgentHome, relayedRefuterRequest) and four tests for the fallback. It also changes the existing missing-review-refuter refusal test to run against an isolated agent home with no Pi default, so that test still exercises the refusal. Each change connects directly to the #2271 objective. No unrelated changes appear in the diff. The docs/review-integration.md note in the PR description is a statement, not a change in this diff.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 too large.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Alan-TheGentleman
Alan-TheGentleman merged commit 3197505 into main Oct 23, 2026
9 checks passed
@Alan-TheGentleman
Alan-TheGentleman deleted the fix/review-role-default-model branch October 23, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug:(ODD) Reviews call the retired review-refuter and review-validator agent roles

1 participant