Skip to content

fix: negotiate unsupported temperature capability on current orchestration line - #779

Merged
seonghobae merged 10 commits into
fix/auto-reasoning-effort-contract-rebasedfrom
fix/provider-temperature-capability-negotiation-on-765-v2
Aug 20, 2026
Merged

fix: negotiate unsupported temperature capability on current orchestration line#779
seonghobae merged 10 commits into
fix/auto-reasoning-effort-contract-rebasedfrom
fix/provider-temperature-capability-negotiation-on-765-v2

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • retry the same chat or raw Responses endpoint once when HTTP 400/422 explicitly reports that temperature is unsupported
  • retain the negotiated omission across later transient retries instead of restoring the rejected field
  • preserve the provider 4xx body for downstream diagnostics and keep invalid-value/unrelated 4xx responses fail-closed
  • add ADR 0012, real local-HTTP transport coverage, and the exact Azure default-only regression

Evidence

  • exact base: contextual-orchestrator#765 current head d3f9a9b96523ed572b908c8abba1afa527eb49dc
  • exact head: cf4a4501fa5057f89b21cad5033c5925755cd150
  • focused transport and sampling tests: 15 passed in 4.59s
  • full suite on the refactor: publish gateway-only paper-grounded reasoning contract #765 integration line: 1472 passed in 546.95s
  • python -m compileall -q contextual_orchestrator
  • git diff --check

This is a successor to #774. It intentionally does not retarget or force-push #774, whose original base is a closed integration branch and therefore has no hosted required Checks.


Open in Devin Review

@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: a2db003c-e864-48b1-b1d6-9aee6eeb7427

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

Current-head review at beb15a7d77d3d47b8de711928faf76ab5d4f7ef7:

  • The added regressions preserve unrelated 4xx bodies, reject invalid caller temperatures as non-negotiation, and keep temperature omitted across subsequent transient retries after a provider capability rejection.
  • No additional source defect found in the current exact diff. The PR remains stacked on refactor: publish gateway-only paper-grounded reasoning contract #765 with no independent required Checks until its base is integrated.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@OpenCode review exact HEAD cf4a450. Verify the Azure default-only temperature diagnostic, 4xx body preservation, and that transient retries retain the negotiated omission. Full local suite: 1472 passed.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Live exact-head runtime evidence for cf4a450: the authenticated provider-neutral gateway returned HTTP 200 with one choice, and a direct validated transport call pinned to gpt-5.6-sol with temperature 0.2 completed successfully. Synthetic prompt only; response content was not persisted.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent Please publish a formal review bound to exact head cf4a4501fa5057f89b21cad5033c5925755cd150. Verify the Azure default-only temperature diagnostic, preservation of unrelated 4xx bodies, and persistence of negotiated temperature omission through later transient retries. Exact local evidence remains 1,472 passed plus an authenticated provider-neutral gpt-5.6-sol synthetic transport success with caller temperature 0.2. This PR is stacked on #765; do not approve stale heads or bypass its base gate.

…iation (#791)

* feat: auto-select embedding agents

* fix: align embedding selection API contract

* test: cover embedding capability selection boundaries

* test: declare embedding capability in embedding fixtures

* style: normalize embedding HTTP fixture imports

* docs: assign unique embedding selection ADR number
devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@devin review exact current HEAD 5a6f563. The valid ADR collision finding is fixed: optional sampling capability negotiation is now ADR 0016, while the accepted gateway-only provider contract remains ADR 0012; references were re-scanned and protocol regression tests pass (21 passed). Revalidate this exact HEAD and report only current findings.

* feat: opt in to client-owned tool loops

* test: repair stale standalone test entrypoints
devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@devin review exact current HEAD dbf46e5. Fixed the current finding: Responses tool-loop passthrough now runs only after non-empty input and stream validation, so stream=true returns invalid_stream instead of a silent non-streamed 200; missing input returns invalid_input. Added HTTP regressions; focused suite 19 passed with Ruff/diff clean. Revalidate this exact head.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@devin review exact current HEAD ef7b1fb. Current remote integration includes the parent stack, #787, the Responses tool-loop stream/input fix, and total inbound-body deadline enforcement. Exact-head focused integration tests pass (38 passed) with changed-file Ruff/diff clean. Revalidate this full SHA only and report any current finding.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment on lines +5999 to +6001
if time.monotonic() >= read_deadline:
self.close_connection = True
raise RequestError(408, "request_read_timeout", "request body read timed out")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Fully received requests can be rejected as timed out

A complete request body is rejected as timed out (raise RequestError(408,...) at contextual_orchestrator/server.py:5999-6001) after the final chunk that completes the body is read, so a valid, fully-received request is thrown away as an error.
Impact: On a slow connection whose last byte arrives right around the deadline, a request whose entire body was already received is returned a 408 error instead of being processed.

Deadline re-check runs even once the body is complete

The loop condition is while len(chunks) < body_size (contextual_orchestrator/server.py:5990). The deadline check at the top of the loop (5991-5993) correctly only fires when more data is still needed. However, the second check added after chunks.extend(chunk) (5999-6001) runs unconditionally after each read — including the read that completes the body. If time.monotonic() >= read_deadline at that moment, a 408 is raised even though len(chunks) == body_size and the full body is available in chunks. The top-of-loop check already covers the incomplete case on the next iteration, so the post-extend check only adds the spurious rejection of an already-complete body. Guarding it with len(chunks) < body_size avoids discarding a fully-read request.

Suggested change
if time.monotonic() >= read_deadline:
self.close_connection = True
raise RequestError(408, "request_read_timeout", "request body read timed out")
if len(chunks) < body_size and time.monotonic() >= read_deadline:
self.close_connection = True
raise RequestError(408, "request_read_timeout", "request body read timed out")
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current exact HEAD: 5774e1d2e878bf111de3229c3468d2071908ec81.

  • Stacked base was merged into the PR head with a normal merge commit; no force-push was used.
  • Root cause fixed: a slow byte trickle could extend inbound request reads beyond the configured total deadline; the shared read path now checks the monotonic deadline before and after each chunk.
  • Exact-head local proof: 1501 passed in 587.79s; changed test/security regression proof 76 passed in 29.63s; Ruff, compileall, and diff checks pass.
  • Current hosted checks: CodeRabbit pass; Devin pending.

@devin-ai-integration please revalidate only this exact HEAD. Normal auto-squash merge is armed; no approval or bypass is being used.

@seonghobae
seonghobae merged commit a4e9a78 into fix/auto-reasoning-effort-contract-rebased Aug 20, 2026
1 of 2 checks passed
@seonghobae
seonghobae deleted the fix/provider-temperature-capability-negotiation-on-765-v2 branch August 20, 2026 17:26
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