Skip to content

refactor(protocols): make annotations and __all__ agree with what the code stores (#2317) - #2377

Merged
JarryShaw merged 1 commit into
mainfrom
refactor/2317-annotation-all-mismatch
Oct 27, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
refactor/2317-annotation-all-mismatch

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • refactor — code refactoring

Description

Closes #2317.

  • Problem: nine declarations described a value the code never produces, so mypy and the rendered docs were wrong about it — CALIPSOOption.cmpt_bitmap typed bytes where _read_opt_calipso stores a tuple of ints, SMFHashBasedDPDOption.post_process declaring its sibling's schema, Optional on an RR/SSR route that is always built, protochain declaring a ProtoChain that NoPayload does not have, the pack_flags constructor stub typing the EPB flags, ARP._make_addr_resolve omitting the bytearray its body handles, an undeclared entry_raw, two HIP parameters missing from both __all__ lists, and a version one _read_param_* took positionally.
  • Fix: each declaration now matches the value. NoPayload overrides protochain as -> 'None', the way it narrows name; entry_raw is declared under TYPE_CHECKING, as IPv4 declares trailer; OverlayTTLParameter and ViaRVSParameter join the HIP __all__ lists and the package re-exports those lists require. Parsing and rebuilding are unchanged.
  • Test: tests/protocols/test_declarations_match_values_2317_unit.py pins each declaration against the value it describes — a parse for what is stored, and the source for a declaration that lives only under TYPE_CHECKING. All nine cases fail on the parent commit.

… code stores (#2317)

Problem: nine declarations described a value the code never produces, so mypy
and the rendered docs were wrong about it -- `CALIPSOOption.cmpt_bitmap` typed
`bytes` where `_read_opt_calipso` stores a tuple of ints,
`SMFHashBasedDPDOption.post_process` declaring its sibling's schema, `Optional`
on an `RR`/`SSR` `route` that is always built, `protochain` declaring a
`ProtoChain` that `NoPayload` does not have, the `pack_flags` constructor stub
typing the EPB flags, `ARP._make_addr_resolve` omitting the `bytearray` its body
handles, an undeclared `entry_raw`, two HIP parameters missing from both
`__all__` lists, and a `version` one `_read_param_*` took positionally.

Fix: each declaration now matches the value. `NoPayload` overrides `protochain`
as `-> 'None'`, the way it narrows `name`; `entry_raw` is declared under
`TYPE_CHECKING`, as IPv4 declares `trailer`; `OverlayTTLParameter` and
`ViaRVSParameter` join the HIP `__all__` lists and the package re-exports those
lists require. Parsing and rebuilding are unchanged.

Test: tests/protocols/test_declarations_match_values_2317_unit.py pins each
declaration against the value it describes -- a parse for what is stored, and
the source for a declaration that lives only under `TYPE_CHECKING`. All nine
cases fail on the parent commit.
@JarryShaw JarryShaw added the refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix) label Oct 27, 2026
@github-actions github-actions Bot added this to the PyPCAPKit v1.5 milestone Oct 27, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.58% (unit tier, Python 3.15, 6a4584ad1, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19601 881 2620 867 92.12%
pcapkit/corekit 2596 74 818 37 96.51%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 3029 23 1050 25 98.82%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17462 218 4598 185 98.16%
pcapkit/toolkit 914 36 334 10 95.51%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4432 1988 1018 167 51.87%

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

@JarryShaw

Copy link
Copy Markdown
Owner Author

Review: GOOD TO GO at 6a4584ad1 (Sonnet). All 9 declarations independently verified against real parsed/constructed values (including a standalone mypy consumer snippet showing 7 errors on main, 0 on head). AST-diff confirms the fix is almost entirely non-executable — the two exceptions (NoPayload.protochain becoming a real property, _read_param_relay_hmac's keyword-only params) were checked and found behaviorally inert (29-case differential, all identical). All 9 test cases fail on main, pass on head. Full regression clean (2725 vs 2712 passed, delta = exactly the new file; same 61 fixture-dependent failures on both trees). HIP re-export pairs confirmed complete via the existing #1807 test.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge.

Review: GOOD TO GO (Sonnet) at 6a4584ad1.
CI: Required checks passed, 0 failures (only the non-blocking experimental GraalPy leg still pending).
Mergeable, 0 unresolved threads.

@JarryShaw JarryShaw added the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 27, 2026
@JarryShaw
JarryShaw merged commit ecace57 into main Oct 27, 2026
65 checks passed
@JarryShaw
JarryShaw deleted the refactor/2317-annotation-all-mismatch branch October 27, 2026 15:12
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 27, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 27, 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.

refactor(protocols): annotations and __all__ disagree with the values the code stores

1 participant