Skip to content

fix(cost-map): carry openai reasoning annotations onto their chatgpt twins - #42923

Closed
v0ropaev wants to merge 1 commit into
BerriAI:mainfrom
v0ropaev:chatgpt-reasoning-annotations
Closed

v0ropaev wants to merge 1 commit into
BerriAI:mainfrom
v0ropaev:chatgpt-reasoning-annotations

Conversation

@v0ropaev

Copy link
Copy Markdown

Closes #41394.

All fourteen chatgpt/* rows in the cost map carry zero reasoning annotations, while the openai rows they proxy carry the full set. Twelve of them have an openai twin to copy from; this fills those twelve in.

Why the fallback does not rescue them

_get_model_info_from_generalization (litellm/utils.py:5660) refuses to generalize when any candidate is an exact cost-map key:

Rules lose to exact entries: if ANY candidate is an exact litellm.model_cost key … the model is known rather than unmapped

chatgpt/gpt-5.6-sol is an exact key, so the empty row wins over the annotated twin it proxies. Measured through get_model_info, with LITELLM_LOCAL_MODEL_COST_MAP=True so the local map is the one under test:

main this branch
chatgpt/gpt-5.6-sol supports_reasoning=None, xhigh=None True, True
chatgpt/gpt-5.4 supports_reasoning=None, xhigh=None True, True
chatgpt/gpt-5.3-instant None, None None, None (no twin — left alone)

Why these values are ours to assert

Not a vendor claim read off a docs page. ChatGPTConfig subclasses OpenAIConfig (litellm/llms/chatgpt/chat/transformation.py:16) and ChatGPTResponsesAPIConfig subclasses OpenAIResponsesAPIConfig (litellm/llms/chatgpt/responses/transformation.py:39), which is where reasoning reaches allowed_keys and reasoning.encrypted_content is forced into include. A chatgpt row's reasoning behaviour is whatever the openai row describes, by inheritance in this repository's own code.

Each of the twelve rows takes exactly the reasoning keys its twin has, values included — so chatgpt/gpt-5.6-sol gets supports_minimal_reasoning_effort: false, not a blanket true.

Two rows are deliberately untouched: chatgpt/gpt-5.3-codex-spark and chatgpt/gpt-5.3-instant have no bare twin at all, so there is nothing to copy and anything I put there would be a guess.

The test

test_openai_reasoning_family_entries_carry_supports_reasoning already guards this class of hole, but it filters on entry.get("litellm_provider") == "openai" (test_model_prices_schema.py:258), so chatgpt-provider rows fall straight through it — which is how fourteen of them got here.

The new test pins the twin invariant rather than the twelve values, so the next chatgpt/* row added cannot repeat this. On main's data it fails with every mismatch named:

chatgpt/gpt-5.5.supports_reasoning is None, gpt-5.5.supports_reasoning is True
chatgpt/gpt-5.5.supports_minimal_reasoning_effort is None, gpt-5.5.supports_minimal_reasoning_effort is False
...

It is scoped to openai twins on purpose. The azure registry already diverges here on main — azure/gpt-5.4 has supports_minimal_reasoning_effort: true where the bare gpt-5.4 has false, and azure/gpt-5.2, azure/gpt-5.3-codex and azure/gpt-5.4-pro carry supports_reasoning without the effort flags their openai twins have. A wider invariant would fail on pre-existing data and would assert something this repository does not control.

Verification

pytest tests/test_litellm/test_model_prices_schema.py   →  62 passed
pytest tests/test_litellm/test_cost_map_guard.py        →  24 passed

Reverting only the two JSON files to main while keeping the test: 1 failed, 61 passed.

Both cost-map files are updated, so the backup-drift guard stays quiet, and python ci_cd/generate_model_prices_schema.py is a no-op — all five reasoning keys were already in the schema.

The JSON was edited textually rather than round-tripped through json.dump, which would have reformatted floats (0.00005 → 5e-05) across the whole 2.8 MB file. The diff is 50 added lines and 12 changed ones, and those twelve are only the trailing commas.

…twins

Every chatgpt/* row in the cost map lacked the reasoning annotations its openai
twin carries, so /model/info reported twelve reasoning models as non-reasoning.

The generalization fallback cannot rescue them: _get_model_info_from_generalization
returns None when any candidate is an exact cost-map key, and chatgpt/<model> is
one, so the unannotated row wins over the annotated twin it proxies.

The flags are ours to assert rather than a vendor claim - ChatGPTConfig subclasses
OpenAIConfig and ChatGPTResponsesAPIConfig subclasses OpenAIResponsesAPIConfig, so
a chatgpt row's reasoning behaviour is whatever the openai row describes.

Two rows are left alone because they have no openai twin to copy from:
chatgpt/gpt-5.3-codex-spark and chatgpt/gpt-5.3-instant.

The new test pins the twin invariant rather than the twelve values, so the next
chatgpt row added cannot repeat this. It is scoped to openai twins: the azure
registry already diverges here and is not this repository's to pin.
@greptile-apps

greptile-apps Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The PR appears safe to merge, with a non-blocking gap in the new regression test

Findings

  1. P2 Extra flags escape the guard ▶

Summary

The PR copies reasoning annotations from OpenAI models to twelve matching ChatGPT cost-map entries and adds a test for that relationship

  • Both cost-map copies receive the same annotations
  • The new test catches missing twin annotations, but not extra ChatGPT annotations

Reviews (1) · Last reviewed commit: "fix(cost-map): carry openai reasoning an..."

mismatched = []
for name, bare in twins:
for key in REASONING_ANNOTATION_KEYS:
if key in prices[bare] and prices[name].get(key) != prices[bare][key]:

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.

P2 Extra flags escape the guard The test checks only keys on the OpenAI twin, so an extra ChatGPT effort flag passes and can falsely advertise support

Suggested change
if key in prices[bare] and prices[name].get(key) != prices[bare][key]:
if (key in prices[name], prices[name].get(key)) != (key in prices[bare], prices[bare].get(key)):

@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing v0ropaev:chatgpt-reasoning-annotations (05eb57c) with main (1c8a0ff)

Open in CodSpeed

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Folded into the rolling registry PR #42951 at field level, with your test and a Co-authored-by trailer. Closing this one as superseded, thanks

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.

[Bug]: All chatgpt/* models missing reasoning annotations present on their openai/ and azure/ twins

1 participant