fix(responses): repair sparse terminal output for Grok Build - #3756
Conversation
…tion Carry #3706 onto the verified opaque recovery foundation, keep canonical quota ownership and preserve caller cancellation at both recovery sites. Co-authored-by: yxr1995-maker <257504378+yxr1995-maker@users.noreply.github.com>
…ase-244-combo-recovery-07c0
…ase-244-combo-recovery-07c0
…ease-244-combo-recovery-07c0
…ated items Carry #3388 with a dedicated tracker and shared leaf codec; preserve existing provider repair, stream ordering and unmarked-client behavior. Co-authored-by: Maple <hzlhu@qq.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (25)
📝 WalkthroughWalkthroughThe PR adds deferred encrypted combo recovery, canonical provider quota handling, and cancellation coverage. It also adds Grok-specific sparse terminal snapshot repair, shared snapshot helpers, extensive validation tests, localized documentation, and release follow-up plans. ChangesRecovery and snapshot repair
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Sequence Diagram(s)Encrypted combo recoverysequenceDiagram
participant Caller
participant handleComboResponses
participant ComboResolver
participant RecoveryHelper
participant RoutedProvider
Caller->>handleComboResponses: submit encrypted NEW_TASK
handleComboResponses->>ComboResolver: select eligible native target
ComboResolver-->>handleComboResponses: no eligible native target
handleComboResponses->>RecoveryHelper: recover task once
RecoveryHelper-->>handleComboResponses: readable task
handleComboResponses->>RoutedProvider: dispatch recovered task
Grok terminal snapshot repairsequenceDiagram
participant GrokClient
participant ResponsesCore
participant GrokSnapshotRewrite
participant ResponsesStream
GrokClient->>ResponsesCore: send x-opencodex-grok: 1 request
ResponsesCore->>GrokSnapshotRewrite: enable terminal repair
ResponsesStream->>GrokSnapshotRewrite: send output_item.done events
GrokSnapshotRewrite-->>GrokClient: send reconstructed response.completed output
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 72 / 80이 PR은 release-244 플랜의 지금 HEAD의 rewrite 순서는 custom-tool restore → tool-search restore → Copilot → (여기 없음) provider snapshot → field backfill → function repair → undeclared-tool guard입니다. 이 PR은 그 사이에 Grok tracker를 provider snapshot 바로 앞에 끼웁니다. 새 파일 복구 규칙은 빡셉니다. done 항목은 타입별 좁은 validator를 통과해야 하고, index는 유일·연속·0부터여야 하며, open 항목이 남아 있거나 tainted면 손대지 않습니다. 플랜 우선순위 72인 이유다. combo 다음 필수 칸이고 Grok Build 빈 completed 출력 구멍을 막습니다. 다만 경로/심볼 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaa005ae0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "custom_tool_call", | ||
| "web_search_call", | ||
| "code_interpreter_call", | ||
| "mcp_call", | ||
| ]); |
There was a problem hiding this comment.
Handle restored tool-search calls in terminal repair
When a marked Grok request declares tool_search on a noncanonical Responses route, createRoutedToolSearchRestoreBlockRewrite runs immediately before this tracker and rewrites the added/done item to tool_search_call. That type is absent from this allowlist, so the added event taints the tracker and the done event is also rejected; a sparse terminal therefore remains empty and Grok can repeat the billable turn. Validate and retain the restored type, counting it as visible client work, while preserving the final client-facing shape.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| // Keep the spent Pool budget sticky even after a recovered routed child: | ||
| // no later failure may reopen ordinary combo/native account hopping. | ||
| adoptFailedChildLog(childLog); | ||
| return lastFailure; |
There was a problem hiding this comment.
Cool a failed routed target before the sticky return
After a stored Pool 401 refresh/replay is exhausted, this branch can recover the encrypted task and continue with a routed target. If that target then returns a hop-class failure such as 429 or 503, the sticky flag remains set and this return bypasses advanceComboAfterFailure, which is where noteComboFailure and coolComboTarget run. The failed backup consequently remains immediately selectable on later child requests instead of respecting combo cooldown/failover; record its failure and cooldown while still suppressing additional same-request hops.
Useful? React with 👍 / 👎.
Summary
Co-authored-by: Maple hzlhu@qq.com
Verification
Current head
eaa005ae0ba4ec48e8106df70b7c2f0b286ef32bincludes the stored Pool recovery boundary repair and is awaiting its own hosted CI. Earlier green runs do not verify this head. No local tests, typecheck or build were run.Independent plan audit passed. Source/protocol/module review is in progress; hosted exact-head CI will run asynchronously.
Unit and server fixtures cover text/function/custom-plus-message recovery, invalid call IDs, bounds, terminal precedence, provider-repair coexistence and the same-provider marker/absent-marker control.
Existing provider snapshot test bodies and the f121348 sparse JSON/function-repair replay test are unchanged.
No local test suite, typecheck or build was run. A local static AST scan could not start because TypeScript is not installed; no dependency was installed and no static scan success is claimed. No live Kiro or Grok request was used.
Checklist
Native stack #3759 was dissolved at the owner's explicit request. The PR head is preserved; integration continues bottom-up with ordinary PRs and admin merge after current-head CI.
Summary by CodeRabbit