fix(registry): report the silent schema overwrites, and name what the code-keyed registrars displaced

[JarryShaw]

Fixes #692. Generalises #681, per the ask:

i think we should apply what #681 added to other registry as well.

Re-verified inventory

Measured with an AST walk over pcapkit/, not a grep — a grep range terminated early on the first attempt and gave a false reading. 12 register() implementations. Classified by what each does on a key collision, which is a more useful split than warn/silent:

Behaviour	Count	Sites
Warns, on presence alone	7	protocols/protocol.py:758, internet/internet.py:136, link/link.py:116, misc/pcap/frame.py:123, misc/pcapng.py:857, transport/transport.py:72, transport/sctp.py:593
Silently overwrites	2	schema/schema.py:1115, schema/misc/pcapng.py:588
Raises RegistryError	1	corekit/context.py:125
Unkeyed sequence, cannot collide	2	foundation/reassembly/reassembly.py:397, internet/esp.py:916

Zero used #681's refined guard. That reconciles with the five-silent framing this work started from, but three of those five turn out not to need a guard, each for its own sufficient reason:

• ContextRegistry.register already raises — raise RegistryError(f'context already registered for protocol: {index}'). Stricter than a warning; relaxing it would be a regression.
• Reassembly.register appends a callback to __callback_fn__, a list. No key, no overwrite.
• ESPContext.register appends to __associations__, also a list — and duplicate SPIs are a designed feature: the class documents its associations as supplied "in order of preference for otherwise equal matches", and ESPContext.match scores them. A guard would flag a supported path.

schema/misc/pcapng.py:588 is the fifth. It is a staticmethod override with a different signature that never delegates to EnumSchema.register, so this PR cannot reach it — it is #678's, handled separately.

What changed

EnumSchema.register (schema/schema.py) — was a bare cls.__enum__[code] = schema. It is the schema half of 14 public registrars, and the defect is an asymmetry within a single call: each register_* helper registers a parser class and a schema class, the parser half has warned on an overwrite for as long as it has existed (internet/ipv4.py:482, hopopt.py:367, ipv6_opts.py:378, hip.py:651, mh.py:1483/1496/1509, ipv6_route.py:355, application/httpv2.py:367, transport/sctp.py:618/632/645/658), and the schema half assigned bare. So one register_ipv4_option replacing a built-in named the parser it displaced and said nothing about the schema.

EnumSchema.__init_subclass__ (same file) — assigns cls.__enum__[code] directly, so class MyOption(Option, code=...), the documented way to add a schema, bypassed register entirely. Guarding only the method would have left that path silent, recreating the very asymmetry being fixed. Its two branches are folded into one loop so the guard is written once. It deliberately does not delegate to cls.register: schema/misc/pcapng.py's Option overrides register with an incompatible signature, so cls.register does not mean the same thing for every subclass.

Presence is a faithful test here only because of #555 — _EnumRegistry.__missing__ returns a miss without recording it. On a plain defaultdict, parsing one packet with an unknown code would have made the next legitimate registration for that code warn about an entry nobody asked for. _EnumRegistry's own docstring already anticipated this guard in those words. A test pins the composition of the two fixes.

The seven code-keyed registrars — message only. They now name the displaced entry and its replacement; they said 'protocol {code} already registered, overwriting' and stopped, so a caller learned something was displaced and never which class.

The refinement does not transfer — decided per registrar, and all seven stay presence-only

Registrar	Key	Decision
protocol.py:758 ProtocolBase.register	explicit int	presence-only
link.py:116	explicit EtherType	presence-only
internet.py:136	explicit TransType	presence-only
frame.py:123	explicit LinkType	presence-only
misc/pcapng.py:857	explicit LinkType	presence-only
transport.py:72	explicit port	presence-only
sctp.py:593	explicit PPID	presence-only

Three independent reasons, any one sufficient:

1. fix(registry): report the protocol-name collision register_protocol hid (#675) #681's licence does not apply. Its guard is narrow because its key is derived from the value (cls.__name__.upper()) and it is the single funnel nine registrars end in, so one class under two codes reaches it twice with nothing displaced. All seven take a caller-supplied code independent of the value, so one class under two codes yields two distinct keys — the spurious-warning case cannot arise. A repeat for one code is a caller mistake worth reporting even when the value is unchanged.
2. It is not even decidable. __proto__ is pre-seeded with unresolved ModuleDescriptor values (link.py:76, internet.py:91, frame.py:90, misc/pcapng.py:557, sctp.py:355, tcp.py:314, udp.py:88), so an incumbent may be a two-string descriptor while the replacement is the very class it names. "A different class" would need incumbent.klass — an eager import on the register path, defeating the laziness the descriptor exists for, purely to decide whether to warn.
3. fix(registry): report the protocol-name collision register_protocol hid (#675) #681 already made this an executable decision. tests/foundation/registry/test_protocols.py:230 asserts the siblings still warn on an identical re-registration and says it "fails if anyone ever 'harmonises' the siblings onto the guard used above". Harmonising would break it by construction.

So the transferable part of #681 is the message, not the condition. The firing condition of all seven is byte-for-byte unchanged.

Evidence

Exit codes read from files, never from a pipeline — and note one run reported 9 passed on its header line while 8 subtests SUBFAILED, so the tally was read too.

pcapkit.__file__ on every measurement: /local/home/jarryx/GitHub/PyPCAPKit/.claude/worktrees/agent-a37137696e200684c/pcapkit/__init__.py, asserted before any other import, with the worktree at sys.path[0] and PYTHONSAFEPATH=1. This matters more than usual here because registry state is process-global, so a shadowed tree makes every observation wrong. One intermediate measurement did resolve to the shadowing install and was discarded.

import pcapkit emits zero new warnings. Before and after are byte-identical: 1 warning total, a third-party DeprecationWarning (dictdumper/vuejs.py:89, "VueJS is deprecated"), and 0 RegistryWarning.

That was verified twice, because the first attempt was wrong in a way worth recording: a script that imported pcapkit.protocols.schema.schema in order to patch _EnumRegistry had already run pcapkit/__init__.py, so every write happened before the patch. It reported 0 writes, 0 warnings — and the known 1-warning baseline is what exposed it. Re-done with a meta-path finder that instruments the module in the window after its body executes: 326 _EnumRegistry writes during import, 0 onto an already-present key. That is also what establishes the __init_subclass__ guard is safe, since a first-time declaration is the case that governs whether import is noisy.

Failing-then-passing, per guard. Each was reverted independently:

Reverted	Result	Exit
EnumSchema.register guard	2 failed, 8 passed — exactly the two tests asserting it	1
__init_subclass__ guard	1 failed, 9 passed — exactly the declaration test	1
the seven messages	8 subtests SUBFAILED, one per registrar	1
nothing (restored)	10 passed	0

Suites. tests/foundation/registry/ 13 → 14 tests, 87 → 95 subtests, exit 0. tests/protocols/schema/ + foundation/registry/ 49 passed / 100 subtests, exit 0. The broader affected areas — protocols/link/, protocols/misc/, protocols/transport/, protocols/schema/, foundation/registry/ — 307 passed / 485 subtests, exit 0; and the top-level dispatch, registry and round-trip tests 72 passed / 456 subtests, exit 0.

An earlier run of those areas showed 25 failures, all FileNotFoundError: sample capture '<name>' not found. Environmental: the captures are generated by examples/generators/make_samples.py, and CI's own make test excludes *_runtime.py for exactly this reason. Generated them, re-ran, all pass.

EXPECTED_FAILURES imported rather than grepped (it is built with ** unpacking): 44 entries before and after. No entry moved.

Static analysis, both at parity. mypy 112 errors before and after, over the whole package (the two new [index] errors the first draft introduced on its new subscript reads are gone — the reads were hoisted onto their own lines with the # type: ignore[index] this file already uses on its assignments).

pylint 364 messages before and after, exit 30 both — scoped to the eight touched modules, with the repo's own PYLINT_FLAGS, against a pristine git archive of HEAD. Stating the scope because the number is meaningless without it: a full make pylint over the package is in the 5,900s, and is not a usable before/after metric, because R0401 (cyclic-import) varies run to run on an unchanged tree — measured at 106 and then 119 on two consecutive runs of the same commit. The eight-module figure is deterministic; it reproduced at 364 on four separate runs.

One honest pylint note: within that scope the composition shifted by one in each direction — C0325 down one (folding __init_subclass__'s duplicated (cls) assignment removed a superfluous-parens) and R0801 up one, a new duplicate-code cluster spanning Internet.register and ProtocolBase.register. Those two methods were already near-identical; giving them the same guard shape tipped the similar region past the threshold. Varying the seven docstrings to name each registry's own key kind was tried and did not remove it, which confirms the similarity is the method bodies, not the prose. Deduplicating them is #514's territory, not this PR's.

Coverage does not go backwards. Each tree measured with its own tests, via coverage run -m pytest:

module	before	after
schema/schema.py	99% (329 stmts, 1 miss)	99% (334 stmts, 1 miss)
misc/pcapng.py	26% (1014 miss)	27% (1009 miss)
transport/sctp.py	34% (361 miss)	35% (356 miss)
protocol.py	95% (19 miss)	95% (19 miss)
internet.py / link.py / frame.py / transport.py	82 / 79 / 61 / 69%	unchanged
total	49% (1463 miss)	49% (1453 miss)

schema.py gains 5 statements — the guards — with misses flat at 1, so every new statement executes. Its one miss is the pre-existing raise KeyError(key) in _EnumRegistry.__missing__. For the seven, the changed line already executed before, so coverage there is flat by construction and the subtest counts above are the measure. pcapkit.py and sctp.py improved because the new sibling test reaches their register for the first time; the accompanying rise in partial branches is the expected signature of newly-reached code, not a regression.

Labels, and the breaking question

Labelled fix + test, matching #681.

The case for breaking: EnumSchema.register and the declaration path were silent and now warn. A downstream consumer running under -W error that replaces a built-in schema — a supported thing to do — now gets an exception where it previously got silence. That is a behaviour change on a supported path, and it is real.

The case against, which is why it is not applied: this repo defines the label as "Alters public API or wire output", and nothing here does. No signature changes, no wire output changes, no registry contents change. The seven keep their firing condition byte-for-byte; only their message text differs, which no test pins (the four references to the old text in the suite are prose in docstrings and comments, and the string already registered, overwriting is still a substring of the new message, so they stay accurate). #681 faced the identical question for the identical reason and was labelled fix + test. Applying breaking here and not there would be the inconsistency.

Worth noting the -W error exposure is smaller than it looks: the warning fires only where something was genuinely displaced, and pcapkit.utilities.warnings.warn logs on the pcapkit logger before calling warnings.warn, so a consumer that filters the category still sees the complaint.

How this constrains #514

#514's staging names the register_protocol collision as its c1 prerequisite, landed by #681. This is not c1 and does not extend it: no key-space change, so #682's finding that every __proto__ reader degrades silently on a miss is untouched, and re-keying stays entirely with #514/#682.

What it does constrain is c2/c3. It settles with tests that the two guard shapes are deliberately different, and why — so a later re-keying cannot quietly harmonise them on the way past, and does not have to rediscover the ModuleDescriptor argument. The one place it adds work: if c3's deferred-registration drain ever routes schema registration through EnumSchema.register more than once for the same code, that would now warn, so the drain needs to be idempotent or to register once. Worth knowing before the drain is written rather than after.

Fixed after cross-review

A cross-review on a different model found a real docs defect, since fixed in the amended commit: the new Note: in EnumSchema.register wrapped a long :func: role across two lines with a trailing backslash. That idiom is used widely in this package, but it is only safe inside a raw docstring, where the backslash survives for docutils to treat as its own line-join escape. EnumSchema.register's docstring is a plain """, so Python's compiler consumed the backslash at compile time and left the next line's indentation as literal spaces inside the role target — measured as '~pcapkit.foundation.registry. + 12 spaces + protocols.register_protocol', which cannot resolve.

Both of this PR's backslash-continued roles are now the text <target> form instead, which is safe regardless of raw-ness because the resolver only reads what is inside the angle brackets. Audited all nine touched docstrings: 0 broken role targets, 48 roles total.

One correction to that review's framing, checked rather than taken on trust: it reported the same defect as pre-existing at schema/schema.py's _EnumRegistry docstring. It is not the same defect. That role is the text <target> form, so its backslash join lands in the link text and the target inside the angle brackets is intact on one line. The effect there is one cosmetic space in a displayed name, not a broken reference, so it is left alone.

---

I am not claiming CI green.

[JarryShaw]

GOOD TO GO — with two items it raised, both now addressed. Relaying an independent cross-review run on a different model (Claude Sonnet) from the one that authored the change, briefed to falsify rather than confirm, read-only. Verdicts are its own; the two follow-ups and one correction below are mine.

Claim-by-claim

#	Claim	Verdict
1	import pcapkit emits zero new warnings	CONFIRMED — 1 total (DeprecationWarning, dictdumper/vuejs.py:89), 0 RegistryWarning, on both this tree and a separate pre-PR worktree at f0999858e
2	The seven registrars' firing condition is byte-for-byte unchanged	CONFIRMED — only the message text differs; every if code in cls.__proto__: and all control flow untouched
3	Presence-only is right for the seven; #681's narrower guard must not transfer	CONFIRMED, all three sub-reasons survived attack
4	A RegistryWarning on EnumSchema.register is safe	CONFIRMED — __missing__ never assigns; in never invokes it; every new subscript read happens only after a confirmed-present check
5	The __init_subclass__ restructure preserves semantics	CONFIRMED — could not construct a diverging input
6	Each new test fails without its guard	CONFIRMED — 2 / 1 / 8-subtest failures reproduced, exit 1 each
7	Tests restore registry state, absence included	CONFIRMED — no leak, including into the real shipped TCP-option registry
8	EXPECTED_FAILURES unchanged at 44	CONFIRMED, and stronger: identical key sets, not just counts
9	mypy and pylint at parity	mypy CONFIRMED; pylint DISPUTED (see below)
10	Declining breaking is right	CONFIRMED — verified the label's description via the API; no missed public-API or wire-output change

It specifically attacked claim 3 by chasing register_apptype, the one registrar that reaches Transport.register twice per call, and independently confirmed the two calls land in TCP.__proto__ and UDP.__proto__ — different dicts, so no double-write path exists that presence-only could warn on spuriously. Same for register_linktype across Frame and PCAPNG.

On claim 5 it went further than the PR did, checking a generator code (consumed exactly once either way, since the new form binds the same object), a str (both versions apply the identical isinstance check and iterate character-wise), and a scalar enum member.

What it disputed, and what I changed

A genuinely broken Sphinx role — its best catch, and a real defect. The new Note: in EnumSchema.register wrapped a long :func: role across two lines with a trailing backslash. That idiom appears on ~523 lines package-wide, but it is only safe inside a raw docstring, where the backslash survives for docutils to interpret as its own line-join escape. EnumSchema.register's docstring is a plain """, so Python's compiler consumed the backslash at compile time and left the following line's indentation as literal spaces inside the role target. Reproduced directly: the target came out as '~pcapkit.foundation.registry. + 12 literal spaces + protocols.register_protocol', which cannot resolve.

Fixed in the amended commit. Both of this PR's backslash-continued roles are now the text <target> form, which is safe regardless of raw-ness because the resolver reads only what is inside the angle brackets. Audited all nine touched docstrings afterwards: 48 roles, 0 broken targets.

One correction to its framing. It reported the identical defect as pre-existing in the same file's _EnumRegistry docstring. Checked rather than taken on trust, and it is not the same defect: that role is already the text <target> form, so the backslash join lands in the link text while the target inside the angle brackets stays intact on one line. The effect is one cosmetic space in a displayed name, not a broken reference. Left alone rather than swept into this diff.

pylint, partially disputed — and it is right that the figure was under-specified. It ran a full make pylint over the package and got 5,895 vs 5,914, not 364. Both numbers are real: mine was scoped to the eight touched modules, which the PR body failed to say. Now stated explicitly. Its more valuable finding is that the whole-package total is not a usable before/after metric at all: R0401 (cyclic-import) varies run to run on an unchanged tree — it measured 106 then 119 on two consecutive runs of the same commit. The two deltas the PR actually rests on reproduced exactly under its own invocation: C0325 −1 and R0801 +1.

What it could not verify

• No full Sphinx build, so the exact warning CI would emit for the broken role is unquoted; it verified the mechanism against docutils directly instead.
• It did not check the coverage figures, having prioritised the claims flagged as highest-risk.

It left the working tree exactly as found — git status --porcelain empty, HEAD unmoved — which I re-verified independently before amending.