feat(routing): add filtered providers and stripped params to routing metadata#1657
feat(routing): add filtered providers and stripped params to routing metadata#1657
Conversation
…ing metadata Track which provider mappings were excluded during routing due to unsupported features (tools, vision, web_search, json_output, etc.) and which request parameters were stripped because the selected provider doesn't support them. This provides visibility into why certain providers were not considered. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds provider filtering reason tracking and parameter stripping tracking throughout the routing system. A new ChangesProvider Filtering & Parameter Stripping Observability
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/gateway/src/chat/chat.ts (1)
1004-1038:⚠️ Potential issue | 🟠 MajorAdd reasoning flags to fallback/direct filter options.
Line 1008 and Line 1153 define
filterOpts/directFilterOptswithout reasoning fields, sogetProviderFilterReasonswon’t filter providers that lack reasoning support (or honorno_reasoning) in fallback/direct paths. This can route to unsupported providers and omit reasoning-related reasons infilteredProviders.🛠️ Suggested fix
const filterOpts = { webSearchTool: !!webSearchTool, responseFormatType: response_format?.type, hasImages, hasTools: tools !== undefined || tool_choice !== undefined, + reasoningEffort: reasoning_effort, + reasoningMaxTokens: reasoning_max_tokens, + noReasoning: no_reasoning, }; const directFilterOpts = { webSearchTool: !!webSearchTool, responseFormatType: response_format?.type, hasImages, hasTools: tools !== undefined || tool_choice !== undefined, + reasoningEffort: reasoning_effort, + reasoningMaxTokens: reasoning_max_tokens, + noReasoning: no_reasoning, };Also applies to: 1114-1116, 1149-1178, 1222-1224
There was a problem hiding this comment.
Pull request overview
Adds richer routing introspection by recording (1) which provider mappings were excluded during routing and why, and (2) which request parameters were stripped due to provider capability limitations, then surfaces this data in the dashboard log UI.
Changes:
- Extend routing metadata schema/types to include
filteredProviders(with reasons) andstrippedParameters. - Record filtered provider mappings during routing and record stripped request params based on
supportedParameters. - Render the new routing metadata sections in the log card and log detail views.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/db/src/schema.ts | Extends log.routingMetadata JSON shape with filteredProviders and strippedParameters. |
| packages/actions/src/get-cheapest-from-available-providers.ts | Updates shared RoutingMetadata interface to include new optional fields. |
| apps/gateway/src/chat/chat.ts | Collects filter reasons during routing and records stripped parameters into routing metadata. |
| apps/ui/src/components/dashboard/log-card.tsx | Displays filtered providers + stripped parameters on the log card. |
| apps/ui/src/app/dashboard/[orgId]/[projectId]/activity/[logId]/log-detail-client.tsx | Displays filtered providers + stripped parameters in the detailed log view. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| webSearchTool: !!webSearchTool, | ||
| responseFormatType: response_format?.type, | ||
| hasImages, | ||
| hasTools: tools !== undefined || tool_choice !== undefined, |
There was a problem hiding this comment.
filterOpts.hasTools uses tools !== undefined, which is true even when tools is an empty array (e.g. web-search-only requests), and can incorrectly filter out providers with webSearch support but no function tools. Also, getProviderFilterReasons supports reasoning-related filtering, but this filterOpts omits reasoningEffort, reasoningMaxTokens, and noReasoning, so routing can still pick a provider mapping that can’t satisfy requested reasoning params. Compute hasTools from tools.length > 0 (plus tool_choice) and include the reasoning fields when calling getProviderFilterReasons.
| hasTools: tools !== undefined || tool_choice !== undefined, | |
| hasTools: | |
| (Array.isArray(tools) && tools.length > 0) || | |
| tool_choice !== undefined, | |
| reasoningEffort, | |
| reasoningMaxTokens, | |
| noReasoning, |
| webSearchTool: !!webSearchTool, | ||
| responseFormatType: response_format?.type, | ||
| hasImages, | ||
| hasTools: tools !== undefined || tool_choice !== undefined, |
There was a problem hiding this comment.
directFilterOpts.hasTools uses tools !== undefined, which is true even when tools is an empty array (e.g. web-search-only requests), and can incorrectly filter out providers with webSearch support but no function tools. Additionally, this directFilterOpts doesn’t include reasoningEffort, reasoningMaxTokens, or noReasoning, so provider mappings can be selected that don’t support requested reasoning params. Compute hasTools from tools.length > 0 (plus tool_choice) and include the reasoning fields when calling getProviderFilterReasons.
| hasTools: tools !== undefined || tool_choice !== undefined, | |
| hasTools: | |
| (Array.isArray(tools) && tools.length > 0) || | |
| tool_choice !== undefined, | |
| reasoningEffort, | |
| reasoningMaxTokens, | |
| noReasoning, |
| // Attach stripped parameters to routing metadata | ||
| if (strippedParameters.length > 0 && routingMetadata) { | ||
| routingMetadata.strippedParameters = strippedParameters; | ||
| } |
There was a problem hiding this comment.
New routingMetadata.strippedParameters behavior isn’t covered by the existing gateway tests (no matches for strippedParameters in apps/gateway/src tests). Add/extend an e2e/spec test that sends a request with an unsupported parameter for a provider (based on supportedParameters) and asserts the resulting log’s routingMetadata.strippedParameters contains the stripped param(s).
| const autoFilterOpts = { | ||
| webSearchTool: !!webSearchTool, | ||
| responseFormatType: response_format?.type, | ||
| hasImages, | ||
| hasTools: tools !== undefined || tool_choice !== undefined, |
There was a problem hiding this comment.
hasTools is computed as tools !== undefined || tool_choice !== undefined, which treats an empty tools array as “tools requested”. After extracting the web_search tool, tools can be [] for web-search-only requests, which would incorrectly filter out providers that support webSearch but not function tools (search-only models). Compute hasFunctionTools = Array.isArray(tools) && tools.length > 0 and set hasTools from hasFunctionTools || tool_choice !== undefined.
| const autoFilterOpts = { | |
| webSearchTool: !!webSearchTool, | |
| responseFormatType: response_format?.type, | |
| hasImages, | |
| hasTools: tools !== undefined || tool_choice !== undefined, | |
| const hasFunctionTools = Array.isArray(tools) && tools.length > 0; | |
| const hasTools = hasFunctionTools || tool_choice !== undefined; | |
| const autoFilterOpts = { | |
| webSearchTool: !!webSearchTool, | |
| responseFormatType: response_format?.type, | |
| hasImages, | |
| hasTools, |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/gateway/src/chat/chat.ts (1)
431-438: 🛠️ Refactor suggestion | 🟠 MajorAvoid
any/as anyin web-search tool extraction; narrow the union type directly.The
toolsparameter is a Zod-validated discriminated union offunctionandweb_searchtypes. You can remove bothtool: anyandas anyby using a type guard on thetypefield to narrow the union.Type-safe refactor
if (tools && Array.isArray(tools)) { const webSearchToolIndex = tools.findIndex( - (tool: any) => tool.type === "web_search", + (tool) => tool.type === "web_search", ); if (webSearchToolIndex !== -1) { - // Cast to any to access properties since the schema allows both function and web_search tools - const foundTool = tools[webSearchToolIndex] as any; - webSearchTool = { - type: "web_search", - user_location: foundTool.user_location, - search_context_size: foundTool.search_context_size, - max_uses: foundTool.max_uses, - }; + const foundTool = tools[webSearchToolIndex]; + if (foundTool.type === "web_search") { + webSearchTool = { + type: "web_search", + user_location: foundTool.user_location, + search_context_size: foundTool.search_context_size, + max_uses: foundTool.max_uses, + }; + } tools.splice(webSearchToolIndex, 1); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/chat/chat.ts` around lines 431 - 438, The current extraction uses `any` and `as any` for `tools` items; replace that with a proper type guard to narrow the Zod discriminated union so you don't need `any`. Create a predicate like `isWebSearchTool(tool): tool is WebSearchTool` that checks `tool.type === "web_search"`, use it with `Array.prototype.find` (or `findIndex` + array access) instead of `(tool: any)`, and then assign the result directly to `foundTool`/`webSearchTool` (no `as any` casting). Ensure the `tools` parameter is typed as the union type and use the guard so `foundTool`, `webSearchToolIndex`, and `webSearchTool` are inferred with the concrete `WebSearchTool` shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 2146-2149: The current attach of strippedParameters to
routingMetadata only happens once and can become stale after retries; update the
retry code paths so that after each call to resolveProviderContext (both in the
streaming and non-streaming retry loops) you merge that call's
strippedParameters into routingMetadata.strippedParameters (e.g., append/union
per-attempt values) instead of only setting it once, or modify
resolveProviderContext to return cumulative stripped params and assign that to
routingMetadata.strippedParameters; ensure you update all places that call
resolveProviderContext so routingMetadata.strippedParameters reflects every
attempt.
- Around line 805-813: The hasTools check currently treats an empty tools array
as true and over-filters providers; update the hasTools expression in
autoFilterOpts, filterOpts, and directFilterOpts to detect non-empty tool lists
by using (Array.isArray(tools) && tools.length > 0) || tool_choice !== undefined
so that a removed web_search (represented by an empty tools array) doesn’t count
as having tools; keep webSearchTool as its own flag (webSearchTool) and apply
this corrected hasTools logic to the hasTools fields in the objects named
autoFilterOpts, filterOpts, and directFilterOpts.
---
Outside diff comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 431-438: The current extraction uses `any` and `as any` for
`tools` items; replace that with a proper type guard to narrow the Zod
discriminated union so you don't need `any`. Create a predicate like
`isWebSearchTool(tool): tool is WebSearchTool` that checks `tool.type ===
"web_search"`, use it with `Array.prototype.find` (or `findIndex` + array
access) instead of `(tool: any)`, and then assign the result directly to
`foundTool`/`webSearchTool` (no `as any` casting). Ensure the `tools` parameter
is typed as the union type and use the guard so `foundTool`,
`webSearchToolIndex`, and `webSearchTool` are inferred with the concrete
`WebSearchTool` shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 94707c74-f6c0-42db-80d0-c1fecd8bf459
📒 Files selected for processing (1)
apps/gateway/src/chat/chat.ts
| const autoFilterOpts = { | ||
| webSearchTool: !!webSearchTool, | ||
| responseFormatType: response_format?.type, | ||
| hasImages, | ||
| hasTools: tools !== undefined || tool_choice !== undefined, | ||
| reasoningEffort: reasoning_effort, | ||
| reasoningMaxTokens: reasoning_max_tokens, | ||
| noReasoning: no_reasoning, | ||
| }; |
There was a problem hiding this comment.
hasTools currently over-filters providers when only web_search is used.
At Line 809 / Line 1057 / Line 1231, hasTools is derived from tools !== undefined. After removing the web_search tool from tools, an empty array still makes hasTools true, which can incorrectly add "tools not supported" and exclude valid providers.
Proposed fix
+ const hasFunctionTools =
+ Array.isArray(tools) && tools.some((tool) => tool.type === "function");
const autoFilterOpts = {
webSearchTool: !!webSearchTool,
responseFormatType: response_format?.type,
hasImages,
- hasTools: tools !== undefined || tool_choice !== undefined,
+ hasTools:
+ hasFunctionTools ||
+ (tool_choice !== undefined && tool_choice !== "none"),
reasoningEffort: reasoning_effort,
reasoningMaxTokens: reasoning_max_tokens,
noReasoning: no_reasoning,
};Apply the same hasTools expression to filterOpts and directFilterOpts.
Also applies to: 1053-1061, 1227-1235
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/gateway/src/chat/chat.ts` around lines 805 - 813, The hasTools check
currently treats an empty tools array as true and over-filters providers; update
the hasTools expression in autoFilterOpts, filterOpts, and directFilterOpts to
detect non-empty tool lists by using (Array.isArray(tools) && tools.length > 0)
|| tool_choice !== undefined so that a removed web_search (represented by an
empty tools array) doesn’t count as having tools; keep webSearchTool as its own
flag (webSearchTool) and apply this corrected hasTools logic to the hasTools
fields in the objects named autoFilterOpts, filterOpts, and directFilterOpts.
| // Attach stripped parameters to routing metadata | ||
| if (strippedParameters.length > 0 && routingMetadata) { | ||
| routingMetadata.strippedParameters = strippedParameters; | ||
| } |
There was a problem hiding this comment.
strippedParameters metadata can be stale after retry fallback switches provider.
Line 2147 attaches strippedParameters once, but retry paths call resolveProviderContext (see apps/gateway/src/chat/tools/resolve-provider-context.ts Line 215-Line 249), which strips parameters again per selected retry provider. Final logs can miss parameters stripped during retries.
Please merge per-attempt stripped parameters into routingMetadata.strippedParameters after each resolveProviderContext call (streaming and non-streaming retry loops), or return cumulative stripped params from resolveProviderContext.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/gateway/src/chat/chat.ts` around lines 2146 - 2149, The current attach
of strippedParameters to routingMetadata only happens once and can become stale
after retries; update the retry code paths so that after each call to
resolveProviderContext (both in the streaming and non-streaming retry loops) you
merge that call's strippedParameters into routingMetadata.strippedParameters
(e.g., append/union per-attempt values) instead of only setting it once, or
modify resolveProviderContext to return cumulative stripped params and assign
that to routingMetadata.strippedParameters; ensure you update all places that
call resolveProviderContext so routingMetadata.strippedParameters reflects every
attempt.
Conflicts in apps/gateway/src/chat/chat.ts and apps/ui/src/components/dashboard/log-card.tsx Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Resolved merge conflicts with origin/main: apps/gateway/src/chat/chat.ts (8 conflicts):
apps/ui/src/components/dashboard/log-card.tsx (2 conflicts):
|
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit