test(fastwire): A0 characterization — lock current service-tier behavior (#1886) - #1892
Conversation
…ctor Phase A0 of the FastWire umbrella (lidge-jun#1886): characterization suites only, zero production changes. Locks the three-state fastMode contract, the unclassified passthrough matrix, exact-model foreign-tier forwarding, the mixed requestedServiceTier logging semantics, routing/fingerprint projections, catalog tier-field byte goldens, and two known bugs (native chat ignores exact-model false; chat-to-responses conversion drops service_tier) as current behavior. Full suite at this commit: 12746 pass / 10 skip / 0 fail. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already 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 (2)
Included review availability: Your plan includes up to 10 reviews per rolling hour; 9 remain after this review. 📝 WalkthroughWalkthroughAdded FastWire characterization tests for service-tier routing, compatibility projection, catalog metadata, request forwarding, fast-mode behavior, request logging, raw-body observation, and two known bugs. ChangesFastWire characterization
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to This change only adds tests that document existing FastWire behavior; no actionable merge-blocking risk remains after normal checks and review. 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 |
…B gate The audit caught a false statement I wrote: lidge-jun#1892 and lidge-jun#1904 are not disjoint, they modify the same two test files. The pair is safe for a better reason - lidge-jun#1904 contains lidge-jun#1892's commit 0cdd07d, so git resolves through the common ancestor instead of seeing two unrelated additions, and the one blob that differs is the intentional A0 flip. Verified both directions, and a sequential merge of all five onto origin/dev in a scratch worktree produced five clean merges. Two things the plan had backwards. lidge-jun#1888's sponsorship label is its third blocker, not its first - it is also CONFLICTING against dev and carries CHANGES_REQUESTED. And the reason not to self-apply that label is sharper than an agent not unblocking itself: MAINTAINERS.md requires explicit security review for auth surfaces, and the label is the record that the review happened, so applying it without doing the review makes the record false rather than merely skipping a step. The train's real gate was never merge order. All five sit behind maintainer approval under Protect dev. Recording per-PR dispositions: lidge-jun#1884 and lidge-jun#1892 are ready, lidge-jun#1902 has no exact-head CI on production routing code, lidge-jun#1904 is a draft with unticked boxes, and lidge-jun#1898 is missing two of the five tests this plan required - account appears zero times in its diff.
…ampaign branch
I wrote that lidge-jun#1902 merged after its suite went green. It did not. The merge landed at 00:36:18Z and test 2/4 reported at 00:36:23, test 4/4 at 00:36:30, npm-global windows at 00:37:32, with macos still running. Everything passed - the run now reads completed/success with all four shards and macos green - so the outcome is sound and the prior round's concern was genuinely answered. The claim was still ahead of the evidence, and on production routing code that is the specific gap the round had flagged. Approving the fork's CI runs was legitimate: action_required protects runners from untrusted code rather than gating merges, and the diff touched no workflow files. The error was the twelve seconds, not the approval. Also recording two things I did not notice at the time. lidge-jun#1892 merged with no exact-head test CI at all, which is the same deficiency lidge-jun#1902 was held for - two characterization test files is a reason to accept the gap, not a reason to miss it. And none of the three carries an approving review artifact; they went through the admin bypass, which this document had just called the train's real gate.
Three of six landed: lidge-jun#1884, lidge-jun#1892, lidge-jun#1902. Three carried forward, each with a reason that belongs to the PR rather than to the wave - lidge-jun#1904 is a draft whose author has not ticked its readiness boxes, lidge-jun#1898 is missing the two pacing tests this plan required, and lidge-jun#1888 has three independent blockers including an unsponsored auth surface. Focused verification on the merged tree covers the replay, fastwire and router suites: 54 pass, 0 fail. Dev's own CI at 2a9f083 is still in progress, and the two runs before it were cancelled by supersession, so the branch has no completed green run on its current head yet. That matters for WP9's promotion, not for these merges.
Summary
Phase A0 of the FastWire umbrella (#1886): characterization tests only — zero production changes. These suites lock the current Fast-mode behavior byte-for-byte before the A1 refactor lands, including two known bugs locked deliberately as current behavior.
What is locked
tests/fastwire-characterization-wire.test.ts(19 cases):fastModecontract on supported routes (trueoverrides caller tier withpriority;falseremoves any tier;undefinedpreserves caller values).support === undefined × fastMode {true,false,undefined} × caller {priority,fast,flex}matrix: unclassified routes pass caller values through untouched —fastModenever intervenes.modelSupportsServiceTier: trueforwards any foreign caller tier on Chat routes (flexand an unknown string), withoutchatServiceTier.requestedServiceTierlogging is mixed-semantics today: the raw caller value survives a fast-mode override, but an unsupported-route strip recordsundefined.false.service_tier.tests/fastwire-characterization-routing.test.ts(6 cases):require.serviceTierseessupportsServiceTier: true + chatServiceTier: falseChat routes as unsupported.default_service_tier/service_tiersincl. the current description string /additional_speed_tiers) and their omission when capability isfalse/unset.Verification
6b17d6233: 12748 pass / 10 skip / 0 fail (823 files).bun x tsc --noEmitclean.src/changes; no existing test touched.devate60d5506cbefore final verification.devadvanced 6 commits during the 696-second suite, still within the repository's enforced 10-commit readiness window.Part of #1886 (Phase A0). The A1 refactor PR stacks on this branch.
🤖 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