Skip to content

fix(autoreview): fall back to Fast when Ultrafast is not advertised - #343

Open
MatiasGigena wants to merge 1 commit into
openclaw:mainfrom
MatiasGigena:fix/autoreview-fast-fallback
Open

MatiasGigena wants to merge 1 commit into
openclaw:mainfrom
MatiasGigena:fix/autoreview-fast-fallback

Conversation

@MatiasGigena

@MatiasGigena MatiasGigena commented Oct 27, 2026 •

Copy link
Copy Markdown

Summary

When the selected Codex model advertises Fast but not Ultrafast, the current default requests Ultrafast and Codex can omit the unsupported tier, leaving the review on standard service. This change probes the model catalog using the isolated Codex runtime and requests Fast for that case.

implicit default + catalog advertises Fast only → request Fast
explicit speed, custom command-auth route, or unavailable catalog → preserve existing selection

The probe runs before the review, never consumes a review result, and preserves explicit CLI, environment, and raw configuration overrides. Catalog metadata describes advertised model support; it does not prove account entitlement or the tier actually delivered by the provider. The choice is made separately for each model attempt.

Evidence

  • A live local P2 review using the proposed helper selected gpt-6.1-sol, reported that Ultrafast was not advertised, requested Fast, and correctly identified the seeded shipping-boundary regression. This observes the requested tier, not a provider receipt.
  • Focused catalog selection and command-auth route tests: 33 passed. The isolated-runtime/auth regression passes with the additional catalog invocation and unchanged auth contents/link count.
  • scripts/validate-skills, git diff --check, npm run typecheck, and the Node test lane (120 tests) pass.
  • Full python3 scripts/check-skills rerun passed: 663 Python tests (7 skips), repository validators, typecheck, and 120 Node tests. The first run exposed an assertion that expected two Codex invocations; the updated isolation test now checks all three and verifies unchanged auth. It also saw an intermittent executable-discovery failure that reproduced on unmodified base 2882b90; the full rerun passed.

Merge Danger

Door: Two-way. Reverting restores the existing default selection.

Blast Radius: Codex reviews using the implicit default speed. A catalog probe can add up to 10 seconds per attempt; probe failure leaves the existing default unchanged. Existing explicit speed settings and command-auth routes are unchanged.

@clawsweeper

clawsweeper Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 27, 2026
@clawsweeper

clawsweeper Bot commented Oct 27, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge.

What this changes

This PR requests Fast when an implicitly selected Codex speed is Ultrafast but the model catalog advertises only Fast.

Example: A gpt-6.1-sol catalog entry advertises additional_speed_tiers=["fast"], with no explicit speed override.

  • Before: Autoreview requests Ultrafast; documented Codex behavior can leave the review on standard service.
  • After: Autoreview requests Fast for that model attempt.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The focused patch has no identified correctness defect, but inspectable runtime proof is absent and the default-selection direction awaits an owner decision.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Preference · Worth it: Needs a maintainer decision · Fix scope: Complete
User problem: Users seeking fast reviews can receive standard service when their model advertises Fast but not the implicitly requested Ultrafast.
Reason: The proposal changes an intentional, documented fallback; the speed owner must decide whether automatic tier substitution is desirable.

Merge readiness

⛔ Blocked before merge - 2 items remain

Keep this PR open: its catalog-based fallback is distinct work absent from current main, and no introduced correctness defect was identified.

Priority: P2
Reviewed head: 98d8dac6bd01a1cb2db6b6c873e62534cfb28898
Owner decision: Required. See Decision needed.

Decision needed

  • Question: Should an implicit Ultrafast request automatically become Fast when the model catalog advertises only Fast?
  • Recommendation: Approve capability-based fallback: Allow this narrow provider-specific selection while preserving explicit settings and unavailable-catalog behavior.
  • Why: Current documentation deliberately permits standard service for an unsupported tier, and no owner decision authorizes changing that fallback.

Before merge

  • Add real behavior proof - The body describes gpt-6.1-sol selecting Fast during a live local review, but provides no inspectable terminal output, logs, recording, or artifact connecting the catalog probe to the shipped review invocation. Attach that trace with CLI version and environment, including fresh-install and latest-stable upgrade behavior with explicit settings intact; redact credentials, IPs, and non-public endpoints. After adding proof, update the PR body, then request a new review with @clawsweeper re-review. The PR author or someone with repository write access can request it.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

None.

Tests

  • Low-value test skills/autoreview/scripts/autoreview_test.py: Remove or replace test_selection_applies_to_command_without_changing_explicit_settings: it manually sets the internal selection attribute and checks a helper rather than constructing a review command, while explicit precedence already has coverage.
  • Missing end-to-end proof: Supply the described real CLI trace showing Fast-only catalog selection and successful review, plus fresh-install and latest-stable upgrade runs preserving explicit speed settings; base-fail/head-pass regression evidence is not supplied.
Agent review details

How this fits together

Autoreview builds isolated Codex CLI invocations from reviewer settings and authentication configuration, then consumes the resulting review report.

flowchart TD
  A[Reviewer settings] --> B[Isolated Codex runtime]
  B --> C[Model catalog probe]
  C --> D[Per attempt speed selection]
  A --> D
  D --> E[Codex review invocation]
  E --> F[Review report]
Loading

Technical review

Best possible solution:

Adopt catalog-based fallback only with the speed owner's approval, preserving explicit settings and the existing unavailable-catalog behavior.

Do we have a high-confidence way to reproduce the issue?

Source confirms that current main always requests Ultrafast implicitly, and its documentation describes standard service when unsupported; actual provider delivery was not independently observed.

Is this the best way to solve the issue?

The implementation is focused and preserves existing controls, but automatic Fast selection changes documented behavior rather than repairing a demonstrated contract violation.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 2882b9054f3b.

Provenance checked

  • Implicit Codex service-tier selection changes intended behavior with a stated reason (ee74808: Deliberately default reviews to medium reasoning and Ultrafast while preserving explicit speed and raw service-tier overrides.)
  • Unsupported-tier documentation changes intended behavior with a stated reason (3d3034a: Expose Ultrafast explicitly and document Codex's standard-service fallback when model metadata does not advertise it.)
  • Isolated Codex runtime and authentication regression keeps the original intent (fix(autoreview): validate Codex isolation preflight #194: Ensure startup probes and review execution use the same isolated environment without changing source authentication.)

Testing

Proof path: unit tests only.

Security

None.

Evidence

What I checked:

Likely related people:

  • Peter Steinberger: Raw commit ee74808 adds skills/autoreview/scripts/autoreview:5345 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: ee748080a927; files: skills/autoreview/scripts/autoreview)
  • Vincent Koc: Raw commit 1b52afd adds skills/autoreview/scripts/autoreview:3427 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 1b52afd09545; files: skills/autoreview/scripts/autoreview)

Labels

Label changes:

  • add P2: This is a limited improvement to implicit review service-tier selection; no broken review workflow or urgent regression is established.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Label justifications:

  • P2: This is a limited improvement to implicit review service-tier selection; no broken review workflow or urgent regression is established.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Attach the real CLI evidence described in the proof assessment.
  • Resolve the automatic tier-selection decision with the speed owner.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 27, 2026, 8:50 AM ET / 12:50 UTC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant