Conversation
…ota evidence Combines two fork PRs on the web-search path: isolate the replay cache by request context (#563) and preserve quota evidence when Retry-After would outlive the sidecar deadline (#568), rebased onto current dev. The #568 test's unrecorded-destination case is adapted to the current foldDeveloperRoleToSystem fixture semantics. bun test: web-search-bridge-replay + web-search-sidecar-429 + server-key-failover-e2e + chat-inline-document-bytes: 65 pass
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe pull request scopes web-search replay restoration to caller and serving identities, binds restoration after route refresh, and bounds OpenAI sidecar 429 retries by the remaining deadline. Tests and documentation cover the updated behavior. ChangesWeb-search replay scoping
OpenAI sidecar retry handling
Chat document role coverage
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Client
participant ResponsesRequest
participant RouteSelection
participant BridgeReplayCache
participant Upstream
Client->>ResponsesRequest: submit responses request
ResponsesRequest->>RouteSelection: refresh adapter and credential
RouteSelection-->>ResponsesRequest: refreshed serving identity
ResponsesRequest->>BridgeReplayCache: restore using bound replay scope
BridgeReplayCache-->>ResponsesRequest: matching replay or unchanged search cell
ResponsesRequest->>Upstream: send rebuilt request
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 61 / 80웹 검색 길에서 서로 겹치던 고침 두 개를 한 PR로 모았습니다. 첫째, 브리지가 대신 돌려 준 검색 결과를 다음 턴에 다시 끼워 넣을 때, 예전에는 공급자 주소( 라인 - 본문의 라인 - 라인 - 상태 — draft이고 메인테이너의 판단이 필요한 지점 루프백에서 검색 재생 주체를 전부 너의 추천 방향(재생 칸을 대화·서빙 신원으로 좁히기, 키 교체 후 다시 묶기, 데드라인 밖 429 유지)은 맞습니다. 머지 전에 본문 이슈 번호를 포크 URL로 고쳐 hygiene을 풀고, 루프백 주체 규칙을 한 줄로 문서/코드에 맞추세요. 그다음 Ready로 올리면 됩니다. 이 댓글은 grok-bot이 작성했습니다 |
|
Review feedback applied on |
|
Consolidated into #5562 as a single related-function aggregate. Source head: Both source contributions are preserved by exact stable patch IDs and original author/date metadata. Final-tree review retains replay isolation by principal, thread and serving identity, credential rebinding before restoration, and original sidecar 429 evidence. The combined regression also checks that repaired search output cannot replay under another principal, conversation or credential. The final unchanged eight-file test scope passed 199/199 tests with 976 assertions after fixing fixture-owned cancellation and spend-lease cleanup; type, structure, privacy and ratchet checks passed. Updated-head hosted CI and security review remain tracked on the draft replacement. Closing this duplicate standalone review entry as part of the requested consolidation after verifying coverage. This is not a merge or release claim; remaining integration checks and reviews are tracked on the replacement. Original branches are retained. |
|
Thanks @luvs01. The replay-isolation and deadline-safe 429 work from this PR landed on dev in #5575 ( Two parts changed on the way in. A caller with no opencodex API key now gets no replay scope instead of the shared |
Summary
Web-search replay results could be reused outside the request context that produced them. Scope the replay cache to the conversation, caller, selected model/provider, and serving credentials, and restore replay data using the current serving identity.
When an upstream 429 supplies a Retry-After longer than the remaining sidecar deadline, preserve the original quota evidence instead of dropping it. This keeps downstream quota and failover decisions tied to the upstream response.
The changes cover the replay cache, request preparation and transport, and web-search executor. Regression coverage includes request-context isolation, credential changes, and Retry-After values that exceed the sidecar deadline. The inline-document fixture explicitly records its developer-message destination to match the current role-folding contract.
Verification
Previously recorded implementation validation:
bun test tests/web-search/web-search-bridge-replay.test.ts tests/web-search/web-search-sidecar-429.test.ts tests/server/server-key-failover-e2e.test.ts tests/responses/chat-inline-document-bytes.test.ts- 65 passed.Checklist
Summary by CodeRabbit
Bug Fixes
Documentation