Skip to content

fix: merge chat and completion provider routing instead of replacing it - #605

Closed
LukasParke wants to merge 3 commits into
mainfrom
claude/merge-chat-provider-routing
Closed

LukasParke wants to merge 3 commits into
mainfrom
claude/merge-chat-provider-routing

Conversation

@LukasParke

@LukasParke LukasParke commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #587

Bug. In the chat model, providerOptions.openrouter.provider replaced the configured provider routing instead of merging into it. doGenerate/doStream built the body as { ...this.getArgs(options), ...restOpenrouterOptions }. Here getArgs() sets provider: this.settings.provider (and then spreads both extraBodys), so a runtime provider object overrode all of it. The completion model had the same pattern for extraBody.provider. Completion has no typed provider setting.

Repro.

const model = openrouter.chat('google/gemini-2.5-flash', {
  provider: { order: ['google'], allow_fallbacks: false, sort: 'price' },
});
await generateText({ model, prompt: 'hi', providerOptions: { openrouter: { provider: { order: ['anthropic'] } } } });
// sent provider: { order: ['anthropic'] } (allow_fallbacks and sort are lost)

Fix. Same semantics as the image model (#551): a shallow merge with precedence settings.provider < config.extraBody.provider < settings.extraBody.provider < providerOptions.openrouter.provider. Non-object values (null, arrays, primitives) are ignored.

  • New helper src/utils/merge-provider-routing.ts (mergeProviderRouting). The image model keeps its own inline merge (same semantics), see Merge coordination below.
  • Chat: doGenerate and doStream.
  • Completion: doGenerate and doStream, using the two extraBody sources plus runtime.

Behavior change (noted in the changeset): a runtime provider object no longer drops configured keys it doesn't set. To override a key, set it explicitly in the runtime object.

Not changed. Embedding, reranking and evaluation spread providerOptions.openrouter differently, and the issue doesn't cover them. I left them alone to keep this PR scoped. I can follow up if we want the same semantics there.

Tests.

  • src/chat/index.test.ts: the issue's example in doGenerate and doStream, full precedence across all four sources, and a null runtime provider being ignored.
  • src/completion/index.test.ts: provider and model extraBody merged with runtime, in doGenerate and doStream.
  • src/utils/merge-provider-routing.test.ts: unit tests for the helper.
  • e2e/issues/issue-587-provider-routing-merge.test.ts: issue regression test (pnpm test:issues, mocked fetch). It runs the issue's example through generateText.

I checked that the 6 new chat/completion tests and the issue test fail on origin/main and pass with this fix.

Checklist

  • I have run pnpm stylecheck and pnpm typecheck
  • I have run pnpm test and all tests pass
  • I have added tests for my changes (if applicable)
  • I have updated documentation (if applicable)

Changeset

  • I have run pnpm changeset to create a changeset file

Merge coordination

Merges cleanly with #603 and #597 in either order, so no hand resolution is needed. The image model is no longer refactored onto mergeProviderRouting. Its new import landed on the same line as #603's user-agent import, and the refactor didn't change behaviour, so src/image/index.ts stays as on main. I checked this with local integration merges of #590–#609 in both orders.

🤖 Generated with Claude Code

A runtime providerOptions.openrouter.provider object replaced the configured
provider routing wholesale. Chat and completion now shallow merge it like the
image model (#551), through a shared mergeProviderRouting helper that the
image model also uses.

Fixes #587

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

LukasParke and others added 2 commits October 22, 2026 23:29
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…lict with #603

The image model already shallow merges provider routing (#551); moving it
onto mergeProviderRouting was a pure refactor, and its new import landed on
the same line as #603's user-agent import. Keep image/index.ts as on main so
the two PRs merge in either order. No behaviour change.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
LukasParke added a commit that referenced this pull request Oct 23, 2026
Behaviour is unchanged; only where the stripping happens moves, so this PR
merges cleanly with #605 and #608 in either order:
- chat: drop reasoningReplay where openrouterOptions is read, leaving the
  cacheControl destructure as on main (#605 extends that destructure).
- completion: drop it from the assembled args in a separate statement,
  leaving the openrouterOptions line as on main (#605 rewrites it).
- convert: put a blank line between the candidate selection and the replay
  filter so the hunk does not touch the findFirstReasoningDetails call
  (#608 renames it).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@LukasParke

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #628 (merged), which merges provider routing in the chat and completion models through the shared mergeProviderRouting helper and closed #587.

@LukasParke LukasParke closed this Nov 10, 2026
LukasParke added a commit that referenced this pull request Nov 10, 2026
Behaviour is unchanged; only where the stripping happens moves, so this PR
merges cleanly with #605 and #608 in either order:
- chat: drop reasoningReplay where openrouterOptions is read, leaving the
  cacheControl destructure as on main (#605 extends that destructure).
- completion: drop it from the assembled args in a separate statement,
  leaving the openrouterOptions line as on main (#605 rewrites it).
- convert: put a blank line between the candidate selection and the replay
  filter so the hunk does not touch the findFirstReasoningDetails call
  (#608 renames it).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.

Chat model replaces configured provider routing instead of merging (consistency with image model, #551)

1 participant