fix(reg,corekit): retype the registry NULL sentinel and raise ProtocolError from ModuleDescriptor.klass

[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change — did not run the full suite (it OOMs on this host); ran the scoped tests/foundation/registry/, tests/corekit/test_module.py and tests/const/test_const_apptype_split_unit.py instead, and new test cases cover the change
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A, changelog 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 — 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
• chore — anything else

---

Description of your pull request and other information

Closes #832
Closes #833

NULL was a plain str compared by identity, defined independently in protocols.py and foundation.py; an equal-but-distinct '(null)' could take a different branch than the sentinel itself (#833), and let an omitted class_ reach ModuleDescriptor.klass's bare getattr, surfacing as a stdlib AttributeError naming '(null)' — the same bare error any bad class name leaks across all nine register_* sites (#832).

Adds one NullType/NULL in pcapkit.corekit.module, shared by both registry modules. ModuleDescriptor.klass now raises ProtocolError for a class name that resolves to nothing, and, ahead of getattr, for a class_ that was never given. Updates #815's AttributeError assertion to ProtocolError, keeping its class-name-not-transport check.

[JarryShaw]

NEEDS CHANGES — cross-review (opus) plus my own re-derivation. The substance is sound; two fixes in pcapkit/corekit/module.py.

1. NullType is not a singleton, and deepcopy identity is a regression this PR introduces. Measured on 375b50c85 vs 3118ed796, each in its own worktree:

PR:    NullType() is NULL -> False   deepcopy(NULL) is NULL -> False   copy(NULL) is NULL -> False
stock: deepcopy('(null)') is '(null)' -> True   copy -> True

The consequence, with a module that really imports (pcapkit.protocols.transport.tcp):

PR    original .klass -> ProtocolError: missing class name for module '…tcp'
PR    deepcopy .klass -> TypeError: attribute name must be string, not 'NullType'
PR    copy     .klass -> ProtocolError  (unaffected — a shallow copy keeps the same NULL reference)
stock original / deepcopy / copy -> all AttributeError "…has no attribute '(null)'"

On stock, deepcopy was no worse than the original. On this head it degrades to a bare stdlib TypeError — the exact failure class #832 removes. Only deepcopy is affected, not copy, so a __deepcopy__/__copy__ pair plus a __new__ guard and __reduce__ is the fix; pcapkit/corekit/fields/numbers.py:686-830 is in-repo precedent. @final does not help — it only sets __final__, and NullType subclasses fine at runtime. Nothing in pcapkit/ deepcopies a descriptor, so this is not an in-tree break, but ModuleDescriptor is public and the registries hold them.

2. module.__all__ is still ['ModuleDescriptor'] — confirmed by import on both trees — while NullType now appears in the public annotation of 15 exported register_* signatures and is cross-referenced as :class:~pcapkit.corekit.module.NullType`` from protocols.py. Add `NULL` and `NullType`.

Also worth a line in the body, not a blocker: ProtocolError is a loud BaseError, so a klass failure now logs CRITICAL and sets sys.tracebacklimit = 0 process-globally (pcapkit/utilities/exceptions.py:191-201) where a bare AttributeError did neither.

Confirmed unchanged: no import cycle (6 fresh interpreters, each importing a different module first, NULL identical through all three paths); #815's coupling test still discriminates, since RegistryError is BaseError, TypeError and disjoint from ProtocolError; zero hasattr(_, 'klass'), zero getattr(_, 'klass', default), zero try-wrapped .klass across ~37 accesses. One correction to the PR's own claim: test_register_apptype_null_sentinel_still_means_absent_with_a_class_module passes on stock — it is a regression lock, not a failing-first test.

[JarryShaw]

Fix pushed as 67b45ad2f (amended, one commit on 3118ed796). I verified it independently rather than taking the report:

__all__                 ['NULL', 'NullType', 'ModuleDescriptor']
NullType() is NULL      True      copy / deepcopy identity   True
pickle p=0 / 1 / 2 / 5  True      (p=1 works too — only 0/2/5 were claimed)
descriptor original / copy / deepcopy .klass  ->  same clean ProtocolError
ModuleDescriptor('…tcp', 'TCP') deepcopy .klass -> <class '…tcp.TCP'>   (well-formed path intact)

Fail-before proof holds. On the prior head 375b50c85 with production files untouched, its three tests fail for the right reasons:

ERROR  …test_klass_raises_protocolerror_not_typeerror_after_deepcopying_the_descriptor
       TypeError: attribute name must be string, not 'NullType'
FAIL   …test_null_sentinel_identity_survives_copy_and_pickle   AssertionError: <NULL> is not <NULL>
FAIL   …test_null_sentinel_is_not_a_string                     AssertionError: <NULL> is not <NULL>
Ran 10 tests — FAILED (failures=2, errors=1)

Only pcapkit/corekit/module.py and tests/corekit/test_module.py changed from the prior head; the other three files are byte-identical. __reduce__ returns a module-level getter, which is what covers pickle protocols 0/1 — those bypass __new__ via copyreg._reconstructor, so the instance guard alone would not have been enough. One caveat that remains and is fine: NullType is still subclassable at runtime, since @final is advisory.

Back to review: pending for the new head; a delta cross-review is running.

[JarryShaw]

NEEDS CHANGES on 67b45ad2f — docs only. The delta re-review (opus) found both earlier findings properly fixed and the new machinery sound; one hand-written doc page is now wrong because of this diff.

docs/source/pcapkit/corekit/module.rst is an autoclass … :no-members: page, not an automodule, so nothing regenerates it. Verified:

module.rst:21-24   .. property:: name  /  :type: str     <-- this PR changes it to 'str | NullType'
this PR adds 8 cross-references to pcapkit.corekit.module.NullType
                   — no `.. autoclass:: NullType` and no `.. data:: NULL` target exists

Both names are now in __all__ and in the public signature of 15 register_* functions, so the refs should resolve. Severity is limited and I resolved the reviewer's open question on it: nitpicky is unset in docs/source/conf.py, docs/Makefile has SPHINXOPTS ?= empty, and deploy-pages.yml runs make -C docs html with no SPHINXOPTS/O — no -W, so the build will not fail. The refs render as plain text and :type: str renders as a confident falsehood.

Everything else confirmed on this head: NullType(1) and NullType(x=1) still raise TypeError, so the permissive __new__ is permissive about multiplicity only; a subclass cannot mint an impostor (it inherits the non-None _instance, so Sub() is NULL), which is the safe direction; __reduce__'s getter round-trips cross-process at all six protocols including the pure-Python pickler; __deepcopy__ correctly takes memo and not memoising a self-copy matches CPython's own if y is not x guard; containers and self-referential structures holding NULL deepcopy fine. pylint 10.00/10, pycodestyle clean, mypy 114 with zero errors in module.py.

Two non-blocking notes: the per-protocol assertions sit inside subTest, which this repo's pytest-subtests can undercount; and importlib.reload(pcapkit.corekit.module) desynchronises the sentinel from its importers — structural to sharing it, unreachable in-tree, and the str sentinel had the same fragility.

[JarryShaw]

Reviewer's final verdict on 67b45ad2f is GOOD TO GO on the code, and it resolved its own open items: tests/project/test_public_api.py 10/10 OK with from pcapkit.corekit.module import * binding all three names, and cross-process unpickling confirmed at protocols 0–5 including the pure-Python pickler.

It reclassified the module.rst staleness as a follow-up issue rather than a change here. I am keeping it in this PR. The page says .. property:: name :type: str while this diff makes the field str | NullType at module.py:163 — a falsehood this PR introduces, and since nothing regenerates that hand-written page and there is no -W, nothing will ever surface it. Shipping it and filing an issue against ourselves is the worse trade for a change this small. So review: needs-changes stands until the doc page lands; the code itself is cleared.

One correction to my own earlier note, for the record: the reviewer's deepcopy finding was measured correctly. The probe it quoted in its first report was a discarded one that died on ModuleNotFoundError; the finding came from a second probe that registered types.ModuleType('demo.real') into sys.modules first and did reproduce the TypeError. My objection applied to the probe as written, not to the finding, which was sound — and which I had independently confirmed with a real module anyway.

Two notes carried forward as non-blocking: the per-protocol assertions sit inside self.subTest, which this repo's pytest-subtests can report green when only subTests fail; and importlib.reload(pcapkit.corekit.module) desynchronises the sentinel from modules that already imported it — structural to sharing it, unreachable in-tree, and the old str sentinel had the same fragility.

[JarryShaw]

GOOD TO GO — amended to 37c21438a. The code verdict from the opus delta review carries, because this round changed no executable code:

git diff 67b45ad2f 37c21438a -- pcapkit/corekit/module.py  ->  +15 lines, ALL docstring prose
whole round: docs/…/module.rst (+11/-2), pcapkit/corekit/module.py (+15), tests/corekit/test_module.py (+20/-1)

The doc page is fixed: :type: is now str | pcapkit.corekit.module.NullType, matching module.py:163, and an "Auxiliaries" section adds .. autoclass:: NullType and .. autodata:: NULL, following the NoValueType/NoValue precedent at docs/source/pcapkit/corekit/fields/field.rst:25-27. It deliberately omits :no-value: there, unlike NoValue, because repr(NULL) == '<NULL>' is a tested part of the contract rather than a memory address worth hiding — I agree with that call. Worker's docs build: warnings 58 → 56 with both autodoc: failed to import lines gone and nothing new; it hit the editable-install shadowing first and fixed it with PYTHONPATH, which is the known trap for building docs from a worktree.

Both non-blocking notes were taken rather than deferred. test_null_sentinel_identity_survives_copy_and_pickle now collects bad_protocols and asserts once outside any subTest, so a regression confined to one protocol fails the parent node the ordinary way. And NullType's docstring now records the importlib.reload caveat, including why it is structural rather than a hole in the singleton guarantee.

Fail-before re-proven by me on the pre-fix head 375b50c85 with production files untouched — Ran 10 tests … FAILED (failures=2, errors=1), the error still TypeError: attribute name must be string, not 'NullType'. One honest caveat: the reworked out-of-loop pickle assertion is not itself demonstrated failing there, because the copy.copy assertion fires first; the test as a whole fails correctly. On origin/main all six fail, since NULL does not exist in that module yet.

The test rework was verified by me, not by the cross-reviewer, which saw only the previous head.

[JarryShaw]

Merged-together integration check — clean. CI only ever proved each PR alone: #834, #835 and #836 were all cut from 3118ed796, so whichever of them merges second and third meets a main it was never tested against. That matters for this pair specifically, because #836 established that pcapkit/foundation/registry/protocols.py needed no change against main — not against #835's rewritten version of it.

Merged all three into a scratch worktree at origin/main: no conflict, 15 files, +1306 −295. Targeted suites on the combined tree, pcapkit.__file__ asserted inside it:

tests/foundation/registry                      19 tests   OK
tests/const                                    77 tests   OK
tests/corekit                                 240 tests   OK (skipped=16)
tests/dumpkit/test_common_unit                 14 tests   OK
tests/dumpkit/test_nameless_enum_rendering      6 tests   OK
tests/dumpkit/test_plist_escaping_regression    4 tests   FAILED (errors=6)  <-- pre-existing

That last one is not this change. Verified with a stock control — a clean origin/main worktree gives the same 6 errors and the same cause: FileNotFoundError: sample capture 'test.pcapng' not found … most of the sample captures are generated, not committed. A fresh worktree simply has no fixtures until python examples/generators/make_samples.py runs.

So 356 tests pass on the combined tree with zero failures attributable to the merge, and the three may be merged in any order.