fix(responses): give tool_search_call and custom_tool_call their own id prefixes - #2173
Conversation
…id prefixes The release audit found this by composing two changes that are each correct alone. A routed tool_search lowering is restored as a tool_search_call with no id (#2145), and the universal output-item id backfill then names it (#2142) -- but the backfill's prefix table had no entry for the type, so it fell through to the generic "item_". That is not cosmetic. stripInvalidItemIds in the Responses adapter deletes any id whose prefix does not match its type, and it lists tsc_ as the only valid prefix for tool_search_call. So the synthesized id survived the turn it was created in and was silently dropped on the next one, leaving the client an item it could not correlate. Neither PR's focused suite caught it because neither composes missing-id restoration with the backfill. custom_tool_call had the same gap and is fixed with it: the serializer enforces ctc_, the backfill did not know the type.
|
✅ Deterministic PR hygiene checks passed. |
|
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 (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe Responses field backfill now generates ChangesResponses ID backfill
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This narrowly corrects synthesized tool-call ID prefixes with focused tests and no actionable merge-blocking risk remains. Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
리뷰 · 우선순위 70 / 80#2145랑 #2142를 붙이면 나오는 구멍임. 라우티드 tool_search가 백필 prefix가 시리얼라이저가 강제하는 집합의 상위집합이어야 함. 테이블 주석으로 박은 거 맞음. types.ts/config.ts 안 건드림. MERGEABLE. 점수는 70임. 2.28에 넣어야 함. #2145+#2142가 이미 해결방안: 머지. tsc_/ctc_ 테스트 유지. 새 output 타입 넣을 때 두 테이블 같이. 이 댓글은 grok-bot이 작성했습니다 |
Summary
Release audit of
origin/main..origin/devfound this by composing two changes that are each correct alone — the shape the audit was specifically looking for.The defect. A routed
tool_searchlowering is restored as atool_search_callwith no id (#2145), and the universal output-item id backfill then names it (#2142). The backfill's prefix table had no entry for the type, so it fell through to the genericitem_:That is not cosmetic.
stripInvalidItemIdsin the Responses adapter deletes any id whose prefix does not match its type, and it liststsc_as the only valid prefix fortool_search_call. So the synthesized id survived the turn that created it and was silently dropped on the next one, leaving the client an item it could not correlate.Neither PR's focused suite caught it, because neither composes missing-id restoration with the backfill. That is exactly why this needed a cross-PR pass rather than per-PR CI.
custom_tool_callhad the identical gap and is fixed alongside it: the serializer enforcesctc_, the backfill did not know the type.The two tables are now a stated invariant — the backfill's prefixes must remain a superset of the ones the serializer enforces — recorded in a comment at the table so the next type added does not reopen it.
Verification
src/server/responses/responses-field-backfill.tsfails exactly the two new tests (20 pass / 2 fail).bun x tsc --noEmit— exit 0.bun test --isolate tests/responses-field-backfill.test.ts— 22 pass / 0 fail.bun run testonssh lidge— 13717 pass / 15 skip / 0 fail across 866 files.bun run privacy:scan— passed.Checklist
No credential or auth surface is touched; this only changes a synthesized id prefix.
Summary by CodeRabbit
Bug Fixes
Tests