fix(responses): recall completed combo routes during compaction - #3971
Conversation
…ombo switch When Codex compacts a conversation that was switched to a different combo mid-session, it sends the bare native model of the new combo target (e.g. gpt-5.6-terra) rather than the combo/<id> selector. Without recall, the bare model hits routeCompactionModel and lands on the configured default provider (or 404s) instead of routing through the combo failover path. Added a bounded session-lane recall map that records the combo target on every successful combo turn. Both compaction entry points (v2 compaction_trigger in handleResponsesInner and v1 /responses/compact in handleResponsesCompact) rewrite a bare model back to the remembered combo selector when it exactly matches the last-served combo target on that session lane. Review round 1 (#3891): the v1 compact endpoint now also syncs the routed identity (compactModel) with the recall rewrite - previously only raw.model was rewritten, so a bare model with no canonical openai row could still 404, and a bare model whose target lives on a native-compact provider (openai / openai-apikey) resolved straight onto the native /responses/compact endpoint, bypassing combo dispatch. Two regression tests cover both routing holes. Safety properties: - Only fires for compaction requests (compaction_trigger present) - Only fires for bare models (no provider/ prefix) - Only fires when the bare model exactly matches the combo target - Different session lanes never borrow each others recall - Explicit combo/provider selectors are never touched The rewrite in handleResponsesInner runs BEFORE comboIdFromRawBody so the combo dispatch path engages. The compact endpoint rewrites raw.model before the non-native dispatch falls through to handleResponses. (cherry picked from commit 52e66f6)
Use existing parsed completion callbacks and an independent child gate slot. Preserve explicit combo aliases, current target ownership, config-generation rejection, and final client-visible model identity across failover and compaction. Verification: source review and git diff --check only. Local product tests, typecheck, builds, installs, scripts, servers, and live API calls NOT RUN by user instruction. Co-authored-by: x3M3x <amroeid1999@gmail.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 (11)
📝 WalkthroughWalkthroughChangesThe PR adds process-local combo recall per conversation lane. Completed combo responses record their final client-visible model. Later bare-model compaction requests can route through the remembered combo when the lane, model, configuration, and ownership remain valid. Combo session recall
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CompactHandler
participant ComboRecallStore
participant ComboRouter
participant Provider
Client->>CompactHandler: compact with bare model
CompactHandler->>ComboRecallStore: lookup session lane and model
ComboRecallStore-->>CompactHandler: remembered combo id or no match
CompactHandler->>ComboRouter: route as combo/<id> when matched
ComboRouter->>Provider: send compact request through combo path
Suggested reviewers: ✨ 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. |
리뷰 · 우선순위 58 / 80이 PR은 대화 중간에 콤보를 바꾼 뒤, 클라이언트가 공급자 접두사 없는 맨모델 이름으로 compact를 요청하면 방금 성공한 콤보 대신 그 맨모델로만 들어가 페일오버·선택이 깨지는 문제를 고칩니다. 새로 생기는 지금 우선순위는 콤보+Codex compact를 쓰는 사용자 체감이 커서 중상입니다. 다만 기억이 프로세스 로컬이라 여러 서버 인스턴스·재시작 뒤에는 예전처럼 맨모델 compact로 떨어질 수 있고, 본문도 그 한계를 숨기지 않습니다. 로컬 product 테스트는 금지되어 hosted CI와 독립 감사가 머지 전 증거입니다. #3891 leftover는 머지 후 landed-via-maintainer로 닫는 패턴이 맞습니다. 라인 src/server/responses/combo-session-recall.ts · rememberComboForLane - 라인 src/server/responses/combo-session-recall.ts · recallComboForLane - TTL·disabled provider·타깃 부재· 라인 src/server/responses/compact.ts · handleResponsesCompact - bare model이고 fast-row/alias가 아닐 때만 recall합니다. 명시 라인 src/server/responses/core.ts · compaction_trigger / completed 콜백 - 수락된 completed만 remember하고, model-less terminal은 recall을 영구 거절합니다. 실패·취소가 성공 기록을 덮지 않는 계약이 여기 있습니다. 경로 #3891 - 같은 버그의 contributor 원본입니다. 이 PR 머지 후 경로 compactHandoffRoutes (기존 compact.ts) - 이름이 비슷한 두 맵이 공존합니다. 문서에 “핸드오프 vs 콤보 recall” 한 줄이 더 있으면 이후 기여자가 덜 헷갈립니다. 필수는 아닙니다. 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
|
Final evidence correction: CI34173074703 actually checked out synthetic merge b12a0bb. Its parents are destination dc5ee2f and candidate797a74c646b554a1716905c3721dd13dcef0d579, and its tree e5a6d802123cb463d7f7ecd172504ff85dff8ff6 exactly equals landed900567af3f38ad6e07e4723c2eedeeffb95c57ab. Thus the final integration including the later B/C changes was tested successfully; the earlier conservative statement that it was untested is superseded. The separate dev-push run34174143615 remains separate evidence. Windows runtime shards and macOS unsharded control were skipped in candidate CI and are not counted as passes. |
Summary
After switching combos, Codex can compact with the last emitted bare model instead of the combo selector. Recall the same lane's last successfully completed combo so v1 and v2 compaction re-enter normal combo selection and failover.
Carry #3891 from
52e66f672d9d30e5dd1188590e669c2bac119540, preserving x3M3x. Correct the unlanded patch's acceptance boundaries: record only accepted completed responses and their client-visible model; preserve explicit configured aliases; reject stale/missing combo targets and late writers after configuration replacement. Failed/incomplete/cancelled attempts retain the previous successful selection. Recall remains process-local, bounded to 256 lanes/30 minutes and stores no credentials.Reuse existing parsed completion hooks, child commit/discard gating and state-store reconciliation. No additional stream reader, timer or generic subsystem. Add activated compaction, terminal, failover and generation regressions plus English/Korean guidance. This PR contains one coherent bug repair; its size includes tests for the original patch's four correctness gaps.
Verification
797a74c646b554a1716905c3721dd13dcef0d579passed Cross-platform CI34173074703:19successful jobs,2explicit skips (Windows runtime shards/macOS unsharded control). Linux and macOS shards passed. Explicit log evidence includes v1/v2 current-account re-selection and admission refusal, subsequent 429/402 alternates, alias precedence, TTL/257-entry eviction, virtual Pro identity, successful/failed/cancelled completion and hidden duplicate-terminal cases. Linux ignores the eager-mode option, actual macOS output also passed all16 hidden-completion variants, including explicit eager relay with terminal recording on/off.Checklist
Co-authored-by: x3M3x amroeid1999@gmail.com
Pre-merge destination
dc5ee2f49dde307f1c7ece31829915072aa9bde5adds C documentation, the independent Santiago fixture correction and B's opt-in reset-credit journal serialization after candidate basebbea77a48. No combo recall/terminal/compaction source, its tests, or build manifests/workflows changed. Existing account/admission/config-reconciliation owners remain unchanged. Candidate CI is exact-head evidence for all four A fixes; the final combination with later B/C changes is not claimed as a pre-merge execution. We verify the full expected integration tree and leave the automatic dev push CI as separate evidence.Summary by CodeRabbit
New Features
Documentation
Tests