feat(review): route trivial diffs to a cheaper model - #6590
Draft
guyoron1 wants to merge 1 commit into
Draft
Conversation
Author type is a proxy for complexity, not complexity itself. A one-line human edit to versions.json runs on the same model as a refactor, because the harness fixes the model before anything has looked at the diff — the $0.40 approval in fullsend-ai#5777. A step ahead of the review agent classifies the diff and, when it is provably trivial, exports FULLSEND_MODEL so the review runs on a cheaper tier. It lives in the dispatch layer rather than a harness, so it applies to every enrolled repo without a per-repo change, and it keys on the diff rather than the author, so it catches human and bot PRs alike. Trivial means every changed file is a modified data/config file and the whole diff is under the threshold. The exclusions carry the safety: added, removed and renamed files are structural even when small; code never qualifies whatever its size; dependency lockfiles never qualify, because npm resolves from the lockfile and a one-line integrity swap is a supply-chain change wearing a trivial diff; and paths that execute or govern never qualify — a deliberately smaller list than REVIEW_PROTECTED_PATHS, which decides whether a review may auto-approve where this decides whether a cheaper model may form the opinion. Two repository variables tune it, since the issue is explicit that the thresholds need tuning: REVIEW_TRIVIAL_MODEL (default sonnet, 'off' disables) and REVIEW_TRIVIAL_MAX_LINES (default 10). Routing runs before setup-agent-env.sh, so an explicit REVIEW_FULLSEND_MODEL or FULLSEND_MODEL repository variable overwrites it — configuration outranks inference. Every uncertain path leaves the model untouched: an unreadable file list, a malformed threshold, a truncated listing, an empty diff. The failure mode of cost routing must be a more expensive review, never a missing one. Signed-off-by: guy oron <goron@redhat.com>
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
Site previewPreview: https://3f87bc3c-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
PR Summary by QodoRoute trivial review diffs to a cheaper model
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
1. PR files query misprojects
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+86
to
+87
| if ! FILES_JSON=$(gh api "repos/${SOURCE_REPO}/pulls/${PR_NUMBER}/files" \ | ||
| --paginate -F per_page=100 --jq '{filename, status, changes}' 2>/dev/null); then |
There was a problem hiding this comment.
2. Pr files query misprojects 🐞 Bug ≡ Correctness
The files endpoint returns arrays, but --jq '{filename, status, changes}' projects fields from
each page array instead of iterating .[], producing null-valued records that are always blocked.
The -F per_page=100 field also needs an explicit GET method or a query parameter; otherwise
routing fails open and never selects the cheaper model.
Agent Prompt
## Issue description
The production GitHub API invocation does not emit one `{filename, status, changes}` object per changed file. This prevents trivial PRs from being routed even though the tests pass.
## Issue Context
The pull-request files endpoint returns page arrays. Existing repository usage iterates those arrays with `.[]`, while the new test stub bypasses the real response shape and ignores command arguments.
## Fix Focus Areas
- .github/scripts/route-review-model.sh[86-87]
- .github/scripts/route-review-model-test.sh[25-26]
- .github/workflows/e2e.yml[133-138]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+68
to
+69
| def is_guarded: test("(^|/)\\.[^/]+/") or test("^(scripts|hack|agents|skills|harness|images|plugins|policies|profiles|providers|api-servers)/") | ||
| or test("^(CODEOWNERS|AGENTS\\.md|CLAUDE\\.md|Dockerfile|Containerfile)$"); |
There was a problem hiding this comment.
3. Governing configs route cheaply 🐞 Bug ≡ Correctness
is_guarded omits root dotfiles and unlisted root manifests, so small edits to executable or policy-bearing files such as action.yml, .goreleaser.yml, .pre-commit-config.yaml, and renovate.json qualify as trivial. This violates the routing safety rule that paths which execute or govern must never use the cheaper tier.
Agent Prompt
## Issue description
The guarded-path matcher is an incomplete denylist and allows multiple executable, release, validation, and dependency-automation configurations to be classified as trivial.
## Issue Context
The accepted extension list includes YAML and JSON. Root hidden files do not match the hidden-directory pattern because it requires a trailing slash, and operational manifests such as `action.yml` and `renovate.json` are absent from the root-file alternatives.
## Fix Focus Areas
- .github/scripts/route-review-model.sh[65-80]
- .github/scripts/route-review-model-test.sh[54-82]
- action.yml[70-79]
- renovate.json[1-17]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+163
to
+167
| # Complexity-based model routing (#5777). Runs before the setup step | ||
| # below on purpose: setup-agent-env.sh re-exports FULLSEND_MODEL from | ||
| # the REVIEW_FULLSEND_MODEL / FULLSEND_MODEL repository variables, so a | ||
| # repo that pins a model keeps it — explicit configuration outranks an | ||
| # inferred route. |
There was a problem hiding this comment.
4. Pinned model override ignored 🐞 Bug ≡ Correctness
reusable-review.yml now writes the inferred FULLSEND_MODEL, but its subsequent setup step does not receive FULLSEND_REPO_VARS, so REVIEW_FULLSEND_MODEL or FULLSEND_MODEL repository pins cannot overwrite it. Repositories using this workflow can therefore run reviews on the cheap model despite an explicit model selection.
Agent Prompt
## Issue description
The reusable review workflow activates inferred model routing without passing repository variables to the setup script that is supposed to restore explicit model pins.
## Issue Context
`setup-agent-env.sh` only processes model overrides when `FULLSEND_REPO_VARS` is populated. The dispatch workflow passes `${{ toJSON(vars) }}`, but the reusable review workflow does not.
## Fix Focus Areas
- .github/workflows/reusable-review.yml[163-183]
- .github/workflows/reusable-dispatch.yml[927-936]
- internal/scaffold/fullsend-repo/.github/scripts/setup-agent-env.sh[41-56]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
guyoron1
marked this pull request as draft
September 1, 2026 12:15
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.
Closes #5777.
Author type is a proxy for complexity, not complexity itself. The harness fixes the review model before anything has looked at the diff, so the one-line
versions.jsonbump in #5777 — human-authored, created through the web UI — was reviewed on Opus for $0.40 and correctly approved in about a minute. The verdict was right; the price was not.The existing cheap-review proposals (#2842, #2639, #3240, #3347) all gate on bot authorship, so none of them would have caught that PR. #1900 covers human-authored changes but only for docs/prompt-only Markdown. This routes on the diff instead, which catches both.
What qualifies
A step ahead of the review agent classifies the changed files and, when the diff is provably trivial, exports
FULLSEND_MODELso the review runs on a cheaper tier. It lives in the dispatch layer, so it applies to every enrolled repo without a per-repo harness change.Trivial means every changed file is a modified data/config file (
.json,.ya?ml,.toml,.txt,.md,.ini,.cfg,.conf,.properties) and the whole diff is under the line threshold.The exclusions are where the safety lives:
integrityswap is a supply-chain change wearing a trivial diff.github/,.claude/),scripts/,hack/,skills/,harness/,agents/,policies/, and root files likeCODEOWNERS,Dockerfile,CLAUDE.mdThat last list is deliberately smaller than
REVIEW_PROTECTED_PATHS. The two answer different questions: protected paths decide whether a review may auto-approve; this decides whether a cheaper model may form the opinion in the first place.Tuning and precedence
The issue is explicit that the thresholds need tuning, so both are repository variables:
REVIEW_TRIVIAL_MODELsonnetoffdisables routing entirely.REVIEW_TRIVIAL_MAX_LINES10Routing runs before
setup-agent-env.sh, which re-exportsFULLSEND_MODELfrom theREVIEW_FULLSEND_MODEL/FULLSEND_MODELrepository variables. A repo that pins a model therefore keeps it — explicit configuration outranks inferred routing, and the ordering is what enforces that rather than a special case in the script.Failure mode
Every uncertain path leaves the model untouched, so the harness default (the more capable model) stands: an unreadable file list, a malformed threshold, a truncated listing (GitHub caps the files endpoint at 3000 and stops paginating without erroring), an empty diff, a missing PR number. The failure mode of cost routing must be a more expensive review, never a missing one.
Testing
.github/scripts/route-review-model-test.shstubsghand runs the real script — not a paraphrase of its logic — across 16 cases, wired intomake script-test. It covers the issue's own example, each exclusion class, threshold accumulation across files, both repository variables, the opt-out, and every degrade-safely path.shellcheckandactionlintare clean; I diffed actionlint against pristineorigin/mainand this adds zero new findings (the 31 existing ones are thejob.workflow_shacontext actionlint doesn't know).go test ./internal/scaffold/passes, which covers the cross-workflow alignment since the step is mirrored intoreusable-review.ymlas well.On the validation criteria
Criteria 1, 2 and 4 are observable once this runs. Criterion 3 (≥50% cost drop) is worth measuring rather than assuming — the
$0.40baseline is a single data point from July, and per-dimension model tiering has landed since. The review agent's ownmetrics.jsonalready records per-run cost, so the number is available from the next trivial PR that goes through this path.