Repository navigation
fix(core): wait for the catalog refresh when an explicit model is missing - #57101
reynold-hu wants to merge 1 commit into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fixes #56119. I ran the issue's reproduction with a 3s delay on the catalog. On the base, opencode/repro-only-model is rejected with provider.no-route. With this PR the model resolves locally and the request reaches upstream. A model that exists nowhere still fails with ModelUnavailableError. A failed refresh still opens the latch, so nothing hangs. The changed tests pass and packages/core typechecks.
Public API: unchanged. ModelsDev.Interface.ready and the optional awaitCatalog parameter are internal to packages/core.
One concern, in the inline comment: the extra wait applies to every missing model, not only during startup.
Related: #51737
| // The bundled catalog is only the boot floor; a model added since the last snapshot is | ||
| // supplied by the initial refresh. Wait for that attempt, then give the provider catalog a | ||
| // moment to apply it before reporting the selection as unavailable. | ||
| yield* awaitCatalog().pipe(Effect.timeout("10 seconds"), Effect.ignore) |
There was a problem hiding this comment.
After the first refresh has settled, ready returns immediately, but every miss still polls for the full second before failing. So the extra latency doesn't stay limited to the startup race. A model that is really missing (wrong id, provider not authenticated) takes about 1s longer to fail for the rest of the process. SessionContext.selectTitle also resolves the preferred small model through this path with orElseSucceed. When that model isn't available, title selection now pays the 1s on every session, and up to about 11s during a slow first refresh.
Could the wait and re-sample only happen when the first refresh hadn't settled at the time of the miss? For example, check the latch state first and fail right away once it's open. Then steady-state misses fail fast, and only the race in #56119 waits. Separately, the re-sample uses Date.now() and a fixed 1s window, a guess at how long the provider/model catalog takes to apply Refreshed. If the models.dev plugin's update is ever slower, this will be flaky.
3b9f8b6 to
09972c6
Compare
Issue for this PR
Closes #56119
Type of change
What does this PR do?
The bundled models.dev snapshot is only the boot floor; the initial refresh runs in the background. Explicit model resolution sampled the catalog once, so a fresh
run --standalone --model …could reject a valid model withprovider.no-routewhile that refresh was still in flight.ModelsDevexposesready, which resolves after the first refresh attempt settles (or immediately when no refresh is scheduled, e.g.fetch: falseor a file catalog).SessionRunnerModel.resolvetakes an optionalawaitCatalogcallback. When the selected model is missing, it waits for that attempt (capped at 10s), then re-samples the catalog for up to 1s before reportingModelUnavailableError. Bundled models still resolve without waiting, and a refresh failure opens the latch so the run fails instead of hanging.SessionContextpassesModelsDev.readyfor the explicit session model path. Compaction's agent-model resolution keeps its current behavior.How did you verify your code works?
packages/core/test/models.test.ts:readycompletes after the first refresh attempt and the mock catalog fetch ran.packages/core/test/location-layer.test.ts: a real catalog model; the first availability sample returns empty (the pre-refresh race),resolvewaits and retries, then resolves after the catalog supplies it. On the parent the same resolution rejects withModelUnavailableError.packages/coresuite: 5551 pass, 24 fail; the remaining failures are the same environment/timeout set as an unmodified checkout on this Windows machine.packages/coretypecheck passes.SessionContextnow replaceModelsDevwith the offline fixture (title, generate, runner, recorded), so the new dependency cannot attempt network access.Screenshots / recordings
No UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #51737