Skip to content

refactor(corekit): merge the two DeferredPacket classes into corekit.packet (#1516) - #1532

Merged
JarryShaw merged 1 commit into
mainfrom
refactor/1516-deferred-packet
Oct 10, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
refactor/1516-deferred-packet

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

This merges the two DeferredPacket mixins into pcapkit.corekit.packet.DeferredPacket, as ruled on #1516.

  • The two classes had AST-identical method bodies. They differed only in which module's Deferred their isinstance check named. Each subclass now names it in __deferred__, so corekit imports nothing from foundation or protocols.
  • __init_subclass__ refuses a subclass that is missing 'packet' in __additional__, or is missing __deferred__. Either omission used to fail silently on the first read.
  • ip.Datagram, tcp.Datagram and the traceflow Index now take the mixin from corekit. Both Deferred classes stay where they are.
  • DeferredPacket can still be imported from its three old public paths, pcapkit.foundation.reassembly.data and both data.data modules. Each re-exports the one class and keeps it in __all__, and a test pins all three as the same object. pcapkit.corekit does not export it, because corekit/__init__.py belongs to refactor(corekit)!: Info.to_dict() returns an OrderedMultiDict; add MultiInfo and OrderedMultiInfo (#1484) #1523.
  • New page: docs/source/pcapkit/corekit/packet.rst. The reassembly and traceflow pages link to it instead of documenting a copy.

Output is byte-identical. I compared canonical to_dict() JSON and every trace file written, for all 23 examples/captures in four passes: pcap with trace_analyse, json, and pcap with the dpkt and scapy engines. That is 588 files, and diff -r against 16fbea054 is clean. Laziness is unchanged: .packet resolves once, and iteration, in and len do not resolve it.

Tests run, all green: the new test, tests/foundation/{reassembly,traceflow,engines}, tests/toolkit/test_engine_pcap_trace_runtime.py, tests/corekit, tests/project, tests/test_docstring_contract.py and tests/test_tier_guard.py. On main all 12 new tests fail. Each of 5 mutations of packet.py makes at least one test fail, and so does dropping a re-export. Rebased over #1511, keeping its FrameRecord.

Refs #1516

@JarryShaw JarryShaw added refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 9, 2026
@JarryShaw
JarryShaw force-pushed the refactor/1516-deferred-packet branch from 3159f4e to 19a217f Compare October 9, 2026 23:03
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

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

  • AST: all 7 methods are identical to both old classes once Deferred is rewritten as self.__deferred__. __init_subclass__ is the only new method.
  • Subclass hook: the library has exactly 3 DeferredPacket subclasses, each naming its own Deferred. ReassemblyData and TraceFlowData don't inherit the mixin. All 486 modules import cleanly.
  • Laziness: on 11 captures, 27 object ops and 11 container ops, analyze/submit counts match main; the only difference is dir() now listing __deferred__. http.pcap gives T=1 R=2, as claimed. Pickles cross-load between the two trees.
  • Byte identity: 413 to_dict(), json and tree outputs across 10 captures are identical.
  • Back-compat: the 3 old import paths resolve to the same class and keep it in __all__. A full Sphinx build produces the same 41 warnings as main.
  • Tests: fail-before is 12. 17 of 19 mutants are caught.
  • The fix(engines): write real PCAP traces for dpkt and scapy (#1507) #1511 conflict is one hunk in traceflow/data/data.py: drop the old class and keep FrameRecord. On that merge, both PRs' tests pass.

Non-blocking test gap: two mutants survive, __contains__ not delegating and type() is replacing isinstance. One assertIn/assertNotIn pair plus a Deferred-subclass case would catch both.

@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: 90.02% (unit tier, Python 3.14, 9fb7ad06b, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 842 2356 756 92.09%
pcapkit/corekit 2240 67 722 34 96.32%
pcapkit/dumpkit 258 2 90 2 98.85%
pcapkit/foundation 2751 127 974 47 94.26%
pcapkit/interface 108 7 36 5 91.67%
pcapkit/protocols 16992 229 4546 190 98.03%
pcapkit/toolkit 635 98 198 5 83.31%
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

Copy link
Copy Markdown
Owner Author

Resolve conflicts

@JarryShaw

Copy link
Copy Markdown
Owner Author

On it. The worker is rebasing onto main (c3d412015) and resolving the traceflow/data/data.py conflict with #1511. It is keeping #1511's FrameRecord.time_epoch and checking for the semantic clash that hit #1511/#1529 earlier. The rebased head gets a delta review.

…packet (#1516)

- pcapkit.corekit.packet.DeferredPacket replaces the copies in
  reassembly/data/data.py and traceflow/data/data.py. Their method bodies
  were AST-identical; the one difference, which module's Deferred the
  isinstance named, is now the __deferred__ class attribute each subclass
  sets, so the module imports nothing from foundation or protocols.
- __init_subclass__ refuses a subclass that does not list 'packet' in
  __additional__ or name a class in __deferred__, either of which would
  otherwise fail silently on the first read.
- ip.Datagram, tcp.Datagram and traceflow Index import it from corekit and
  set __deferred__ to their own Deferred, which stays where it was.
- DeferredPacket stays importable from its three old public paths, now
  re-exported from corekit.packet. New docs page corekit/packet.rst; the two
  foundation pages point to it instead of documenting a copy.
- tests/corekit/test_deferred_packet_1516_unit.py pins laziness, the hook,
  the guard, the import layering, the three models' wiring and the old paths.

Refs #1516
@JarryShaw
JarryShaw force-pushed the refactor/1516-deferred-packet branch from 19a217f to 9fb7ad0 Compare October 10, 2026 02:09
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 10, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 9fb7ad06b. This was a Sonnet delta review; the author ran Opus. The rebase over #1511 is clean.

@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 10, 2026
@JarryShaw
JarryShaw merged commit de72d01 into main Oct 10, 2026
40 checks passed
@JarryShaw
JarryShaw deleted the refactor/1516-deferred-packet branch October 10, 2026 03:34
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant