Skip to content

fix: negotiate unsupported sampling controls across transports - #774

Closed
seonghobae wants to merge 2 commits into
feat/paper-grounded-auto-orchestration-20260820from
fix/provider-temperature-capability-negotiation
Closed

fix: negotiate unsupported sampling controls across transports#774
seonghobae wants to merge 2 commits into
feat/paper-grounded-auto-orchestration-20260820from
fix/provider-temperature-capability-negotiation

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • retry once without temperature when the selected provider rejects that optional control with HTTP 400/422
  • apply the same capability negotiation to Chat Completions, Responses, and raw/proxy transport
  • keep model selection, fallback tables, and reasoning policy upstream-owned; no model-name heuristic

Evidence

  • uv run pytest tests/test_provider_protocol.py tests/test_provider_integration.py -q: 16 passed
  • full upstream suite on this feature head: 459 passed
  • git diff --check: passed

Decision record

  • docs/planning/adrs/0012-optional-sampling-capability-negotiation.md

This PR is intentionally scoped as a stack on feat/paper-grounded-auto-orchestration-20260820; it does not modify or force-push draft PR #765. Hosted Checks are not materialized for this non-default base, so this evidence is not a protected-main merge claim.

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 894b08de-ff9d-48b3-8005-a3b39f2841df

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Reviewed and fixed the current stack head. The shared is_provider_capability_error remains generic for structured-output negotiation; temperature retries now require a 400/422 response whose reason/body explicitly says temperature is unsupported. This prevents retrying invalid temperature values while preserving existing JSON-schema/json-object fallback. Exact HEAD 8977384; focused provider/protocol/passthrough tests: 40 passed; full suite: 460 passed; compileall and git diff --check passed. Please review this exact SHA only.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review: head 8977384 is based on closed branch feat/paper-grounded-auto-orchestration-20260820 (PR #766), so GitHub reports no required Checks. The current successor #765 does not contain the old provider_protocol field; retargeting without a rebase would expand this into a 72-file diff. Please rebase/cherry-pick the two capability-negotiation commits onto the current #765/main integration line, preserve the 16 focused tests, and make the exception-chain walk honor suppress_context so raise-from-none cannot trigger an unrelated temperature retry. Re-run hosted Checks and obtain independent approval before merge.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Successor PR #779 is open on the exact current #765 integration head. It reapplies only the temperature capability negotiation at the current common chat/raw transport boundary, with 1468 full-suite passes. #774 remains intentionally unretargeted and unmerged because its closed-base history has no hosted required Checks; no force-push or protection bypass was used.

Copy link
Copy Markdown
Contributor Author

Superseded by #779, which rebases the same optional-temperature capability negotiation onto the current #765 integration line and restores hosted exact-head Checks. Closing this stale-base duplicate unmerged; no evidence from this head transfers to #779.

@seonghobae seonghobae closed this Aug 20, 2026
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