release: promote dev → main (settings model override fix)#59
release: promote dev → main (settings model override fix)#59namastex888 merged 4 commits intomainfrom
Conversation
`rlmx config set model.provider openai` saved to settings.json but was ignored when a local rlmx.yaml existed — the yaml model was always used. Now the documented priority (CLI flags > settings.json > rlmx.yaml > defaults) is actually enforced for model.provider, model.model, and model.sub-call-model across run, cache, batch, and benchmark commands.
fix: settings model override not applied to CLI
📝 WalkthroughWalkthroughThis PR bumps the package version from 0.260409.1 to 0.260409.3 and introduces global model settings override functionality in the CLI, applying global model provider configurations to per-project settings across query, cache, batch, and benchmark commands. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: f561305eca
ℹ️ 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".
| const s = settings ?? _globalSettings; | ||
| const provider = s["model.provider"]; | ||
| const model = s["model.model"]; | ||
| const subCallModel = s["model.sub-call-model"]; |
There was a problem hiding this comment.
The new override path reads model.sub-call-model, but the CLI documentation for rlmx config tells users to set model.sub_call_model; because runConfig stores keys verbatim, users following the documented key will never override config.model.subCallModel and sub-calls will continue using the YAML/default model unexpectedly. This can skew cost/latency experiments that rely on a cheaper sub-call model.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to apply model overrides from global settings, ensuring that settings defined in ~/.rlmx/settings.json take precedence over local rlmx.yaml configurations. The review identified a high-severity bug where the key used for sub-call models is inconsistent with the documentation, preventing the override from functioning correctly. Additionally, a duplicate and misplaced JSDoc comment was noted in src/cli.ts.
| const s = settings ?? _globalSettings; | ||
| const provider = s["model.provider"]; | ||
| const model = s["model.model"]; | ||
| const subCallModel = s["model.sub-call-model"]; |
There was a problem hiding this comment.
The key model.sub-call-model is inconsistent with the documentation in CONFIG_HELP (line 665), which specifies model.sub_call_model (using an underscore). Since the rlmx config set command uses the keys defined in the help text, the settings stored in settings.json will use underscores, causing this override logic to fail to find the value.
| const subCallModel = s["model.sub-call-model"]; | |
| const subCallModel = s["model.sub_call_model"]; |
| /** | ||
| * Apply model overrides from global settings (~/.rlmx/settings.json). | ||
| * Priority: CLI flags > settings.json > rlmx.yaml > hardcoded defaults. | ||
| * | ||
| * This ensures `rlmx config set model.provider openai` actually takes effect | ||
| * even when a local rlmx.yaml exists with its own model defaults. | ||
| */ |
There was a problem hiding this comment.
This JSDoc block is a duplicate of the one provided for the applySettingsModelOverrides function at lines 29-35. It is also misplaced here as it precedes the _globalSettings variable rather than the function it describes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/cli.ts (1)
19-35: Remove duplicated header comment block for override helper.Lines 19-25 and 29-35 repeat the same doc content. Keeping one block improves readability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 19 - 35, Remove the duplicated module-level doc comment block that repeats the "Apply model overrides..." header; keep a single copy immediately above the module-level reference declaration so the module-level ref comment for _globalSettings: GlobalSettings remains documented once. Ensure the retained comment explains the override priority and why it exists, and delete the redundant second block without changing the _globalSettings identifier or its type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli.ts`:
- Around line 36-40: The applySettingsModelOverrides function only reads
s["model.sub-call-model"], so settings keyed with model.sub_call_model are
ignored; update the logic to accept both forms by reading subCallModel from
s["model.sub-call-model"] ?? s["model.sub_call_model"] (and similarly handle any
other model keys with hyphen/underscore variants) and then use that value in the
same places as the current subCallModel variable so underscore-named settings
are applied consistently alongside hyphen-named ones.
---
Nitpick comments:
In `@src/cli.ts`:
- Around line 19-35: Remove the duplicated module-level doc comment block that
repeats the "Apply model overrides..." header; keep a single copy immediately
above the module-level reference declaration so the module-level ref comment for
_globalSettings: GlobalSettings remains documented once. Ensure the retained
comment explains the override priority and why it exists, and delete the
redundant second block without changing the _globalSettings identifier or its
type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c35c867-e4a4-4934-ac5c-12dcd4d26b45
📒 Files selected for processing (3)
package.jsonsrc/cli.tssrc/version.ts
| function applySettingsModelOverrides(config: RlmxConfig, settings?: GlobalSettings): void { | ||
| const s = settings ?? _globalSettings; | ||
| const provider = s["model.provider"]; | ||
| const model = s["model.model"]; | ||
| const subCallModel = s["model.sub-call-model"]; |
There was a problem hiding this comment.
Support both model.sub-call-model and model.sub_call_model keys to avoid silent misses.
Line 40 only reads model.sub-call-model, but this file’s own help text (Line 665) still advertises model.sub_call_model. Settings created with underscore won’t be applied.
🔧 Proposed compatibility fix
function applySettingsModelOverrides(config: RlmxConfig, settings?: GlobalSettings): void {
const s = settings ?? _globalSettings;
const provider = s["model.provider"];
const model = s["model.model"];
- const subCallModel = s["model.sub-call-model"];
+ const subCallModel = s["model.sub-call-model"] ?? s["model.sub_call_model"];
if (typeof provider === "string" && provider) {
config.model.provider = provider;
}
if (typeof model === "string" && model) {
config.model.model = model;
}
if (typeof subCallModel === "string" && subCallModel) {
config.model.subCallModel = subCallModel;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function applySettingsModelOverrides(config: RlmxConfig, settings?: GlobalSettings): void { | |
| const s = settings ?? _globalSettings; | |
| const provider = s["model.provider"]; | |
| const model = s["model.model"]; | |
| const subCallModel = s["model.sub-call-model"]; | |
| function applySettingsModelOverrides(config: RlmxConfig, settings?: GlobalSettings): void { | |
| const s = settings ?? _globalSettings; | |
| const provider = s["model.provider"]; | |
| const model = s["model.model"]; | |
| const subCallModel = s["model.sub-call-model"] ?? s["model.sub_call_model"]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli.ts` around lines 36 - 40, The applySettingsModelOverrides function
only reads s["model.sub-call-model"], so settings keyed with
model.sub_call_model are ignored; update the logic to accept both forms by
reading subCallModel from s["model.sub-call-model"] ?? s["model.sub_call_model"]
(and similarly handle any other model keys with hyphen/underscore variants) and
then use that value in the same places as the current subCallModel variable so
underscore-named settings are applied consistently alongside hyphen-named ones.
2 participants
Summary
rlmx config set model.providernow actually takes effect over rlmx.yaml defaultsTest plan
tsc --noEmitcleanopenai/gpt-5.4-minivia settings overrideSummary by CodeRabbit
New Features
Chores