Skip to content

fix(corekit): finalise each non-final Info subclass on its own state (#1490) - #1496

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1490-info-child-finalisation
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1490-info-child-finalisation

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

Info.__new__ now reads __finalised__ from the class's own __dict__ (pcapkit/corekit/infoclass.py:424), so a non-final child is finalised on its first construction whatever its parent's state. Before, it inherited the parent's BASE, kept the __excluded__ copied at declaration, and leaked __map__, __map_reverse__, __multi__ from to_dict(), len() and iteration.

  • New tests/corekit/test_info_child_finalisation_1490_unit.py: parent-first and child-first, each in a fresh interpreter, over a synthetic pair and six real models (hopopt/ipv6_opts QuickStartOption, SMFDPDOption, ipv4.QSOption, tcp.MPTCPJoin). On main's infoclass.py all six parent-first cases fail; child-first passes on both.
  • test_info_roundtrip_unit.py: the fix(corekit): a non-final Info subclass built after its parent leaks internal keys from to_dict() #1490 Gap row is gone and the root-cause pin asserts ['a', 'b'].
  • test_final_enforcement.py: two behaviour changes follow. A @final class under a BASE ancestor is now refused, closing the gap that test documented. A child declared before its parent was finalised now gets its own __init__.

Ran: the new test, tests/corekit, tests/protocols/internet, tests/protocols/transport, test_capture_roundtrip_runtime.py, tests/project, test_docstring_contract.py, test_final_enforcement.py, all green. repr(frame.info.to_dict()) hashes over examples/captures (23 files) are identical to main.

Closes #1490

@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 9e86cf01f. Cross-reviewed on Sonnet; the author ran on Opus.

  • The fix works. I measured it myself: on origin/main sources the new tests give 7 failed, 1 passed. At head, the new file, test_final_enforcement.py and test_info_roundtrip_unit.py give 40 passed, 1458 subtests.
  • It is order-independent. Building all 489 Info subclasses parent-first leaks internal keys from 32 classes on main, and from none at head. Child-first gives 0 on both. The generated __init__ qualnames match between the two orders at head.
  • The @final refusal is a fix, not a contract break. The old test was added by design: #514 recorded two rulings it never implemented — the Base/public collapse and the enforcing @final #778 as a documented gap, not a guarantee; I checked its docstring at ccf5f623b. Importing all 485 modules and building every Info subclass raises 0 InfoError on either tree.
  • Regressions: the per-capture to_dict() hashes are identical across all 23 captures. tests/corekit plus enforcement: 501 passed. Internet plus transport: 692 passed. util/run_unittest_leg.py corekit: 660 tests, 0 failures.

Non-blocking: a user subclass with no __init__, under a parent with a hand-written one that has already been built, now gets a generated __init__ rather than inheriting the custom one. That matches what main already did in the other build order, and no shipped class is affected. It is worth a line in the body.

Schema has the same inherited-state check; that is filed separately.

@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
…1490)

Info.__new__ read __finalised__ through inheritance, so once a non-final
parent had been built its non-final children read the parent's BASE and
were never finalised. They kept the __excluded__ copied at declaration,
and to_dict(), len() and iteration leaked __map__, __map_reverse__ and
__multi__. Read the state from the class's own __dict__ instead.

- tests/corekit: new per-order tests, each order in a fresh interpreter,
  over a synthetic pair and six real models; drop the #1490 Gap row and
  flip the root-cause pin.
- tests/test_final_enforcement.py: a @Final class under a BASE ancestor
  is now refused, and a child predating its parent's finalisation gets
  its own __init__.
@JarryShaw
JarryShaw force-pushed the fix/1490-info-child-finalisation branch from 9e86cf0 to e7d02d1 Compare October 9, 2026 20:14
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict carried to e7d02d195: 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 (f18a09f7cc93).

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Coverage: 89.65% (unit tier, Python 3.14, e7d02d195, 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 2115 66 680 33 96.17%
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.

@JarryShaw
JarryShaw merged commit 934c343 into main Oct 9, 2026
36 checks passed
@JarryShaw
JarryShaw deleted the fix/1490-info-child-finalisation branch October 9, 2026 20:37
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
JarryShaw added a commit that referenced this pull request Oct 9, 2026
…#1498) (#1499)

Schema.__new__ read __finalised__ through inheritance, so once a non-final
parent had been built its non-final children read the parent's BASE and
were never finalised: they kept the __excluded__ copied at declaration and
to_dict() leaked __map__, __map_reverse__, __buffer__ and __updated__.
Read the class's own __dict__ instead, as #1496 does for Info. This also
closes the documented gap where a bare @Final under a BASE ancestor
escaped the guard, so its Warning becomes a Note.

The shipped instances are the Quick-Start option schemas of HOPOPT,
IPv6-Opts and IPv4.
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): a non-final Info subclass built after its parent leaks internal keys from to_dict()

1 participant