fix(agent): Change 'agent' tool model to be the small model#2365
Open
taoeffect wants to merge 8 commits intocharmbracelet:mainfrom
Open
fix(agent): Change 'agent' tool model to be the small model#2365taoeffect wants to merge 8 commits intocharmbracelet:mainfrom
taoeffect wants to merge 8 commits intocharmbracelet:mainfrom
Conversation
The `buildAgent()` function always passed the large model to sub-agents, ignoring the `Agent.Model` config field entirely. This meant the `agent` tool sub-agent would use the large model (e.g. Opus) even when the user had configured a different small task model (e.g. Gemini Flash), causing the sub-agent to incorrectly identify as the large model's identity. Now `buildAgent()` selects the primary model based on `agent.Model`, and `AgentTask` defaults to `SelectedModelTypeSmall` so the task sub-agent uses the user's configured small model for inference, system prompt construction, and provider prefix resolution. Assisted-by: Crush
taoeffect
requested review from
aymanbagabas,
Copilot and
raphamorim
and removed request for
a team
This was referenced Mar 5, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the default model used by the agent tool’s “Task” sub-agent to prefer the configured small model (to reduce cost/latency) and adjusts agent construction so the agent’s configured model selection is respected.
Changes:
- Switch the default
Taskagent inConfig.SetupAgents()to useSelectedModelTypeSmall. - Update
coordinator.buildAgent()to select the primary model based onagent.Modelwhen constructing aSessionAgent. - Update
prompt.Build(...)andSystemPromptPrefixselection to use the chosen primary model/provider.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/config/config.go | Changes the default Task agent model from large to small. |
| internal/agent/coordinator.go | Makes buildAgent() respect the agent’s configured model when selecting the primary model and building prompts/prefixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ildAgent Address two issues from Copilot PR review: 1. Convert SessionAgentOptions initialization from positional to keyed struct literal for readability and resilience to field reordering. 2. Always pass the real large model as LargeModel so that generateTitle's small→large fallback remains functional when the sub-agent is configured to use the small model for primary execution. Assisted-by: Opus 4.6 via Crush <crush@charm.land>
The Copilot review fix (251dc5a) regressed issue charmbracelet#2358 by always passing the real `large` model as `LargeModel` to `NewSessionAgent`. Since `Run()` dispatches inference via `a.largeModel`, the sub-agent always used the large model regardless of `agent.Model`. Fix: pass `primary` (which resolves to `small` when `agent.Model == SelectedModelTypeSmall`) as `LargeModel`, matching the pattern already used by `agentic_fetch_tool.go`. Assisted-by: Claude via Crush <crush@charm.land>
Contributor
Author
|
The CLA Assistant failed because of some network error - note I did sign the CLA in a separate PR. |
taoeffect
changed the title
Member
|
Thanks for the PR, @taoeffect. This may need to be something that's configurable, but maybe we should try this PR our first and evaluate the results (with a few of the normal big-small model combinations). P.S. I much appreciate the NOTE portion—thanks! |
Reconcile the buildAgent() conflict between the issue-2358 branch and main. The branch introduced a `primary` model variable that selects between large/small based on agent config, while main refactored ConfigStore access to use .Config() and added new SessionAgentOptions fields (SystemPrompt, Tools, Notify). The resolution preserves both: the primary model selection logic and main's updated API surface.
Contributor
Author
|
Note: one test is failing after merging |
taoeffect
added a commit
to taoeffect/crush
that referenced
this pull request
Mar 19, 2026
…tool Merge the agent model selection feature (PR charmbracelet#2365) with the LLM-driven compaction feature (PR charmbracelet#2333). The two branches independently modified `buildAgent()` in coordinator.go: - issue-2358 introduced per-agent model selection via a `primary` variable that picks the large or small model based on each agent's config, allowing the `agent` tool to default to the small (cheaper/faster) model. - new_session_tool added compaction flag derivation via `compactionFlags()`, which reads the configured `CompactionMethod` to control auto-summarize and context-status behavior for LLM-driven compaction. The resolution combines both: the primary model is selected per agent config, its provider config is used for the system prompt prefix, and the compaction flags are derived from the compaction method — preserving the full intent of both branches. Also retains `.tldr/` and `.tldrignore` gitignore entries from new_session_tool that were absent in issue-2358.
taoeffect
added a commit
to taoeffect/crush
that referenced
this pull request
Mar 19, 2026
Merge changes from taoeffect2 which includes: - feat: LLM and user-controlled compaction via `new_session` tool (charmbracelet#2333) Adds a `new_session` tool for customizable compaction, a compaction method switching menu in the UI, and context status injection into the system prompt when using LLM/user-driven compaction. - fix(agent): change agent tool model to use the small model (charmbracelet#2365) Switches the `agent` tool from the large task model to the small task model for cost savings and improved speed. Additionally merges Docker MCP support (enable/disable toggle in the commands menu, config persistence, and MCP client lifecycle management), session continuation on startup via --continue/--session flags, and RefreshTools on the Coordinator interface. Conflicts resolved (preferring taoeffect2): - internal/agent/coordinator.go: removed extra blank line before NewSessionAgent (cosmetic only) - internal/ui/model/ui.go: removed redundant cfg nil-check in compaction dialog handler, now uses m.com.Store() directly
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I don't know if it is intentional that the
agenttool uses the large task model but to me this was a surprise. I assumed it used the small task model, and to me this seems to make more sense and gives a way to save on cost while also improving speed/performance. In the case that you agree theagenttool should use the small task model I offer this PR as a fix for #2358.As an alternative I could work on a PR that offers the user a menu setting to allow the user to select which model the
agenttool uses.Closes #2358
NOTE: code written by Opus 4.6 under my supervision.