fix(agent): route GPT-5+ model-service FQNs to Responses - #7358
Conversation
Signed-off-by: Fizz <400e8babadcee6a7f420103f10a2849d84c4a9c71d5bd04f3948c814216648a3@buzz.block.builderlab.xyz>
🔐 Codex Security Review
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1da1910833
ℹ️ 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".
| databricks_v2_wire_route: if model_service_fqn && fqn_requires_responses(raw_model_id) { | ||
| DatabricksV2Route::OpenaiResponses |
There was a problem hiding this comment.
Include Responses-only GPT services in discovery
When a GPT-5+ UC service advertises a non-empty supported_api_types list containing only mlflow/v1/responses, this route is unreachable through normal discovery: parse_uc_model_services_page still retains only services advertising mlflow/v1/chat/completions (catalog.rs lines 872–895), and its existing responses-only fixture confirms such entries are dropped. Because successful discovery is authoritative and does not fall back, Desktop and ACP omit precisely the Responses-capable service this branch can now invoke; update the discovery predicate to accept Responses-capable FQNs as well.
AGENTS.md reference: desktop/src/features/agents/AGENTS.md:L309-L309
Useful? React with 👍 / 👎.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Verdict: no material blockers found in this route-only change
Reviewed head 1da19108336570cb3287f407386ba8d6b3701e5f against base f038cbbb0d4092a72ffd93f17916f84d2b39bb43.
The service-only classifier and Rust/TypeScript parity check out. FQNs retain their neutral effort profile, the full model ID survives selection and request serialization, and completion plus handoff summarization use the same route decision. This is a COMMENT review, not an approval.
Existing finding and residual risks
- Responses-only discovery is a real, pre-existing limitation, not an established P1 regression here.
catalog.rs:857-896excludes services advertising onlymlflow/v1/responses; its existing fixture confirms that. This does not make the new route unreachable: chat-advertising services (including chat + responses), missing/empty capability metadata, and explicitly configured custom model IDs can reach it. Keep Responses-only discovery as a separate follow-up; align admission with the transport actually supported for that FQN rather than admitting all Responses-only services. - Live gateway acceptance remains unverified. The HTTP capture proves the client sends the unchanged FQN to
/ai-gateway/openai/v1/responses, not that Databricks accepts it. A live smoke test should include a tool-call continuation, not just a text answer. - Existing Responses reasoning continuity is not covered by this fix. The unchanged parser/replay path drops opaque reasoning output items. OpenAI's function-calling guide says to pass them back, while its reasoning guide describes this as strongly recommended for continuity and efficiency. I verified the omission but not a rejection of Buzz's reconstructed, call-ID-only history. I am not presenting that as a newly demonstrated high-severity failure or requiring a transport refactor in this routing PR.
Validation
Clean source at the reviewed head:
- Rust FQN unit/HTTP checks: 5 passed; FQN integration checks: 2 passed.
- Rust capability checks: 14 passed, 1 intentionally ignored corpus writer; Desktop corpus: 10 passed. Both interpreters cover the same 158 vectors.
- Existing Responses checks: 28 passed; discovery-filter regression: 1 passed. Test groups overlap.
git diff --checkpassed. Existing GitHub Actions Rust, Desktop, security and integration checks report success. No broad local CI rerun or live inference performed.
GitHub currently reports MERGEABLE with merge state BLOCKED; that is separate from this code verdict. The Codex security-review status comment also says a current-range review is required.
…-enforcement * origin/main: feat(mesh): upgrade to mesh-llm 0.76.0-rc8 and recommend Qwen3.8 27B (#6189) fix(agent): route GPT-5+ model-service FQNs to Responses (#7358) fix(buzz-acp): wake held ACP threads and fence forked sessions (#7340) fix(mobile): style inline code with the app mono face (#6631) chore(release): release Buzz Desktop version 0.5.23 (#7381) fix(desktop): keep packaged frontendDist relative so Windows embeds assets (#7177) fix(sidebar): simplify unread indicators and emphasize priority activity (#7134) Signed-off-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz>
* origin/main: feat(mesh): upgrade to mesh-llm 0.76.0-rc8 and recommend Qwen3.8 27B (#6189) fix(agent): route GPT-5+ model-service FQNs to Responses (#7358) fix(buzz-acp): wake held ACP threads and fence forked sessions (#7340) fix(mobile): style inline code with the app mono face (#6631) chore(release): release Buzz Desktop version 0.5.23 (#7381) fix(desktop): keep packaged frontendDist relative so Windows embeds assets (#7177) fix(sidebar): simplify unread indicators and emphasize priority activity (#7134) Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com>
* origin/main: (29 commits) fix(acp): pace targeted overflow recovery on consumer capacity (#7325) fix(link-preview): keep composer fetches user-paced (#7211) feat(mesh): upgrade to mesh-llm 0.76.0-rc8 and recommend Qwen3.8 27B (#6189) fix(agent): route GPT-5+ model-service FQNs to Responses (#7358) fix(buzz-acp): wake held ACP threads and fence forked sessions (#7340) fix(mobile): style inline code with the app mono face (#6631) chore(release): release Buzz Desktop version 0.5.23 (#7381) fix(desktop): keep packaged frontendDist relative so Windows embeds assets (#7177) fix(sidebar): simplify unread indicators and emphasize priority activity (#7134) Add generic information-flow control core (#7293) feat(buzz-acp): update base prompt; add buzz context and skills to Pi agents (#7335) fix(desktop): restore mention chip identity icons (#7338) Persist video playback speed preference (#7336) Verify ACP relay events before prompt routing (#7010) fix(buzz-acp): bound busy-owner hold to prevent cross-channel starvation (#7337) feat(desktop): invite owned agents from standalone forums (#7125) fix(desktop): authorize remote mentions at publication (#7124) fix(acp): rename system tag to agent-instructions (#7332) fix(desktop): bind duplicate mention selections to exact recipients (#7133) refactor(relay): extract NIP-29 membership authorization (#7285) ... Signed-off-by: Tom Brow <tomb@block.xyz>
…stody * origin/main: fix(acp): pace targeted overflow recovery on consumer capacity (block#7325) fix(link-preview): keep composer fetches user-paced (block#7211) feat(mesh): upgrade to mesh-llm 0.76.0-rc8 and recommend Qwen3.8 27B (block#6189) fix(agent): route GPT-5+ model-service FQNs to Responses (block#7358) fix(buzz-acp): wake held ACP threads and fence forked sessions (block#7340) fix(mobile): style inline code with the app mono face (block#6631) Signed-off-by: Joel Robotham <jrobotham@squareup.com>
* origin/main: (77 commits) fix(acp): pace targeted overflow recovery on consumer capacity (#7325) fix(link-preview): keep composer fetches user-paced (#7211) feat(mesh): upgrade to mesh-llm 0.76.0-rc8 and recommend Qwen3.8 27B (#6189) fix(agent): route GPT-5+ model-service FQNs to Responses (#7358) fix(buzz-acp): wake held ACP threads and fence forked sessions (#7340) fix(mobile): style inline code with the app mono face (#6631) chore(release): release Buzz Desktop version 0.5.23 (#7381) fix(desktop): keep packaged frontendDist relative so Windows embeds assets (#7177) fix(sidebar): simplify unread indicators and emphasize priority activity (#7134) Add generic information-flow control core (#7293) feat(buzz-acp): update base prompt; add buzz context and skills to Pi agents (#7335) fix(desktop): restore mention chip identity icons (#7338) Persist video playback speed preference (#7336) Verify ACP relay events before prompt routing (#7010) fix(buzz-acp): bound busy-owner hold to prevent cross-channel starvation (#7337) feat(desktop): invite owned agents from standalone forums (#7125) fix(desktop): authorize remote mentions at publication (#7124) fix(acp): rename system tag to agent-instructions (#7332) fix(desktop): bind duplicate mention selections to exact recipients (#7133) refactor(relay): extract NIP-29 membership authorization (#7285) ... Signed-off-by: Sol <478bb5a31222ea2b28a3d1afb8b1d598940628f19c2a87efc3c4b822299eeec6@buzz.block.builderlab.xyz> # Conflicts: # desktop/src-tauri/src/commands/media_download.rs # desktop/src-tauri/src/lib.rs
Summary
Route Databricks Unity Catalog model services to OpenAI Responses when their service name matches GPT-5 or newer. These models can reject tools plus reasoning on Chat Completions.
Match only the service component, using the existing family-token boundaries and a numeric major version. Catalog and schema names cannot select the protocol. Keep neutral effort capabilities and the full model ID unchanged; other services still use MLflow Chat Completions.
Keep the Rust and desktop resolvers in sync, add shared boundary cases and a captured-HTTP regression for completion and summarization, and update the documented FQN rule.
Related issue
No duplicate found in searches for “FQN responses” PRs or “astra” issues. Related: #6918 introduced Unity Catalog discovery.
Originating conversation: buzz://message?channel=0b881928-a3a6-4c01-b981-8e64268f01ce&id=770949343bc96a9ed88acd90a1b37d358a0efc52c79237d0fdb491ce02b8d4ed
Testing
No live Databricks inference test. The gateway must accept the full model-service ID on its OpenAI Responses route; this remains the integration risk.
The local
just ciattempt exceeded its five-minute deadline duringmobile-check, so the full repository gate was not completed. All push-hook checks passed.Generated with Codex