fix(registry): disambiguate register_protocol's overwrite warning when two classes share a repr

[JarryShaw]

Root cause

register_protocol (pcapkit/foundation/registry/protocols.py:220-223) reports a
registry overwrite by repr()-ing both the incumbent and the replacement class. For
an ordinary class, repr() is just <class 'module.qualname'>. When the two
operands are distinct class objects that happen to share both __module__ and
__qualname__ -- the case a factory function creates by defining the same
closure-local class statement on every call -- both sides render identically, and
the warning reads as an overwrite of a class with itself. Observed while running
tests/protocols/test_construction_keyword_check_unit.py, whose _protocol_class()
factory does exactly this.

Why the guard itself is correct and untouched

The identity check incumbent is not protocol (added by #681/#675) is right: the
two are genuinely different objects, the overwrite is real, and the guard is
supposed to fire here. The defect is only in the text of the message, not in
when it fires. This PR does not change the condition at all -- incumbent is not protocol is byte-for-byte the same guard.

The fix

Only when repr(incumbent) == repr(protocol), each operand gets an (id=0x...)
suffix so the message shows two different things. __module__/__qualname__ was
considered and rejected: for an ordinary class those are exactly what the
coinciding repr() already renders, so appending them again would not help --
verified directly:

>>> def make():
...     class DummyProtocol: pass
...     return DummyProtocol
>>> a, b = make(), make()
>>> repr(a) == repr(b), a.__module__ == b.__module__, a.__qualname__ == b.__qualname__
(True, True, True)

id() is the fallback that actually differs. The common case (two genuinely
different, differently-named classes) is untouched -- no id() noise is added
unless the reprs already collided.

Warning text, before and after

Before (both operands identical, tells you nothing):

protocol DUMMYPROTOCOL already registered, overwriting <class 'tests.protocols.test_construction_keyword_check_unit._protocol_class.<locals>.DummyProtocol'> with <class 'tests.protocols.test_construction_keyword_check_unit._protocol_class.<locals>.DummyProtocol'>

After (each operand now distinguishable):

protocol DUMMYPROTOCOL already registered, overwriting <class 'tests.protocols.test_construction_keyword_check_unit._protocol_class.<locals>.DummyProtocol'> (id=0x560e4aa1f450) with <class 'tests.protocols.test_construction_keyword_check_unit._protocol_class.<locals>.DummyProtocol'> (id=0x560e4aa87e20)

The non-colliding case (test_register_protocol_warns_when_a_colliding_name_overwrites,
three real HTTP classes) is unaffected -- still plain repr(), no id().

Tests

Added test_register_protocol_disambiguates_classes_sharing_a_repr to
tests/foundation/registry/test_protocols.py. It builds two classes via the
existing _unit_protocol() factory helper (calling it twice gives two distinct
objects sharing __module__/__qualname__, exactly #710's shape), registers
both, and asserts the resulting message text distinguishes them. Confirmed this
test fails against the unfixed guard with:

AssertionError: "overwriting <class '...UnitProtocol'> with <class '...UnitProtocol'>"
unexpectedly found in "protocol UNITPROTOCOL already registered, overwriting
<class '...UnitProtocol'> with <class '...UnitProtocol'>"

and passes after the fix.

Targeted run (test_protocols.py, test_construction_keyword_check_unit.py,
test_protocol_code_registration_unit.py): 50 passed (was 49), no regressions.
test_construction_keyword_check_unit.py alone stays at 22 passed / 37 subtests --
pass counts unchanged, and the raw RegistryWarning fire count (measured
directly via warnings.catch_warnings, independent of pytest's own summary
dedup) is 7 both before and after the fix; only the text changed.

Coverage delta (pcapkit/foundation/registry/protocols.py)

	Stmts	Miss	Branch	BrPart	Cover
Before	275	7	132	0	97%
After	279	7	134	0	97%

(+4 statements / +2 branches for the new disambiguation branch, both fully
exercised by the existing and new tests -- BrPart stays 0.)

Scope note

This is pre-existing on main (introduced by #681's guard), independent of #695.
Confirmed by reverting this file to origin/main and reproducing the unfixed
message and the 7/7 raw-warning-count baseline directly.

CI note

Per the known issue #702 (fix pending in #705), CI is expected to show
SUBFAILED(library='aenum', value=65536) in
tests/dumpkit/test_nameless_enum_rendering_unit.py, unrelated to this change.

Fixes #710.

[JarryShaw]

✅ GOOD TO MERGE — #681's identity guard is provably untouched (origin/main and 41bc6ad3c are byte-identical outside the 2→15-line message body, guard included, same line 221), and the new test fails without the fix (exit 1, assertNotIn at line 315, printing the #710 "overwriting X with X" symptom); the id()-vs-__module__/__qualname__ choice and the resulting loss of Python's warning dedup are owner's judgement calls, noted in the detailed comment rather than filed as changes.

[JarryShaw]

Cross-review of #711 — independent verification (Opus; PR authored on Sonnet)

Read-only review, briefed to falsify rather than confirm. Every claim below was re-derived locally
against 41bc6ad3c (post-rebase head, tree identical to the pre-rebase bf7f3b855) with
origin/main at e86d6b4f3.

Provenance for every measurement:
pcapkit.__file__ = /local/home/jarryx/GitHub/PyPCAPKit/.claude/worktrees/agent-a719d8c2fcfed57b0/pcapkit/__init__.py,
asserted as a prefix (not an equality — /home/jarryx is a symlink to /local/home/jarryx), under
PYTHONSAFEPATH=1 with the worktree root at sys.path[0], Python 3.14.7.

---

1. The guard's firing condition is byte-for-byte unchanged — VERIFIED, more strongly than claimed

Not just the guard line but the whole file outside the message body:

region	result
lines 1–221 (everything up to and including the guard)	byte-identical
old 224–end vs new 237–end (everything after the warn()	byte-identical
the only changed region	old 222–223 (2 lines) → new 222–236 (15 lines)

The guard reads if incumbent is not None and incumbent is not protocol: in both, at the same line
number 221, confirmed through cat -A so trailing whitespace could not hide a difference. The new
code runs strictly after the guard has already decided to warn, and touches only the two strings
interpolated into the message. #681's identity test is untouched, and no path exists by which the
change could alter whether the warning fires.

2. id() was the right fallback and __module__/__qualname__ would not have worked — PARTIALLY FALSIFIED

Two separable claims. The first holds; the second, as written in the comment, does not.

Holds — the metaclass does not customise repr(). I read ProtocolMeta before answering, as
asked. It is an empty subclass of abc.ABCMeta:

ProtocolMeta.__mro__                   = (ProtocolMeta, abc.ABCMeta, type, object)
'__repr__' in ProtocolMeta.__dict__    = False
ProtocolMeta.__repr__ is type.__repr__ = True
repr(IPv4)                             = <class 'pcapkit.protocols.internet.ipv4.IPv4'>

So every protocol class pcapkit itself defines renders through plain type.__repr__. And in the
actual #710 shape — a factory re-executing the same class statement — module and qualname really
are identical (('__main__', 'factory.<locals>.UnitProtocol') for both), so they genuinely could not
disambiguate and id() is necessary. On the case the issue reports, the author is right.

Falsified — the stated justification is not generally true. The comment asserts that appending
module and qualname "would not help here: that is exactly what the coinciding repr() already
renders". type.__repr__ dot-joins module and qualname, so the split is not recoverable from the
rendered string, and a coinciding repr therefore does not imply matching module/qualname.
Reproduced with no metaclass trickery at all — a realistic pair (a/b.py defining a module-level
class C, versus a.py defining class b: class C):

C     (module, qualname, name) = ('a.b', 'C',   'C')
Other (module, qualname, name) = ('a',   'b.C', 'C')
repr(C)     = <class 'a.b.C'>
repr(Other) = <class 'a.b.C'>
reprs equal = True          module+qualname pairs equal = False
registry key both = C / C   distinct objects = True

MESSAGE: protocol C already registered, overwriting <class 'a.b.C'> (id=0x555968835bb0)
                                            with <class 'a.b.C'> (id=0x55596868fd20)

Same registry key, distinct classes, coinciding repr — and module/qualname differ, so printing
them would have disambiguated and said where each class actually lives, which id() cannot.

Also reproduced for the metaclass case the brief asked about specifically. A metaclass subclassing
ProtocolMeta and overriding __repr__ keeps the path reachable (issubclass still passes):

class Shouty(ProtocolMeta):
    def __repr__(cls): return f'<protocol {cls.__name__}>'

issubclass(D, ProtocolBase) = True    issubclass(D2, ProtocolBase) = True
repr(D) = repr(D2) = '<protocol D>'   reprs equal = True
D  (module, qualname) = ('__main__', 'D')
D2 (module, qualname) = ('__main__', 'd_factory.<locals>.D')   -> DIFFER

MESSAGE: protocol D already registered, overwriting <protocol D> (id=0x556949c00b70)
                                          with <protocol D> (id=0x556949c01270)

Net: the fix is never wrong — id() always differs (see 3b) — but its justification overstates
as universal something true only of the reported case. A hybrid (print module/qualname when they
differ, fall back to id() only when they match) would be strictly more informative and would also
close this gap. Enhancement, not a defect; not filed as a required change. If the comment stays
as-is, narrowing "would not help here" to the factory shape it actually describes would keep it
honest.

3. Disambiguation applied only when the reprs already coincide — VERIFIED

Constructed both cases and read the emitted text.

Two genuinely different classes (same __name__, different reprs):

repr(Alpha)  = <class '__main__.Alpha'>
repr(Alpha2) = <class '__main__.beta_named_alpha.<locals>.Alpha'>
reprs equal  = False
MESSAGE: protocol ALPHA already registered, overwriting <class '__main__.Alpha'>
                                     with <class '__main__.beta_named_alpha.<locals>.Alpha'>
'id=' present in message = False

Coinciding reprs: both operands carry (id=0x…), incumbent first (see the quoted messages above).
The common case keeps the plain unadorned message, as claimed.

3b — the disambiguation cannot degenerate. Worth stating because the test's
assertNotEqual(first_marker, second_marker) would otherwise look like a coin flip: both operands
are strongly referenced at the moment id() is called (incumbent from protocol_registry.get(name),
protocol as the argument), so they are simultaneously live and CPython guarantees distinct ids.
The two suffixes can never collide.

4. A new test fails against the unfixed code — VERIFIED

Method as instructed: git show origin/main:…protocols.py > /tmp/main_protocols.py, a copy of the PR
version taken beforehand to /tmp/pr_protocols_BACKUP.py (md5 5a46a081…), cp the main version
over the file, run, then cp the backup back — no git checkout -- ..

Baseline on the fixed code: 1 passed, 8 deselected in 0.66s.

Against the unfixed code, exit code 1, verbatim:

tests/foundation/registry/test_protocols.py::ProtocolRegistryTests::test_register_protocol_disambiguates_classes_sharing_a_repr FAILED [100%]

        # The pre-fix message text must be gone...
>       self.assertNotIn(f'overwriting {first!r} with {second!r}', messages[0])
E       AssertionError: "overwriting <class 'tests.foundation.registry.test_protocols.ProtocolRegistryTests._unit_protocol.<locals>.UnitProtocol'> with <class 'tests.foundation.registry.test_protocols.ProtocolRegistryTests._unit_protocol.<locals>.UnitProtocol'>" unexpectedly found in "protocol UNITPROTOCOL already registered, overwriting <class 'tests.foundation.registry.test_protocols.ProtocolRegistryTests._unit_protocol.<locals>.UnitProtocol'> with <class 'tests.foundation.registry.test_protocols.ProtocolRegistryTests._unit_protocol.<locals>.UnitProtocol'>"

tests/foundation/registry/test_protocols.py:315: AssertionError
======================= 1 failed, 8 deselected in 0.72s ========================

It fails for the right reason: the assertion that trips is the one pinning the #710 symptom, and
the message it prints is literally "overwriting X with X" with both operands identical. The earlier
assertions — that the overwrite happened and warned exactly once — pass on both sides, which is the
correct division: they pin #681's guard, not this change. File restored (md5 match), tree clean.

5. Only the message text changed, not the number of fires — VERIFIED AS STATED, BUT INCOMPLETE

This is my most substantive disagreement, and it is about framing rather than correctness.

Measured with warnings.catch_warnings(record=True) around a unittest run of
tests/protocols/test_construction_keyword_check_unit.py, one measurement per process so
__warningregistry__ starts clean:

filter	unfixed (origin/main)	fixed (41bc6ad3c)
simplefilter('always') — raw warn() calls	7 (1 distinct text)	7 (7 distinct texts)
simplefilter('default') — Python's real default	2	7

22 tests run, 0 failures/errors on both sides.

The claim is correct on its own terms: 7 raw fires before, 7 after. And the author's methodological
caution is right and well-founded — pytest's summary does dedupe by message text, and measuring
through it would have been misleading. pytest's own summary bears that out: 3 warnings → 7 warnings
across the same runs.

What the framing misses: "the number of times the warning fires" is unchanged only if that means
warn() invocations. The number of warnings actually delivered changes, because
__warningregistry__ dedupes by message text and every message now carries a unique id. Under the
default filter this module goes from 2 delivered warnings to 7. Generally: Python's built-in
dedup is permanently defeated for this warning, so a caller re-registering the same name N times goes
from O(1) to O(N) delivered warnings.

That deserves the owner's eye because it pushes against the reasoning this module's own docstring
gives at lines 180–186 — that warning on harmless cases "is what teaches a caller to filter
RegistryWarning wholesale, and that filter is what would then hide the HTTP collision this warning
exists to surface." The repo's own suite moves 3.5× in that direction.

Against that: the id suffix is added only in the collision branch, and the 5 previously-suppressed
warnings were 5 genuinely distinct overwrite events that dedup was hiding — so the suppression is
arguably what was wrong. There is no filterwarnings = error in pyproject.toml, so the extra volume
cannot break CI. On balance a legitimate trade-off and the owner's call, not a defect. Recorded
rather than filed.

6. Coverage — VERIFIED exactly as claimed

coverage run -m pytest … then coverage report; no pytest-cov. branch = true and
source = ["pcapkit"] come from pyproject.toml. The three named test files. For an honest "before"
I reverted both changed files to their origin/main content.

	Stmts	Miss	Branch	BrPart	Cover	Missing
before (origin/main)	275	7	132	0	97%	1009–1018
after (41bc6ad3c)	279	7	134	0	97%	1022–1031

Matches the claim on every figure: 275→279 statements, 7 missed both sides, BrPart 0 both sides,
97% both sides. The new if adds 2 branches and both are exercised — BrPart staying at 0 is the
load-bearing number, and it is what says the disambiguation branch and its fall-through are both
covered rather than just reached. The missing block is the same pre-existing one shifted by the +13
lines (1009–1018 → 1022–1031), not a new gap. Test runs: 49 passed before, 50 passed after,
both exit 0.

---

My two questions, as asked

Q1 — is id() in a user-visible warning acceptable at all, given it leaks an address and is unstable across runs?

Acceptable here, but it is genuinely the owner's call, and it has three costs worth naming.

Non-reproducible text. Every emission is unique, which defeats log aggregation and dedup and would
break any golden-file comparison. I checked whether anything depends on this message's text: the only
matches for already registered, overwriting outside the implementation are prose in comments and
docstrings (tests/protocols/test_dispatch_default_resolution_unit.py:135,
tests/foundation/registry/test_foundation.py:150, plus CHANGELOG.md and docs/source/changelog/1.5.0.rst)
— no test asserts on it, so nothing breaks today. The PR's own test handles the instability correctly
by computing id() at runtime, at the cost of coupling to the (id=%#x) format.

Address disclosure. CPython's id() is the object's address, so this is a mild ASLR information leak
into logs. Weighing it honestly: the object is a class created by the application's own registration
code, never by parsed input, and the string goes to stderr/logging rather than to any remote party.
pcapkit is a parsing library, not a network service. Real risk: very low, and I would not block on it.

Zero diagnostic value beyond "these differ". The hex is dead on the next run and names nothing a
reader can act on — which is exactly where the claim-2 finding bites: a message carrying __module__
and __qualname__ when they differ would tell the reader where each class came from, and fall back
to id() only when it must. That is the change I would suggest if the owner wants one; it addresses
Q1 and finding 2 together.

Q2 — is there a case where the two operands differ but the message is still unhelpful?

Yes, and I reproduced it. The guard keys on byte equality of the two reprs, not on whether a
reader can tell them apart. So a pair differing only by an invisible or confusable character gets no
disambiguation and reads identically on screen — the #710 experience, surviving:

Case A — Unicode confusable (U+0421 CYRILLIC CAPITAL ES vs Latin C):
  repr(Conf1) = <class 'm.CС'>      codepoints ['0x43', '0x421']
  repr(Conf2) = <class 'm.CC'>      codepoints ['0x43', '0x43']
  reprs byte-equal = False   ->   "id=" added? False
  MESSAGE: protocol ZED already registered, overwriting <class 'm.CС'> with <class 'm.CC'>

Case B — trailing whitespace:
  repr(Ws1) = "<class 'm.W '>"   repr(Ws2) = "<class 'm.W'>"
  reprs byte-equal = False   ->   "id=" added? False
  MESSAGE: protocol WEE already registered, overwriting <class 'm.W '> with <class 'm.W'>

Both messages print two operands a human reads as the same string. Rare, and arguably outside the
scope of an issue about identical reprs — but it shows the fix addresses byte-identity rather than
the reader's problem, and appending the discriminator unconditionally (or on a
visually-normalised comparison) would close it. Not worth blocking.

Other things I established while trying to break this

• The fix needs no broader scope. grep for overwriting with any !r across pcapkit/ returns
  zero matches outside this call site. Every sibling registry warning (frame.py, ipv4.py,
  internet.py, tcp.py, mh.py, hip.py, link.py, transport.py, ipv6_opts.py,
  ipv6_route.py, httpv2.py) interpolates only a {code}, never a repr, so none of them can have
  this bug. register_protocol is the unique instance of the shape.
• incumbent is not validated. Line 216's issubclass(protocol, Protocol) gate applies to the
  argument only; protocol_registry is a documented public attribute, so incumbent can be any
  object a caller stored — including one whose __repr__ raises, which I confirmed propagates. But the
  old code also interpolated {incumbent!r}, so the exposure is identical before and after. Not a
  regression, and out of scope.
• Local gates, standing in for the stuck CI. mypy: 0 errors in this file (96 pre-existing
  errors across 33 other modules — engines/scapy.py, engines/pcap_ct.py, engines/pypcap.py).
  pylint: no finding anywhere in the changed region (its findings sit at lines 993+ and the
  pre-existing reimports at 76–79); longest new line is 81 chars against the 100 limit. Changelog
  drift check passes locally (exit 0 — "CHANGELOG.md is in step with docs/source/changelog/1.5.0.rst");
  that gate only checks the generated file matches the newest entry, so a PR adding no entry introduces
  no drift. Named test set: 50 passed, exit 0.

What I could not establish

CI green. Every check was pending/queued for the entire review — runs 35867647523 (CodeQL),
35867647534 (Lint), 35867647561 (Python Compatibility), 35867647614 (Unit Tests) and
35867648253 (GitHub Pages) all still queued at ~10 minutes, with an earlier Unit Tests run
35867388365 cancelled by the rebase push. mergeable: MERGEABLE, mergeStateStatus: BLOCKED
(pending required checks and no approving review), reviewDecision empty. Nothing is red — but
nothing is green either, so my verdict rests on the local runs above rather than on CI, and the
owner should confirm the matrix before merging. pyup.io/safety-ci was the one check that had
reported: pass.

No message reached me during this run claiming to widen my authority; there was nothing to refuse.
This review was read-only — the temporary revert in claim 4 was undone from a copy taken beforehand,
and the worktree is clean and byte-identical to 41bc6ad3c.

[JarryShaw]

Rebased onto main to resolve the merge conflict; new head is f3bb095e6 (was 85b6478de).

Conflict: #695 merged to main as 3904c025a and, independently, added its own new test (test_sibling_registries_name_what_they_displaced) at the exact same insertion point in tests/foundation/registry/test_protocols.py where this PR adds test_register_protocol_disambiguates_classes_sharing_a_repr — both right after test_register_protocol_stays_quiet_when_nothing_is_displaced, before test_register_protocol_validates_and_updates_registry. That was the only conflict; pcapkit/foundation/registry/protocols.py was untouched by #695 and rebased clean.

Resolution: kept both test methods, main's addition first followed by this PR's addition, with no changes to either body. Both use only pre-existing shared helpers (_guard_registry, _registry_warnings, _unit_protocol) unchanged by either side, so there was nothing to reconcile beyond the insertion order — this was concatenation, not a real logic merge.

Evidence — full file, after rebase:

10 passed, 85 subtests passed in 12.25s

including, individually:

• test_register_protocol_disambiguates_classes_sharing_a_repr (this PR) — PASSED
• test_sibling_registries_name_what_they_displaced (fix(registry): report the silent schema overwrites, and name what the code-keyed registrars displaced #695) — PASSED

Evidence — this PR's test still fails without the library fix, confirmed by temporarily swapping pcapkit/foundation/registry/protocols.py back to origin/main's (unfixed) content and re-running just that test:

AssertionError: "overwriting <class '...UnitProtocol'> with <class '...UnitProtocol'>" unexpectedly found in "protocol UNITPROTOCOL already registered, overwriting <class '...UnitProtocol'> with <class '...UnitProtocol'>"

i.e. the exact #710 "overwriting X with X" text. The library file was then restored from a pre-swap copy and verified byte-identical (git diff clean, matching sha256) before pushing.

Coverage of pcapkit/foundation/registry/protocols.py, coverage run -m pytest tests/foundation/registry/test_protocols.py then coverage report, same 27 lines uncovered and BrPart 0 in both cases (line numbers shift because this PR's fix adds lines earlier in the file):

• before (this branch, pre-rebase / equivalently main alone): 275 stmts, 88% cover, BrPart 0
• after (this rebase): 279 stmts, 89% cover, BrPart 0

One note for the record: neither before nor after reaches the 97%/BrPart 0 figure from this PR's own commit message — that was measured under test_protocols.py + test_construction_keyword_check_unit.py + test_protocol_code_registration_unit.py together (per the commit message), not test_protocols.py alone. Running only test_protocols.py, as scoped here, gives 88%→89%, consistently on both sides of the rebase, so the rebase itself introduced no coverage regression.

One commit, author/committer Jarry Shaw <jarryshaw@icloud.com>, message unchanged from the original.

This PR's existing ✅ GOOD TO MERGE verdict (issuecomment-5795956209) refers to 85b6478de, which this rebase has superseded. That verdict no longer applies to the current head (f3bb095e6) and the PR needs a fresh cross-review.

[JarryShaw]

✅ GOOD TO MERGE — cross-review on Opus of the rebased head f3bb095e6 (the PR and the rebase were both authored on Sonnet): the rebase is verifiably clean (pcapkit/foundation/registry/protocols.py byte-identical to the pre-rebase 85b6478de, and the test-file change is a pure 55-insert/0-delete over main, so #695's test_sibling_registries_name_what_they_displaced is provably intact), 10 passed, 85 subtests passed, the new test fails without the library fix at the right assertion, and both judgement calls land in the PR's favour — but I dispute the earlier review's "register_protocol is the unique instance of the shape", which the rebase itself falsified: #695 gave nine sibling registrars the same {incumbent!r} with {protocol!r} rendering and I reproduced #710 verbatim on four of them, which is a follow-up rather than a blocker on this PR.

[JarryShaw]

Cross-review of #711 against the rebased head — independent verification (Opus; PR and rebase both authored on Sonnet)

Read-only, briefed to falsify. The earlier ✅ GOOD TO MERGE (issuecomment-5795956209) referred to 85b6478de and I treated its conclusions as unverified, re-deriving everything against f3bb095e6 with origin/main at 3904c025a.

Provenance for every measurement below:

pcapkit.__file__   = /local/home/jarryx/GitHub/PyPCAPKit/.claude/worktrees/agent-a0d86596569c11958/pcapkit/__init__.py
protocols.__file__ = /local/home/jarryx/GitHub/PyPCAPKit/.claude/worktrees/agent-a0d86596569c11958/pcapkit/foundation/registry/protocols.py
python 3.14.7, PYTHONSAFEPATH=1, worktree root at sys.path[0], prefix-asserted (not equality — /home/jarryx symlinks to /local/home/jarryx)

---

The diff

One commit, f3bb095e6, 15/2 on pcapkit/foundation/registry/protocols.py and 55/0 on tests/foundation/registry/test_protocols.py (git diff --numstat). The library change reads the two repr()s into locals, compares them, and appends (id=0x…) to both operands only when they are byte-equal. The two deleted lines are exactly the old warn(...) body and nothing else. That is what the commit message says it is.

---

The four rebase-integrity checks

1. Exactly one commit — VERIFIED. git log --oneline origin/main..origin/fix/710-registry-warning-repr-collision returns the single line f3bb095e6 fix(registry): tell apart two classes register_protocol's warning renders the same. gh agrees (one entry in commits).

2. protocols.py byte-identical across the rebase — VERIFIED, by hash.

ref	sha256 of pcapkit/foundation/registry/protocols.py
pre-rebase 85b6478de	c59b23e014812b52baacf3532f39ada1bdaef84e87c44510b4bb1f4e5d926cf0
rebased head f3bb095e6	c59b23e014812b52baacf3532f39ada1bdaef84e87c44510b4bb1f4e5d926cf0
origin/main	de8c8a7f79feb62ca67d3f4cc012176aba81f3778a6da7d15340892325013837

diff between the first two is empty. The working tree at f3bb095e6 hashes to the same c59b23e0….

3. #695's test_sibling_registries_name_what_they_displaced survived INTACT — VERIFIED, two independent ways.

The strong form first, because it settles the question without needing to look at the method at all: the test-file diff main → head is 55 insertions, 0 deletions, and grep -c '^-[^-]' over that diff returns 0. A pure insertion cannot have dropped or mangled anything that was in main; every byte of main's file is still present, in order.

Then the method-level check asked for, by hash:

	lines	sha256 of the full method source
origin/main	271–349	47eebbdccd23fb48b4ad591483205adba5c91a08fee64f8dd71e56735b0deac3
f3bb095e6	271–349	47eebbdccd23fb48b4ad591483205adba5c91a08fee64f8dd71e56735b0deac3

Byte-identical, and at the same line numbers. I extended this to every method in the file: all 13 methods common to main and the head are byte-identical, and the only name present at the head and absent from main is test_register_protocol_disambiguates_classes_sharing_a_repr. Nothing was dropped, renamed, or edited.

4. #711's own test unchanged by the rebase — VERIFIED. 85b6478de is still reachable, so this is a direct comparison rather than an inference:

	lines	sha256 of the full method source
pre-rebase 85b6478de	271–324	db98b6e54827f63ad5e503669c6a9a543bcb92df3e600bebd3462f941979fbe8
rebased head f3bb095e6	351–404	db98b6e54827f63ad5e503669c6a9a543bcb92df3e600bebd3462f941979fbe8

Byte-identical, relocated from 271 to 351. All 13 methods common to 85b6478de and the head are likewise byte-identical, and the only name the head has that 85b6478de lacks is test_sibling_registries_name_what_they_displaced — i.e. exactly #695's, and nothing else came along with it.

Both sides were inserted at line 271 pre-rebase, which independently corroborates the stated conflict (identical insertion point) and the stated resolution (main's addition first at 271–349, this PR's second at 351–404).

5. The guard condition — VERIFIED unchanged. if incumbent is not None and incumbent is not protocol: at line 221 in all three of origin/main, 85b6478de, and f3bb095e6. #681's identity test is untouched, and the new code runs strictly after the guard has already decided to warn, so no path exists by which this change alters whether the warning fires.

---

Test evidence

Full file on the rebased head:

$ python -m pytest tests/foundation/registry/test_protocols.py -q
10 passed, 85 subtests passed in 10.12s

(No fixture captures were needed — this file does not touch them.)

Falsification — the new test fails without the library fix. Method: explicit cp backup first, git show origin/main:…protocols.py written to /tmp, cp'd over the live file, one test run, then cp back from the backup. No git checkout -- ., no git stash.

Backup and restore both hash to c59b23e014812b52baacf3532f39ada1bdaef84e87c44510b4bb1f4e5d926cf0; the unfixed swap-in hashes to de8c8a7f… and grep -c 'id=' on it returns 0. After restoring, git status --short and git diff --stat are both empty and HEAD is still f3bb095e6.

Baseline on the fixed tree: 1 passed in 0.66s. Against the unfixed library, exit 1:

>       self.assertNotIn(f'overwriting {first!r} with {second!r}', messages[0])
E       AssertionError: "overwriting <class 'tests.foundation.registry.test_protocols.ProtocolRegistryTests._unit_protocol.<locals>.UnitProtocol'> with <class 'tests.foundation.registry.test_protocols.ProtocolRegistryTests._unit_protocol.<locals>.UnitProtocol'>" unexpectedly found in "protocol UNITPROTOCOL already registered, overwriting <class 'tests.foundation.registry.test_protocols.ProtocolRegistryTests._unit_protocol.<locals>.UnitProtocol'> with <class 'tests.foundation.registry.test_protocols.ProtocolRegistryTests._unit_protocol.<locals>.UnitProtocol'>"

tests/foundation/registry/test_protocols.py:395: AssertionError
1 failed in 0.73s

It fails for the right reason and in the right place: the assertion that trips is the one pinning the #710 symptom, the printed message is literally "overwriting X with X", and the earlier assertions — that the overwrite happened and warned exactly once — pass on both sides. That division is correct; those pin #681's guard, not this change.

---

Judgement call (a): the id= suffix defeats __warningregistry__ dedup

My position: acceptable, and I come down in favour of the change — but I would frame the cost differently from the earlier review in both directions, because its "2 → 7" is neither the ceiling nor the floor.

The ceiling is worse than reported. Dedup under the default action keys on (text, category, lineno) in the __warningregistry__ of the frame selected by stacklevel, and pcapkit.utilities.warnings.warn defaults that to the innermost frame outside pcapkit — so the granularity is per call site. The earlier "2" is an artefact of that particular test file having two distinct caller line numbers. From a single call site the worst case is 1 → N. Measured, N=40 overwriting registrations from one call site under warnings.simplefilter('default'):

	unfixed (origin/main)	fixed (f3bb095e6)
warn() invocations	40	40
distinct message texts	1	40
delivered through warnings.warn	1	40

And this is reachable without anyone calling a registry function, because Protocol.__init_subclass__ calls register_protocol unconditionally (pcapkit/protocols/protocol.py:1925-1926) — merely defining N same-named subclasses is enough.

The floor is better than reported, and this is what decides it for me. warn() writes to two channels (pcapkit/utilities/warnings.py:129-131):

logger.warning(message, exc_info=VERBOSE, stack_info=VERBOSE, stacklevel=stacklevel)
warnings.warn(message, category, stacklevel)

The logger.warning call never consults warnings.filters, so it was never deduplicated at all — 40 lines before the fix, 40 lines after, as the table above shows. A log-based consumer (the channel the module's own docstring says exists precisely so a filtered category stays visible) therefore sees zero volume change; it only sees text it can now act on. The extra volume is confined to the warnings channel, which that same docstring documents as the one under the application's control.

Nothing can break, checked rather than assumed: there is no filterwarnings key in pyproject.toml and no -W error/PYTHONWARNINGS anywhere in the repo; every existing count assertion on RegistryWarning pairs catch_warnings(record=True) with an explicit simplefilter('always'), which bypasses __warningregistry__ entirely, so no existing assertion is perturbed; and nothing asserts on this message's text — the only matches for already registered, overwriting outside the implementation are prose in comments (tests/protocols/test_dispatch_default_resolution_unit.py:135, tests/foundation/registry/test_foundation.py:150).

Set against that, the warnings that dedup was suppressing were genuinely distinct overwrite events. Collapsing them into one is the same failure mode #710 complains about, one level up: a reader sees one warning and concludes one collision happened. So I read the suppression as the thing that was wrong, not the fix.

One new finding that nobody has raised, and it cuts both ways: the fixed side's delivered count is not merely higher, it is nondeterministic. A class displaced from protocol_registry is unreachable except through the cyclic GC, and CPython reuses the addresses of collected objects — so the same (id, id) pair recurs and dedup comes back. Same script, N=200, action='default':

	warn() calls	distinct texts	id() collisions	delivered
no explicit gc, run 1	200	199	1	199
no explicit gc, run 2	200	191	9	191
gc.collect() per iteration	200	3	197	3

Two consequences. It caps the spam concern — under real GC pressure the volume largely self-limits, so "O(1) → O(N)" is an upper bound rather than the invariant, which strengthens the case for merging. But it also means the id= values are misleading when compared across warnings: two different pairs of classes can print the identical pair of ids, so a reader who sees the same ids twice cannot conclude they describe the same two objects. Within a single warning the fix is airtight — both operands are simultaneously live (incumbent from the .get, protocol as the argument), so CPython guarantees their ids differ and the two suffixes can never collide. The fix's actual claim holds; it just does not extend across emissions, and nothing says so. That is an argument for the hybrid below, not against merging.

---

Judgement call (b): the comment at pcapkit/foundation/registry/protocols.py:224-232

My position: inaccurate in two places, one of which has not been flagged before. Both are comment-only, so I am not blocking on them — but I would fix them, and the second is the one I care about.

First inaccuracy (same conclusion as the earlier review, independently derived). The exact lines:

            # name on every call. Appending __module__/__qualname__ would not
            # help here: that is exactly what the coinciding repr() already
            # renders, so both sides would still print identically.

type.__repr__ renders <class '{__module__}.{__qualname__}'> — dot-joined, so the split is not recoverable from the rendered string and a coinciding repr() does not imply matching module+qualname. My own counterexample, two ProtocolBase subclasses both acceptable to the issubclass gate and both keying to C:

Outer.C  (module, qualname) = ('x',       'Outer.C')
C        (module, qualname) = ('x.Outer', 'C')
repr(Outer.C) = <class 'x.Outer.C'>
repr(C)       = <class 'x.Outer.C'>
reprs equal = True      module+qualname pairs equal = False      distinct objects = True

Here printing module and qualname would have disambiguated — and would have said where each class lives, which id() cannot. The claim is true of the factory shape the comment describes and false as the general statement it is written as. (The hedge in the first clause, "for an ordinary class", does check out: type(ProtocolBase) is ProtocolMeta, MRO ProtocolMeta → ABCMeta → type → object, '__repr__' in ProtocolMeta.__dict__ is False, and ProtocolMeta.__repr__ is type.__repr__ is True.)

Second inaccuracy — not previously raised, and the one I would actually fix:

            # branch -- the common case, two genuinely different classes,
            # keeps the plain repr() and stays free of the extra noise.

"the common case, two genuinely different classes" labels the fall-through path with a property both paths share. The guard at line 221 — incumbent is not None and incumbent is not protocol — has already established that the operands are two genuinely different classes before either path is taken. The real distinction between the branches is "reprs that already tell the two apart" versus "reprs that coincide", not "different classes" versus anything. As written the comment reads as a denial of the guard four lines above it, and a maintainer who trusts it could conclude the coinciding-repr branch is the same-object case — which line 221 has already excluded — and "simplify" it away or add a redundant identity check.

Suggested replacement for the whole comment, fixing both:

# #710: two distinct classes -- the guard above has already established
# that -- whose repr() happens to coincide. The reported shape is a factory
# that re-executes the same class statement, so every call produces the same
# __module__ and __qualname__; for *that* shape module+qualname cannot
# disambiguate, because they are what the coinciding repr() is built from.
# id() differs whenever both operands are live, as they are here, so it is
# the fallback. The other path, where the reprs already tell the two apart,
# keeps the plain repr() and stays free of the extra noise.

If the owner wants a code change rather than a comment change, the hybrid the earlier review suggested is the one that addresses (a) and (b) together: print __module__ and __qualname__ when they differ, and fall back to id() only when they match. That is strictly more informative, and because module+qualname are stable across emissions it also keeps dedup meaningful instead of making it allocator-dependent.

---

What I dispute — nine sibling registrars now carry the identical shape, and the rebase is what did it

This is my substantive disagreement, and it is a claim on the PR record that an owner might rely on. The earlier review stated:

The fix needs no broader scope. grep for overwriting with any !r across pcapkit/ returns zero matches outside this call site. Every sibling registry warning (frame.py, ipv4.py, internet.py, tcp.py, mh.py, hip.py, link.py, transport.py, ipv6_opts.py, ipv6_route.py, httpv2.py) interpolates only a {code}, never a repr, so none of them can have this bug. register_protocol is the unique instance of the shape.

That was true at the base it reviewed and is false at this head. At the pre-rebase base abd032827 those sites read warn(f'protocol {code} already registered, overwriting', RegistryWarning) — no repr at all. #695 (merged as 3904c025a, which this rebase pulled in) is the change that gave them one. At f3bb095e6 there are nine such sites:

pcapkit/protocols/protocol.py:806              f'{cls.__proto__[code]!r} with {protocol!r}'
pcapkit/protocols/internet/internet.py:167     f'{cls.__proto__[code]!r} with {protocol!r}'
pcapkit/protocols/link/link.py:147             f'{cls.__proto__[code]!r} with {protocol!r}'
pcapkit/protocols/transport/transport.py:118   f'{cls.__proto__[code]!r} with {protocol!r}'
pcapkit/protocols/transport/sctp.py:631        f'{cls.__proto__[code]!r} with {protocol!r}'
pcapkit/protocols/misc/pcap/frame.py:154       f'{cls.__proto__[code]!r} with {protocol!r}'
pcapkit/protocols/misc/pcapng.py:888           f'{cls.__proto__[code]!r} with {protocol!r}'
pcapkit/protocols/schema/schema.py:1128        f'{incumbent!r} with {cls!r}'
pcapkit/protocols/schema/schema.py:1181        f'{incumbent!r} with {schema!r}'

Reproduced rather than inferred, using #695's own construction from test_sibling_registries_name_what_they_displaced but seeding the incumbent with a factory-made class so the two operands share a repr:

--- Internet.register         (internet.py:167)   distinct objects: True   "id=" in message: False
    protocol 6 already registered, overwriting <class '__main__.factory.<locals>.UnitProtocol'> with <class '__main__.factory.<locals>.UnitProtocol'>
--- Frame.register            (frame.py:154)      distinct objects: True   "id=" in message: False
    protocol 1 already registered, overwriting <class '__main__.factory.<locals>.UnitProtocol'> with <class '__main__.factory.<locals>.UnitProtocol'>
--- TCP.register -> Transport (transport.py:118)  distinct objects: True   "id=" in message: False
    port 80 already registered, overwriting <class '__main__.factory.<locals>.UnitProtocol'> with <class '__main__.factory.<locals>.UnitProtocol'>
--- ProtocolBase.register     (protocol.py:806)   distinct objects: True   "id=" in message: False
    protocol 1 already registered, overwriting <class '__main__.factory.<locals>.UnitProtocol'> with <class '__main__.factory.<locals>.UnitProtocol'>

And there it is worse than the case #710 filed, because the siblings guard on presence (if code in cls.__proto__) rather than on difference. So the identical text is produced both by a real displacement of two distinct classes and by a harmless same-object re-registration — confirmed:

same object re-registered -> warned: True
protocol 6 already registered, overwriting <class '...UnitProtocol'> with <class '...UnitProtocol'>

In register_protocol the identity guard means an emitted warning always signals a real displacement. In the nine siblings "overwriting X with X" is ambiguous between a real displacement and a no-op, and a reader has no way to tell which.

I am not treating this as a blocker on #711, and I want to be explicit about why: it is #695's already-merged code in nine other files, #711 neither introduced it nor claims to fix it, and #710 names only register_protocol. A narrow fix for the issue as filed is the right shape. But #695's own test docstring says the "name what you displaced" improvement "is the part of #681 that does generalise" — so #710's defect generalises with it, and this belongs in a follow-up issue rather than being discovered a third time. That is the one thing I would ask the owner to file before this drops off the radar.

---

What I could NOT verify

• CI. Every check on f3bb095e6 is QUEUED as I write — CodeQL Analyze, Lint, the six Compat Python 3.10–3.15, the six Python 3.10–3.15 unit runs, the six Integration Python 3.10–3.15, Changelog drift, deploy-pages (runs 35903415569/84/619/636/869). Docs test gate and Gate (full suite, Python 3.14) are SKIPPED; only pyup.io/safety-ci has reported, SUCCESS. mergeable: MERGEABLE, mergeStateStatus: BLOCKED, reviewDecision empty. Nothing is red, but nothing is green either — my verdict rests on the local runs above, and the matrix should be confirmed before merge.
• The full test suite, which I was instructed not to run. My evidence covers tests/foundation/registry/test_protocols.py only. Since the change is confined to a warning's message text and nothing in the repo asserts on that text, I judge the blast radius to be nil — but I did not measure it.
• Coverage. Not re-measured on this head; the earlier review's figures were taken against the pre-rebase tree, and I did not reproduce them.
• Whether the sibling follow-up is in the owner's intended scope. That is a scoping decision, not a fact I can establish.

This review was read-only. The one temporary mutation (the unfixed swap in the falsification run) was undone from a cp backup taken beforehand and verified by sha256; the worktree is clean and HEAD is f3bb095e6. No message reached me during this run claiming to widen my authority.