feat: publish the export accounting valplay was recounting - #1
Merged
Conversation
The manifest already carried plenty of counters, but none of them was usable as a gate. The reference replay reports skipped_bits 19,135,006 and 7,889 failed RPC streams while being healthy, so any consumer gating on a raw counter would mark every match degraded forever and the signal would become noise nobody reads. lost_content_blocks() nets the preserved payloads back out, leaving only the lines USAGE.md says must be zero. oracle and manifest now call the same function instead of restating the arithmetic.
The _ClassNetCache suffix and the unresolved-payload field name were literals in both Rust and Python with nothing tying them together, so a rename here broke valplay while this suite stayed green. The test parses the adapter's assignment statements rather than grepping its text: both values also appear in its docstring, so a substring search would be satisfied by prose alone and could never fail. A third test feeds the scanner a decoy docstring to prove it reads the assignment.
The adapter emitted six manifest keys and dropped everything else, including the whole quality object. valplay therefore derived every coverage verdict by recounting the NDJSON instead of reading the accounting this tool already computes, and the two could disagree silently. quality and net_field_export_groups now pass through verbatim -- no summarizing, no defaults. A missing quality writes null rather than an empty object, because zeros would read as "nothing was lost" when the truth is "nobody counted". The adapter's own measurements go in a separate 'adapter' object so they can never be mistaken for upstream numbers. players[] is deliberately NOT forwarded: valplay builds a richer version of the same table from the same rows, keeping character GUIDs as a set so revives and late joins keep their kill attribution. Forwarding a thinner copy would spread account PUUIDs into one more file and offer an attractive way to lose that. A test pins the omission. Event order is (packet_id, time_ms) stable-sorted. time_ms is not monotonic in packet_id -- vrf-frame substitutes 0 for a non-finite frame time -- so a regression is counted and published rather than sorted away, since sorting it away would move the stream off wire order.
The patch file described a fix to valplay's compute_metrics.py that was never applied, so Swiftplay replays silently produced empty metrics. The change now lives in valplay; leaving an applied patch on disk only invites a second application. Its reasoning moved into the code it describes. Also refreshes the test counts quoted in the docs -- the full check_docs.py caught them, which is why it is not run with --fast.
Owner
Author
|
Downstream: yakisoba0728/valplay#4 — the consumer of this accounting. Ship together. |
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
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.
Why
vrfkit already computes precise completeness accounting —
chunks_processed,event_rows,movement_rows,event_layout_mismatches, error buckets, the nestednetandsinkobjects — andquality_json_names_every_load_bearing_counterpins its keys.The Python adapter emitted six manifest keys and dropped all of it. So valplay, the only consumer, derived every coverage and reconciliation verdict by recounting the NDJSON rather than reading the accounting this repo produces. The two could disagree silently, and its published quality model rested on the recount.
Nothing tested the seam either: renaming a column in
vrf-export/src/schema.rsbroke valplay while this suite stayed green.What changed
A loss counter a consumer can actually gate on. The existing counters are unusable as gates — the reference replay reports
skipped_bits19,135,006 and 7,889 failed RPC streams while being healthy, so gating on a raw counter marks every match degraded forever.NetStats::lost_content_blocks()nets the preserved payloads back out, leaving only the linesdocs/USAGE.mdsays must be zero.oracleandmanifestnow call the same function instead of restating the arithmetic.The accounting crosses the seam.
qualityandnet_field_export_groupspass through verbatim — no summarizing, no defaults. A missingqualitywritesnull, not an empty object: zeros would read as "nothing was lost" when the truth is "nobody counted". The adapter's own measurements go in a separateadapterobject so they can never be mistaken for upstream numbers.players[]is deliberately not forwarded. valplay builds a richer version of the same table from the same rows, keeping character GUIDs as a set so revives and late joins keep their kill attribution. Forwarding a thinner copy would spread account PUUIDs into one more file and offer an attractive way to lose that. A test pins the omission.Ordering is defined here.
time_msis not monotonic inpacket_id—vrf-framesubstitutes 0 for a non-finite frame time — so events are(packet_id, time_ms)stable-sorted and a regression is counted and published rather than sorted away, since sorting it away moves the stream off wire order.The duplicated constants are pinned. The
_ClassNetCachesuffix and the unresolved-payload field name were literals in both languages with nothing tying them together. The test parses the adapter's assignment statements rather than grepping: both values also appear in its docstring, so a substring search would be satisfied by prose alone and could never fail. A third test feeds the scanner a decoy docstring to prove it reads the assignment.The Swiftplay patch is applied.
docs/swiftplay-metrics.patchdescribed a fix to valplay that was never applied, so Swiftplay replays silently produced empty metrics. The change now lives in valplay and the patch file is removed.Verification
Every new gate was mutation-tested: reverting each one turns the intended test red.
Deploy note
This must ship together with the valplay PR. The bundle schema goes to 2 and valplay's
_validate_bundlerequires it, so an old checkout on either side fails loudly by design. Existing bundles are invalid and the corpus needs re-extraction (~22s per replay for the adapter stage alone).