Skip to content

Conversation

@PineArt
Copy link

@PineArt PineArt commented Dec 21, 2025

… variable support

  • Add environment variable support for Ollama configuration (OLLAMA_BASE_URL, OLLAMA_SERVER_URL, OLLAMA_API_KEY, OLLAMA_NUM_CTX, OLLAMA_MODEL_ID)
  • Implement proper configuration priority: environment variables > VS Code settings > defaults
  • Update lmstudio.ts to use refreshModels with baseUrl parameter
  • Refactor webviewMessageHandler to use new refreshModels function for forced cache refresh
  • Add normalizeToolSchema to ensure JSON Schema compatibility with Ollama's Go unmarshaler
  • Improve comments to clarify configuration behavior

🤖 Generated with Claude Code

Related GitHub Issue

Closes: #

Roo Code Task Context (Optional)

Description

Test Procedure

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Additional Notes

Get in Touch


Important

Refactor Ollama and LM Studio configurations to support environment variables, prioritize them over settings, and improve cache management with refreshModels.

  • Configuration:
    • Add environment variable support for Ollama (OLLAMA_BASE_URL, OLLAMA_SERVER_URL, OLLAMA_API_KEY, OLLAMA_NUM_CTX, OLLAMA_MODEL_ID) in native-ollama.ts.
    • Prioritize configuration: environment variables > VS Code settings > defaults.
  • Cache Management:
    • Replace flushModels with refreshModels in lmstudio.ts and webviewMessageHandler.ts for cache refresh with baseUrl.
  • Utilities:
    • Add normalizeToolSchema in native-ollama.ts for JSON Schema compatibility with Ollama's Go unmarshaler.
  • Comments:
    • Improve comments for configuration behavior clarity.

This description was created by Ellipsis for b1e7777. You can customize this summary. It will automatically update as commits are pushed.

… variable support

- Add environment variable support for Ollama configuration (OLLAMA_BASE_URL, OLLAMA_SERVER_URL, OLLAMA_API_KEY, OLLAMA_NUM_CTX, OLLAMA_MODEL_ID)
- Implement proper configuration priority: environment variables > VS Code settings > defaults
- Update lmstudio.ts to use refreshModels with baseUrl parameter
- Refactor webviewMessageHandler to use new refreshModels function for forced cache refresh
- Add normalizeToolSchema to ensure JSON Schema compatibility with Ollama's Go unmarshaler
- Improve comments to clarify configuration behavior

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@PineArt PineArt requested review from cte, jr and mrubens as code owners December 21, 2025 04:35
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 21, 2025
@roomote
Copy link
Contributor

roomote bot commented Dec 21, 2025

Rooviewer Clock   See task on Roo Cloud

Reviewed the environment variable support changes. Found one issue to address before merging.

  • Error message in ECONNREFUSED handler shows incorrect URL when env vars are set (native-ollama.ts:162-167)

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

Comment on lines +162 to +167
// Priority: environment variables > VS Code settings > default
const host =
process.env.OLLAMA_BASE_URL ||
process.env.OLLAMA_SERVER_URL ||
this.options.ollamaBaseUrl ||
"http://localhost:11434"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this new priority chain, the error message in the ECONNREFUSED handler (line 334) becomes inconsistent. It still references this.options.ollamaBaseUrl || "http://localhost:11434" but the actual host used could differ when environment variables are set. Users debugging connection failures would see a misleading URL in the error message.

Consider storing the resolved host as an instance property (e.g., this.resolvedHost) so the error message can reference the actual URL being used.

Fix it with Roo Code or mention @roomote and request a fix.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Dec 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

2 participants