fix(api): validate max_tokens when preferred budget is omit - #641
fix(api): validate max_tokens when preferred budget is omit#641cursor[bot] wants to merge 29 commits into
Conversation
…closed otherwise Chat history: message-level audio and legacy function_call are null/empty omit no-ops; non-empty fail closed with named errors (including tools passthrough). Tip substrate from #577 assistant refusal/annotations honesty. Local full unit: 940 passed.
…ed otherwise OpenAI fine-tune style message weight is not applied on this gateway. Accept null/0/1 as honest no-ops; reject other types and values with invalid_message_weight. Tip substrate from #578. Local full unit: 943 passed.
…ion role Reject unsupported message keys with named unknown_message_fields (not silent strip or tools-passthrough smuggle). Reject legacy function role with invalid_message_role migration to tool. Tip substrate from #579. Local full unit: 947 passed.
OpenAI partial-assistant prefix flag is not applied on this gateway. null/false are honest no-ops; true and non-booleans fail closed with invalid_message_prefix. Tip substrate from #580. Local full unit: 950 passed.
…therwise Named invalid_max_tool_calls on /v1/chat/completions instead of opaque unknown_fields. Aligns with Responses max_tool_calls honesty; gateway has no multi-step tool loop.
…losed otherwise Legacy /v1/completions treated max_tool_calls as unknown_fields. Accept the key for named invalid_max_tool_calls (null/empty/whitespace omit-equivalent), matching chat/Responses honesty so SDKs get a clear migration path.
SDK clients often send include_usage/include_obfuscation as JSON null. Drop null flag values before validation so null (and null+false mixes) match omit / all-false no-ops on chat, Completions, and Responses. True flags remain fail-closed with invalid_stream_options.
…age honesty Null include_usage/include_obfuscation stay omit-equivalent, but unknown stream_options keys no longer become no-ops just because their value is null. Weight, prefix, refusal, annotations, developer role, empty user/system content, and participant name now use the same named errors on the tools passthrough path as on orchestration. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Hoist stream, required model, stream_options, and temperature/top_p range checks before proxy_completion so a tools or response_format body cannot return a billed JSON completion when the SDK asked for SSE, or silently pick a pool model when model is omitted. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Hoist stream, required model, stream_options, and temperature/top_p range checks before proxy_completion so a tools or response_format body cannot return a billed JSON completion when the SDK asked for SSE, or silently pick a pool model when model is omitted. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Hoist attribution and routing validation before proxy_completion so a tools or response_format body cannot bill a sync completion with an unknown spend dimension or a batch/latency_tolerant hint. Tools passthrough has no batch job plane. Buyer next action: send known sync attribution; omit routing.channel=batch and latency_tolerant=true on tool-calling requests. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Call the orchestration message, max_tokens, attribution, and routing validators before proxy_completion so a tools or response_format body cannot bill a completion with no prompt, crash on a non-object entry, or silently drop unknown spend/routing keys. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Match _validate_messages: tools + user content 123 must 400 invalid_message instead of billing a sync completion. Assistant/tool JSON null stays omit-equivalent. Buyer next action: send user/system content as a non-empty string or a content-parts array. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Match the tools-path honesty contract to the invalid_message behavior so SDK clients send a string or content-parts array. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Passthrough has no batch job plane. Reject routing.channel=batch and latency_tolerant=true before proxy_completion so a tool-calling body cannot bill a silent sync completion. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Hoist the remaining chat request knobs before proxy_completion so an OpenAI SDK tool-calling body cannot bill a sync completion for seed, stop, n>1, logprobs, logit_bias, out-of-range token/penalty values, unsupported reasoning_effort, or a non-default service_tier. Buyer next action: omit those fields on tool-calling requests; they are not applied on this gateway. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Call _validate_messages before proxy_completion so tools + [] / omitted / null / non-list / non-object messages raise invalid_message instead of billing a completion with no prompt. Buyer next action: always send a non-empty messages array of objects on tool-calling requests. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Hoist the orchestration request-knob validators before proxy_completion so a tools or response_format body cannot bill a completion that silently drops seed, stop, n>1, logprobs, logit_bias, or out-of-range penalties. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
stream_chat defaulted to 0.2 even after the HTTP path wrote default_temperature from the request. A streamed invoice summary at temperature=0.8 no longer silently falls back to 0.2. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
… proxy SDK optional defaults serialize omitted fields as JSON null. Accepting those keys without popping them is not omit-equivalent: proxy_completion forwards the body and several providers reject a null JSON Schema object. Pop the keys in place, keep non-null wrong types on invalid_tools, and assert omit-real via mock echo on chat and Responses. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
OpenAI SDK tool callers send stream=true. Returning 400 was honest but left every streaming tool client failing. Pipe a single pool agent as chat.completion.chunk frames so concatenated mock content matches the non-stream JSON body, and pipe live provider text so tool_calls deltas survive. Keep include_usage, empty messages, and unsupported knobs fail-closed. Buyer next action: send stream=true on tool-calling requests; omit stream_options.include_usage. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep tools/response_format SSE proxy from this branch and take #601's null tool.function omit-pop, request-knob HTTP cases, and streamed route temperature. Conflict resolution keeps stream=true as SSE, not 400. CHANGELOG records the combined unreleased slice. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Unknown assistant tool_calls entry/function fields, non-boolean include_orchestration_trace, and unknown mode used to bill a JSON or SSE completion on the tools proxy. They now return the same named 400s as the orchestration path before the first byte. Buyer next action: send only id/type/function/optional index on assistant tool_calls; send a boolean or omit include_orchestration_trace; send auto/route/conduct or omit mode. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
mock:// lookup_balance now returns assistant tool_calls and streams delta.tool_calls with finish_reason=tool_calls so offline SDK clients match the live _stream_raw path. Invoice identifiers bind from the user text (default INV-9). Buyer next action: send stream=true on tool-calling requests and include the invoice id in the user message. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
stream_chat recorded temperature but omitted the request-scoped top_p, presence_penalty, and frequency_penalty that chat() already forwards. A streamed invoice summary at top_p=0.1 no longer drops those knobs. Buyer next action: send the nucleus and penalty values you want on stream=true route requests; streaming matches the JSON path. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Tools / response_format proxy is the route hop. mode=conduct and include_orchestration_trace=true billed a silent chat.completion with no Conductor workflow and no TRINITY trace. Reject them with the same named errors as orchestration, including mixed orchestration=route + mode=conduct and the SSE tools path. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
SDK bodies often send max_completion_tokens: null with a sibling max_tokens. Treating key presence as precedence skipped the legacy validator, so max_tokens=0 still billed on tools, chat, and Completions. JSON false for top_logprobs was also omit-as-zero because False == 0. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Buyer next action
Do not send max_tokens=0 on /v1/chat/completions or /v1/completions even when max_completion_tokens is JSON null. That pair is now invalid_max_tokens. JSON top_logprobs: false is invalid_top_logprobs (not omit-as-zero). Integer top_logprobs=0 remains omit. Valid max_tokens>=1 with a null preferred budget still succeeds.
Unique-slice verdict (6135ab1 on #624 13ced1e)
SOUND. No Critical or Important defects.
_resolve_chat_output_token_budget uses independent ifs, not key-presence elif. Omit-equivalent preferred (JSON null / empty) falls through to sibling max_tokens. Invalid preferred (0 / bool) raises invalid_max_completion_tokens and does not fall through. The helper is wired on tools passthrough, chat, and Completions. Boolean top_logprobs is checked before == 0 / != 0 at all three sites.
Responses ~4496 already uses three independent ifs (max_tokens / max_completion_tokens / max_output_tokens) on the parent — out of scope and already safe.
Evidence (this tip)
python3 tests/test_passthrough_request_knobs_http_honesty.py— okpython3 tests/test_chat_max_completion_tokens_http_honesty.py— okpython3 tests/test_completions_max_completion_tokens_http_honesty.py— okpython3 tests/test_completions_top_logprobs_reject_http_honesty.py— okpython3 tests/test_chat_top_logprobs_http_honesty.py— okpython3 tests/test_paper_contracts.py/test_api_contract.py/test_self_check.py/test_conventions.py— ok
CodeRabbit CLI 0.7.3 is not_authenticated (auth login --agent timed out). Unique-slice review is from the isolated 6135ab1 diff plus the HTTP tests above.
Merge rule
Do not merge this 156-file honesty stack onto main. This is the omit-budget landing vehicle over #625 / #605 / #606 / #609 — do not merge those stacks either. Independent non-author APPROVE + Full unit / Semgrep are still required. This run does not self-approve.
Residual (next loop — do not open a third omit-budget PR)
- Empty-string preferred + sibling
max_tokens=0is validator-covered, not a new HTTP case. - Tools/chat still copy
top_logprobsinline instead of calling_validate_completions_top_logprobs(bool-first, soelif tlp != 0is not a False==0 trap). - Official Responses
text.formatlanding is #657. Token / allowlist / bind / TLS / sqlite / Clearfolio remains #621 / #637.json_schema.nameremains #654.
Sent by Cursor Automation: Fix Issues
| def _resolve_chat_output_token_budget(body: dict[str, Any]) -> int | None: | ||
| """Prefer ``max_completion_tokens``; fall back to ``max_tokens`` when omit. | ||
|
|
||
| OpenAI chat prefers ``max_completion_tokens`` when both keys are present. | ||
| JSON null or empty on the preferred key is omit-equivalent, so a sibling | ||
| ``max_tokens=0`` must still fail closed instead of billing a completion | ||
| with no output budget. | ||
| """ | ||
| preferred = None | ||
| if "max_completion_tokens" in body: | ||
| preferred = _validate_chat_max_completion_tokens(body) | ||
| if preferred is not None: | ||
| return preferred | ||
| if "max_tokens" in body: | ||
| return _validate_completions_max_tokens(body) | ||
| return None |
There was a problem hiding this comment.
SOUND. Independent ifs: omit-equivalent preferred (None / empty from _validate_chat_max_completion_tokens) falls through to sibling max_tokens. Invalid preferred (0 / bool) raises inside that helper and cannot fall through. This is the unique omit-budget close vs the parent elif "max_tokens" leftover on #624.
| if "top_logprobs" in body: | ||
| tlp = body.get("top_logprobs") | ||
| if tlp is None or (isinstance(tlp, str) and not tlp.strip()): | ||
| pass | ||
| elif isinstance(tlp, bool) or not isinstance(tlp, int): | ||
| raise RequestError( | ||
| 400, | ||
| "invalid_top_logprobs", | ||
| "top_logprobs must be an integer", | ||
| ) | ||
| elif tlp != 0: | ||
| raise RequestError( | ||
| 400, | ||
| "invalid_top_logprobs", | ||
| "top_logprobs is not supported on /v1/chat/completions", | ||
| ) |
There was a problem hiding this comment.
Residual only — not a False==0 trap. isinstance(tlp, bool) runs first, so JSON false cannot take elif tlp != 0. Same pattern at the chat route (~4186) and Completions helper (~940). Next honesty slice should call one shared helper with endpoint_path instead of copying this block. Do not open a third omit-budget PR for that DRY.
Pull request was closed
|
Superseded by tip substrate ≥ #691 (cumulative OpenAI/gateway honesty band + auto-merge tip). Closing to free product-gate runners (Full unit + Semgrep). |


Buyer next action
On
/v1/chat/completionsor/v1/completions, do not sendmax_tokens=0even whenmax_completion_tokensis JSON null. That pair used to bill a completion with no output budget. It now returnsinvalid_max_tokens. JSONtop_logprobs: falseisinvalid_top_logprobs(not omit-as-zero).Valid
max_tokens>=1with a null preferred budget still succeeds. Integertop_logprobs=0remains an omit-equivalent no-op. Tool-callingstream=truestays an SSE proxy on this tip.Why this PR exists
#625 (
2b6a984) is the same unique fix on the #605 146-file stack. Do not merge #625. This lands the fix on #62413ced1e(tools-honesty + SSE + conduct/trace).#624 still used
elif "max_tokens"in_validate_chat_passthrough_request_knobsandtlp != 0(False==0) in the tools hoist and chat route handler. Those leftover traps are closed here.Unique commit:
6135ab1on #624. Do not merge in parallel with #625 / #605 / #606 / #609.Evidence
python3 tests/test_passthrough_request_knobs_http_honesty.py— invoice-lookup tools, omit-preferred +max_tokens=0andtop_logprobs: falsepython3 tests/test_chat_max_completion_tokens_http_honesty.py— orchestration pathpython3 tests/test_completions_max_completion_tokens_http_honesty.py— Completions pathpython3 tests/test_completions_top_logprobs_reject_http_honesty.py— boolean rejectpython3 tests/test_chat_top_logprobs_http_honesty.py— chattop_logprobs: falseokdocs/architecture.md,docs/rest_api_design.md,conductor/tracks.mdResidual (next loop)
Do not open a third omit-budget PR. Token/allowlist landing remains #621. After #624+#621: remaining
__main__.pybind/TLS/sqlite/Clearfolio flags on the #621 tip only.Do not self-approve. Independent non-author APPROVE + Full unit/Semgrep required.