fix: integrate hosted image tool preferences - #837
Conversation
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29e98c6706
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return `${field}.${key} cannot prefer ${tool}: the model does not support it`; | ||
| } | ||
| } | ||
| let effectiveWire = resolveEffectiveWire(key, registry?.adapter ?? provider.adapter); |
There was a problem hiding this comment.
For deepseek/deepseek-v4-flash on a Responses inbound request, the canonical registry declares an openai-responses model-wire default and resolveWireProtocolOverride selects it at runtime, but this validator initializes effectiveWire from the provider-wide registry.adapter (openai-chat) and never consults providerModelWireDefault. Consequently, both config loading and the management API reject a hosted-tool preference that would actually execute through the required Responses adapter; derive the effective wire through the same inbound-aware registry path used by the runtime.
AGENTS.md reference: src/AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
| const preferredTools = (selectedModelId ? provider.modelPreferHostedTools?.[selectedModelId] : undefined) | ||
| ?? provider.modelPreferHostedTools?.[modelId]; | ||
| if (!preferredTools?.includes(HOSTED_IMAGE_GENERATION_TOOL) || !isPlainObject(body)) return body; |
There was a problem hiding this comment.
When modelPreferHostedTools is present and a request routes to a valid model ID matching an inherited Object.prototype property, such as constructor or toString, this lookup returns the inherited function rather than an array and the subsequent .includes(...) call throws a TypeError before upstream dispatch. Model IDs with these values are accepted by routing, so use an own-property lookup and verify that the selected entry is an array before reading it.
AGENTS.md reference: src/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
|
Integration is now green on Ubuntu, macOS, Windows, all three npm-global jobs, React Doctor, and the target gate. The original contributor commit remains intact as the first commit. @lidge-jun @Wibias, ready for the independent maintainer approval required before merge. |
2 participants
Summary
devmodelPreferHostedToolsdevThe original PR branch does not allow maintainer edits and is now conflicting, so this integration branch carries the reviewed change without rewriting or squashing the contributor commit.
Test plan
bun test tests/openai-responses-passthrough.test.ts tests/management-provider-validation.test.ts tests/openai-api-virtual-models.test.ts tests/config.test.ts— 228 passedbun run typecheckDecision Log
maintainerCanModifyis disabled.dev, resolve only current-dev integration conflicts, and retain the original commit as the first commit in this PR.Supersedes #616 after merge.