Skip to content

fix(chat): use the model passed in via params - #3422

Merged
olimorris merged 2 commits into
mainfrom
fix/adapters-params
Oct 1, 2026
Merged

olimorris merged 2 commits into
mainfrom
fix/adapters-params

Conversation

@olimorris

@olimorris olimorris commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

Description

When looking at implementing a new edit_file tool, I noticed some issues in how my tool testing script was running and it led to the identification that models passed in through parameters on the chat may not resolve correctly, especially in the case of the CoPilot adapter.

Supporting Documentation

AI Usage

Claude Code + Opus 5.5

Related Issue(s)

Screenshots

Checklist

  • I've read the contributing guidelines and have adhered to them in this PR
  • I confirm that this PR has been majority created by me, and not AI (unless stated in the "AI Usage" section above)
  • I've run make all to ensure docs are generated, tests pass and StyLua has formatted the code
  • (optional) I've added test coverage for this fix/feature
  • (optional) I've updated the README and/or relevant docs pages

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Refactors how model parameters flow through the adapter system.

The PR appears safe to merge, though completion callbacks lose HTTP failure details when automatic compaction subsequently fails.

Findings

  1. P2 Compaction loses request errors ▶
Fix with agent prompt
### Issue 1
lua/codecompanion/interactions/chat/init.lua:1627
When an HTTP request fails and automatic context compaction starts, `done` returns before passing the request error to `finish`. If compaction also fails, its abort path calls `finish()` without that error. The `on_completed` callback then reports the failure without its details, making the original request failure harder to diagnose.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR routes chat model parameters through adapter resolution, gives explicit ACP model and mode options precedence over configured session defaults, and adds failure details to completion callbacks.

  • Tests cover model selection and HTTP and ACP completion errors.
  • The HTTP error detail is still lost if automatic compaction starts and then fails.

Reviews (2) · Last reviewed commit: "updates"

Comment thread lua/codecompanion/init.lua
Comment thread lua/codecompanion/init.lua
Comment thread lua/codecompanion/init.lua
Comment thread lua/codecompanion/interactions/chat/init.lua Outdated
Comment thread lua/codecompanion/interactions/chat/init.lua Outdated
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Deploying codecompanion with  Cloudflare Pages  Cloudflare Pages

Latest commit: e24c4ff
Status: ✅  Deploy successful!
Preview URL: https://66d6aa0f.codecompanion.pages.dev
Branch Preview URL: https://fix-adapters-params.codecompanion.pages.dev

View logs

return
end
self:finish()
self:finish({ error = opts.error })

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Compaction loses request errors

When an HTTP request fails and automatic context compaction starts, done returns before passing the request error to finish. If compaction also fails, its abort path calls finish() without that error. The on_completed callback then reports the failure without its details, making the original request failure harder to diagnose.

Prompt To Fix With AI
This is a comment left during a code review.
Path: lua/codecompanion/interactions/chat/init.lua
Line: 1627

Comment:
**Compaction loses request errors**

When an HTTP request fails and automatic context compaction starts, `done` returns before passing the request error to `finish`. If compaction also fails, its abort path calls `finish()` without that error. The `on_completed` callback then reports the failure without its details, making the original request failure harder to diagnose.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@olimorris
olimorris merged commit 170c287 into main Oct 1, 2026
9 checks passed
@olimorris
olimorris deleted the fix/adapters-params branch October 1, 2026 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant