Repository navigation
Conversation
ghost
mentioned this pull request
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
github-actions
Bot
added
the
awaiting-review
edelauna
left a comment
edelauna
left a comment
There was a problem hiding this comment.
Has a test comment, and implementation comment, to see if we could reduce duplication.
| if (!this.modelFetchPromise) { | ||
| this.modelFetchPromise = getOllamaModels(this.options.ollamaBaseUrl, this.options.ollamaApiKey) | ||
| .then((models) => { | ||
| this.models = models | ||
| return this.getModel() | ||
| }) | ||
| .finally(() => { | ||
| this.modelFetchPromise = undefined | ||
| }) | ||
| } | ||
|
|
||
| return this.modelFetchPromise |
There was a problem hiding this comment.
This single-flight fetch looks like a near-verbatim copy of RouterProvider.fetchModel/ensureModelFetched at router-provider.ts:59-86 — same modelFetchPromise guard, .then/.finally shape. Both providers extend BaseProvider. Worth extracting a shared helper so the two stay in sync, or is keeping them side-by-side intentional?
| }) | ||
| }) | ||
|
|
||
| describe("ensureModelFetched", () => { |
There was a problem hiding this comment.
This unit test proves ensureModelFetched populates contextWindow, but no test asserts the fetched value actually reaches context management — Task.spec stubs getModel to a fixed 200k, so nothing guards against the 128k fallback being used instead of the fetched window, which is exactly the symptom this PR fixes. Worth an integration assertion that the fetched value reaches modelInfo.contextWindow?
github-actions
Bot
added
awaiting-author
github-actions
Bot
added
awaiting-author
… integration test Address PR review comments: - Extract the single-flight model metadata fetch into BaseProvider.fetchModelWithSingleFlight and use it from both NativeOllamaHandler and RouterProvider so the two stay in sync. - Add a Task integration test asserting the fetched model context window reaches manageContext instead of the 128k fallback.
github-actions
Bot
added
awaiting-author
github-actions
Bot
added
has-conflicts
Review statusThanks for contributing. This comment tracks the review sequence and the next action. Current step: Resolve the merge conflicts. The review sequence resumes after the branch is mergeable. Review-state labels are managed by this workflow; do not edit them manually. |
|
This PR has been awaiting author changes for 14 days and will be automatically closed in 7 days. Please address the review comments or leave a comment if you need more time. |
|
Hi @navedmerchant and @edelauna! I'd like to follow up on this fix, as the issue is still affecting native Ollama users with large-context models. My setup uses ollama cloud with a reported context window of 1,000,000 tokens. The UI correctly detects 1M, but after restarting VS Code or restoring a task, context condensation can happen around 125k due to the 128k fallback. As of v3.92.0, the underlying issue appears to remain unresolved. I noticed that this PR has been blocked by merge conflicts for over two months and is now at risk of automatic closure. Could someone rebase/update this PR, or could another contributor take it over? The fix and regression tests already seem largely implemented, and losing context prematurely is quite disruptive for long coding sessions. Thanks for your work on Zoo Code! |
What changed
Native Ollama now fetches model metadata before task context management reads the model limits. The fetch is single-flight, avoids repeat requests once metadata is populated, and can recover after a failed request.
Focused regression coverage verifies that a detected 1M context window replaces the 128k fallback before a model request, while preserving duplicate suppression and retry behavior.
Why this change was made
After VS Code restarted or rebuilt the API handler, existing Ollama tasks could condense around 125k because context management ran before native Ollama fetched
/api/showmetadata.Closes #1151.
Impact
Ollama tasks now use the model's detected context window for condensing decisions from the start of a resumed task, avoiding premature context loss for large-context models.
Related PRs