Repository navigation
fix(tui): align provider doc conformance guards (sampling, Gemini signature gate, DeepSeek note) - #93
Conversation
Fresh review (from scratch, 2026-10-08)Rebase check: All three provider claims were re-verified against the live docs today, and the two behavior fixes are correct and complete. One required finding on the DeepSeek commit. Verified
Required
Should fix
Optional
Non-blocking disclosures that check out: no Bedrock/Vertex transport exists in this repo (custom gateways go through the same seam); the OpenAI-compat proxy path forwards |
The Anthropic sampling restriction blankets every model released after Opus 4.6: the Messages API reference states those models "do not support" temperature/top_p beyond their backwards-compatible defaults (1.0 / >= 0.99), and the migration guides restate it per family — "Setting temperature, top_p, or top_k to any non-default value on Claude Opus 4.7 and later models, including Claude Opus 5.5, returns a 400 error" — covering the Claude 5 generation, Haiku 5.5 (defaults only: temperature 1, a lone top_p 0.99), and the Fable/Mythos lines. The gate only knew opus-4-7/4-8 and fable/mythos, so user-configured temperature/top_p reached Claude 5 and Haiku 5.5 models and 400'd the request. Dropping the params entirely is always safe; the gate extends its own family list and deliberately stays independent of model_supports_reasoning's exact-id list, which answers a different question (thinking capability, not sampling rejection). Pinned across opus-4-7/4-8, opus-5(-5) plus a datestamped spelling, sonnet-5(-5), haiku-5-5, fable-5, and mythos-5, with the older opus-4-6/sonnet-4-6/haiku-4-5 passthrough asserted. Docs re-checked 2026-10-08. Signed-off-by: asto18089 <asto18089@126.com>
8b1a7c2 to
d06356e
Compare
|
Fix wave pushed as d06356e (force-push, same 3-commit structure — each fix folded into its own commit, DCO intact):
Local gates on d06356e: |
JensenChen28
left a comment
There was a problem hiding this comment.
Reviewed current head d06356e703af35c7e3bb844ee21447dacbe3b7e4. The three changes are bounded and follow their runtime call paths: Claude 5-family sampling fields are omitted while 4.6 remains unchanged; Gemini 3 image variants bypass only the missing-signature fail-closed check; and deepseek-flash participates in replay, stream classification, and separate-field reasoning. The three targeted regressions pass locally (3/3), the branch merges cleanly with current pinvou3-clean (the intervening hunk is the unrelated Anthropic timeout envelope), and all current CI checks including gate are green. Approved.
JensenChen28
left a comment
There was a problem hiding this comment.
Reviewed d06356e. Holding approval pending evidence for the Google image-model exception; this is a verification gap, not a reproduced live-service failure.
The new gemini-3*image* exemption is justified by a claim that unsigned image-model turns do not produce a 400. The current Google Gemini 3 guide instead describes strict validation for function calls and image generation/editing, including errors for omitted signatures. The new test only proves the local builder accepts the unsigned history; it does not establish server acceptance. Please provide the applicable current OpenAI-compatible endpoint contract or a sanitized integration result for each supported model, or retain the guard until that exception is established. I am not treating the native API guide alone as proof of the compat endpoint's behavior.
The Anthropic-family and DeepSeek alias changes agree with the current Anthropic Messages reference and DeepSeek quick start.
Independent validation: exact production Google/Anthropic predicates compiled in an isolated Rust harness: test result: 2 passed; 0 failed. This confirms local classification only. A full TUI test build encountered dependency-shape mismatches in the reused build directory; its results are excluded, and I have not established those as a PR defect. No live provider calls or full workspace suite were run. Required checks were rechecked before submission.
The first cut of this commit exempted gemini-3* image-output variants
from the fail-closed replay gate, citing Vertex's thought-signatures
page ("Gemini 3 Pro Image doesn't return a 400 error if a thought
signature isn't returned"). That citation was wrong for this route: the
page belongs to Vertex / Gemini Enterprise Agent Platform, a different
product from the endpoint is_exact_google_chat_route binds to — the
Gemini API at generativelanguage.googleapis.com. That product's
OpenAI-compatibility page defers thought signatures to the Gemini API
thought-signatures guidance, and its Gemini 3 guide keeps image
generation/editing under strict validation: "The API enforces strict
validation on all Model parts including a `thoughtSignature`. Missing
signatures will result in a 400 error" — the same tier as function
calling. The exemption would have waved unsigned replays through to
the very 400 the gate turns into a local diagnostic, so the guard is
retained for every gemini-3 id, image variants included, and the
cross-product doc conflict is documented at the gate.
Pinned across the GA and preview image spellings; restoring the
exemption is the mutation target and turns the pin red.
Docs re-checked 2026-10-12. Review finding on PR #93.
Signed-off-by: asto18089 <asto18089@126.com>
The quick-start's current model name is deepseek-flash (MODEL VERSION DeepSeek-V4.1-Flash), but it matched no arm of requires_reasoning_content and no catalog fallback, so the replay policy dropped assistant reasoning_content and the stream classifier rendered thinking deltas as answer text — the exact replay-or-400 failure the gate exists to prevent on thinking-by-default tool turns. The alias now gates like the rest of the family, and the note states the retirement facts (deepseek-chat / deepseek-reasoner fully retired after 2026-07-24) instead of implying they are still served. Pinned at the matcher, the replay policy, and the stream style; the retired-alias pins are unchanged. Docs re-checked 2026-10-08. Signed-off-by: asto18089 <asto18089@126.com>
d06356e to
ced1064
Compare
|
Gemini image-variant finding — verified, and it's stronger than a verification gap: the exemption contradicted the applicable contract. Guard retained; exemption removed. Re-verified independently against the live docs today (2026-10-12):
What changed (commit 2 rewritten, now purely additive — 39 insertions, 0 deletions, net behavior identical to base; the Anthropic and DeepSeek commits are untouched, the DeepSeek patch byte-identical):
Gates: Re-requesting review. |
JensenChen28
left a comment
There was a problem hiding this comment.
Re-reviewed the incremental change since d06356e against the actual target. The previous validation concern is addressed by removing the Google image-model exemption and restoring the existing fail-closed predicate for all Gemini 3 models. No relaxed provider-contract assumption remains in production. The Anthropic and DeepSeek changes are unchanged from the preceding review. Inspected the updated full request-builder regression covering image variants; two standalone predicate tests passed for the restored Google guard and retained Anthropic model-family behavior, and diff checks passed. I did not execute the full TUI/request-builder suite locally or call a live provider.
No additional actionable defect confirmed. Approval remains pending the running required check and aggregate gate. The parent project pins an older foundation; integration through a future parent gitlink remains separate.
JensenChen28
left a comment
There was a problem hiding this comment.
All required checks, including the aggregate gate, now pass on the unchanged reviewed head. The disputed Google image-model exemption was removed, restoring the existing signature guard; the earlier contract concern is resolved. The prior incremental source review and standalone predicate checks remain applicable. No unresolved blocking finding remains from my review. I did not repeat the full TUI suite or call a live provider; parent-project gitlink integration remains separate.
Summary
Three provider-doc conformance changes found during the PR #91 preserve-thinking review — all pre-existing on
pinvou3-clean, separated from that PR because they are outside its scope.anthropic_model_rejects_samplingonly knew opus-4-7/4-8 and fable/mythos. The sampling restriction blankets every model released after Opus 4.6 — the Messages API reference states those models do not supporttemperature/top_pbeyond their backwards-compatible defaults (1.0 / ≥ 0.99), and the migration guides restate it per family ("Settingtemperature,top_p, ortop_kto any non-default value on Claude Opus 4.7 and later models, including Claude Opus 5.5, returns a 400 error") — so user-configuredtemperature/top_preached Claude 5 and Haiku 5.5 models and 400'd the request. The gate extends its own family list (substring matching like the existing arms); it deliberately stays independent ofmodel_supports_reasoning's exact-id list, which answers a different question (thinking capability, not sampling rejection). Dropping the params entirely is always safe. (platform.claude.com Messages API reference + migration guides, re-checked 2026-10-08, reviewer-confirmed 2026-10-12)gemini-3*image*variants from the fail-closed replay gate on the strength of Vertex's thought-signatures page ("Gemini 3 Pro Image doesn't return a 400 error if a thought signature isn't returned"). Review on this PR caught the product mix-up: that page belongs to Vertex / Gemini Enterprise Agent Platform, while the gate binds only togenerativelanguage.googleapis.com/v1beta/openai— the Gemini API, whose OpenAI-compatibility page defers thought signatures to the Gemini API guidance and whose Gemini 3 guide keeps image generation/editing under strict validation ("The API enforces strict validation on all Model parts including athoughtSignature. Missing signatures will result in a 400 error", the same tier as function calling; only plain text/chat is lenient). The exemption is removed: everygemini-3id stays fail-closed, the cross-product conflict is documented at the gate arm so it doesn't quietly return, and a pin covers both the GA and-previewimage spellings — the compat page itself namesgemini-3-pro-image-previewin its grounding example, so the earlier "shut-down-previewspelling" claim was wrong. (ai.google.dev Gemini 3 guide + OpenAI compatibility page, re-checked 2026-10-12)deepseek-flash(MODEL VERSION DeepSeek-V4.1-Flash), which matched no arm of the reasoning-replay gate — assistantreasoning_contentwas dropped from replay and thinking deltas were rendered as answer text, the documented replay-or-400 failure on thinking-by-default tool turns.deepseek-flashnow gates like the rest of the family, and the replay policy + stream classifier pick it up from the same predicate. The retireddeepseek-chat/deepseek-reasoneraliases keep their gates (fully retired and inaccessible after 2026-07-24, but gated for as long as any route still accepts them). (api-docs.deepseek.com quick-start + thinking-mode guide, re-checked 2026-10-08, reviewer-confirmed 2026-10-12)Testing
deepseek-flasharm turns the respective pin red; for the Gemini gate, restoring the image exemption turnsgoogle_image_variants_stay_fail_closed_on_missing_signaturesred (verified live during this fix).sandbox::process_hardeningno_new_privs child-process tests, which cannot pass inside this execution sandbox (the kernel NNP flag is one-way and already set for the session's process tree; the module is byte-identical to the base commit and they pass on CI runners). Known parallel-load flakes (model_inventory::ollama_default_prefers_live_local_tags,runtime_chat_relay::failed_state_writes…) pass standalone..cargo fmt --checkclean; no clippy findings in this PR's files (a local stable-clippy run flags pre-existing lint-drift errors in files byte-identical to the base commit; CI's toolchain remains authoritative).No-Issue: docs-conformance follow-ups surfaced by the PR #91 review; this repo has issues disabled. Offering
deepseek-flashin the model picker / bundled catalog is left as a follow-up — hand-typed ids are first-class and the gate no longer depends on the picker.