Repository navigation
fix(autoreview): fall back to Fast when Ultrafast is not advertised - #343
MatiasGigena wants to merge 1 commit into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs real behavior proof before merge. What this changesThis 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.
Review scores
ProductKind: Preference · Worth it: Needs a maintainer decision · Fix scope: Complete 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 Decision needed
Before merge
FindingsNone. Tests
Agent review detailsHow this fits togetherAutoreview 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]
Technical reviewBest 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
TestingProof path: unit tests only. SecurityNone. EvidenceWhat I checked:
Likely related people:
LabelsLabel changes:
Label justifications:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale6/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. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 27, 2026, 8:50 AM ET / 12:50 UTC. |
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.
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
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.scripts/validate-skills,git diff --check,npm run typecheck, and the Node test lane (120 tests) pass.python3 scripts/check-skillsrerun 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 base2882b90; 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.