feat: centralized models.json with llm-update script - #798
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
📝 WalkthroughWalkthroughThis PR updates AI model configurations across multiple config files to use newer model versions (Claude 4.6, GPT 5.2/5.3 variants, Gemini 3.x), introduces a new Changes
Sequence DiagramsequenceDiagram
participant Developer
participant models.json
participant llm-update.sh
participant ConfigFiles as Config Files<br/>(openclaw, opencode,<br/>llm, ccs, cliproxy, codex)
Developer->>models.json: Update model aliases<br/>and versions
Developer->>llm-update.sh: Execute automation script
llm-update.sh->>models.json: Read model registry
activate models.json
models.json-->>llm-update.sh: Return alias mappings
deactivate models.json
llm-update.sh->>llm-update.sh: Resolve aliases to<br/>concrete model IDs
llm-update.sh->>ConfigFiles: update_openclaw()
llm-update.sh->>ConfigFiles: update_opencode()
llm-update.sh->>ConfigFiles: update_llm()
llm-update.sh->>ConfigFiles: update_ccs()
llm-update.sh->>ConfigFiles: update_cliproxy()
llm-update.sh->>ConfigFiles: update_codex()
activate ConfigFiles
ConfigFiles-->>llm-update.sh: All files updated
deactivate ConfigFiles
llm-update.sh-->>Developer: Status messages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the management and consistency of Large Language Model (LLM) configurations across the system. By centralizing model aliases and their concrete IDs in a single Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Mesa DescriptionTL;DRCentralized LLM model aliases in What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request introduces a centralized models.json to manage model aliases and a powerful shell script, llm-update.sh, to propagate these settings across various configuration files. This is a great step towards simplifying model management. The script is well-structured, but relies on text-processing tools like awk and perl which can be brittle. My review includes suggestions to improve the script's robustness and maintainability, such as dynamically generating alias lists and strengthening the logic for file updates.
| [[ -f "$file" ]] || { echo "SKIP: $file not found"; return; } | ||
|
|
||
| # Non-thinking aliases for provider model lists | ||
| local provider_aliases='["claude-opus","claude-sonnet","claude-haiku","gpt","gpt-codex","gemini-pro","gemini-flash","glm"]' |
There was a problem hiding this comment.
The provider_aliases variable is hardcoded. This creates a maintenance burden, as it needs to be manually updated if non-thinking model aliases are added or removed from models.json. You can derive this list dynamically from models.json to make the script more robust and truly have a single source of truth.
| local provider_aliases='["claude-opus","claude-sonnet","claude-haiku","gpt","gpt-codex","gemini-pro","gemini-flash","glm"]' | |
| local provider_aliases=$(jq '[keys[] | select(endswith("-thinking") | not)]' "$MODELS") |
|
|
||
| # Strip JSONC: remove comment-only lines, fix trailing commas | ||
| local stripped | ||
| stripped=$(grep -v '^\s*//' "$file" | perl -0777 -pe 's/,(\s*[}\]])/\1/g') |
There was a problem hiding this comment.
The perl command for stripping JSONC features is effective but can be difficult to understand for future maintainers. Consider adding a comment explaining what the regex s/,(\s*[}\]])/�/g does (i.e., removes trailing commas in JSON objects and arrays) to improve the script's readability and maintainability.
| # Only replace the top-level model line (before any [section] header) | ||
| local model | ||
| model=$(resolve gpt-codex) | ||
| awk -v m="$model" '!done && /^model = "/ { print "model = \"" m "\""; done=1; next } {print}' "$file" > "$file.tmp" && mv "$file.tmp" "$file" |
There was a problem hiding this comment.
The awk command used to update config.toml is brittle as it relies on a specific format model = "...". This will fail if the formatting changes slightly (e.g., different spacing, single quotes). For improved robustness, consider using a TOML-aware tool like yqif it's available in your environment. If not, thisawk` command is a reasonable approach but be aware of its limitations.
| if grep -q '^oauth-model-alias:' "$file"; then | ||
| awk -v replacement="$block" ' | ||
| /^oauth-model-alias:/ { found=1; print replacement; next } | ||
| found && /^[^ #]/ { found=0 } |
There was a problem hiding this comment.
The awk script used to replace the oauth-model-alias block is fragile. The condition found && /^[^ #]/ to detect the end of the block will fail if the YAML block contains an empty line, as an empty line matches ^. This would cause the script to prematurely stop skipping lines and corrupt the output file. A more robust condition would be to check for non-empty lines.
| found && /^[^ #]/ { found=0 } | |
| found && /./ && /^[^ #]/ { found=0 } |
There was a problem hiding this comment.
1 issue found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/llm-update.sh">
<violation number="1" location="scripts/llm-update.sh:13">
P2: Fail fast when an alias is missing. As written, `resolve` returns the string `null` for unknown aliases, so the script can silently generate invalid configs instead of erroring.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@scripts/llm-update.sh`:
- Around line 12-13: The resolve() helper currently returns the literal string
"null" when an alias is missing, corrupting downstream model IDs; update the
resolve function (resolve) to make jq emit an empty string for missing keys
(e.g., use the jq fallback like '.[$a] // empty') and then validate the result:
if the resolved value is empty or unset, print a clear error (including the
alias and MODELS context) and exit non‑zero so the script fails fast instead of
propagating "null" into configs.
- Around line 97-111: The current JSONC stripping only removes full-line
comments and leaves inline comments, breaking jq; update the strip step that
sets the local variable stripped (currently using grep + perl) to robustly
remove inline // comments outside string literals—either call a JSONC-aware tool
(e.g., node's strip-json-comments or a small python script that parses strings
and strips // comments) or replace the pipeline with a safe parser before piping
into jq, ensuring the rest of the pipeline using $cliproxyapi_models,
$shunkakinoki_models, $model and $small_model receives valid JSON; keep the same
output behavior (writing back to "$file" and the subsequent awk step).
🧹 Nitpick comments (4)
scripts/llm-update.sh (4)
38-66: HardcodedcontextWindowandmaxTokensfor all models may be inaccurate.Every model entry gets
contextWindow: 200000andmaxTokens: 32000, but GPT and Gemini models have different context windows (e.g., GPT-4 variants typically have 128k, Gemini Pro has 1M+). If OpenClaw uses these values for request sizing or truncation, this could cause issues.Consider whether
models.jsonshould carry per-model metadata (context window, max tokens) or whether OpenClaw ignores these fields in practice.
94-95:modelandsmall_modelare set to the same value — intentional?Both
modelandsmall_modelresolve to the sameglmalias. If OpenCode differentiates between a primary and a lightweight model, this may not be the intended behavior. If it's intentional, a brief comment would clarify.
120-136:update_llm()writes the file without checking if the parent directory exists.Unlike other
update_*functions that guard with[[ -f "$file" ]], this function writes directly to$file. Ifconfig/llm/doesn't exist, the redirect will fail. This is fine if the directory is guaranteed to exist, but for consistency and robustness:🔧 Proposed fix: add a directory guard
update_llm() { local file="$ROOT_DIR/config/llm/extra-openai-models.yaml" + mkdir -p "$(dirname "$file")" local aliases='["claude-sonnet","claude-opus","claude-haiku","glm"]'
219-229: Consider adding a dependency check at script start.The script depends on
jq,perl,awk, andgrep. A quick preflight check would give a clearer error than a mid-run failure, especially for fresh machine setups (which is a common dotfiles scenario).🔧 Example preflight check
[[ -f "$MODELS" ]] || { echo "ERROR: models.json not found" >&2; exit 1; } + +for cmd in jq perl awk grep; do + command -v "$cmd" >/dev/null 2>&1 || { echo "ERROR: $cmd is required but not installed" >&2; exit 1; } +done
| # Resolve alias → concrete model ID | ||
| resolve() { jq -r --arg a "$1" '.[$a]' "$MODELS"; } |
There was a problem hiding this comment.
resolve() silently returns the string "null" for missing aliases, corrupting downstream configs.
If a typo or missing alias is passed to resolve, jq -r '.[$a]' prints the literal string null. This will propagate into config files as model IDs like cliproxy/null, causing silent misconfigurations that are hard to debug.
🛡️ Proposed fix: validate resolve output
-resolve() { jq -r --arg a "$1" '.[$a]' "$MODELS"; }
+resolve() {
+ local val
+ val=$(jq -r --arg a "$1" '.[$a] // empty' "$MODELS")
+ if [[ -z "$val" ]]; then
+ echo "ERROR: unknown alias '$1' in models.json" >&2
+ exit 1
+ fi
+ echo "$val"
+}Using // empty makes jq output nothing for missing keys, and the guard fails fast with a clear error message. This is especially important given set -euo pipefail — an unquoted empty variable would already trigger set -u, but the current code returns the string "null" which bypasses that protection.
📝 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.
| # Resolve alias → concrete model ID | |
| resolve() { jq -r --arg a "$1" '.[$a]' "$MODELS"; } | |
| # Resolve alias → concrete model ID | |
| resolve() { | |
| local val | |
| val=$(jq -r --arg a "$1" '.[$a] // empty' "$MODELS") | |
| if [[ -z "$val" ]]; then | |
| echo "ERROR: unknown alias '$1' in models.json" >&2 | |
| exit 1 | |
| fi | |
| echo "$val" | |
| } |
🤖 Prompt for AI Agents
In `@scripts/llm-update.sh` around lines 12 - 13, The resolve() helper currently
returns the literal string "null" when an alias is missing, corrupting
downstream model IDs; update the resolve function (resolve) to make jq emit an
empty string for missing keys (e.g., use the jq fallback like '.[$a] // empty')
and then validate the result: if the resolved value is empty or unset, print a
clear error (including the alias and MODELS context) and exit non‑zero so the
script fails fast instead of propagating "null" into configs.
| # Strip JSONC: remove comment-only lines, fix trailing commas | ||
| local stripped | ||
| stripped=$(grep -v '^\s*//' "$file" | perl -0777 -pe 's/,(\s*[}\]])/\1/g') | ||
|
|
||
| echo "$stripped" | jq --tab \ | ||
| --argjson cliproxyapi "$cliproxyapi_models" \ | ||
| --argjson shunkakinoki "$shunkakinoki_models" \ | ||
| --arg model "$model" --arg small_model "$small_model" ' | ||
| .model = $model | .small_model = $small_model | | ||
| .provider.cliproxyapi.models = $cliproxyapi | | ||
| .provider.shunkakinoki.models = $shunkakinoki | ||
| ' | awk '/"write": false/ { | ||
| indent=$0; sub(/[^ \t].*/,"",indent) | ||
| print indent "// Disable file modification tools for review-only agent" | ||
| } {print}' > "$file" |
There was a problem hiding this comment.
JSONC stripping only handles full-line comments; inline comments will produce invalid JSON.
Line 99 uses grep -v '^\s*//' which removes lines where // is the first non-whitespace content. If opencode.jsonc contains inline comments (e.g., "key": "value" // explanation), they'll survive into the jq input and cause a parse error.
🔧 Proposed fix: also strip inline comments
- stripped=$(grep -v '^\s*//' "$file" | perl -0777 -pe 's/,(\s*[}\]])/\1/g')
+ stripped=$(sed 's|//.*$||' "$file" | perl -0777 -pe 's/,(\s*[}\]])/\1/g')sed 's|//.*$||' will incorrectly strip // inside string values (e.g., URLs like http://...). A more robust approach would use a proper JSONC parser or a more targeted regex that avoids strings. If inline comments aren't used today, a code comment noting this limitation would suffice.
🤖 Prompt for AI Agents
In `@scripts/llm-update.sh` around lines 97 - 111, The current JSONC stripping
only removes full-line comments and leaves inline comments, breaking jq; update
the strip step that sets the local variable stripped (currently using grep +
perl) to robustly remove inline // comments outside string literals—either call
a JSONC-aware tool (e.g., node's strip-json-comments or a small python script
that parses strings and strips // comments) or replace the pipeline with a safe
parser before piping into jq, ensuring the rest of the pipeline using
$cliproxyapi_models, $shunkakinoki_models, $model and $small_model receives
valid JSON; keep the same output behavior (writing back to "$file" and the
subsequent awk step).
bf6e3ac to
055c22b
Compare
Single source of truth for SOTA model aliases mapped to concrete IDs. The llm-update.sh script propagates changes to all 6 tool configs (openclaw, opencode, llm, ccs, codex, cliproxyapi). - Add models.json with flat alias→ID map (claude-opus-4.6, gpt-5.3-codex, etc.) - Add scripts/llm-update.sh to regenerate model sections in all configs - Update openclaw to SOTA-only models, remove legacy entries - Update CCS profiles (agy→claude-opus-4-6-thinking, codex→gpt-5.3-codex) - Add cliproxy oauth-model-alias for claude-opus-4.6 routing
There was a problem hiding this comment.
Pull request overview
Introduces a centralized models.json mapping of model aliases to concrete model IDs and adds an update script to propagate those IDs into multiple tool configuration templates, keeping model selections consistent across the repo.
Changes:
- Add
models.jsonas a single alias → model ID source of truth. - Add
scripts/llm-update.shto regenerate/update OpenClaw, OpenCode, llm, CCS, Codex, and cliproxyapi configs frommodels.json. - Update existing config templates to newer model IDs and remove legacy entries.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/llm-update.sh | New propagation script that rewrites multiple tool configs based on models.json. |
| models.json | Central alias → concrete model ID mapping used by the update script. |
| config/opencode/opencode.jsonc | Updates provider model lists to current IDs and normalizes formatting. |
| config/openclaw/openclaw.template.json | Refreshes the available model list and default primary/fallback models. |
| config/llm/extra-openai-models.yaml | Regenerates llm’s extra model definitions from aliases. |
| config/cliproxyapi/config.template.yaml | Adds oauth model alias routing for antigravity. |
| config/ccs/agy.settings.template.json | Updates CCS agy profile to new Claude IDs. |
| config/ccs/codex.settings.template.json | Updates CCS codex profile to new GPT Codex ID. |
| config/ccs/gemini.settings.template.json | Updates CCS gemini profile to new Gemini IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [[ -f "$MODELS" ]] || { echo "ERROR: models.json not found" >&2; exit 1; } | ||
|
|
||
| # Resolve alias → concrete model ID | ||
| resolve() { jq -r --arg a "$1" '.[$a]' "$MODELS"; } |
There was a problem hiding this comment.
resolve() returns the literal string null when an alias key is missing in models.json, which would silently propagate invalid model IDs (e.g., cliproxy/null) into multiple configs. Consider making resolve fail fast (exit non-zero) when the lookup is missing/empty so a bad models.json edit can’t produce corrupted config output.
| resolve() { jq -r --arg a "$1" '.[$a]' "$MODELS"; } | |
| resolve() { | |
| local alias="$1" | |
| local value | |
| value=$(jq -r --arg a "$alias" '.[$a] // empty' "$MODELS") | |
| if [[ -z "$value" || "$value" == "null" ]]; then | |
| echo "ERROR: alias '$alias' not found in models.json" >&2 | |
| exit 1 | |
| fi | |
| printf '%s\n' "$value" | |
| } |
| # Only replace the top-level model line (before any [section] header) | ||
| local model | ||
| model=$(resolve gpt-codex) | ||
| awk -v m="$model" '!done && /^model = "/ { print "model = \"" m "\""; done=1; next } {print}' "$file" > "$file.tmp" && mv "$file.tmp" "$file" |
There was a problem hiding this comment.
The comment says this only replaces the top-level model = line (before any [section] header), but the awk currently replaces the first model = "..." it finds anywhere in the file. Tighten the match to stop once a section header is encountered, or adjust the comment so future edits don’t assume behavior that isn’t implemented.
| awk -v m="$model" '!done && /^model = "/ { print "model = \"" m "\""; done=1; next } {print}' "$file" > "$file.tmp" && mv "$file.tmp" "$file" | |
| awk -v m="$model" ' | |
| /^\[/ { in_section=1 } | |
| !done && !in_section && /^model = "/ { | |
| print "model = \"" m "\"" | |
| done=1 | |
| next | |
| } | |
| { print } | |
| ' "$file" > "$file.tmp" && mv "$file.tmp" "$file" |
|
|
||
| # -------------------------------------------------------------------------- | ||
| # Cliproxyapi — config/cliproxyapi/config.template.yaml | ||
| # OAuth alias: antigravity upstream=claude-opus-thinking, alias=claude-opus |
There was a problem hiding this comment.
The header comment describes the oauth alias in terms of aliases (upstream=claude-opus-thinking, alias=claude-opus), but the implementation uses resolve and writes concrete model IDs into the YAML. Update the comment to reflect the actual behavior (IDs vs aliases) to avoid confusion when someone edits models.json or this replacement block later.
| # OAuth alias: antigravity upstream=claude-opus-thinking, alias=claude-opus | |
| # OAuth alias: antigravity, using models.json aliases: | |
| # upstream alias=claude-opus-thinking (resolved to concrete model ID), | |
| # display alias=claude-opus (resolved to concrete model ID) |
|
|
||
| # -------------------------------------------------------------------------- | ||
| echo "Updating tool configs from $MODELS ..." | ||
| echo | ||
| update_openclaw | ||
| update_opencode | ||
| update_llm | ||
| update_ccs | ||
| update_codex | ||
| update_cliproxy | ||
| echo |
There was a problem hiding this comment.
This repo uses ShellSpec for bash scripts (and existing scripts under scripts/ have corresponding spec/*_spec.sh coverage), but this new llm-update.sh doesn’t have a spec. Adding a ShellSpec test for at least “runs successfully” + “idempotent output” (run twice yields no diffs) would help prevent accidental config corruption.
…itution Rewrite llm-update.sh from 230-line per-tool jq transforms to 67-line sed-based __PLACEHOLDER__ substitution from .tpl.* source templates. Entire-Checkpoint: ac82f4d070f9
There was a problem hiding this comment.
1 issue found across 10 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="config/codex/config.tpl.toml">
<violation number="1" location="config/codex/config.tpl.toml:4">
P2: Escape the assistant message before inserting it into the osascript string to avoid malformed notifications or AppleScript injection when the message contains quotes/backslashes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| notify = [ | ||
| "bash", | ||
| "-lc", | ||
| "JSON=\"$1\"; LAST_MESSAGE=$(echo \"$JSON\" | jq -r '.\"last-assistant-message\" // \"Codex task completed\"'); osascript -e \"display notification \\\"$LAST_MESSAGE\\\" with title \\\"Codex\\\"\"", |
There was a problem hiding this comment.
P2: Escape the assistant message before inserting it into the osascript string to avoid malformed notifications or AppleScript injection when the message contains quotes/backslashes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/codex/config.tpl.toml, line 4:
<comment>Escape the assistant message before inserting it into the osascript string to avoid malformed notifications or AppleScript injection when the message contains quotes/backslashes.</comment>
<file context>
@@ -0,0 +1,63 @@
+notify = [
+ "bash",
+ "-lc",
+ "JSON=\"$1\"; LAST_MESSAGE=$(echo \"$JSON\" | jq -r '.\"last-assistant-message\" // \"Codex task completed\"'); osascript -e \"display notification \\\"$LAST_MESSAGE\\\" with title \\\"Codex\\\"\"",
+ "notify",
+]
</file context>
Summary
models.jsonas single source of truth for SOTA model aliases → concrete IDsscripts/llm-update.shthat propagates model changes to all 6 tool configs (openclaw, opencode, llm, ccs, codex, cliproxyapi)oauth-model-aliasforclaude-opus-4.6routing via antigravityWorkflow
Test plan
bash scripts/llm-update.sh— all 9 configs report OKmake build && make switchdeploys without errorsopenclaw agent --local --agent main --message "Say hi"gets a response🤖 Generated with Claude Code
Summary by cubic
Centralizes model aliases in models.json and switches to a template-driven llm-update.sh to sync model IDs across all tools. Updates configs to current models (Claude Opus 4.6 + Thinking, GPT 5.3 Codex, Gemini 3 Pro/Flash) and removes legacy entries.
New Features
Migration
Written for commit 3c87e28. Summary will update on new commits.