Record why the 13-bit row count is right (W4) - #8
Merged
Conversation
The reader takes a page's slot count from the 13-bit field packed at +24. crate-digger models the same bytes differently — an 8-bit count at +24 with a u16 "num_rows_large" at +34 taking over past 255 rows — and if that were right this reader would mis-parse every dense page, silently, with verify unable to notice because both sides share the reader. Fixtures cannot settle it: they are packed by the same assumption the parser makes. A real 3,673-track export can, and does. Across its 997 data pages, 14 carry 284 rows: word & 0x1FFF = 284 on those pages, and they parse correctly — the library converts to 3,872 playlist entries and verifies with 0 discrepancies. the byte at +24 reads 28, which is just 284 & 0xFF. Alone it under-reports. the u16 at +34 is not a row count at all: 283, 84, 14 and 20 across pages that all hold 284 rows, and 0x1FFF on 271 others. This repo names that field transaction_row_index, which fits — 283 is the last index of 284 rows. on the 983 pages holding <=255 rows the two readings agree, which is why the divergence stays invisible on small libraries. Comment only, no behaviour change. Written down so the next reader does not re-open the question from the ksy alone and "fix" a correct parser. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V3aF4y4GdsuJta2bfYUZ8w
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 W4 from
.omc/plans/playlist-phantom-entry-root-cause.md— and the answer is that no code change is needed. Comment only, no behaviour change.The doubt
The reader takes a page's slot count from the 13-bit field packed at
+24. crate-digger models the same bytes differently — an 8-bit count at+24with anum_rows_largeu16 at+34taking over past 255 rows. If crate-digger were right, this reader would mis-parse every dense page, silently — andverifycould never catch it, because both sides share the reader.The plan flagged that fixtures cannot settle this: they are packed by the same assumption the parser makes. Only real bytes can. A 3,673-track export provided them.
The measurement — 997 data pages, 14 carrying 284 rows
word & 0x1FFF+24+34284 & 0xFF.0x1FFFon 271 others. This repo names ittransaction_row_index, which fits — 283 is the last index of 284 rows.Why this is worth a commit
Probe-before-fix is what the plan called for, and it paid: acting on the ksy alone would have "fixed" a correct parser into a broken one. The measurement is now recorded next to
_decode_page_countsso the next reader doesn't re-open the question from the spec alone.🤖 Generated with Claude Code
https://claude.ai/code/session_01V3aF4y4GdsuJta2bfYUZ8w