Update accumulated observable correction on the wide decode path#357
Merged
Conversation
…and expose the wide mask
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.
Found by an adversarial review of the PR #338 merge reconciliation: the wide-observables migration left
StreamingLogicalDecoder's accumulated correction behind. The narrowflush()setaccumulated_obs, but the wideflush_obs()did not — and the Python bindings routeLogicalAlgorithmDecoder.decode()through the wide path, soaccumulated_obs()returned 0 after adecode()that returned a nonzero correction. Only the narrow path had test coverage.Fix (single decode path):
accumulated_obsfield is now a wideObsMask, updated inflush_obs();flush()delegates toflush_obs()and narrows with the same fail-loud error style asdecode_to_observables(no truncation).accumulated_obs() -> Result<u64, _>(errors beyond 64 observables instead of truncating); newaccumulated_obs_mask()returns the lossless mask.accumulated_obsraises past 64; newaccumulated_obs_maskreturns an arbitrary-precision int via the existingobsmask_to_pyhelper.wide_decode_path_updates_accumulated_obscovers the actual bug (wide decode → accumulated state, plus reset), alongside the updated narrow-path tests.Verified: 130 decoder-core tests pass (incl. the new regression), clippy cold-clean on both changed crates, fmt clean.