Skip to content

fix(tui): align provider doc conformance guards (sampling, Gemini signature gate, DeepSeek note) - #93

Merged
asto18089 merged 4 commits into
pinvou3-cleanfrom
fix/provider-doc-conformance
Oct 12, 2026
Merged

asto18089 merged 4 commits into
pinvou3-cleanfrom
fix/provider-doc-conformance

Conversation

@asto18089

@asto18089 asto18089 commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

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 sampling gate: anthropic_model_rejects_sampling only 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 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") — so user-configured temperature/top_p reached 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 of model_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 thought-signature gate — guard retained after review: the first cut exempted 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 to generativelanguage.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 a thoughtSignature. Missing signatures will result in a 400 error", the same tier as function calling; only plain text/chat is lenient). The exemption is removed: every gemini-3 id 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 -preview image spellings — the compat page itself names gemini-3-pro-image-preview in its grounding example, so the earlier "shut-down -preview spelling" claim was wrong. (ai.google.dev Gemini 3 guide + OpenAI compatibility page, re-checked 2026-10-12)
  • DeepSeek aliases: the quick-start's current model name is deepseek-flash (MODEL VERSION DeepSeek-V4.1-Flash), which matched no arm of the reasoning-replay gate — assistant reasoning_content was dropped from replay and thinking deltas were rendered as answer text, the documented replay-or-400 failure on thinking-by-default tool turns. deepseek-flash now gates like the rest of the family, and the replay policy + stream classifier pick it up from the same predicate. The retired deepseek-chat / deepseek-reasoner aliases 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

  • Behavior pins, all mutation-verified: removing the Claude 5 families from the sampling gate or dropping the deepseek-flash arm turns the respective pin red; for the Gemini gate, restoring the image exemption turns google_image_variants_stay_fail_closed_on_missing_signatures red (verified live during this fix).
  • Full lib suite Full lib suite on this branch: 13808 passed / 2 failed / 13 ignored — the 2 failures are the sandbox::process_hardening no_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 --check clean; 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-flash in 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.

@asto18089

Copy link
Copy Markdown
Collaborator Author

Fresh review (from scratch, 2026-10-08)

Rebase check: pinvou3-clean is still at this PR's base (44921cf) — nothing to rebase. git merge-tree against the current heads of #90 (ae36626) and #91 (4e80b10) merges clean in both directions, so merge order is free.

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

  • Anthropic sampling gate (779d637) — the quoted sentence is verbatim on the Opus 5.5 migration guide, and the Messages API reference ("Models released after Claude Opus 4.6 do not support…") establishes the cross-family coverage: Opus 4.7/4.8/5/5.5, Sonnet 5/5.5, Haiku 5.5, Fable 5/5.1, Mythos 5/5.1 — exactly the gate's set (the pre-existing fable/mythos arms cover the latter), and the new substrings match nothing older (no Sonnet 4.7/4.8 or Haiku 5.0 exists). The "non-default" wording is doc-accurate; the old "reject outright" wording was wrong. Wiring is airtight: every Anthropic-Messages route (native, Model Studio, custom wire="anthropic" gateways, streaming / non-streaming / preview) builds through the single build_anthropic_body seam, so nothing bypasses the gate. top_k is part of the documented restriction, but MessageRequest has no top_k field and no code path can emit one, so the gate's omission is moot. Mutation-verified: neutralizing the family array turns the new pin red (claude-opus-5 receives temperature).
  • Gemini image exemption (8bc5a70) — the quote is verbatim-current on docs.cloud.google.com (…/models/thinking/thought-signatures), which is where the text lives now (ai.google.dev moved the page 2026-08-18; the PR body cites docs.cloud.google.com, correct). Text Gemini 3 still 400s on missing signatures, so keeping fail-close there is right; send-when-present matches the docs' "you must still return the thought signatures" instruction for Pro Image. Every documented gemini-3* image id contains "image" (gemini-3-pro-image, gemini-3.1-flash-image, gemini-3.1-flash-lite-image); gemini-nano-banana-2.1 never hits the gemini-3 branch and was never gated anyway. The exemption only removes the requirement — capture/attach stays model-independent. Mutation-verified: reverting to return true turns the pin red with the fail-close bail.
  • DeepSeek comment (8b1a7c2) — provably comment-only (comment-stripped tree is byte-identical); the old chat→v4-flash / reasoner→v4-pro mapping is indeed stale, so the correction was warranted.
  • No smuggling — exactly 3 commits, exactly the claimed files, no lockfile/CI/docs/fingerprint touches, no test deletions, each hunk serves its commit's purpose.
  • Gates (local, this machine) — cargo fmt --check clean; workspace clippy with the CI lint set: 0 warnings; all-features lib suite reproduced exactly: 11955 passed / 0 failed / 13 ignored (11968 total). Two parallel-load flakes (runtime_chat_relay::failed_state_writes… and the already-disclosed ollama_default_prefers_live_local_tags) pass standalone.

Required

  1. The rewritten DeepSeek note documents a contract the code does not honor for the id the docs currently serve (requires_reasoning_content, chat.rs:3310). Today's quick-start names deepseek-flash as the model (MODEL VERSION "DeepSeek-V4.1-Flash"); the "retired aliases served by V4.1-Flash" footnote is about deepseek-v4-flash / deepseek-v4-flash-vision-exp, and deepseek-chat / deepseek-reasoner appear nowhere in the current quick-start (the news entries fully retired them after 2026-07-24). deepseek-flash matches no arm here (no deepseek-v4 substring, no chat/reasoner prefix, no r-marker) and also misses model_supports_reasoning's deepseek && v4 fallback, so the replay policy returns false → assistant reasoning_content is dropped from replay (chat.rs:2773) and thinking deltas are rendered as answer text (chat.rs:3469 → ReasoningStreamStyle::None). That is precisely the replay-or-400 contract the new comment calls stable. Since this PR is the doc-conformance PR, fix it here: add || lower.starts_with("deepseek-flash") plus pin rows — both consumers (chat.rs:3415, 3469) consult this one function, so a single arm repairs replay and stream classification together. The note itself should also state the retirement facts rather than implying deepseek-chat/deepseek-reasoner are still served.

Should fix

  1. "The gate now shares the family matching used by the adaptive-thinking rules" (commit message + PR body) — no shared matcher exists; the PR adds a second inline list next to model_supports_reasoning's exact-id list (models.rs:438), which has different members and semantics. With fix(tui): preserve thinking across turns #91 landing a third copy (anthropic_omitted_thinking_defaults_to_adaptive), a model generation would need three list updates. Reword the claim, or extract a shared const.
  2. Haiku 5.5 parenthetical "admits only temperature: 1" — the docs also admit a lone top_p: 0.99, and reject top_p: 1, any top_k, and any request carrying both params. Behavior is unaffected (the gate drops everything); wording precision only.
  3. The image test pins the retired id gemini-3-pro-image-preview (shut down 2026-06-25). The live GA id is gemini-3-pro-image (plus the gemini-3.1-flash-image family — all covered by the substring). Pin a live id, or both.

Optional

  1. Sampling-test hardening: add a claude-mythos-5 positive and claude-opus-4-6 / claude-haiku-4-5 negative controls (a future contains("opus-4-") regression would pass today's suite), and a datestamped id such as claude-opus-5-20260101.

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 temperature by dialect design and is out of scope; the pre-existing models/-prefix fail-open in the Gemini gate is unchanged by this PR.

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>
@asto18089
asto18089 force-pushed the fix/provider-doc-conformance branch from 8b1a7c2 to d06356e Compare October 8, 2026 10:59
@asto18089

Copy link
Copy Markdown
Collaborator Author

Fix wave pushed as d06356e (force-push, same 3-commit structure — each fix folded into its own commit, DCO intact):

  1. Required — deepseek-flash now gates (requires_reasoning_content): the alias matched no arm, so the replay policy dropped assistant reasoning_content and the stream classifier inlined thinking deltas as answer text. Pinned at the matcher, the replay policy, and the stream style (deepseek_flash_replays_and_streams_reasoning_like_the_family); commit 3 is now fix(tui) and its note states the retirement facts (deepseek-chat / deepseek-reasoner fully retired after 2026-07-24) instead of implying they are still served. legacy_deepseek_alias_reasoning_effort is intentionally untouched — deepseek-flash is a unified thinking-by-default model, so injecting chat's "off" or reasoner's "high" would override the documented default.
  2. "Shares the family matching" claim dropped from the commit message and PR body — the gate extends its own substring list and deliberately stays independent of model_supports_reasoning's exact-id list (different question, different semantics).
  3. Haiku 5.5 parenthetical corrected — admits its defaults (temperature: 1, a lone top_p: 0.99); top_p: 1, any top_k, or both params together 400.
  4. Image test pins live ids (gemini-3-pro-image, gemini-3.1-flash-lite-image) instead of the shut-down -preview spelling.
  5. Sampling test hardened — claude-mythos-5 and datestamped claude-opus-5-20260101 positives, claude-opus-4-6 / claude-haiku-4-5 negative controls (mutation-verified: an over-broad contains("opus-4-") arm now goes red).

Local gates on d06356e: cargo fmt --check clean; workspace clippy with the CI lint set, 0 warnings; all-features lib suite 11956 passed / 0 failed / 13 ignored (the two known parallel-load flakes pass standalone).

@JensenChen28 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@asto18089
asto18089 force-pushed the fix/provider-doc-conformance branch from d06356e to ced1064 Compare October 12, 2026 09:57
@asto18089 asto18089 changed the title fix(tui): align provider doc conformance guards (sampling, Gemini image variants, DeepSeek note) fix(tui): align provider doc conformance guards (sampling, Gemini signature gate, DeepSeek note) Oct 12, 2026
@asto18089

Copy link
Copy Markdown
Collaborator Author

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):

  • The gate binds only to generativelanguage.googleapis.com/v1beta/openai (is_exact_google_chat_route, exact host+path). That is the Gemini API, not Vertex. Its own OpenAI-compatibility page (ai.google.dev/gemini-api/docs/openai, updated 2026-09-02) defers the contract to the Gemini API guidance: "Gemini 3 supports OpenAI compatibility for thought signatures in chat completion APIs. You can find the full example on the thought signatures page."
  • The Gemini 3 guide (ai.google.dev/gemini-api/docs/generate-content/gemini-3, updated 2026-09-03) 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 ("Function Calling (Strict): … Missing signatures will result in a 400 error"). Only plain text/chat is lenient ("Validation is not strictly enforced, but omitting signatures will degrade the model's reasoning and answer quality").
  • The exemption's citation was docs.cloud.google.com (updated 2026-10-09), which belongs to Vertex / Gemini Enterprise Agent Platform — a different product and endpoint family. Its "Gemini 3 Pro Image doesn't return a 400 error if a thought signature isn't returned" relaxation cannot be assumed to govern generativelanguage.googleapis.com, which is exactly the gap you flagged. And here the applicable native guide is strict, so rather than hunt live-service evidence for a relaxation the primary contract does not grant, I've taken your second option: the guard is retained.

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):

  • google_model_requires_thought_signatures returns true for every gemini-3 id again; the Vertex-vs-Gemini-API conflict is documented at the arm so the exemption doesn't quietly return off the Vertex page.
  • New pin google_image_variants_stay_fail_closed_on_missing_signatures covers gemini-3-pro-image, gemini-3-pro-image-preview, gemini-3.1-flash-image, gemini-3.1-flash-lite-image. Mutation-verified: restoring the exemption turns the pin red; pristine code passes.
  • One erratum on my earlier fix-wave comment: the "shut-down -preview spelling" claim was wrong — the current OpenAI-compatibility page itself names gemini-3-pro-image-preview as the image model in its grounding example — so the pin deliberately covers both spellings.

Gates: cargo fmt --check clean; Google signature module 9/9; full all-features lib suite 13808 passed / 2 failed / 13 ignored (the 2 are the sandbox no_new_privs child-process tests — the kernel NNP flag is one-way and already set inside this execution sandbox; module byte-identical to base, they pass on CI runners). Push note: git push to github.com is currently walled from here, so the branch was rebuilt via the Git Data API with byte-identical SHAs verified at every layer (blob 16d1e20aa/666bcfdd, tree 57e52702b/3c6d8cc8, commits f6f6409/ced1064b4).. Local clippy (stable 1.98.1) reports 6 pre-existing drift errors (chunks_exact_to_as_chunks/print_stderr/clone_on_copy/redundant_closure) in files byte-identical to base — none in this PR's files; CI's toolchain remains the authoritative green.

Re-requesting review.

@JensenChen28 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

2 participants