test(subscriptions): gate the three TickDecoder::RESPONSE_MESSAGE_IDS consts - #739
Merged
Merged
Conversation
… consts #738 gave the tick driver the const and the shared `is_undeclared` filter, but `collect_stream_decoder_impls` matched `impl StreamDecoder<` only, so the three `TickDecoder` consts were unchecked in both directions. Reproduced first: `TickDecoder<TickBidAsk>` declaring `[HistoricalTickBidAsk, UserInfo]` passed all 1364 sync tests. The precondition was the job. With no backstop, all three `decode` impls dive straight into `require_proto()` and answer `UnexpectedWireFormat` to every probe, so the check would have read them as handling all 88 scanned discriminants and asserted nothing. Each now narrows with `expect_type` first — the single-arm form the scanner and wsh impls already use. The two traits share `check_decoder`, with the differing `decode` signatures erased by a closure at the two call sites, so the failure taxonomy stays in one copy. `test_decoder_roster_is_complete` counts per trait, so adding a `TickDecoder` while deleting a `StreamDecoder` cannot net out to a passing total. No user-visible change: the driver filters on the const before calling `decode`, so the new narrow is unreachable in production.
/simplify on #739. Four findings applied. The roster length was hand-declared and compared only against the tree, so it never inspected `check_all`: add a decoder, bump the constant, forget the `check_stream::<Foo>` line, and the counts agreed while `Foo` was never probed — exactly the rot the test claims to prevent. The `check_*` calls now tally themselves, so the roster is the thing under test and the two constants are gone. Verified: dropping one `check_all` line now fails, and used to pass. `collect_impls` walked all of src/ once per trait, reading 1.8 MB to look for a different 19-byte prefix each time. It takes every header at once now. The walk itself was a near-identical copy of `collect_sites` in one_shot_pairing_tests.rs — 11 of 18 lines, including the load-bearing `tests.rs` / `*_tests.rs` skip filter, stated twice with no link between them. Both now call `test_utils::source_scan::visit_production_sources`, which is where "production source" is defined once. Also switches `is_dir()` to `file_type()` to drop a stat per entry. `check_decoder` was at four parameters, 130 lines above a comment explaining that a different function stays at three to respect the budget; it returns its failures now instead of taking an out-param. Trims the backstop rationale, which had reached six statements across three files, down to the rule node plus one-line pointers. The node gains the argument against the two designs a reviewer would otherwise re-derive: why the const stays a slice, and why the narrow stays a hardcoded literal rather than reading `Self::RESPONSE_MESSAGE_IDS` (that would make the cross-check circular).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the
TickDecodergap #738 left open, tracked as a follow-up inplans/claude-md-knowledge-graph.md.The gap
#738 extended
RESPONSE_MESSAGE_IDStoTickDecoderso all three drivers share one skip filter, but the gate did not follow:collect_stream_decoder_implsmatchedimpl StreamDecoder<only, leaving the three tick consts unchecked in both directions. Over-declaring one routes a foreign frame's bytes intoproto::HistoricalTicks*with no test failing.Reproduced before fixing —
TickDecoder<TickBidAsk>declaring[HistoricalTickBidAsk, UserInfo]passed all 1364 sync tests. It now fails by name, as does the opposite direction (declaring&[]whiledecodestill handles the type).The precondition was the job
The gate could not simply be pointed at
TickDecoder. With no backstop, all threedecodeimpls dive straight intorequire_proto()and answerUnexpectedWireFormatto every probe — so the check would have read them as claiming an arm for all 88 scanned discriminants and asserted nothing.Each impl now narrows with
message.expect_type(..)?first, the single-arm backstop form the scanner and both wsh impls already use. #738 learned the same fact from the opposite end: it deleted two of those narrows as "a third copy of the same fact" and the gate went red within the hour. A backstop is not defensive coding — it is what makes a decoder's arm set observable from outside.Shape
check_decoderis shared by both traits, with the differingdecodesignatures (StreamDecodertakes a context and&mut;TickDecodertakes&and returns a batch) erased by a closure at the two call sites. The failure taxonomy stays in one copy rather than growing a second that drifts.test_decoder_roster_is_completecounts per trait, so adding aTickDecoderwhile deleting aStreamDecodercannot net out to a passing total.No user-visible change, so no CHANGELOG entry: the driver filters on the const before calling
decode, making the new narrow unreachable in production.Rule graph
docs/rules/wire/proto-only-decoding.mdsaid out loud that the consts were ungated. Per the maintenance protocol that is a missing gate rather than a documentation problem — the claim is rewritten rather than appended to, and #739 is recorded as a precedent alongside #738's counter-example.Verification
cargo fmt; clippy ×3 configs; rustdoc trio;just test(all legs);cargo build --examples×2; both integration crates;just rules-check.