fix(models): strip ollama prefix in ModelSelector; fix order-dependent auth tests - #33
Conversation
![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: d02ce26a3e
ℹ️ 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".
| return .openRouter(modelId: qualified.model) | ||
| } | ||
| if provider == "ollama" { | ||
| return .ollama(parseOllamaModel(qualified.model.lowercased()) ?? .custom(qualified.model.lowercased())) |
There was a problem hiding this comment.
When CLI users pass provider-qualified Ollama IDs that are case-sensitive (for example Hugging Face GGUF IDs or custom tags such as Q4_K_M), this branch parses the already lowercased normalized string and then lowercases the model again, so the OllamaProvider sends a different model value and Ollama can fail to find the registered model. Use the original trimmed modelString/qualified model for the custom fallback and only lowercase for known-name matching.
Useful? React with 👍 / 👎.
…m-provider test helper
ModelSelector.parseModel fell through to the bare-name fallback with the
provider prefix still attached, producing .custom("ollama/<model>") — an id
that does not exist on the Ollama server. Provider-qualified ollama ids now
resolve like LanguageModel.parse already does. Also wraps the custom-provider
test helper in TestEnvironmentMutex: it mutated global profile/registry state
unlocked, making AuthManagerTests order-dependent.
d02ce26 to
3a8593d
Compare
steipete
changed the title
1 participant
Summary
Slimmed after #30/#31 landed GPT-5.6 on main first — this PR now contains only the two changes main still lacks:
ModelSelector.parseModelkeeps theollama/prefix in custom model ids. Provider-qualified unknown names fell through to the bare-name fallback with the prefix attached, producing.custom("ollama/qwen2.5vl:latest")— an id that does not exist on the Ollama server. (LanguageModel.parsealready strips it; the throwing selector now matches.) Found live: Peekaboo's shipped default provider entryollama/qwen2.5vl:latestwas unusable through this path.Custom-provider test helper serialized under
TestEnvironmentMutex. It mutatedTachikomaConfiguration.profileDirectoryNameandCustomProviderRegistry.sharedunlocked, which madeAuthManagerTestsorder-dependent — adding any test could (deterministically, given the right scheduling) fail "openrouter resolves env and credential keys as bearer auth". State is restored indeferand the callback is@Sendable.Tests
ModelSelector strips the ollama prefix from provider-qualified custom models(regression for 1, incl. known-shortcut preservation).