test(google): assert what Vertex sends, not that two settings agree - #1922
Conversation
The post-merge audit of #1739 caught a test that could not fail. It built the same Vertex request twice, once with directGeminiWireRenames unset and once false, and asserted the two matched - which stays true even if Vertex stopped preserving the requested model id, because both sides would be wrong together. These assert the content instead: Vertex puts the requested id on the wire under all three settings and never the -tiered spelling, and its system identity names the requested model rather than a renamed one. That second one covers a real defect the #1739 merge fixed in passing, where Vertex sent the bare id while the identity line claimed -tiered. An ablation settles what the googleMode === vertex arm is worth: deleting it leaves all 24 tests green, because Vertex builds its own aiplatform URL from parsed.modelId and identityModelId only special-cases Cloud Code Assist, so routedModelId never reaches Vertex either way. The arm is defensive rather than load-bearing. It stays as a guard against a refactor that routes Vertex through the shared builder, and the comment says plainly that no test proves it fires today - claiming otherwise would be the unfalsifiable coverage this change exists to remove.
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
|
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 includes up to 10 reviews per rolling hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change adds Vertex-specific tests for URL generation and system identity text. The tests verify that requested model IDs remain unchanged and do not gain the ChangesVertex model coverage
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change strengthens localized Vertex request assertions without changing production behavior, and no actionable merge-blocking risk remains after normal checks and review. 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 |
…ssertions test(google): assert what Vertex sends, not that two settings agree
1 participant
Summary
The post-merge audit of #1739 caught a test that could not fail. It built the same
Vertex request twice — once with
directGeminiWireRenamesunset, oncefalse— andasserted the two matched. That stays true even if Vertex stopped preserving the
requested model id, because both sides would be wrong together.
These assert the content instead: Vertex puts the requested id on the wire under all
three setting values and never the
-tieredspelling, and its system identity namesthe requested model rather than a renamed one. The second covers a real defect #1739
fixed in passing, where Vertex sent the bare id while the identity line claimed
-tiered.An ablation settles what the
googleMode === "vertex"arm is actually worth:deleting it leaves all 24 tests green, because Vertex builds its own
aiplatformURL from
parsed.modelIdandidentityModelIdonly special-cases Cloud CodeAssist, so
routedModelIdnever reaches Vertex either way. The arm is defensiverather than load-bearing. It stays as a guard against a future refactor that routes
Vertex through the shared builder, and the test comment says plainly that no test
proves it fires today — claiming otherwise would be exactly the unfalsifiable
coverage this change removes.
Verification
bun test tests/google-adapter.test.ts— 24 pass, 0 fail.bun test tests/google-adapter.test.ts tests/config.test.ts tests/gemini-37-flash-migration.test.ts— 210 pass, 0 fail, 879 expect() calls.Checklist
devSummary by CodeRabbit
-tieredsuffix.