Make ai_model and ai_base_url actually do something - #171
Conversation
Both settings are declared in Configuration, set in the host initializer, and documented in the host guide's configuration reference. Neither was read by anything. `ai_model` lost to a hard-coded default argument on AiProviders::OpenAi.call, so a host setting gpt-4o-mini still paid for gpt-4o. `ai_base_url` never reached the client at all, which quietly made the documented "point CoPlan at your own provider" impossible — the one setting that matters for running this against Azure, LiteLLM, vLLM, Ollama or an internal gateway rather than OpenAI directly. Both are now read, with the constants as the floor so a host that sets only an API key is unaffected. An explicit `model:` argument still wins, for callers whose prompt needs a particular model. The four specs that matter fail against the previous code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bb82e67a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Used when the host configures neither, so a host that sets only an | ||
| # API key still works. | ||
| DEFAULT_MODEL = "gpt-4o".freeze | ||
| DEFAULT_BASE_URL = "https://api.openai.com/v1".freeze |
There was a problem hiding this comment.
/v1 from the ruby-openai URI base
With the bundled ruby-openai 8.3.0 client, uri_base is the server root and the client adds its configured API version (v1) when constructing the chat endpoint. Now that this value is actually passed into the client, the default becomes https://api.openai.com/v1/v1/chat/completions, so ordinary OpenAI summary requests return 404; the documented custom URLs ending in /v1 have the same problem. Use a root such as https://api.openai.com (and document custom roots accordingly), or explicitly account for the client's API-version handling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Checked against the bundled gem (ruby-openai 8.3.0, pinned by sha in Gemfile.lock): uri skips appending api_version when the base already contains it — elsif @uri_base.include?(@api_version) in lib/openai/http.rb#uri. Verified against the real client, no stubs:
https://api.openai.com/v1 => https://api.openai.com/v1/chat/completions
https://api.openai.com => https://api.openai.com/v1/chat/completions
https://gateway.example.com/openai/v1 => https://gateway.example.com/openai/v1/chat/completions
https://gateway.example.com/openai => https://gateway.example.com/openai/v1/chat/completions
No double /v1 in any configuration, including the documented custom URLs. The base-url spec in this PR was mock-based though, so the concern was fair that nothing pinned the gem's rule — 8da4147 adds two no-stub specs that exercise the real client's URI construction, so a future gem upgrade that changes the version-handling fails in CI instead of 404ing in production.
Review flagged a potential double /v1 (uri_base + the client's own api_version). ruby-openai 8.3.0 skips appending when the base already carries the version (http.rb#uri), so the default is fine — but that rule lives in the gem. Exercise the real client's URI construction so a gem upgrade that changes it fails in CI instead of 404ing in production. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Two of the three AI settings in the configuration reference were decorative.
ai_base_urlandai_modelare declared inCoPlan::Configuration, set in the host initializer, and documented indocs/HOST_APP_GUIDE.md— and read by nothing.ai_modellost to a hard-coded default argument onAiProviders::OpenAi.call, so a host that setgpt-4o-ministill paid forgpt-4o.ai_base_urlnever reached the client at all. That's the one that matters: it quietly made the documented "point CoPlan at your own provider" impossible, so running against Azure OpenAI, LiteLLM, vLLM, Ollama or an internal gateway didn't work no matter what you configured.Both are read now. The constants are the floor, so a host that sets only an API key is unaffected, and an explicit
model:argument still wins for callers whose prompt needs a particular model.ruby-openaionly appends its own/v1when the base URL doesn't already carry one, so the existing default value passes through unchanged.Testing
Four of the six new specs fail against the previous code (the other two cover behaviour that already worked: an explicit model argument, and the default when config is cleared). Full suite green, 1332 examples.
Deleting the two settings was the alternative. Wiring them up seemed better than removing the seam, given self-hosting is a goal.
🤖 Generated with Claude Code