Skip to content

fix(corekit): keep a BitField's unnamed bits as captured (#1487) - #1495

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1487-bitfield-unnamed-bits
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1487-bitfield-unnamed-bits

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

BitField cleared every bit its namespace leaves unnamed on rebuild. post_process now records those bits under an __unnamed__ key, as an int over the whole field with the named bits cleared, and pre_process seeds its buffer from it (pcapkit/corekit/fields/strings.py:277, :301, :329, :363).

Why a key in the value: it is how #1380 (__remainder__) and #1465 (__short_read__) keep what the capture held, and an ordinary key rides through Schema.to_dict/from_dict with no extra plumbing. As there, the key is absent when there is nothing to keep (all unnamed bits zero), so a user-built value packs them as zeros as before, and the record is masked so it can never override a named subfield.

Eight shipped BitFields leave bits unnamed, but all eight are ForwardMatchField look-ahead peeks, which never pack (e.g. schema/transport/tcp.py:792, schema/internet/ipv4.py:598), so no protocol output changes: repr(frame.info.to_dict()) hashes over all 23 examples/captures files are identical on main and here.

Tests: new tests/corekit/test_bitfield_unnamed_bits_1487_unit.py (12 failed on main's strings.py, 9 pass here); the #1487 Gap row is deleted. tests/corekit 477 passed, test_capture_roundtrip_runtime.py 4 passed, tests/project 419 passed / 1 skipped, test_docstring_contract.py 7 passed. pylint/mypy/isort on strings.py show no new findings; the make targets were not run.

Closes #1487

@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at be50fbdb7. Cross-reviewed on Sonnet; the author ran on Opus.

  • The fix works. I measured it myself: on origin/main sources the new tests give 12 failed. At head, the new file plus test_field_roundtrip_unit.py gives 17 passed, and only the Gap(1487…) row is removed.
  • Corrected in the body: eight shipped BitFields do leave bits unnamed, not none. All eight are ForwardMatchField look-ahead peeks, which never pack, so __unnamed__ never reaches packed or public output: 0 hits across every capture's to_dict().
  • The key survives the round trip. It sits inside the field's value dict, so Schema.to_dict/from_dict and deepcopy carry it untouched. A value without it still packs unnamed bits as zero.
  • Inputs: True and bytes are rejected. A named subfield is never overridden. Out-of-width bits are masked off.
  • Regressions: the per-capture to_dict() hashes are identical across all 23 captures. tests/corekit 477 passed; capture round trip 4 passed with 20501 subtests; tests/project 419 passed.
  • Merges cleanly with fix(corekit): an untyped ListField packs the bytes it unpacks (#1489) #1494 (git merge-tree).

Non-blocking:

  • Negative ints are masked in two's complement rather than rejected.
  • A user BitField with gaps now exposes __unnamed__, which is a small visible change.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.65% (unit tier, Python 3.14, 910c898a0, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 958 2356 871 90.99%
pcapkit/corekit 2125 66 686 33 96.19%
pcapkit/dumpkit 242 2 88 2 98.79%
pcapkit/foundation 2648 98 930 48 94.80%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 17086 238 4628 190 97.98%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 431 4 124 4 98.56%
pcapkit/vendor 4409 2342 1006 157 43.25%

Per-file detail: the coverage-html artifact of this run.

BitField.post_process read only the named subfields and pre_process
packed from all zeros, so a bit outside every subfield was cleared on
rebuild. post_process now records those bits under an `__unnamed__`
key, present only when one is set, and pre_process seeds the buffer
from it, masked so it can never override a named subfield. A value
built without the key packs them as zeros, as before.

Deletes the #1487 Gap row from the field round-trip table.

Closes #1487
@JarryShaw
JarryShaw force-pushed the fix/1487-bitfield-unnamed-bits branch from be50fbd to 910c898 Compare October 9, 2026 20:14
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict carried to 910c898a0: GOOD TO GO. This is a pure rebase onto main at 00aa1fc90, after #1493 and #1494 merged. range-diff shows = and the stable patch-id is unchanged (ce5602829185).

@JarryShaw
JarryShaw merged commit 10a30ff into main Oct 9, 2026
36 checks passed
@JarryShaw
JarryShaw deleted the fix/1487-bitfield-unnamed-bits branch October 9, 2026 20:36
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(corekit): BitField drops the bits its namespace does not name

1 participant