fix(xai): stop stripping web_search fields xAI accepts - #2422
Conversation
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request has been marked Ready for Review. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe xAI web-search adapter now removes only ChangesxAI web-search normalization
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🟡 Moderate · up to The change is narrowly scoped to preserve xAI-supported web-search fields and has no supplied regression signal, but the PR remains draft with all readiness gates unchecked and the reported local suite has five failures; merge readiness therefore remains unconfirmed until those checks are resolved or explicitly accepted. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
`normalizeXaiResponsesWebSearch` deleted `user_location` and
`search_content_types` from every xAI web_search declaration. Both are
accepted by the upstream, so this was a silent capability loss on the
API-key path — a caller's location hint and content-type selection never
reached the model.
It also contradicted the sibling layer: `stripOpenAiOnlyWebSearchFields`
removes exactly the two fields xAI refuses and deliberately KEEPS
user_location/filters, with a probe note recording them as accepted
(tests/responses-routed-web-search-fields.test.ts). The two layers
disagreed about the same field, and the normalizer ran first, so it won.
Probed 2026-08-22, one field per request, against BOTH xAI destinations
(api.x.ai and cli-chat-proxy.grok.com), which behave identically:
external_web_access 400 on EVERY value, including `true`
search_context_size 400
user_location 200
search_content_types 200
filters 200
enable_image_search 200
So only the two refused fields are removed now. The image-search mapping
is kept: it compensates for nothing being deleted anymore, but dropping
it would be a separate behavior change.
Two assertions in responses-routed-web-search-fields.test.ts over-specified
the result as a bare `{type:"web_search"}` while that file's own probe note
says user_location is accepted; they now assert it is preserved.
Gate: 14246 pass / 5 fail, and all five also fail on untouched upstream/dev
(baseline: 6 fail, a superset). Zero regressions. The failing families
(CL-07, autostart shim, release helper, shellStreamExec) are flaky and
unrelated.
ddaf497 to
4c6582c
Compare
리뷰 · 우선순위 52 / 80설명: 이 PR 은 엑스아이 웹검색에서 받아 주는 칸을 지워 버리던 것을 그만둔다. 지금 CURRENT 지금 HEAD 의 src/adapters/xai-web-search.ts 63줄부터 68줄까지는 웹검색 도구에서 네 칸을 지운다. 바깥 웹 접근, 검색 맥락 크기, 검색 내용 종류, 사용자 위치다. 69줄은 내용 종류에 그림이 있으면 그림검색 켜기를 붙인다. 그래서 호출자가 위치를 넣어도, 글과 그림만 고르라고 해도, 그 값이 모델까지 가지 않는다. 옆 레이어 src/adapters/openai-responses.ts 1506줄 OPENAI_ONLY_WEB_SEARCH_FIELDS 는 바깥 웹 접근과 검색 맥락 크기 두 칸만 지운다. 1496줄 주석은 2026-08-21 조사에서 사용자 위치와 필터는 받아 준다고 적어 두었다. 같은 파일 시험 tests/responses-routed-web-search-fields.test.ts 32줄도 그 조사를 반복한다. 그런데 엑스아이 전용 정규화기가 먼저 돌아가서, 받아 주는 칸까지 지워 버린다. 두 레이어가 같은 칸을 다르게 본다. 이 PR 은 거절하는 두 칸만 지운다. 작성자가 2026-08-22 에 칸마다 하나씩, api.x.ai 와 cli-chat-proxy.grok.com 둘 다에 찔러 보았다. 둘의 답이 같다. 바깥 웹 접근은 참이어도 400 이다. 검색 맥락 크기도 400 이다. 사용자 위치, 검색 내용 종류, 필터, 그림검색 켜기는 200 이다. 그림검색 켜기 매핑은 남긴다. 이제 내용 종류를 안 지우니 겹칠 수 있지만, 빼는 것은 이번 범위가 아니다. 시험 두 파일은 맨몸 웹검색이 아니라 위치를 남긴다고 잠근다. 호환 시험 이름도 거절 칸만 지운다고 바꿨다. 작성자는 olddonkey 이다. 드래프트가 아니다. bug 와 review-ready 라벨이 있다. 체크리스트 네 칸이 채워져 있다. GitHub 위생, 타깃, 라벨, CodeRabbit 은 통과다. 작성자 로컬은 같은 스위트에서 기준선과 비교해 회귀가 없다고 적었다. Closes 가 없다. 이번 시간에 2421 은 머지 없이 닫혔고 2420 도 닫혔다. 사용자 길이로는 위치와 내용 종류가 모델에 안 가는 구멍이라서 52. 카탈로그 팁은 Ox Alpha x-preview-f-free + deepseek-v4-flash-vision-exp. Cursor 정적 카탈로그는 opus-4-8-fast / opus-5-fast. 2334 CursorCredentialRouter 는 여전히 src/providers/cursor-pool.ts 모듈+테스트만 있고 어댑터에 연결되지 않았다. 2332 H2 는 discovery 전용. 2320 overflow + 2342 는 이미 dev. 2188 사이드카는 이미 dev. 2382 데스크톱 앱 재시작은 이미 dev. 2292 는 아직 연다. src/adapters/xai-web-search.ts 라인 63 - 지금 HEAD 는 바깥 웹 접근, 검색 맥락 크기, 검색 내용 종류, 사용자 위치를 같이 지운다 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
The two assertions this branch changed both built an api.x.ai provider, where
the xAI normalizer strips the fatal fields before the capability gate runs. They
would have passed with the backfill broken, and never touched the OAuth CLI
destination the Responses opt-in actually targets.
Now the OAuth row resolves through resolveProviderTransport("xai", routed),
asserts it reaches cli-chat-proxy.grok.com, and checks the accepted fields
survive there; an unclassified control asserts the fatal fields are RETAINED
without classification, so a broken backfill fails.
|
Updated after review. The two assertions this PR changed both built an Now the OAuth row resolves through Gate: 14433 pass / 2 fail, zero regressions against untouched |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Approved on exact head 415aec65f77ef92958f755d909ca98f95a2e71a2. The change is narrowly scoped to preserving xAI-accepted user_location and search_content_types while continuing to remove only external_web_access and search_context_size; the existing enable_image_search mapping remains unchanged. I verified the two focused suites (15/15), strict typecheck, and the full repository Cross-platform CI including macOS, all Linux shards, package smokes, hygiene, and CodeRabbit. The updated OAuth control also proves the registry capability backfill is causal rather than accidentally passing through the public-host normalizer.
3 participants
What
normalizeXaiResponsesWebSearchdeleteduser_locationandsearch_content_typesfrom every xAIweb_searchdeclaration. Both are accepted by the upstream, so this was a silent capability loss on the API-key path — a caller's location hint and content-type selection never reached the model.It also contradicted the sibling layer.
stripOpenAiOnlyWebSearchFieldsremoves exactly the two fields xAI refuses and deliberately keepsuser_location/filters, with a probe note intests/responses-routed-web-search-fields.test.tsrecording them as accepted. The two layers disagreed about the same field, and the normalizer runs first, so it won.Evidence
Probed 2026-08-22, one field per request, against both xAI destinations —
api.x.aiandcli-chat-proxy.grok.com— which behave identically:external_web_accessArgument not supported— on every value, includingtruesearch_context_sizeArgument not supporteduser_locationsearch_content_typesfiltersenable_image_searchSo only the two refused fields are removed now.
The
search_content_types:["image"] -> enable_image_searchmapping is kept. It compensated for a deletion that no longer happens, so it is arguably redundant, but removing it would be a separate behavior change and is out of scope here.Tests
Two assertions in
tests/responses-routed-web-search-fields.test.tsover-specified the result as a bare{type:"web_search"}while that same file's probe note saysuser_locationis accepted. They now assert it is preserved.tests/xai-web-search-compat.test.tsis updated for the same reason and its test renamed to say what it actually checks.Gate
bun test --isolate --parallel ./tests/— 14246 pass / 5 fail. All five also fail on untouchedupstream/dev(baseline run: 6 fail, a superset of mine). Zero regressions.The suite is flaky — three runs of identical code gave 18, 5 and 6 failures — so this is reported as a set difference against a baseline rather than an absolute count. The failing families (
CL-07 task effectiveness,Codex autostart shim,release helper,shellStreamExec) are timing/filesystem sensitive and unrelated to this change.🤖 Generated with Claude Code
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit