fix: lengthen requestRetry hold so grok-4.6 thinking is not withheld - #989
Merged
chenyme merged 10 commits intoAug 20, 2026
Merged
Conversation
Usage.reasoning_tokens, empty reasoning items, and the Chat SSE stub are not proof of thinking. Degraded upstreams fill those fields and then dump visible tokens, so the hold used to fail-open as 200. Require reasoning/summary deltas or encrypted_content before delivering. An empty stub waits until terminal so ciphertext that arrives at the end is not withheld.
Grok CLI treats annotations as a required field. A missing key fails deserialization with "missing field annotations" and the TUI retries the turn. Keep existing citation arrays untouched.
Grok TUI declares a tools array on every agent request, including the first thinking turn. Skipping hold on that schema forwarded missing-thinking streams as HTTP 200. Keep the replay-safety skip for in-flight tool output (function_call_output, tool_result, role=tool). Compact skip and previous_response_id pinning are unchanged.
response.completed / [DONE] with zero tokens used to keep the quality hold in Wait until the idle timeout. The client already had HTTP 200 headers and saw a 50–120s Retrying spinner. Finish the peek immediately and surface errQualityEmptyStream so the attempt loop rotates. Open empty hangs still wait for idle and do not fail-open as HTTP 200.
Empty SSE idle timeouts currently write a hardcoded 24h cooldown via qualityIdleAccountCooldown. Toggling enabled does not reset health, so a single-account pool stays on 429 upstream_cooling until the operator edits sqlite and restarts. Add POST /api/admin/v1/accounts/:id/clear-cooldown (API + UI) which zeros failure_count/cooldown_until/last_error. UpdateHealth already publishes InvalidationAccountHealthChanged, so runtimeStore=memory selector overlays are replaced. Expose qualityGuard.requestRetry.idleAccountCooldown (1m–168h, default 24h) independently of missing-thinking accountCooldown. PATCH enabled while cooling returns enabledDoesNotClearCooldown so the UI can warn. Closes chenyme#976
Grok TUI tools run locally. The next /v1/responses body already contains function_call_output; retrying it on another account only regenerates the model turn and does not re-execute tools. Skipping hold on that body let 0-thinking dumps through on the common agent loop.
TUI 0.2.93 treats any response.incomplete as fatal max_tokens_truncation, ignoring incomplete_details.reason, so stream aborts killed the turn. serde also requires model on response.failed / completed; a missing key fails deserialization. Always emit response.failed and fill model from stream metadata or the last seen response event.
3s hold cuts encrypted thinking that arrives at end-of-stream, which fills audits with HTTP 200 · error and quality_degraded retries. Match the production lab defaults: 30s hold, minOutput 8, 12h missing-thinking cooldown, 15m idle cooldown, grok-4.6 probe model. requestRetry.enabled stays false in built-in Go defaults so existing configs do not flip on upgrade; config.example.yaml now recommends it on.
# Conflicts: # backend/internal/application/gateway/quality_retry.go # backend/internal/application/gateway/quality_retry_scan.go # backend/internal/application/gateway/quality_retry_test.go # backend/internal/infra/config/config.go # backend/internal/infra/config/config_test.go # backend/internal/transport/http/account/handler.go # backend/internal/transport/http/account/handler_test.go # backend/internal/transport/http/inference/handler.go # backend/internal/transport/http/inference/handler_test.go # config.example.yaml
mcheiyue
added a commit
to mcheiyue/grok2api
that referenced
this pull request
Aug 20, 2026
…me#989 thinking-hold / chenyme#983 audit，保留 Console team 熔断）
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
qualityGuard.requestRetrycurrently defaults toholdTimeout: 3sandminOutputTokens: 32. grok-4.6 (and grok-4.5 high-effort) often emitencrypted_contentat the end of the stream. A 3s peek sees ≥32 visible tokens with no ciphertext, classifies the account as missing-thinking, and writes:200 · 错误/quality_degradedThat is a false withhold, not a client-visible “fake thinking” leak. Production lab (
v3.1.4-18183-hold-tools) already uses a 30s hold (the allowed maximum) and does not see this audit storm on the same code.What changed
When
requestRetryfields are omitted (and inconfig.example.yaml):holdTimeoutminOutputTokensaccountCooldownidleAccountCooldownmodelrequestRetry.enabled: true.Enabledstays false, so existing configs withoutrequestRetrydo not flip on upgrade.qualityGuard.enabledstays false;mode: hybridis unchanged. This PR is request-path hold defaults only.Upgrade
Already-running installs that copied the old 3s example keep 3s until they edit
config.yamland restart:New copies of
config.example.yamlget the 30s hold. Clients should allow ≥120s; 6 × 30s hold plus retries will otherwise 499.