chore: expand llm-update template coverage - #1077
Conversation
- add templated sources for aichat, pi settings, llm defaults, and fish wrappers - extend llm-update mappings and coverage checks for the new templates - add fish template tests and keep Claude settings out of llm-update tracking Entire-Checkpoint: 3782180a0e72
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR introduces configuration and shell function templates for AI model integration (including AIChat and Pi), updates existing shell function descriptions to remove model-specific references, extends the llm-update build script with new template mappings, and adds corresponding test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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, 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 expands the templating capabilities of the 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;DRExpanded What changed?
Description generated by Mesa. Update settings |
![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 expands the llm-update templating system to cover more configuration files, including for aichat, pi, and several fish wrapper functions. The changes are logical and the introduction of new templates is a good step towards centralizing model configuration. The new templates and updates to the llm-update.sh script are correct. I've noticed a few areas for improvement in the newly added tests to enhance their robustness and correctness, which are detailed in my comments.
| It 'includes fish wrapper templates in template mappings' | ||
| When run bash -c "grep '_ocxe_function.tpl.fish' '$SCRIPT' && grep '_pixe_function.tpl.fish' '$SCRIPT'" | ||
| The output should include '_ocxe_function.tpl.fish' | ||
| The output should include '_pixe_function.tpl.fish' | ||
| End |
There was a problem hiding this comment.
This test for fish wrapper templates is incomplete as it misses _ocxeh_function.tpl.fish and _pixeh_function.tpl.fish. Also, the test pattern of chaining grep with && is flawed because only the output of the last command is checked. A simpler and more correct approach is to cat the script and check for the presence of all required strings in its output.
Here is a suggested refactoring:
It 'includes fish wrapper templates in template mappings'
When run cat "$SCRIPT"
The output should include '_ocxe_function.tpl.fish'
The output should include '_ocxeh_function.tpl.fish'
The output should include '_pixe_function.tpl.fish'
The output should include '_pixeh_function.tpl.fish'
End| # ── no args: interactive mode ───────────────────────────── | ||
| set log1 (mktemp) | ||
| function opencode; echo $argv >> $log1; end | ||
|
|
||
| _ocxe_function | ||
|
|
||
| @test "no args calls opencode with templated model" (grep -c "cliproxyapi/__GLM__" $log1) -ge 1 | ||
| @test "no args skips run subcommand" (grep -c "^run " $log1) -eq 0 | ||
|
|
||
| # ── with args: run mode ────────────────────────────────── | ||
| set log2 (mktemp) | ||
| function opencode; echo $argv >> $log2; end | ||
|
|
||
| _ocxe_function hello world | ||
|
|
||
| @test "with args uses run subcommand" (grep -c "^run " $log2) -ge 1 | ||
| @test "with args builds prompt" (grep -c "hello world" $log2) -ge 1 | ||
|
|
||
| rm -f $log1 $log2 |
There was a problem hiding this comment.
To improve test robustness and maintainability, it's better to avoid redefining the mock function (opencode). You can define it once and then reset the state (the log file) between test groups. This makes the test easier to read and less prone to errors if tests are reordered or added.
Here is a suggested refactoring:
# ── no args: interactive mode ─────────────────────────────
set log (mktemp)
function opencode; echo $argv >> $log; end
_ocxe_function
@test "no args calls opencode with templated model" (grep -c "cliproxyapi/__GLM__" $log) -ge 1
@test "no args skips run subcommand" (grep -c "^run " $log) -eq 0
# ── with args: run mode ──────────────────────────────────
> $log # Reset log
_ocxe_function hello world
@test "with args uses run subcommand" (grep -c "^run " $log) -ge 1
@test "with args builds prompt" (grep -c "hello world" $log) -ge 1
rm -f $log| # ── no args: interactive mode ───────────────────────────── | ||
| set log1 (mktemp) | ||
| function pi-agent; echo $argv >> $log1; end | ||
|
|
||
| _pixe_function | ||
|
|
||
| @test "no args calls pi-agent with templated preset" (grep -c "openrouter-preset/@preset/__GLM_NONDOT__" $log1) -ge 1 | ||
|
|
||
| # ── with args: builds prompt ────────────────────────────── | ||
| set log2 (mktemp) | ||
| function pi-agent; echo $argv >> $log2; end | ||
|
|
||
| _pixe_function hello world | ||
|
|
||
| @test "with args builds prompt" (grep -c "hello world" $log2) -ge 1 | ||
|
|
||
| rm -f $log1 $log2 |
There was a problem hiding this comment.
Similar to the other new test file, you can improve robustness by defining the mock function (pi-agent) once and resetting the log file between test groups. This avoids function redefinition and makes the test cleaner.
Here is a suggested refactoring:
# ── no args: interactive mode ─────────────────────────────
set log (mktemp)
function pi-agent; echo $argv >> $log; end
_pixe_function
@test "no args calls pi-agent with templated preset" (grep -c "openrouter-preset/@preset/__GLM_NONDOT__" $log) -ge 1
# ── with args: builds prompt ──────────────────────────────
> $log # Reset log
_pixe_function hello world
@test "with args builds prompt" (grep -c "hello world" $log) -ge 1
rm -f $logThere was a problem hiding this comment.
Pull request overview
Expands scripts/llm-update.sh template coverage so more tool and shell configs can be generated from models.json, with accompanying spec coverage and new fishtape tests for the fish template wrappers.
Changes:
- Added new template→output mappings for aichat config, Pi settings, and
llmdefault model text. - Introduced templated fish wrapper sources for
ocxe/ocxehandpixe/pixeh, aligning generated.fishwrappers with the templates. - Added fishtape tests for the new fish template wrappers and extended the ShellSpec mapping-coverage checks.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spec/llm_update_spec.sh | Adds mapping-coverage assertions for new templates in llm-update.sh. |
| scripts/llm-update.sh | Extends the TEMPLATES map to include aichat, Pi settings, llm default model, and additional fish wrapper templates. |
| config/aichat/config.tpl.yaml | New aichat config template parameterized by __GLM__. |
| config/pi/settings.tpl.json | New Pi settings template parameterized by __GLM__. |
| config/llm/default_model.tpl.txt | New single-value default model template (__GLM__). |
| home-manager/programs/fish/functions/_ocxe_function.tpl.fish | New templated ocxe fish wrapper using __GLM__. |
| home-manager/programs/fish/functions/_ocxeh_function.tpl.fish | New templated ocxeh fish wrapper using __GLM__. |
| home-manager/programs/fish/functions/_pixe_function.tpl.fish | New templated pixe fish wrapper using __GLM_NONDOT__. |
| home-manager/programs/fish/functions/_pixeh_function.tpl.fish | New templated pixeh fish wrapper using __GLM_NONDOT__. |
| home-manager/programs/fish/functions/_ocxe_function.fish | Updates description/comment text to match the templated wrapper intent. |
| home-manager/programs/fish/functions/_ocxeh_function.fish | Updates description text to match the templated wrapper intent. |
| home-manager/programs/fish/functions/_pixe_function.fish | Updates description/comment text to match the templated wrapper intent. |
| home-manager/programs/fish/functions/_pixeh_function.fish | Updates description text to match the templated wrapper intent. |
| spec/fish/_ocxe_function.tpl_test.fish | New fishtape tests validating templated ocxe wrapper behavior. |
| spec/fish/_ocxeh_function.tpl_test.fish | New fishtape tests validating templated ocxeh wrapper behavior. |
| spec/fish/_pixe_function.tpl_test.fish | New fishtape tests validating templated pixe wrapper behavior. |
| spec/fish/_pixeh_function.tpl_test.fish | New fishtape tests validating templated pixeh wrapper behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 17 files
Prompt for AI agents (unresolved 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="spec/llm_update_spec.sh">
<violation number="1" location="spec/llm_update_spec.sh:50">
P2: This only checks that the template path appears somewhere in the script, not that the `TEMPLATES` entry maps it to the correct output file.</violation>
</file>
<file name="home-manager/programs/fish/functions/_pixeh_function.tpl.fish">
<violation number="1" location="home-manager/programs/fish/functions/_pixeh_function.tpl.fish:11">
P2: This wrapper never enables Pi's non-interactive mode, so `pixeh` will launch the interactive UI instead of returning a headless response.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| Describe 'mapping coverage' | ||
| It 'includes aichat config in template mappings' | ||
| When run bash -c "grep 'config/aichat/config.tpl.yaml' '$SCRIPT'" |
There was a problem hiding this comment.
P2: This only checks that the template path appears somewhere in the script, not that the TEMPLATES entry maps it to the correct output file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spec/llm_update_spec.sh, line 50:
<comment>This only checks that the template path appears somewhere in the script, not that the `TEMPLATES` entry maps it to the correct output file.</comment>
<file context>
@@ -45,6 +45,29 @@ The output should include '.tpl.'
+Describe 'mapping coverage'
+It 'includes aichat config in template mappings'
+When run bash -c "grep 'config/aichat/config.tpl.yaml' '$SCRIPT'"
+The output should include 'config/aichat/config.tpl.yaml'
+End
</file context>
| return 1 | ||
| end | ||
|
|
||
| pi-agent "$prompt" -m 'openrouter-preset/@preset/__GLM_NONDOT__' |
There was a problem hiding this comment.
P2: This wrapper never enables Pi's non-interactive mode, so pixeh will launch the interactive UI instead of returning a headless response.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/programs/fish/functions/_pixeh_function.tpl.fish, line 11:
<comment>This wrapper never enables Pi's non-interactive mode, so `pixeh` will launch the interactive UI instead of returning a headless response.</comment>
<file context>
@@ -0,0 +1,12 @@
+ return 1
+ end
+
+ pi-agent "$prompt" -m 'openrouter-preset/@preset/__GLM_NONDOT__'
+end
</file context>
Entire-Checkpoint: 3782180a0e72
Summary by cubic
Expands llm-update coverage to include aichat config, Pi settings, LLM default model, and fish wrapper templates so model upgrades stay in sync across tools. Adds tests to guard mappings and wrapper behavior.
_ocxe,_ocxeh,_pixe,_pixeh).scripts/llm-update.sh.spec/llm_update_spec.sh.Written for commit 5a1508e. Summary will update on new commits.