Conversation
|
🔍 OpenCodeReview found 2 issue(s) in this PR.
|
| Protocol: ProtocolOpenAIChatCompletions, | ||
| BaseURL: "https://openrouter.ai/api/v1", | ||
| EnvVar: "OPENROUTER_API_KEY", | ||
| OpenModelList: true, |
There was a problem hiding this comment.
Documentation sync required. Per project rules, any modification of a provider entry must be accompanied by updates to the built-in provider table in all four documentation files (en/configuration.md, zh/configuration.md, ja/configuration.md, ru/configuration.md). The docs currently list OpenRouter without mentioning that its model list is non-gating (open catalog). Please update all four docs to reflect this behavioral difference.
| Protocol: ProtocolOpenAIChatCompletions, | ||
| BaseURL: "https://openrouter.ai/api/v1", | ||
| EnvVar: "OPENROUTER_API_KEY", | ||
| OpenModelList: true, |
There was a problem hiding this comment.
Missing test coverage for new field. The existing TestLookupProvider_OpenRouterDetails test verifies Protocol, BaseURL, and EnvVar but does not assert OpenModelList == true. Since this new field changes resolver behavior (disabling model-list gating), add an assertion like if !p.OpenModelList { t.Error("expected OpenModelList to be true") } to prevent accidental regression.
Qiyuanqiii
left a comment
There was a problem hiding this comment.
We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
|
done |
Qiyuanqiii
left a comment
There was a problem hiding this comment.
Re-reviewed 3a63e3e.
The OpenRouter override change is appropriately scoped: its preset/configured model entries remain picker suggestions, while unlisted per-run IDs can reach the provider. Closed-list presets retain their validation, and the existing ambient-auth exception is preserved.
The earlier bot requests are addressed in this revision: the OpenRouter preset flag has a direct assertion, the resolver test covers an unlisted model alongside configured suggestions, and the four provider-table documentation pages have been updated.
I found no code-level blocker.
Description
The built-in OpenRouter preset lists a handful of models for
ocr config model. The resolver also treated that list, combined with configured suggestions, as an allowlist for--model, so valid OpenRouter IDs absent from the list failed before a request reached OpenRouter.This change marks OpenRouter's model list as open for per-run overrides. The listed models remain picker suggestions; an unlisted ID passes through for OpenRouter to validate. Closed-list providers and the existing Bedrock exception keep their current behavior. The CLI reference documents the distinction.
Type of Change
How Has This Been Tested?
make testpasses locally (race enabled; Go 1.25.5, macOS arm64)The new resolver test selects the built-in OpenRouter provider with a configured model suggestion and passes an unlisted ID via
--model. Existing tests cover rejection by closed-list providers.go generate ./internal/llmproduced no catalog changes;make check,make build, andgit diff --checkpassed. No live OpenRouter request was made. The requiredocr review --audience agentcommand was attempted with the locally built CLI but could not resolve an LLM endpoint because none is configured in this environment.Checklist
go fmt,go vet)AI/LLM disclosure: Codex using GPT-6 Sol assisted with investigation, implementation, tests, documentation, and this description. A human contributor review and CLA signature are not claimed here.
Related Issues
Closes #1609.