Skip to content

UN-4224 [FIX] Stop sending temperature to GPT-6 models on Bedrock, OpenAI and Azure OpenAI - #2310

Open
praveen-formido wants to merge 6 commits into
mainfrom
UN-4224-gpt6-temperature-strip
Open

praveen-formido wants to merge 6 commits into
mainfrom
UN-4224-gpt6-temperature-strip

Conversation

@praveen-formido

@praveen-formido praveen-formido commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

What

  • Stop sending temperature to OpenAI GPT-6 models (gpt-6-luna, gpt-6-sol, gpt-6-astra, gpt-6.1-sol) from the AWS Bedrock, native OpenAI and Azure OpenAI LLM adapters. Azure AI Foundry is covered through the existing strip.

Why

  • GPT-6 Luna on AWS Bedrock failed every call with temperature not permitted for this model. GPT-6 models reject the parameter.

  • Our adapters always pass a temperature: the pydantic default (0.1, or 1 for Azure OpenAI), and 1 when reasoning / extended thinking is on.

  • What reaches AWS depends on the route. I captured the outgoing request on LiteLLM 1.96.2 with drop_params=True:

    Model id Route Before After
    openai.gpt-6-luna Bedrock Mantle "temperature": 0.1, the reported failure omitted
    us. / global.openai.gpt-6-luna Bedrock Converse omitted by LiteLLM omitted
    openai.gpt-5.6-terra Bedrock Mantle omitted by LiteLLM's GPT-5 rule omitted
  • GPT-5.6 worked only because LiteLLM drops a non-1 temperature for gpt-5 names. That rule doesn't match GPT-6.

  • Not a regression from UN-4020 [FIX] Route AWS Bedrock Mantle models (GPT-5.6 Terra) via bedrock_mantle #2248 (UN-4020). That PR didn't handle temperature for any Mantle model.

How

  • _SAMPLING_DEPRECATED_MODEL_STEMS gets a gpt-6 stem. This strip already handles Claude Opus 4.7+ and is called by the Bedrock, Azure AI Foundry, Vertex and Anthropic adapters.
    • The existing trailing-edge anchor plus .→- normalisation matches every Bedrock encoding: bedrock/, bedrock_mantle/, us./global. profiles and gpt-6.1-*.
    • It excludes gpt-5.6-* (which normalises to gpt-5-6-*), gpt-60 and gpt-oss-*.
  • The native OpenAI adapter now calls the strip on its return path.
  • Azure OpenAI needed more than the strip:
    • The deployment name need not name the model, so GPT-6 is also detected from the optional Model field, which is carried as cost_model.
    • LLM sets cost_model aside at construction. A new LLM._revalidate helper, used by all four completion paths, passes it back into re-validation. It goes in after the per-call kwargs, so a caller passing temperature= cannot displace it. The cloud agentic workers do pass temperature=; this was flagged by review.
    • Azure detection therefore holds on every pass, and the sampling params are popped like in every other adapter.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • No change for any non-GPT-6 model. The new stem is anchored, and negative tests pin GPT-5.x, GPT-OSS, gpt-60 and older Claude models to keep their temperature.
  • The native OpenAI adapter now runs the shared strip. It matches only Claude Opus 4.7+ and GPT-6, so other OpenAI models are untouched.
  • Azure OpenAI only changes when GPT-6 is detected.
  • LLM._revalidate passes cost_model back to adapter.validate(). No parameter model declares it, Pydantic ignores unknown keys, and every call site already pops cost_model before LiteLLM, so no other adapter's request changes.
  • One limitation, the same as the existing Claude strip on Azure AI Foundry: an Azure OpenAI deployment whose name hides GPT-6 and whose Model field is empty is not detected.

Database Migrations

  • None

Env Config

  • None

Relevant Docs

  • None

Related Issues or PRs

Dependencies Versions

LiteLLM 1.104.0 is the first release whose bundled registry includes the GPT-6 Bedrock Mantle entries. With it, air-gapped deployments now route openai.gpt-6-luna to Mantle and price GPT-6 calls instead of recording $0. LiteLLM ≥ 1.100 hard-requires boto3>=1.43.1, so the storage stack moves with it:

Package Before After
litellm 1.96.2 1.104.0
boto3 / botocore 1.34.x 1.43.106 (exact; aiobotocore 3.9.2 accepts 1.43.101–106)
aiobotocore 2.x 3.9.2
s3fs / fsspec 2024.10.0 2026.9.0
gcsfs / adlfs 2024.10.0 / 2024.7 2026.10.0 / 2026.8.0
google-cloud-storage 2.9.0 3.16.0 (every gcsfs that works with fsspec 2026.x needs 3.x)

Follow-on changes:

  • workers requires-python is now bounded to <3.13, like every other project. The open bound made uv resolve Python 3.14, where protobuf ≥ 6.31 conflicts with the connectors' secret-manager/bigquery protobuf<5 caps. The image runs 3.12.
  • MinioFS: the UN-3487 walk() override is removed. fsspec 2026.x's DirFileSystem.walk() now relpaths entry names itself, so the override relpathed twice and tripped fsspec's assertion.
  • cohere timeout patch: re-pointed to 1.104.0 after re-diffing upstream. The sync path is still untimed upstream.
  • azure-datalake-store (ADLS Gen1) drops out with adlfs 2026; we only use AzureBlobFileSystem.

MinIO older than RELEASE.2025-01-20. boto3 ≥ 1.36 sends a CRC32 checksum instead of Content-MD5 on DeleteObjects, and older MinIO rejects that with MissingContentMD5. That includes the enterprise chart's bitnami-minio:2024.12.18. s3fs routes every rm through DeleteObjects.

  • Fix: a before-call.s3.DeleteObjects handler re-adds Content-MD5 over the final (XML-escaped) body. It is appended to botocore's BUILTIN_HANDLERS, so boto3, botocore and the aiobotocore sessions that s3fs creates all get it. S3 Express, which rejects MD5, is skipped.
  • Where it lives: one module, unstract/sdk1/patches/s3_delete_objects_md5.py. It's loaded by the sdk1 file-storage helper and imported by the MinIO connector. Connectors already depends on sdk1 at runtime, via unstract.filesystem in unstract_file_system.py.
  • Why not the alternatives: AWS_REQUEST_CHECKSUM_CALCULATION=when_required doesn't help, because DeleteObjects requires a checksum. botocore's deprecated conditionally_calculate_md5 skips requests that carry a flexible checksum.
  • No server-side change needed: the fix is client-side, so it works the same on the enterprise chart's standalone (Bitnami 2024-12-18) and HA (upstream 2024-12-18) MinIO, and on customer-run MinIO. No chart, image or operator change (a chart bump, Zipstack/unstract-cloud#1825, was closed). OSS compose already runs MinIO 2026-09-22 and is unaffected.

Notes on Testing

  • sdk1 suite on LiteLLM 1.104.0: 755 passed. Connectors: 72 passed (8 live-integration skips). Workers: 1358 passed.
  • Live storage scenario on the new stack against bitnami-minio:2024.12.18 and MinIO 2026-09-22, with the hook: FileStorage.rm(recursive=False), raw fsspec single and bulk rm, and MinioFS rm (file and recursive) all succeed with no fallback warnings. Without the hook, the single-file and raw-fsspec deletes fail on 2024.12.18. MinioFS ls/walk names stay bucket-relative on both.
  • Hook unit tests (sdk1) capture a real botocore DeleteObjects request: the digest matches the XML-escaped body, the handler runs after escape_xml_payload, and registration is idempotent. With the registration removed, 3 tests fail.
  • Full ETL workflow on locally built images (LiteLLM 1.104.0, boto3 1.43.106): MinIO source connector → exported Prompt Studio tool → MinIO destination connector, with MinIO as platform file storage. It completed, wrote the correct output to the destination bucket, and cleaned up its execution directory.
  • Same workflow with platform storage on the chart's exact bitnami-minio:2024.12.18:
    • With the hook: completed, correct output, execution directory cleaned up, no MissingContentMD5.
    • Control without the hook: Bulk delete failed with MissingContentMD5 in the logs; it only completed because of the UN-3421 fallback. New tests in tests/test_sampling_strip.py cover:
    • detection positives and negatives;
    • Bedrock validate for the unprefixed id and both explicit routes, with and without extended thinking, plus re-validation;
    • native OpenAI with and without reasoning;
    • Azure OpenAI with an opaque deployment name through the real LLM._revalidate, including a per-call temperature=0.5, a deployment that names the model, and non-GPT-6 controls (which keep a per-call temperature);
    • Azure AI Foundry.
  • With the new wiring disabled, the new tests fail (12 for the stem, 5 for the OpenAI/Azure wiring).
  • Local docker stack rebuilt from this branch (backend + worker-unified). Live Test Connection on the Bedrock adapter:
    • openai.gpt-6-luna on Mantle: 200, completion returned (25/32 tokens). This is the route that failed before.
    • global.openai.gpt-6-luna and us.openai.gpt-6-luna on Converse: 200.
    • Note that openai.gpt-6-luna on Mantle returns "model does not exist" in ap-south-1. That's regional availability, not this change.
  • ruff 0.3.4 check and format are clean.

Screenshots

  • N/A

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

…enAI and Azure OpenAI

GPT-6 models (luna, sol, astra, 6.1-sol) reject the `temperature`
parameter. Our LLM adapters always pass one (the pydantic default, or 1
when reasoning is on), and LiteLLM forwards it on the Bedrock Mantle
route, so every GPT-6 Luna call on Mantle failed with "temperature not
permitted for this model". GPT-5.6 was never affected because LiteLLM
has a GPT-5 rule that drops a non-1 temperature; that rule matches
`gpt-5` names only. The Converse route (`us.`/`global.` profile ids)
already drops it.

- Add a `gpt-6` stem to `_SAMPLING_DEPRECATED_MODEL_STEMS`, the strip
  already used for Claude Opus 4.7+. The Bedrock, Azure AI Foundry,
  Vertex and Anthropic adapters call it on their return path. The
  trailing-edge anchor keeps `gpt-5.6-*` (normalised to `gpt-5-6-*`),
  `gpt-60` and `gpt-oss-*` out.
- Call the strip from the native OpenAI adapter too.
- Azure OpenAI: the deployment name need not name the model, so detect
  GPT-6 from the optional Model field as well. `LLM` sets `cost_model`
  aside and re-validates without it, where Azure's default temperature
  of 1 would return, so pin `temperature` to None instead of popping it
  (LiteLLM omits a None temperature) and stop reasoning from forcing 1.

LiteLLM is intentionally not upgraded here: the first release whose
bundled registry carries the GPT-6 Mantle entries (1.104.0) needs
boto3>=1.43.1, which conflicts with the boto3 1.34 / s3fs 2024.x pins.
See UN-4224 for the routing gap that leaves on air-gapped deployments.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

via Greptile

RetriggerConfidence Score: 5/5

[High risk] Updates multiple dependencies and patches S3 delete behavior.

The PR appears safe to merge; no outstanding finding or new actionable issue remains.

Summary

The PR removes unsupported sampling parameters for GPT-6 requests across the affected LLM adapters, upgrades the supporting dependencies, and restores Content-MD5 for older MinIO servers. The latest changes consolidate the MinIO hook in sdk1 without changing its registration behavior.

Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[LLM call] --> B[Adapter validation]
  B --> C{GPT-6 detected?}
  C -->|Yes| D[Remove sampling parameters]
  C -->|No| E[Keep supported parameters]
  D --> F[LiteLLM provider request]
  E --> F
Loading

Reviews (5) · Last reviewed commit: "UN-4224 [FIX] Keep one copy of the Delet..."

Comment thread unstract/sdk1/src/unstract/sdk1/adapters/base1.py Outdated
…e per call

Review (Greptile P1): for an Azure OpenAI deployment whose name hides
GPT-6, the only record of the model that survived `LLM`'s re-validation
was the pinned `temperature: None`. `complete()` merges per-call kwargs
over the stored ones, so a caller passing `temperature=` (the cloud
agentic_table / agentic_extraction workers do) overwrote that marker and
the temperature reached a model that rejects it.

Detect from the real model id instead of a caller-writable marker.
`LLM._revalidate` now feeds the `cost_model` it set aside back into
re-validation, after the per-call kwargs so they cannot displace it, and
the four completion paths use it. Azure prefers that `cost_model` as the
original model id, so detection holds on every pass and the sampling
params are simply popped like every other adapter -- the `None` pin and
its sticky-marker check are gone.

Other adapters are unaffected: none declares `cost_model`, Pydantic
ignores unknown keys, and every call site already pops `cost_model`
before LiteLLM sees the kwargs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ge stack

LiteLLM 1.104.0 is the first release whose bundled model registry carries
the GPT-6 Bedrock Mantle entries, so air-gapped deployments (which fall
back to the bundled registry) now route `openai.gpt-6-luna` to Mantle and
price GPT-6 calls instead of recording $0.

LiteLLM >= 1.100 hard-requires boto3 >= 1.43.1, which drags the storage
stack with it; these move together:

- litellm 1.96.2 -> 1.104.0
- boto3 / botocore 1.34.x -> 1.43.106, pinned exactly in backend, workers
  and sdk1: it must stay inside the botocore range aiobotocore 3.9.2
  accepts (1.43.101-1.43.106)
- s3fs / fsspec 2024.10.0 -> 2026.9.0 (s3fs drops its `[boto3]` extra)
- gcsfs 2024.10.0 -> 2026.10.0, adlfs 2024.7 -> 2026.8.0
- google-cloud-storage 2.9.0 -> 3.16.0: every gcsfs compatible with
  fsspec 2026.x needs google-cloud-storage 3.x. Our direct use (Client,
  bucket, get_blob, md5_hash, upload_from_*) is unchanged in 3.x.

Follow-on fixes:
- workers: bound `requires-python` to <3.13 like every other project. The
  open bound made uv resolve Python 3.14, where google-api-core >= 2.27
  needs protobuf >= 6.31 but the connectors' secret-manager / bigquery
  pins cap it below 5. The image runs Python 3.12.
- MinioFS: drop the UN-3487 `walk()` override. fsspec 2026.x's
  `DirFileSystem.walk()` relpaths each entry's `name` itself, so the
  override relpathed twice and tripped fsspec's assertion. The UN-3487
  regression test now guards the upstream behaviour.
- cohere embed timeout patch: re-pointed to 1.104.0 after re-diffing
  upstream; the sync `embedding()` still builds an untimed HTTPHandler.

Known gap, deliberately not fixed here: boto3 >= 1.36 stops sending
Content-MD5 on DeleteObjects, which MinIO older than RELEASE.2025-01-20
rejects. Verified live against the enterprise chart's
bitnami-minio:2024.12.18: single-file deletes (recursive=False, raw
fsspec) fail with MissingContentMD5, directory cleanup survives through
the UN-3421 fallback. MinIO 2026-09-22 (OSS compose) is unaffected.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment thread backend/pyproject.toml
praveen-formido and others added 3 commits October 6, 2026 10:04
…than 2025-01-20

botocore 1.36+ sends a CRC32 flexible checksum instead of Content-MD5 on
DeleteObjects. MinIO older than RELEASE.2025-01-20 -- including the
enterprise chart's bitnami-minio:2024.12.18 -- rejects that with
MissingContentMD5, and s3fs routes every `rm` through DeleteObjects, so
with the boto3 1.43 upgrade single-file deletes (recursive=False, raw
fsspec) and connector deletes failed on those servers. The UN-3421
fallback only covered recursive deletes through sdk1 FileStorage.

Register a `before-call.s3.DeleteObjects` handler that adds Content-MD5
over the final (XML-escaped) body. It is appended to botocore's
BUILTIN_HANDLERS, so every session created afterwards gets it -- boto3,
botocore and the aiobotocore sessions s3fs creates. AWS S3 and new MinIO
accept both headers; S3 Express (which rejects MD5) is skipped.

`AWS_REQUEST_CHECKSUM_CALCULATION=when_required` does not help (the
operation requires a checksum), and botocore's deprecated
`conditionally_calculate_md5` skips requests that carry a flexible
checksum, so neither could be reused.

unstract-connectors does not depend on sdk1, so each carries an
identical, idempotent copy: sdk1 loads it from the file-storage helper,
connectors from the MinIO connector. Whichever loads first registers.

Verified live on the new stack against bitnami-minio 2024.12.18 and
MinIO 2026-09-22: FileStorage.rm(recursive=False), raw fsspec single and
bulk rm, and MinioFS rm (file and recursive) all succeed with no
fallback warnings.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
SonarCloud failed the quality gate on duplicated new code (39.2% vs 3%):
connectors carried a verbatim copy of sdk1's hook and its tests, on the
assumption that connectors does not depend on sdk1.

It does at runtime: `unstract_file_system.py`, the base class every
filesystem connector extends, imports `unstract.filesystem`, which
depends on sdk1. So the MinIO connector now imports
`unstract.sdk1.patches.s3_delete_objects_md5` directly, and the copy and
its duplicate tests are gone. Importing the connector still registers the
handler (verified), and connector deletes against MinIO 2024-12-18 still
succeed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Oct 6, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Unstract test results

Per-group results

Status Group Tier Passed Failed Errors Skipped Duration (s)
✅ e2e-api-deployment e2e 3 0 0 0 8.6
✅ e2e-coowners e2e 1 0 0 0 1.6
✅ e2e-etl e2e 1 0 0 0 14.4
✅ e2e-login e2e 2 0 0 0 1.2
✅ e2e-prompt-studio e2e 1 0 0 0 11.5
✅ e2e-smoke e2e 2 0 0 0 1.4
✅ e2e-workflow e2e 1 0 0 0 12.3
✅ frontend unit 620 0 0 0 14.2
✅ integration-backend integration 603 0 0 26 60.4
✅ integration-connectors integration 1 0 0 7 8.7
✅ integration-workers integration 164 0 0 1 40.1
❌ ui e2e 0 1 0 0 0.0
✅ unit-backend unit 1422 0 0 1 36.8
✅ unit-connectors unit 72 0 0 0 8.3
✅ unit-core unit 281 0 0 0 2.4
✅ unit-platform-service unit 15 0 0 0 2.0
✅ unit-rig unit 120 0 0 0 3.4
✅ unit-runner unit 10 0 0 0 3.4
✅ unit-sdk1 unit 753 0 0 0 29.8
✅ unit-workers unit 1383 0 0 1 116.0
TOTAL 5455 1 0 36 376.6

Critical paths

⚠️ Critical paths not yet covered

  • workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)
✅ Covered critical paths
  • auth-login — covered by e2e-login
  • adapter-register-llm — covered by integration-backend
  • workflow-author — covered by integration-backend
  • co-owner-manage — covered by integration-backend, e2e-coowners
  • workflow-create-execute — covered by e2e-workflow
  • api-deployment-provision — covered by integration-backend
  • api-deployment-auth — covered by integration-backend
  • api-deployment-run — covered by e2e-api-deployment
  • mcp-server-auth — covered by integration-backend
  • mcp-platform-auth — covered by integration-backend
  • platform-key-whoami — covered by integration-backend
  • prompt-studio-author — covered by integration-backend
  • prompt-studio-fetch-response — covered by e2e-prompt-studio
  • connector-register-test — covered by integration-backend
  • pipeline-etl-execute — covered by e2e-etl
  • usage-aggregate-read — covered by integration-backend
  • usage-token-tracking — covered by e2e-api-deployment
  • callback-result-delivery — covered by e2e-api-deployment

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.

3 participants