Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (7 skipped: 7 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: extending hosted-search history projection to native Lite compaction requests.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (7 skipped: 7 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
3/4 boxes ticked. The PR is more than 10 commits behind |
|
Scoped review at |
|
Thanks @Ingwannu. I've expanded the Verification section for I also made the limits explicit: the native result handler test uses a mocked upstream, and the existing referenced-item test covers an ordinary turn through the shared projection block, not a separate native-trigger/reference case. The earlier live compaction checks were from my patched local installation; I haven't rerun live requests for this update. The 147 passing local tests used Bun 1.3.14, and the full suite and pinned 1.4.3 runner are still unverified. I agree that #7250's ordinary non-Lite report is outside this fix and have stated that in the summary. The current-head Cross-platform CI, Catalog async contracts, and React Doctor runs still show |
|
Thanks for separating the current-head mocked fixtures, predecessor live receipts and the Bun 1.3.14 limitation. The branch is unchanged, so I have not repeated the earlier review or counted the expanded prose as a new execution. The ordinary non-Lite report remains outside this fix. I cannot approve the pending aggregate under the current maintenance execution scope: its gates job runs |
|
Thank you @EricFeng9 for isolating this and for the careful regression coverage. Your commit is cherry-picked with authorship kept into #7353, which extends it. That PR also projects hosted-search history whenever the outbound request declares no search tool (#7250), so native v2 compaction with an empty tool list is covered even without the opt-in, and it fixes routed models losing search results on replay (#7269). The maintainer will close this PR when #7353 merges. |
…) (#7353) * fix: project hosted-search history during native Lite compaction * fix(responses): project hosted-search history when no search tool is declared (#7250) Native Sol answers a replayed web_search_call without a matching web_search declaration with a generic server_error. Project unreferenced cells into the existing bounded reference notes whenever the final body declares no hosted search, on every Responses destination; declared requests keep structural history. Also translates the comments carried over from #7268 into English. * fix(web-search): replay sidecar search results to routed models (#7269) Codex replays a hosted web_search_call cell without its result text, and the parser dropped it for routed providers, so a model whose turn ended on a client tool call forgot it had searched and repeated the search and the tool. Rebuild the cell as a web_search call/result pair when search is declared, and let the sidecar loops record the result under the emitted cell id so the next turn restores it, scoped to caller and conversation. * fix(web-search): rebuild replayed searches at the input level and drop them without a sidecar Review round 1: the parse-time pair could call an undeclared web_search tool when no sidecar plan resolves, split a parallel client batch, and collide with client call ids. Rewrite cells into function_call/output items before the item loop, defer the output past the following call batch, allocate collision-free call ids mapped back to cell ids, and remove the pairs when the search plan is undefined. * fix(web-search): answer a replayed search before the next client batch Review round 2: a deferred search output crossed into the following assistant turn when consecutive client batches followed the cell, and a restored failed search lost its error flag. Flush the deferred output when a call follows the batch outputs, and set the flag independently of the restored text. --------- Co-authored-by: Junming Feng <12311031@mail.sustech.edu.cn>
|
Thank you, @EricFeng9! Your native Lite compaction projection was absorbed into #7353 (merged into |
Summary
I ran into a repeatable remote-compaction failure after switching an existing conversation to the official Codex GPT-6.1 Sol route. A new conversation worked, but the existing conversation failed with an upstream 500
server_errorduring compaction.I isolated the trigger to hosted-search history. With
projectResponsesLiteHostedSearchHistory: true, ordinary Responses Lite turns already converted that history to reference notes, but the final adapter condition explicitly excluded_compactionRequest. Native v2 compaction therefore still sent the structuralweb_search_callitems that reproduced the failure.This PR removes that exclusion so native v2
compaction_triggerrequests use the same existing opt-in projection. The canonical destination check, explicit Lite signal, default-off setting, bounded untrusted notes, and same-input reference handling stay in place. The native trigger, reasoning items, client tool call/result pairs, and native compaction output are preserved. Native v1/responses/compactand portable/routed compaction retain their existing behavior.I added regression coverage for the real trigger shape with both HTTP-header and WS-metadata Lite signals, and for the disabled setting. I also updated the configuration comments, architecture contract, and English/Simplified Chinese/Traditional Chinese documentation.
Related: #6742. This is scoped to native v2 Lite compaction with the existing option enabled. #7250's ordinary non-Lite missing-declaration report remains outside this PR.
Verification
The local check results below apply to the code in the current head,
ca0a9ee3229d37477739eb08d0dc72712c489b5f. I have expanded the test references here without changing the branch or rerunning checks that had already passed.What the focused tests cover
compaction_triggerinput with both HTTP-header and WS-metadata Lite signals, with projection enabled and disabled. It checks that the trigger, reasoning item (including its opaque content), empty tool list, and client call/result pairs survive. Only the eligible hosted-search history is converted.item_referencein the same input and converts only the other three search items. It also checks a second pass is unchanged. This existing test exercises an ordinary turn. Native v2 now reaches the same final projection block and referenced-ID exemption; I am not counting the ordinary-turn test as a separate native-trigger/reference test. The adjacent store:false case checks the existing reference-removal behavior.true; the configuration tests check that the option stays off in defaults and reject malformed values. The native trigger case above also covers the disabled option.Local commands and results
bun test tests/adapters/openai/openai-native-hosted-search-history.test.ts tests/responses/responses-native-compact-search-history.test.ts tests/responses/responses-compaction-override.test.ts tests/config/config-hosted-search-projection.test.ts: 147 passed, 0 failed (Bun 1.3.14). Before the source change, the updated regression tests had 3 failures../node_modules/.bin/tsc --noEmit: passed.bun scripts/structure-ssot.ts: passed.bun scripts/privacy-scan.ts: passed.node node_modules/astro/bin/astro.mjs buildindocs-site/: passed; 577 pages built. Existingsshconfigsyntax-highlighting warnings remained.git diff --check: passed.Earlier live reproduction
I also tested the change in my locally patched installed package through the official account route. One historical search item reproduced the failure before the fix; after the fix, native Lite compaction returned
response.completedwith a nativecompactionitem. A separate compaction-only request using the affected conversation's reconstructed history also succeeded with 355,808 input tokens. I did not overwrite the original conversation history or include private payloads or credentials in this PR.Those live checks used my patched local installation. I have not rerun live requests for this update; the test references above identify the automated coverage at
ca0a9ee322.Full-suite exception: I kept local validation to the affected adapter/configuration/compaction regressions given the size of the repository-wide suite and the scope of this one-condition change. I have not run the full suite or
test:changed. The local Bun version was 1.3.14, not the repository-pinned test runner 1.4.3. Broader regression coverage and validation with that runner are still pending CI.The current-head
Cross-platform CI,Catalog async contracts, andReact Doctorruns reportaction_required. I am not treating those runs, or the passing quality/bot checks, as functional CI passing.Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
/responses/compactrequests retain their existing behavior.