test(routing): lock policy evidence parity across inbound protocols - #1280
Conversation
📝 WalkthroughWalkthroughThe PR adds routing-policy parity tests for Responses, Chat Completions, and Claude Messages. The tests compare rich routing evidence for tool-and-image requests and verify no hard evidence for plain-text requests. ChangesRouting policy parity
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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 |
github-actions
Bot
added
the
chore
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/routing-policy-surface-parity.test.ts`:
- Around line 73-101: Update the routing policy parity tests to exercise each
surface through its actual routing entry point rather than calling
evidenceFromBody directly. For Responses, Chat Completions, and Claude Messages,
send representative requests through their dev handlers and assert the evidence
received by policy evaluation, while retaining translator unit coverage where
useful.
🪄 Autofix
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8c2e379f-6b88-429e-b608-bbaccf38f4f0
📒 Files selected for processing (1)
tests/routing-policy-surface-parity.test.ts
| expect(evidenceFromBody(responsesBody)).toEqual(EXPECTED_RICH_EVIDENCE); | ||
| expect(evidenceFromBody(chatBody)).toEqual(EXPECTED_RICH_EVIDENCE); | ||
| expect(evidenceFromBody(claudeBody)).toEqual(EXPECTED_RICH_EVIDENCE); | ||
| }); | ||
|
|
||
| test("plain text without tools produces no hard routing evidence on every surface", () => { | ||
| const responsesBody = { | ||
| model: MODEL, | ||
| input: [{ | ||
| type: "message", | ||
| role: "user", | ||
| content: [{ type: "input_text", text: "hello" }], | ||
| }], | ||
| }; | ||
|
|
||
| const chatBody = chatCompletionsToResponsesBody({ | ||
| model: MODEL, | ||
| messages: [{ role: "user", content: "hello" }], | ||
| }); | ||
|
|
||
| const claudeBody = anthropicToResponsesTranslation({ | ||
| model: MODEL, | ||
| max_tokens: 128, | ||
| messages: [{ role: "user", content: "hello" }], | ||
| }).body; | ||
|
|
||
| expect(evidenceFromBody(responsesBody)).toEqual({}); | ||
| expect(evidenceFromBody(chatBody)).toEqual({}); | ||
| expect(evidenceFromBody(claudeBody)).toEqual({}); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Exercise each routing entry point.
These assertions call evidenceFromBody() directly after calling the translators. They cannot fail if a Responses, Chat Completions, or Claude Messages dev handler stops passing its normalized body to evidenceFromBody(), passes the raw body, or drops the resulting evidence before policy evaluation.
Drive one request for each surface through its routing entry point. Assert the evidence received by the routing policy. Keep these assertions as translator unit tests if they are useful.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/routing-policy-surface-parity.test.ts` around lines 73 - 101, Update
the routing policy parity tests to exercise each surface through its actual
routing entry point rather than calling evidenceFromBody directly. For
Responses, Chat Completions, and Claude Messages, send representative requests
through their dev handlers and assert the evidence received by policy
evaluation, while retaining translator unit coverage where useful.
Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
1 participant
Summary
devalready passes translated bodies throughevidenceFromBody()on all three surfacesWhy
The earlier audit identified cross-surface policy evidence as a risk. Current
devalready fixed the production call sites, so this PR adds regression coverage instead of duplicating routing logic.Scope
Verification
CI should run the new
tests/routing-policy-surface-parity.test.tswith the normal suite.Base inspected:
devat3ad5bb6bd3f76f6879d84b78ea39edd3e01ec296.Summary by CodeRabbit