fix(api): fail-closed seed, spend knobs, and batch hints on tools path - #605
fix(api): fail-closed seed, spend knobs, and batch hints on tools path#605cursor[bot] wants to merge 16 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.
Validate weight/prefix/refusal/annotations before the tools early-return so SDK tool-calling bodies cannot smuggle unsupported message fields. Strip omit-equivalent max_tool_calls before provider passthrough and name-reject the field on /v1/completions. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Reject non-OpenAI keys on assistant tool_calls objects with named unknown_tool_call_fields / unknown_tool_call_function_fields. Accept optional non-negative index (null omit) for stream-assembled histories. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Developer role, empty user/system content, multimodal part shape, and participant name were only checked in _validate_messages, which the tools/response_format early-return skips. SDK tool-calling bodies could therefore proxy unsupported values. Run the same fail-closed checks before passthrough and cover the realistic tools+field HTTP cases. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Completions and chat applied temperature, top_p, penalties, and max_tokens by mutating shared ModelClient defaults. Concurrent ThreadingHTTPServer workers could observe another request's knobs. request_sampling stores overrides on threading.local and restores them on exit; chat/stream_chat read the calling thread first. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Tools/response_format still billed a completion when messages was [], omitted, null, or a non-list. Require the same non-empty array as _validate_messages before the early-return. stream_chat now reads the same thread-local sampling knobs as chat so route streaming cannot drop top_p or penalties. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep Fugu/TRINITY/Conductor titles for paper-contract search while recording APA 7th citations. Split the optional tool_calls index case so omit-key is actually tested instead of the previous tautology. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Tools/response_format still silent-selected a worker when model was omitted and returned JSON 200 for stream=true. Require a pool model and reject stream=true before the early-return so SDK tool-calling bodies cannot hide the deployment or lie about SSE. HTTP honesty tests cover tools and response_format; named-model + stream=false still proxies. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Those validators sat after the tools/response_format early-return, so SDK tool-calling bodies could request include_usage or smuggle a non-object attribution/routing and still bill a completion. Run the same named checks before proxy_completion. All-false stream_options stays an omit-equivalent no-op. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Sampling-range checks sat after the tools/response_format early-return, so temperature=9 and top_p=2 were forwarded and billed. Run the same Completions range validators before proxy_completion. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Tools/response_format still billed a completion when seed, out-of-range penalties, zero max_tokens, n>1, or batch routing hints were set. Hoist the same named checks used on the orchestration path before proxy_completion. HTTP honesty tests cover the realistic tools+field cases. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Those validators still sat after the tools/response_format early-return, so SDK tool-calling bodies could request stop sequences, blank user attribution, or token logprobs and still bill a completion. Run the same named chat-path checks before proxy_completion. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
There was a problem hiding this comment.
Stale comment
Review (PR #605)
Do not merge this draft. It is a 146-file honesty stack on the #599 substrate. Landing vehicle remains #601 (
7e9a339). Cherry-pick the unique commits onto that head; do not land #591/#594/#596/#597/#599/#605 in parallel.Unique slice (reviewed)
109b9d2+6ef1405hoistseed, penalties,max_tokens/max_completion_tokens,n,user,stop,logprobs/top_logprobs, andlogit_biasbeforeproxy_completion, and rejectrouting.channel=batch/latency_tolerant=trueon the tools path. Named errors match the orchestration path. HTTP cases intests/test_chat_spend_knobs_tools_passthrough_http_honesty.pycover a realistic invoice-lookup + tools body, including omit-equivalent no-ops (seed: null,stop: null,logprobs: false, emptylogit_bias).The PR body is stale: it still lists
stop/user/logprobs/logit_biasas residual. Those landed in6ef1405.Residual that still billed on this head
On
6ef1405,tools+store=truestill returned200 chat.completion(prompt_tokens: 0) while store was dropped. The same fail-open applied tomodalities=["audio"],prediction,reasoning_effort=high,service_tier=flex, and non-stringmetadata. Those validators still sat after the early-return.Successor unique commit
7c99a27hoists them. Prefer cherry-picking that commit onto #601, not merging this stack.Buyer next action
Omit
seed,n>1,stop,logprobs=true, nonzerologit_bias, batch routing hints,store=true, non-textmodalities,prediction,reasoning_effortother thannone, andservice_tierother than auto/default on tool-calling requests. Keep SDK-default nulls as honest no-ops.Independent non-author APPROVE + Full unit/Semgrep still required. Do not self-approve. Draft #606 already owns the SSE-proxy residual — do not open a third stream=true stack.
Sent by Cursor Automation: Fix Issues
|
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.
Review (PR #605)
Do not merge this 146-file stack. Unique commits 109b9d2 + 6ef1405 correctly hoist seed, penalties, max_tokens / max_completion_tokens, n, batch / latency_tolerant, stop, user, logprobs / top_logprobs, and logit_bias before proxy_completion. Named errors match the orchestration path. Invoice-lookup + lookup_balance HTTP cases in tests/test_chat_spend_knobs_tools_passthrough_http_honesty.py are real.
The PR body is stale: it still lists stop / user / logprobs / logit_bias as residual. Those landed in 6ef1405.
Defect in this unique slice
if "max_completion_tokens" in body wins even when the value is JSON null. A tools body with max_completion_tokens: null and max_tokens: 0 still returned 200 chat.completion (usage.prompt_tokens: 0, echo max_tokens: 0). top_logprobs: false was omit-as-zero because False == 0.
Successor unique commit 2b6a984 (_resolve_chat_output_token_budget) fixes both on tools, chat, and Completions. Cherry-pick that commit onto #606, not this stack.
Residuals already owned elsewhere
On 6ef1405, store / modalities / prediction / reasoning_effort / service_tier / metadata still sit after the early-return — #609 7c99a27. Mode / include_orchestration_trace — #610 / #613. SSE tools — #606 / #617.
Do not land #591–#605 in parallel. Independent non-author APPROVE + Full unit/Semgrep required. Do not self-approve.
Buyer next action
On /v1/chat/completions with tools, omit seed, n>1, stop, logprobs=true, nonzero logit_bias, and batch routing hints. Do not send max_tokens=0 even when max_completion_tokens is null — that pair used to bill; take 2b6a984 for the fail-closed repair. Keep SDK-default nulls as honest no-ops.
Sent by Cursor Automation: Fix Issues
| those, so an SDK tool-calling body could bill a seeded, over-range, or | ||
| multi-choice completion the gateway cannot honor. | ||
| """ | ||
| if "max_completion_tokens" in body: |
There was a problem hiding this comment.
if "max_completion_tokens" in body wins even when the value is JSON null/empty (omit in _validate_chat_max_completion_tokens). A tools body with max_completion_tokens: null and max_tokens: 0 never reaches _validate_completions_max_tokens and still bills 200 chat.completion.
Successor 2b6a984 falls back to the legacy validator when the preferred key is omit-equivalent. Do not grow this stack to land that fix.
| ) | ||
| if "top_logprobs" in body: | ||
| tlp = body.get("top_logprobs") | ||
| if tlp is not None and tlp != 0: |
There was a problem hiding this comment.
tlp is not None and tlp != 0 treats JSON false as omit because False == 0. That silent-strips a wrong type and bills. Fail-closed is invalid_top_logprobs (integer required). top_logprobs=5 is already hoisted and tested. Repair is in 2b6a984.
| _validate_routing(body.get("routing")) | ||
| _validate_completions_temperature(body) | ||
| _validate_completions_top_p(body) | ||
| _validate_chat_passthrough_spend_knobs(body) |
There was a problem hiding this comment.
This unique slice correctly hoists spend knobs + batch reject immediately before proxy_completion. The early-return at 3865 still skips the orchestration-only block, so store / modalities / prediction / reasoning_effort / service_tier / metadata (3934–3945) and mode / include_orchestration_trace (3867–3884) remain fail-open on tools bodies. Those remainders are already on #609 and #610/#613 — do not grow this 146-file stack to hoist them here.
| _validate_chat_stop(body) | ||
| _validate_chat_n(body) | ||
| _validate_chat_logprobs(body) | ||
| if "store" in body: |
There was a problem hiding this comment.
These validators still sit after the tools early-return. store=true, modalities=["audio"], prediction, reasoning_effort=high, service_tier=flex, and non-string metadata still bill proxy_completion on head 6ef1405. Unique commits did not hoist them — #609 7c99a27 already does. Do not merge this stack to pick that up.
| """Chat does not apply seed; tools bodies must not bill a seeded proxy.""" | ||
| server, thread, port = _server() | ||
| try: | ||
| status, body = _post(port, _tools_body(seed=42)) |
There was a problem hiding this comment.
Realistic HTTP: live build_server, invoice-lookup prompt, lookup_balance tools. Asserts 400 + invalid_seed / not supported / chat. The file does not assert no-bill against a ledger (400-before-proxy_completion is the implicit signal). Missing on this head: max_completion_tokens: null + max_tokens: 0, and top_logprobs: false — those cases are in successor 2b6a984.
|
Superseded by #607 through #601. The current #607 lane preserves the later honesty-stack request-knob validation (including the spend/batch fail-closed boundary) and adds the KV-backed provider-host policy on the newer substrate. Keeping #605 open would retain an older overlapping landing vehicle. No checks, reviews, or approvals transfer. |


Buyer next action
On
/v1/chat/completionswithtoolsorresponse_format, do not sendseed,n>1, out-of-range penalties,max_tokens=0,routing.channel=batch, orrouting.latency_tolerant=true. Those used to bill a sync completion. They now fail closed with the same named errors as the orchestration path.Valid in-range penalties,
max_tokens>=1,n=1, nullseed, androuting.channel=syncstill proxy.Why this PR exists
#599 (
7a576c2) hoisted model/stream/temperature/top_p. Spend knobs and batch hints still sat after the tools early-return. Track 003 next slice.This branch is a successor of #599 with one unique commit (
109b9d2). Do not merge #599, #591, #594, #596, or this stack in parallel with #597. Prefer cherry-picking109b9d2onto #597 if that remains the landing vehicle.Evidence
python3 tests/test_chat_spend_knobs_tools_passthrough_http_honesty.py— 13 HTTP cases (invoice lookup + tools)docs/architecture.md,docs/rest_api_design.md,conductor/tracks.mdResidual (next loop)
stop,user,logprobs/top_logprobs,logit_biasstill run after the early-return.Do not self-approve. Independent non-author APPROVE + Full unit/Semgrep required.