test(protocol): extend the golden decoder oracle past frame decode — and fix the four u32 narrowings it found - #869
Open
vishk23 wants to merge 4 commits into
Open
Conversation
vishk23
changed the title
ryanbr
pushed a commit
that referenced
this pull request
Jul 27, 2026
…ites check (#889) Filed by @tanarchytan in #775, who asked for exactly this: a JVM/CI test, no device, that builds both schemas and asserts matching column names, NOT NULL / default flags and primary keys for every shared table. CLAUDE.md's parity contract already said Room and GRDB "must agree on the resulting schema" and that "column order in a Room CREATE TABLE must match the entity field order" — but nothing enforced it, so a migration added on one side could diverge until someone noticed by hand. That already happened once: rrInterval's key was widened on Room v18 and only later on GRDB v24-rr-seq. MECHANISM. schema_oracle.json, committed in two byte-identical copies with a cross-copy identity test — the decoder_oracle.json pattern, one layer down. SchemaOracleTests.swift runs the real DatabaseMigrator into an in-memory database and asserts PRAGMA table_info / index_list against the fixture; SchemaOracleTest.kt asserts the schema Room itself generates against the same fixture. The Android half needed exportSchema = true plus a room.schemaLocation KSP arg, which makes Room's processor write one JSON file per version describing the exact CREATE TABLE it emits. No Robolectric, no device, no emulator, no new dependency, no lockfile change. The export is NOT committed: it is a derived answer regenerated by KSP every build, exactly like the decoded values the decoder oracle compares — committing it would mean two copies of the same facts and churn on every migration. 31 tables pinned, both suites reporting every mismatch in a run rather than the first, and the divergence ledger fails in BOTH directions — a new divergence, and a recorded one that has quietly been fixed — so it can only shrink deliberately. VERIFIED ON REVIEW. Both oracle copies are byte-identical (same md5). The pins still match CURRENT main rather than this branch's base: roomVersion 25 matches, and the 31-entry grdbMigrations list is identical to main's in order and content, so nothing goes stale on landing despite #888 and #890 merging since. Both spot-checked divergences exist in the code as recorded. exportSchema false->true is build-time only; the runtime path is unchanged. NOT VERIFIED. The 75-line build.gradle.kts change and SchemaOracleTest.kt are checked by nothing: android.yml is disabled_manually, so no CI job compiles Kotlin or runs Gradle. test (WhoopStore) passing proves the Swift oracle works and says nothing about the Android half. That matters more than for a normal Kotlin change because this one edits the BUILD — a broken config would not surface until a release, and fork-release.yml pushes the version bump before it builds. The three Gradle hazards documented in the file (stale export under UP-TO-DATE, the UnitTest KSP round wiping the directory, stale-output cleanup removing it) are not findable without executing it, so there is good evidence it was run locally; it is simply not reproducible in CI today. FOLLOW-UP FILED for the ppg-hr-bpm-integer-on-android reason text, which describes the affinity split as having "a data consequence" and being the same class as the #869 decoder bug. It is neither: both platforms round to whole bpm before storing (PpgHr.swift:151 says so, citing #219 as the change that put them in lockstep), the domain type is Int on both sides, and the read path CASTs and ROUNDs anyway. The entry belongs in the ledger — the affinity split is real — but what it is said to mean overstates it, and a committed fixture asserting a data bug will send someone looking for one.
ryanbr
added a commit
that referenced
this pull request
Jul 27, 2026
The divergence ledger #889 introduced is careful — of its eight entries, seven state plainly that no stored row differs and what closing the gap would cost. One overstates. `ppg-hr-bpm-integer-on-android` called the REAL/INTEGER split "a data consequence", said the estimate is "fractional by construction, so Android stores a truncated bpm for the same input", said "the Swift row is `Double`", and filed the whole thing under the decoder-oracle (#869) class of bug. None of that holds, and the last part is the crispest to check: the Swift ROW type is `Int`. PpgHr.swift declares `public let bpm: Int` and comments it "the same Int domain as the measured HRSample.bpm and the Android PpgHr.Estimate.bpm, so the stored value and type match across platforms (#219)". Only the SQLite COLUMN differs. Both platforms also round to whole bpm before that point — Swift `(fsD * 60 / refinedLag).rounded()` then `Int(est.bpm)`, Kotlin `Math.round(fsD * 60 / refinedLag).toInt()` — and the read path CASTs and ROUNDs again, a no-op on a whole number. There is exactly one writer and one construction site on each side, so nothing else can put a fractional value in the column. #869's class was a real value divergence; this is not one. The entry stays: the declared affinity does diverge, and a bpm that ever became fractional would then diverge silently, which is worth the tripwire. Only its meaning is corrected, plus what closing it costs, in the same shape as the other seven. Worth fixing rather than shrugging at because the text is COMMITTED in a fixture whose purpose is to be authoritative about schema differences. Left alone it sends the next reader looking for a truncation bug that does not exist — or prompts a Room column widening that changes nothing and costs a migration. A ledger that overstates decays the same way one that omits does. Text only. No key renamed, so nothing referencing it changes; both suites assert on the KEYS, not the prose. Both copies stay byte-identical, and the file's own \uXXXX escaping is preserved so the diff is one line per copy rather than churning every em dash in the ledger.
ryanbr
added a commit
that referenced
this pull request
Jul 27, 2026
Follow-up to #889. Text only, one line per oracle copy. The divergence ledger #889 introduced is careful — of its eight entries, seven state plainly that no stored row differs and what closing the gap would cost. One overstates. `ppg-hr-bpm-integer-on-android` called the REAL/INTEGER split "a data consequence", said the estimate is "fractional by construction, so Android stores a truncated bpm for the same input", said "the Swift row is `Double`", and filed the whole thing under the decoder-oracle (#869) class of bug. None of that holds, and the last part is the crispest to check: the Swift ROW type is `Int`. PpgHr.swift declares `public let bpm: Int` and comments it "the same Int domain as the measured HRSample.bpm and the Android PpgHr.Estimate.bpm, so the stored value and type match across platforms (#219)". Only the SQLite COLUMN differs. Both platforms round to whole bpm before that point — Swift `(fsD * 60 / refinedLag).rounded()` then `Int(est.bpm)`, Kotlin `Math.round(fsD * 60 / refinedLag).toInt()` — and the read path `CAST(ROUND(p.bpm) AS INTEGER)` rounds again, a no-op on a whole number. There is exactly one writer and one construction site on each platform (StreamStore.swift + PpgHr.swift; WhoopRepository.kt + HistoricalStreams.kt), so nothing else can put a fractional value in the column. #869's class was a real value divergence; this is not one. THE ENTRY STAYS. The declared affinity does diverge, and a bpm that ever became fractional would then diverge silently, which is worth the tripwire. Only its meaning is corrected, plus what closing it costs, in the same shape as the other seven. Worth fixing rather than shrugging at because the text is COMMITTED in a fixture whose purpose is to be authoritative about schema differences. Left alone it sends the next reader looking for a truncation bug that does not exist — or prompts a Room column widening that changes nothing and costs a migration. A ledger that overstates decays the same way one that omits does. No key renamed, so nothing referencing it changes; both suites assert on the KEYS, not the prose. Both copies stay byte-identical, and the file's own \uXXXX escaping is preserved so the diff is one line per copy rather than churning every em dash in the ledger. Every quote and code snippet in the new text was checked verbatim against the file it is attributed to — a corrective text that misquoted the code would be worse than the error it replaces.
`record_index@11` is an unsigned 32-bit field. The Kotlin decoder narrowed it with `toInt()`, and Kotlin's Int is 32-bit where Swift's is 64-bit, so a value with bit 31 set decoded to -1_062_772_323 on Android and 3_232_194_973 on Swift/macOS from byte-identical bytes. The per-platform fixture-hex tests cannot see this: the wire bytes are the same on both platforms and each suite asserts its own platform's answer, so the divergence lives entirely downstream of the bytes. PR #848 hit the same 32-vs-64-bit split in the v18 storage codec and had to re-widen there; this is the decoder key itself. Carried as a Long, matching the u32 reader's own return type. No main-code consumer reads the key (the only readers are tests and the field census), so the change is confined to the decoded map and its assertion.
`decoder_oracle.json` already made both decoders prove the same output against the same bytes, but it stopped at frame decode. It now pins three layers, each with a twin test in `DecoderOracleTests.swift` and `DecoderOracleTest.kt`, and the file stays byte-identical across the two copies. 1. Decoded VALUES, including the 32-vs-64-bit boundary. Adds `whoop5_v18_synthetic_record_index_high_bit` - a real worn frame with only the u32 `record_index@11` replaced by 0xC0A7619D and the CRC32 recomputed. Bit 31 is where Swift and Kotlin stop agreeing, and no real capture reaches it (observed lifetime counters are ~8-25 million), so it has to be synthesised to be covered at all. 2. Stream ASSEMBLY (`stream_batches`). Six batches run named fixture frames through `extractHistoricalStreams` and pin every stream's row count - zeros included, so a stream materialising on one platform only is a failure - plus the emptiness verdict and the #547 dropped-record counter. Frame decode agreeing does not imply assembly agrees. Because a batch fixture can only cover the streams it happens to populate, a constructive test also builds a one-stream batch per stream and asserts each is non-empty: `insert` early-returns on an empty batch, so a stream missing from the verdict is silent data loss on that platform. 3. A `coverage` manifest, so the oracle cannot quietly stop covering something. It pins every fixture and batch name, every stream, and every asserted decoder key mapped to HOW MANY frames assert it - the count, not just the name, so dropping a key from one frame while another still carries it is also a failure. Every non-derived key must additionally appear in a real decode, the check PR #848 needed after a rename silently broke an extractor. Both suites also assert the stream list equals the array-typed fields `Streams`/`StreamBatch` actually declare, so a new stream added on one platform without oracle coverage fails there. Batches use identity clock refs (the archive-replay seam), so no clock correction applies and they stay reproducible. Refs #647, #775.
`unix` is an unsigned 32-bit field at three separate decode sites (WHOOP 4 v24/v5 @11, WHOOP 4 v25 @11, WHOOP 5/MG v18 @15). All three narrowed it with `toInt()`, and Kotlin's Int is 32-bit where Swift's is 64-bit, so from 2038-01-19 — the second bit 31 first sets — Android decodes a NEGATIVE timestamp where macOS/iOS decodes 2147483648 from byte-identical bytes. For `unix` that is worse than a wrong number. The negative value fails the #547 `MIN_PLAUSIBLE_UNIX` floor in `extractHistoricalStreams`, so the record is dropped: every offloaded record disappears on Android while macOS/iOS keeps ingesting the same bytes, with the UI still reporting a healthy sync. That violates the cross-platform parity contract, and it is silent. Carried as a Long, matching the u32 reader's own return type. The widening alone is NOT sufficient: `intOrNull`'s `is Long -> v.toInt()` branch would have re-narrowed it right back at the read site, so this adds `longOrNull` and reads `unix` through it. Two downstream consumers move with it — `Backfiller`'s SpO2 RE dump (an `as? Int` cast that would otherwise miss on every record and silently stop dumping) and `Spo2ReTrace.recordLine`, whose Swift twin already takes a 64-bit Int, so widening restores the byte-identical-line promise rather than breaking it. The v26 PPG path is widened for consistency only; it was never wrong, because its single consumer re-widened with `and 0xFFFFFFFFL`. That mask is now unnecessary and is removed. It remains the one site the oracle cannot pin: v26 returns null from `decodeHistorical`, so no v26 frame can live in the fixture set. Pinned on both platforms by three new synthetic oracle frames, one per decode site, each its real source capture with only the four `unix` bytes replaced by 0x80000000 and the CRC32 trailer recomputed (the two families checksum different ranges). Three paired stream batches assert the record still assembles into rows instead of vanishing, which is the actual user-visible bug; they need a `wall_now` override because the records are ~12 years in the future, so this also adds that test seam to Swift's `extractHistoricalStreams` (Android already had it). Every production caller passes nil and behaviour is byte-identical. Verified: swift test 350 passed; ./gradlew testFullDebugUnitTest 3096 passed (SyncChipStateTest fails on pristine upstream/main, unrelated). Each of the three decoder lines and the `longOrNull` read was reverted in isolation to confirm the matching fixture fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`testDeclaredStreamsMatchOracleManifest` has been red since 2026-07-27. #848 added `v18_aux` to `Streams` / `StreamBatch` and to both platforms' emptiness verdicts, but not to this branch's oracle manifest, so the check that exists to catch exactly that — a new stream with no oracle counts — caught it. The manifest is the fix, not the test. `v18_aux` belongs in `coverage.streams` rather than `non_stream_lists` because both `Streams.isEmpty` (Streams.swift:646) and `StreamBatch.isEmpty` (WhoopRepository.kt:99) include it: it gates the insert, so a batch carrying only v18_aux rows must not read as empty. Counts are MEASURED, not chosen — each one is what `extractHistoricalStreams` actually produced over the pinned fixture frames: whoop5_v18_worn_pair 2 whoop5_v18_offwrist_single 1 whoop5_v18_high_bit_unix_survives 1 the other six batches 0 Both `streamCounts` helpers gain the stream, so the counts are compared against a real row count rather than silently absent. Both `emptinessVerdictCovers…` one-stream cases gain a `v18_aux` case, which is the half of the #848 defence that a fixture batch cannot provide. The one-stream case carries `recordIndex: 1` rather than a bare `ts`: `V18AuxSample.isEmpty` / `V18AuxRow.isEmpty` is "every slot is null", so an all-null row would still make the list non-empty and pass the check while proving nothing about a row that carries data. Both oracle copies stay byte-identical (`testOracleCopiesAreIdentical`). Swift: 445 tests, 0 failures. Kotlin: 3225 tests, 0 failures, 0 errors, 5 skipped across 393 classes (JUnit XML). DecoderOracleTest 6/6 on each side.
vishk23
force-pushed
the
test/oracle-parity-extend
branch
from
July 29, 2026 04:49
7f04842 to
b5cefb7
Compare
Author
|
Green again. Rebased onto The failure was
The counts are measured, not chosen. I added the stream with placeholder zeros, ran the batch test, and took the numbers the real
Only the three v18 batches carry rows, which is the expected shape — the field is 5/MG v18-only and empty on a 4.0. Three things had to move together, and the suite would have caught any one left behind:
One detail in that last case worth flagging, because the obvious version is a false pass: Verified
|
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.
decoder_oracle.jsonalready exists in two byte-identical copies with a cross-copy identity test, landed explicitly to catch Swift/Kotlin decoder drift. It stops at frame decode. That gap is not theoretical — it let two real cross-platform bugs through in #848, both caught by review rather than by a test:3_232_194_973in Swift and-1_062_772_323in Kotlin. Swift'sIntis 64-bit, Kotlin's is 32-bit. A fixture-hex test cannot catch this: the wire bytes are identical on both platforms and each suite asserts its own platform's answer, so the divergence lives entirely downstream of the bytes. Only comparing the decoded numbers against one shared expectation finds it.StreamBatch.isEmptyomitted a stream in Kotlin but not Swift, andinsertearly-returns on empty — so a batch carrying only that stream banked nothing on Android. Silent data loss, invisible to any frame-decode test.This extends the oracle past frame decode, following the established idiom rather than adding a parallel mechanism — and then fixes the four u32 narrowings it found. Refs #647 (asks for cross-platform golden tests on duplicated policy surfaces) and #775 (filed by @tanarchytan, asks for a mechanical parity guardrail instead of a comment).
What the oracle now pins
Three layers, each with a twin test in
DecoderOracleTests.swiftandDecoderOracleTest.ktreading the same shared file.1. Decoded values, including the 32-vs-64-bit boundary. Four synthetic fixtures, each a real capture with only the four bytes of one u32 replaced and the CRC32 trailer recomputed (the two families checksum different ranges: whoop4
frame[4..length), whoop5frame[8..payloadEnd)):whoop5_v18_synthetic_record_index_high_bitrecord_index@11→0xC0A7619Dwhoop5_v18_real_wornwhoop4_v24_synthetic_unix_high_bitunix@11→0x80000000whoop4_v24_real_wornwhoop4_v25_synthetic_unix_high_bitunix@11→0x80000000whoop4_v25_real_awhoop5_v18_synthetic_unix_high_bitunix@15→0x80000000whoop5_v18_real_worn0x80000000is 2038-01-19T03:14:08Z — the exact second bit 31 of a unix u32 first sets. No real capture can reach bit 31 before then (and observed lifetimerecord_indexcounters are ~8–25 million), so these have to be synthesised to be covered at all. Each is flaggedsynthetic: truewith its provenance in the fixture. The v18 unix frame deliberately keeps its realrecord_index, so it also proves the patch hitunix@15and not the u32 four bytes away.2. Stream assembly (
stream_batches). Nine batches run named fixture frames throughextractHistoricalStreamsand pin every stream's row count — zeros included, so a stream materialising on one platform only is a failure rather than an unasserted extra — plus the emptiness verdict and the #547 dropped-record counter. The batches cover an empty input, a v24 record, three v25 records (gravity but no HR), a worn 5/MG pair, an off-wrist record (HR gated out while skin temp / gravity / steps / sleep state land — the partial-batch shape), a session-range rejection, and three post-2038 batches that assert a high-bit-unix record still assembles into rows instead of vanishing.Those last three need a
wall_nowoverride, because their records are ~12 years in the future and the #547 gate's upper bound is otherwise the live clock — without it they would drop on both platforms for an unrelated reason and assert nothing. Android'sextractHistoricalStreamsalready had that seam; this adds the matching one to Swift (wallNow: Int? = nil). Every production caller passesniland behaviour is byte-identical.Because a batch fixture can only cover the streams it happens to populate, a constructive test also builds a one-stream batch per stream and asserts each is non-empty. That is the exhaustive form of bug 2: any stream left out of the emptiness verdict on either platform fails.
3. A
coveragemanifest, so the oracle cannot quietly stop covering something. It pins every fixture and batch name, every stream, and every asserted decoder key mapped to how many frames assert it — the count, not just the name, so dropping a key from one frame while another still carries it is also a failure. Every non-derived key must additionally appear in a real decode: that is the check #848 needed after a rename had silently broken an extractor. Both suites also assert the stream list equals the array-typed fieldsStreams/StreamBatchactually declare, so a new stream added on one platform without oracle coverage fails there.The bugs found
record_index@11narrowed (first commit).decodeWhoop5Historicalnarrowed the u32 withtoInt()— #848's divergence in the decoder key itself rather than the storage codec. Carried as aLong, matching the u32 reader's own return type. No main-code consumer reads the key (only tests and the field census), so that change is confined to the decoded map and its assertion.unixnarrowed at three sites (second commit) — and this one is worse than a wrong number. A negative timestamp fails the #547MIN_PLAUSIBLE_UNIXfloor, soextractHistoricalStreamsdrops the record entirely: from 2038-01-19 every offloaded record disappears on Android while macOS/iOS keeps ingesting the same bytes, with the UI still reporting a healthy sync. Widened toLongat all three sites.The widening alone is not sufficient, which is the part worth flagging to a reviewer:
intOrNull'sis Long -> v.toInt()branch re-narrows the value at the read site and silently undoes the fix. So this adds alongOrNullaccessor and readsunixthrough it. Two consumers move with it —Backfiller's SpO2 RE dump (anas? Intcast that would otherwise miss on every record and silently stop dumping) andSpo2ReTrace.recordLine, whose Swift twin already takes a 64-bitInt, so widening restores the byte-identical-line promise rather than breaking it.The v26 PPG path is widened for consistency only. It was never actually wrong — its single consumer re-widened with
and 0xFFFFFFFFL— and that now-redundant mask is removed. It stays the one narrowing site the oracle cannot pin: v26 returnsnil/nullfromdecodeHistorical, so no v26 frame can structurally live in the fixture set.Proof the tests can fail
A parity test that cannot fail is worthless, so each new check was verified by reintroducing the real divergence and confirming the failure, then reverting:
record_indexnarrowed back totoInt()(bug 1)oracleFramesDecodeToExpectedOutputFAILED:whoop5_v18_synthetic_record_index_high_bit.record_index expected:<3232194973> but was:<-1062772323>unix@11narrowed back, v24/v5 pathwhoop4_v24_synthetic_unix_high_bit.unix expected:<2147483648> but was:<-2147483648>; batch FAILEDwhoop4_v24_high_bit_unix_survives.rr row count expected:<2> but was:<0>unix@11narrowed back, v25 pathwhoop4_v25_synthetic_unix_high_bit.unixlikewise; batch FAILEDwhoop4_v25_high_bit_unix_survives.gravity row count expected:<1> but was:<0>unix@15narrowed back, v18 pathwhoop5_v18_synthetic_unix_high_bit.unixlikewise; batch FAILEDwhoop5_v18_high_bit_unix_survives.rr row count expected:<2> but was:<0>longOrNullswapped back tointOrNulloracleStreamBatchesAssembleToExpectedShapesleepStatedropped from Kotlin'sStreamBatch.isEmpty(bug 2's shape)emptinessVerdictCoversEveryStreamFAILED:StreamBatch carrying only 'sleep_state' must NOT be emptybpm != 0HR gate removed (a one-sided assembly change)oracleStreamBatchesAssembleToExpectedShapeFAILED:whoop5_v18_offwrist_single.hr row count expected:<0> but was:<1>heart_rateexpectation deleted from both copiesoracleCoverageManifestMatchesFixturesFAILED on both platformsTwo of those rows are the argument for the layers existing at all. The
longOrNullrevert is a live regression that passes every frame fixture — only the stream-assembly layer catches it, because the map value is correct and the loss happens at the read. And theheart_ratedeletion is why the manifest counts frames per field rather than listing names: a set-only manifest passed that deletion, because other frames still assertedheart_rate.Verification
Packages/WhoopProtocol: 350 tests, 0 failuresPackages/WhoopStore: 300 tests, 0 failurescd android && ./gradlew testFullDebugUnitTest: 3096 tests, 1 failure, 5 skipped — the failure iscom.noop.ui.SyncChipStateTest > lastSyncedAt_takesPriorityOverHistorySyncExperimental(IllegalStateException: NoopApplication is not attached), pre-existing on pristineupstream/main, entirely withincom.noop.ui, which this diff does not touch. Ran without--dependency-verification=off; the macOS aapt2 pin from build(android): pin the macOS aapt2 artifact so dependency verification passes off-Linux #854 works.xcodebuild -scheme Strand -destination 'platform=macOS': BUILD SUCCEEDED. Run because the newwallNowparameter is package surface consumed by app-target callers (Strand/Collect/Backfiller.swift,Strand/Collect/RawHistoryArchive.swift), andapp-build.ymlis disabled by default — a package signature change can break an app target that no default CI compiles.No BLE path, analytic, score, gate, UI or export is touched.
Scope, and what this deliberately does not do
This is one engine done well plus the mechanism for others to follow: the type-47 HISTORICAL_DATA decoder and its stream assembly, with a
coveragemanifest and a note indocs/CONTRIBUTING.mdpointing future work at extending the oracle rather than reinventing it.On the store layer — "same rows written for the same input", and #775's Room↔GRDB schema check — the honest answer is that it is worth doing and belongs in its own PR, because it needs a different mechanism and a build change, not more oracle entries. The Android JVM test lane has no Robolectric and no
room.schemaLocationexport, so a JVM unit test today cannot instantiate a Room database or read its generatedCREATE TABLE. The tractable path, and the one that matches this idiom: enable Room's schema JSON export, and have a GRDB-side test run the migrations into an in-memory database and comparePRAGMA table_info/ primary keys against the same committed shared fixture — aschema_oracle.jsonshaped exactly like this file. Happy to follow up if that shape looks right.Still left in place and flagged rather than fixed here:
Framing.ktnarrowsunixthe same way at three more sites (CONSOLE_LOGS@12, and the WHOOP 4 / WHOOP 5 METADATA decoders). Those are a different parsed map with different consumers, andclassifyHistoricalMetaalready re-widens withand 0xFFFFFFFFL, so they are correct-but-fragile rather than broken — a separate concern from the historical-record path this PR covers.