Fix ollama model for plan/agent mode

[ctrysbita]

This PR fixes microsoft/vscode#293071

PR #2782 introduces the issue by passing the wrong variable (the raw model list) into byokKnownModelsToAPIInfo , this PR fixes it by passing the correct knownModels

By the way, this PR also fixes crash on undefined usage when using ollama models

[Copilot]

Pull request overview

This PR fixes a bug introduced in PR #2782 where the Ollama provider was passing the wrong variable type to byokKnownModelsToAPIInfo. The issue prevented Ollama models from working correctly in plan/agent mode.

Changes:

• Fixed the getAllModels method in OllamaLMProvider to pass the correct knownModels variable instead of the raw models array

[Andygol]

@ctrysbita could you accept CLA to give a go to this PR 🙏

[ctrysbita]

@microsoft-github-policy-service agree

[CristianVGdev]

Corrects the input to byokKnownModelsToAPIInfo.

/api/tags is a listing endpoint and was being treated as a normalized model definition. Passing knownModels instead matches the actual data contract and avoids relying on incomplete Ollama metadata.

This is consistent with the function signature, which explicitly expects a BYOKKnownModels map:

export function byokKnownModelsToAPIInfo(
  providerName: string,
  knownModels: BYOKKnownModels | undefined
): LanguageModelChatInformation[] {
  if (!knownModels) {
    return [];
  }

  return Object.entries(knownModels).map(([id, capabilities]) =>
    byokKnownModelToAPIInfo(providerName, id, capabilities)
  );
}

[CristianVGdev]

@ctrysbita Suggestion: avoid bind + permanent no-op for usage

Current:

usage = this.forward(this._wrapped.usage?.bind(this._wrapped) || (() => { }));

Proposal:

usage = this.forward((...args: any[]) => this._wrapped.usage?.apply(this._wrapped, args));

Reason:

• Fixes the original problem (bind on undefined) while maintaining the behavior of flush() through forward.
• Avoids capturing a null operation at build time (if usage is added later in some implementations/flags, this will start working without rebuilding the flow).
• Preserves the correct this without bind.

If this causes any strange behavior in downstream flow, I think your original patch would remain. This is primarily a “worth trying” improvement for optional proxy methods.

[rajasrijan]

Facing the same issue. Is there a work around i can use till the fix is propagated to release?

[ctrysbita]

@hediet Hi, could you have a review on this?

[CristianVGdev]

@ctrysbita

@hediet Hi, could you have a review on this?

The more you push into the pipeline, the more reviews will be required, and you need two.
That said, I mentioned the issue of avoid bind + permanent no-op for usage, and you haven't responded to that either.

[CristianVGdev]

Fixed on #3566

[ctrysbita]

@ctrysbita

@hediet Hi, could you have a review on this?

The more you push into the pipeline, the more reviews will be required, and you need two. That said, I mentioned the issue of avoid bind + permanent no-op for usage, and you haven't responded to that either.

Actually it always need one reviewer with write permission.
For the usage, if the wrapped upstream doesn't have a usage implementation it should remain stable during the session so I just want to keep it simple and consistent with existing code like workspaceEdit above.

[ctrysbita]

Fixed on #3566

Good, let's keep track on that PR.