chore: speed cron model override coverage#106938
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 13, 2026, 8:38 PM ET / July 14, 2026, 00:38 UTC. Summary PR surface: Tests -241. Total -241 across 3 files. Reproducibility: not applicable. This PR addresses duplicated test execution rather than a runtime defect, and the performance comparison is documented with Testbox runs. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused owner-boundary assertions and remove the duplicate temp-home suite once exact-head CI confirms every distinct contract remains covered. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR addresses duplicated test execution rather than a runtime defect, and the performance comparison is documented with Testbox runs. Is this the best way to solve the issue? Yes. Moving distinct assertions to the model-selection and forwarding owner suites, then deleting the overlapping integration harness, is cleaner than retaining duplicate end-to-end-style coverage. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against cae29a079203. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Tests -241. Total -241 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Land-ready proof:
|
|
Merged via squash.
|
What Problem This Solves
The cron model override shard repeated selection and forwarding coverage through a broad temp-home integration suite. That file took 7.58 seconds with warm transforms and 21.32 seconds on a cold Testbox run.
Why This Change Was Made
This removes the overlapping integration file and keeps its distinct contracts at their owner boundaries: Gmail hook precedence in the model-selection suite and default-thinking propagation in the run-forwarding suite. Existing focused tests already cover whitespace, payload/session precedence, invalid refs, catalog reasoning policy, and provider-authoritative stale-catalog behavior.
User Impact
No runtime or user-visible behavior changes. CI executes less duplicate test work while retaining the behavior coverage.
Evidence
pnpm check:changedpassed core-test types, formatting, lint, and static guards for the unchanged patch before the final rebase; exact-base focused tests reran after the rebase.