[JarryShaw]

• Searched for similar pull requests
• Followed the coding style
• make test passes, and a test case covers the change
• Changelog entry: N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

• fix — bug fix

---

Description

Closes #2268.

• Fix: register_protocol_code now flattens every target and checks each key against its destination's enum before registering any — a wrong code can no longer replace a shipped entry, and a list that fails partway no longer leaves its earlier entries registered.
• Fail-without-fix: 6 failed on origin/main. Passing on the rebased head.

[JarryShaw]

Review: NEEDS CHANGES at 928fecc9f (Sonnet) — test-only gap, the code is correct.

• Confirmed: checking happens before any registration (a mixed-bad mapping/list changes nothing); the destination table matches the 11 typed registrars and all 43 shipped keys; Loopback/IPX are correctly left unchecked (they have no typed registrar to mirror); the int and list forms both work. Fail-without-fix: 6 failed on origin/main. On the head, 107+262+150 passed.
• Required: add a table-driven test pinning all 7 _DESTINATION_KINDS rows — mutating 8 of 28 table cells (dropping a row, or flipping number on SCTP/Link/Frame/PCAPNG) currently survives the test selection.
• Non-blocking: Loopback/IPX stay genuinely unchecked by design, letting a numerically-equal wrong enum through with only a warning; a non-class destination mid-list raises a bare AttributeError rather than the house RegistryError; the bool gap is fix(foundation): _check_code(number=True) takes a bool, so register_tcp(True) adds port 1 #2269.

[JarryShaw]

Amended 0ff7478ba → b621dc51b: added test_every_destination_kind_row_is_enforced, which pins all 7 _DESTINATION_KINDS rows against the real registrars — a wrong-enum key, the number flag, and the row's own enum type, plus a set-equality check against the table. 14/14 single-cell mutants (dropping a row, flipping number) are killed. test_protocol_code_registration_unit.py + test_docstring_contract.py + test_tier_guard.py give 169 passed (unittest), and test_dispatch_registry_unit.py run alongside confirms nothing leaked into its exact-count assertions.

[JarryShaw]

Review: GOOD TO GO at b621dc51b (Sonnet, round 2) — test-only, the round-1 gap is closed.

• Confirmed: all 14 single-cell mutants (7 row drops, 7 number flips) are killed, and the harness confirms it can tell a kill from a survivor by reproducing round 1 on the unamended test. No mocking loophole — _check_code runs for real before the mocked register is reached. The diff is test-only; the pcapkit/ tree hash is identical across all three heads.
• Fail-without-fix: 6 distinct tests fail on origin/main (one ImportError plus the five original rejection cases). On the head, 26+8+135 passed.
• Non-blocking: the new test's docstring overstates "by behaviour rather than a copy of the table" — the assertIs/set(...) checks are exactly that, and carry most of the kind-swap coverage; no positive case sends a valid AppType member through the mapping form.

[JarryShaw]

for _CODE_DESTINATIONS and _DESTINATION_KINDS, can we use Type[EnumRegistry] and Type]ProtocolBase] types to replace the type annotations? to be more specific.

[github-actions]

Coverage: 91.58% (unit tier, Python 3.15, 3010e54d1, 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	3036	23	1054	25	98.83%
pcapkit/interface	138	4	52	3	96.32%
pcapkit/protocols	17455	218	4598	185	98.15%
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]

Done for _CODE_DESTINATIONS (enum keys are now Type[EnumRegistry], confirmed every key is an EnumRegistry subclass). _DESTINATION_KINDS's values are also tightened the same way, but its keys stay as a bare type: Link and Internet are genuinely abstract in this codebase (they don't implement name/length/make/__index__/read — only their concrete subclasses like Ethernet and IPv4 do), and mypy's type-abstract check specifically refuses an abstract class where a dict key is typed Type[ProtocolBase] (it doesn't mind one in a tuple value, which is why _CODE_DESTINATIONS was fine). Narrowing the key type there would need a # type: ignore[type-abstract] on Link:/Internet:, which seemed worse than leaving the key bare. Say if you'd rather have the ignore instead.

Pushed as 79b88eee1. mypy on the file is back to its baseline 3 pre-existing errors (unrelated TransportProtocol | str lines), and the registry tests give 82 passed.

[JarryShaw]

i'd still prefer using Type[ProtocolBase] and ignore mypy error if necessary

[JarryShaw]

Ruling noted: Type[ProtocolBase] with a type: ignore on the two abstract keys (Link, Internet). Dispatching the change.

[JarryShaw]

Pushed 79b88eee1 → 3010e54d1: _DESTINATION_KINDS's key is now Type[ProtocolBase], with # type: ignore[type-abstract] on exactly the two abstract keys (Link, Internet). mypy --warn-unused-ignores confirms both are load-bearing. Single-file mypy is back to the 3 pre-existing errors; the whole-package run is error-set-identical to origin/main (100 errors in both). Registry tests: 82 passed.

[JarryShaw]

Review: GOOD TO GO at 3010e54d1 (Sonnet, round 3) — also covers the owner-preferred Type[ProtocolBase]+ignore version pushed mid-review.

• Confirmed: Link/Internet are the only two abstract classes among the destinations; the two # type: ignore[type-abstract] are both load-bearing (dropping either reintroduces exactly one error; a spurious ignore on a concrete row is flagged unused-ignore). Whole-package mypy is error-set-identical to origin/main (100 errors, both). The diff from the already-reviewed head is annotation/import/cast only — bytecode comparison confirms zero behavior change. Fail-without-fix: 7 failed (6 distinct tests) on origin/main. 265+262 passed on the head.
• No cleaner alternative found: a TypeVar bound fails at module level; cast/deriving the table are equally mypy-clean trade-offs, not clearly better.
• Non-blocking: a one-line comment above the two ignored rows explaining why would help, since the reason currently lives only in this thread.

[JarryShaw]

Review: GOOD TO GO at 3010e54d1 (Sonnet, round 4) — a fresh, independent pass, not a rubber-stamp.

• Confirmed: mypy clean to the 3-error baseline, both single-file and whole-package (error sets genuinely identical, mapped through the real diff hunks). Both ignores are exactly load-bearing — stripping either reintroduces one error, adding one elsewhere is flagged unused-ignore. The owner's two comments are both satisfied. 268 passed; a simulated merge onto current main (which now includes fix(foundation): _check_code(number=True) takes a bool, so register_tcp(True) adds port 1 #2269) stays clean at 235 passed.
• Fail-without-fix: 7 failed (6 distinct tests) on origin/main.
• Non-blocking: while a key is abstract, mypy skips checking that row's value tuple — inherent to the design, covered at runtime by the existing test; a one-line comment on the two ignored rows is optional, no house precedent requires one.

[JarryShaw]

Ready to merge.

Review: GOOD TO GO (Sonnet, round 4) at 3010e54d1 — fresh independent re-verification of the owner's final Type[ProtocolBase]/Type[EnumRegistry] preference with ignores.
CI: Required checks passed, 0 failures.
Mergeable, 0 unresolved threads.

[JarryShaw]

On hold for a design change: moving _DESTINATION_KINDS from a module-level mapping to a class attribute declared directly on each of Link/Internet/Frame/PCAPNG/TCP/UDP/SCTP, so the Enum registry and target protocol class _check_code validates against are explicitly specified where each class already declares __proto__, rather than kept in a separate table that has to stay in sync. Will post the revised diff shortly.

[JarryShaw]

This merged before the class-attribute redesign I started above could land on it — opened as a follow-up instead: #2392.