fix(foundation): add the #718 identity guard to the five registrars it missed - #742
Merged
Merged
Conversation
JarryShaw
added
fix
JarryShaw
force-pushed
the
fix/739-extractor-registry-identity-guard
branch
from
September 24, 2026 15:30
6cbcb56 to
f0a1f5b
Compare
JarryShaw
changed the title
Owner
Author
|
Self-dispatched review (Opus), not the independent cross-review — I dispatched this myself, so treat it as weaker evidence than an independently-briefed review; posting per house rule that a sub-agent's verdict has to reach the PR if it's worth reporting at all. Scope: covered only commit Does not cover the two |
Owner
Author
|
❌ NEEDS CHANGES @ |
Owner
Author
|
❌ NEEDS CHANGES @ Independent cross-review — distinct from the self-dispatched Opus review above, which covered only Blocking. Both files carry the identical sentence: "…the identity check compares the incumbent dumper (index Body numbers.
The factory returns a 2-tuple, so
Raised and dismissed: ❌ NEEDS CHANGES @ |
JarryShaw
added
review: needs-changes
JarryShaw
added a commit
that referenced
this pull request
Sep 24, 2026
Library modules imported a *Base class aliased to its public name, so the source read `class PCAP(Engine)` while actually inheriting `EngineBase`. Per the ruling on #514 the split is permanent and library classes inherit the base, so the base is now imported under its own name. * rewrote 28 of the 82 alias imports -- EngineBase 8 of 8, ReassemblyBase 3 of 3, DumperBase 2 of 2, TraceFlowBase 2 of 2, ProtocolBase 13 of 67 * renamed the code and annotation references that followed, including `cast()` targets, `TypeVar(bound=)` and `# type:` comments, but not the string values inside `Literal[...]` * corrected four `docs/source` index pages that named the public class while their own class diagram roots the hierarchy at the base * added `tests/test_base_class_contract.py`, pinning the contract per suite The four non-protocols families are complete. The remaining 54 sites are all `ProtocolBase` ones in paths #726 and #742 own, recorded as prefixes in `PENDING_ALIAS_PATHS` so the entry fails once its pull request lands. No behaviour change: name registry 38 keys before and after, `descendants(Public)` 0 in all five suites, 45 dispatches identical and still identical after clearing the name registry, mypy at its 112-error baseline with none introduced.
12 tasks
…t missed register_engine, register_reassembly, register_traceflow, and both register_dumper copies (Extractor and TraceFlowBase) warned on mere key presence, so re-registering the exact same class or descriptor read as "overwriting X" even though nothing was displaced. #718 fixed this for ten code-keyed registrars but excluded these five because their message wording falls outside its criterion, even though they share the defect. - Replace each `if key in cls.__dict__:` guard with `incumbent = cls.__dict__ .get(key)` + `incumbent is not None and incumbent is not value`, matching the identity-guard shape #726 gives the ten sibling registrars. Message text is unchanged. - register_engine/reassembly/traceflow: all three seed dicts hold only ModuleDescriptors, never a class, so the *first* real registration (descriptor -> class) still legitimately warns; only a *repeat* registration is now silent. Plain dicts, so `.get()`/`in` are both non-inserting. - register_dumper (both copies): __output__ maps format -> (dumper, ext), so the identity check compares the incumbent dumper (index 0), not the pair -- a re-registration that only changes ext is still identity-equal on the dumper and stays silent. __output__ is a `collections.defaultdict`; measured that `.get()` does not invoke the default factory the way a subscript access would (confirmed: len() unchanged on a `.get()`/`in` miss, +1 on a `d[missing]` subscript). `incumbent_entry[0]` is safe on a factory-produced miss too, since the factory itself returns a 2-tuple. - Add tests pinning, per registrar: same-object re-registration is silent, different-object replacement still warns with the same message, and the registry write happens on both paths. Verified all five fail against the unfixed guard. Ran tests/foundation/test_extraction.py + tests/foundation/traceflow/ test_traceflow_base.py: 16 -> 21 passed (28 subtests, unchanged). coverage: extraction.py 367 -> 372 stmts (miss unchanged at 7, 96%), traceflow.py 121 -> 123 stmts (0 miss, 100%) -- all new statements fully exercised. Fixes #739.
JarryShaw
force-pushed
the
fix/739-extractor-registry-identity-guard
branch
from
September 24, 2026 16:06
f0a1f5b to
0071801
Compare
JarryShaw
added
review: pending
Owner
Author
|
Addressed both blockers from the independent review, pushed as Docstring fix, not a code change. The Corrected PR-body numbers, re-derived from a clean Re-ran Label flipped |
Owner
Author
|
✅ GOOD TO MERGE @ |
Owner
Author
|
✅ GOOD TO MERGE @ Independent cross-review, distinct from the self-dispatched one above. Narrow re-check: Blocker resolved. New text, identical in both files: "a re-registration that only changes On pair-vs-dumper — my own judgement, not deference. Comparing
Why the gate was re-run: Non-blocking nits, neither worth a revision on its own:
✅ GOOD TO MERGE @ |
JarryShaw
added
review: good-to-go
JarryShaw
added a commit
that referenced
this pull request
Sep 24, 2026
…one (#750) Library modules imported a *Base class aliased to its public name, so the source read `class PCAP(Engine)` while actually inheriting `EngineBase`. Per the ruling on #514 the split is permanent and library classes inherit the base, so the base is now imported under its own name. * rewrote 28 of the 82 alias imports -- EngineBase 8 of 8, ReassemblyBase 3 of 3, DumperBase 2 of 2, TraceFlowBase 2 of 2, ProtocolBase 13 of 67 * renamed the code and annotation references that followed, including `cast()` targets, `TypeVar(bound=)` and `# type:` comments, but not the string values inside `Literal[...]` * corrected four `docs/source` index pages that named the public class while their own class diagram roots the hierarchy at the base * added `tests/test_base_class_contract.py`, pinning the contract per suite The four non-protocols families are complete. The remaining 54 sites are all `ProtocolBase` ones in paths #726 and #742 own, recorded as prefixes in `PENDING_ALIAS_PATHS` so the entry fails once its pull request lands. No behaviour change: name registry 38 keys before and after, `descendants(Public)` 0 in all five suites, 45 dispatches identical and still identical after clearing the name registry, mypy at its 112-error baseline with none introduced.
JarryShaw
added a commit
that referenced
this pull request
Sep 24, 2026
…arning count Round 8: ran the merge-base/changed-files/cited-path intersection to completion (0c7f2b7..origin/main: 43 commits/124 files; 54 cited paths, 34 of them touched by that range) instead of trusting a tense grep. - :1540/:1542 -- "93 of the 95 sites"/"48 of the 49 record lengths" -> 94 of 95 / 49 of 49; zero old-expression sites remain on main. - :1544-1546, :1566 -- "LOCATOR_SET keeps the old expression ... is currently right" / "is unchanged in both respects" -> past tense; #679 fixed both LOCATOR_SET sites. - :1566 -- "HIP_COPIES stays at two" -> past tense; #679 fixed LOCATOR_SET's Length unit, #689 then dropped HIP_COPIES to one. - :1976-1977 -- "pcapng.txt ... wants a separate refresh" -> past tense; #685 removed it from the index instead of regenerating it. - :2327-2328 -- "55 warnings on main before this change, 56 after" (-b html) -> 53/54; the raw `grep -c WARNING:` double-counts two Scapy import lines as Sphinx warnings, confirmed live on this head with both -b dummy and -b html (real 36/raw 38 with const/reg.rst excluded, same +2 gap either way). - :834 -- stale ``protocol.py:1016`` -> ``:1413`` (the actual ``self._file.read()`` call inside ``_read_fileng``). - :1969 -- the #646 entry's coverage renumbering was wrong twice over (first ``1153 to 1265``, then ``1153 to 1443``, the latter being main's ``def`` line, which always executes and can never be the single miss). Corrected to ``1248 to 1360``, the ``warn(...)`` statement's line before/after #646's own diff. Regenerated CHANGELOG.md from the edited entries. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Follow-up: origin/main advanced through #726/#740/#741/#742 (to 0a3abff) and then #747/#748 (to 074c53e) while this sat at good-to-go; #726 moved three more claims anchored on files it touched. - :834 -- ``protocol.py:1413`` -> ``:1411``; #726 shifted the ``self._file.read()`` call in ``_read_fileng`` by -2 lines. - :2382-83 -- traceflow.py "Line 406" -> "Line 424"; #742 inserted 18 lines above the ``#: Type[Dumper]: Dumper class.`` comment. - :2099-2104, :2187-89 -- the "seven code-keyed parser registrars" and ``Option.register`` are no longer presence-only. #726, fixing #718, gave all seven -- and ``Option.register`` itself -- the same identity guard ``register_protocol`` already had; reworded both passages to say so, confirmed against the guards' own current docstrings. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests.
JarryShaw
added a commit
that referenced
this pull request
Sep 24, 2026
…arning count Round 8: ran the merge-base/changed-files/cited-path intersection to completion (0c7f2b7..origin/main: 43 commits/124 files; 54 cited paths, 34 of them touched by that range) instead of trusting a tense grep. - :1540/:1542 -- "93 of the 95 sites"/"48 of the 49 record lengths" -> 94 of 95 / 49 of 49; zero old-expression sites remain on main. - :1544-1546, :1566 -- "LOCATOR_SET keeps the old expression ... is currently right" / "is unchanged in both respects" -> past tense; #679 fixed both LOCATOR_SET sites. - :1566 -- "HIP_COPIES stays at two" -> past tense; #679 fixed LOCATOR_SET's Length unit, #689 then dropped HIP_COPIES to one. - :1976-1977 -- "pcapng.txt ... wants a separate refresh" -> past tense; #685 removed it from the index instead of regenerating it. - :2327-2328 -- "55 warnings on main before this change, 56 after" (-b html) -> 53/54; the raw `grep -c WARNING:` double-counts two Scapy import lines as Sphinx warnings, confirmed live on this head with both -b dummy and -b html (real 36/raw 38 with const/reg.rst excluded, same +2 gap either way). - :834 -- stale ``protocol.py:1016`` -> ``:1413`` (the actual ``self._file.read()`` call inside ``_read_fileng``). - :1969 -- the #646 entry's coverage renumbering was wrong twice over (first ``1153 to 1265``, then ``1153 to 1443``, the latter being main's ``def`` line, which always executes and can never be the single miss). Corrected to ``1248 to 1360``, the ``warn(...)`` statement's line before/after #646's own diff. Regenerated CHANGELOG.md from the edited entries. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Follow-up: origin/main advanced through #726/#740/#741/#742 (to 0a3abff) and then #747/#748 (to 074c53e) while this sat at good-to-go; #726 moved three more claims anchored on files it touched. - :834 -- ``protocol.py:1413`` -> ``:1411``; #726 shifted the ``self._file.read()`` call in ``_read_fileng`` by -2 lines. - :2382-83 -- traceflow.py "Line 406" -> "Line 424"; #742 inserted 18 lines above the ``#: Type[Dumper]: Dumper class.`` comment. - :2099-2104, :2187-89 -- the "seven code-keyed parser registrars" and ``Option.register`` are no longer presence-only. #726, fixing #718, gave all seven -- and ``Option.register`` itself -- the same identity guard ``register_protocol`` already had; reworded both passages to say so, confirmed against the guards' own current docstrings. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at 948ac49 came back NEEDS CHANGES: the round-12 edit fixed two sites of the harmonisation claim and left its twin, plus its own reasoning, asserting the opposite; and four numbers anchored on files the merges touched had drifted independently of #726. - :1877-1878, :1883-1884 (#675) -- "carries the guarded ``if code in cls.__xxx__: warn(...)``" / "every sibling warns on mere presence" -> past tense, noting #726 later gave all seven the identity guard this entry's own comparison assumes they lack. - :2100-2109 -- dropped the retained "yields two keys and never reaches one key twice" (false: ``Internet.register(TransType.TCP, TCP)`` warns once, incumbent.klass is TCP) and "leaves a different-class test undecidable" (contradicted by :2404-2406's own ``incumbent is not protocol`` definition); replaced with the actual false positive the guard has -- pre-seeded ``ModuleDescriptor`` incumbents never compare equal to the resolved class. - :2192 -- reflowed the ``Option.register`` paragraph (orphan lines fixed alongside). - :2387-2388 -- the ``Type[Dumper]`` quote now matches what is actually at line 424 (post-#709-fix), rather than the pre-fix bare form. - :945 -- ``README.md`` (103) -> (102). - :1136 -- "75 of the 117 modules" -> "77 ... after #647 below adds the same ending to three more" (drifted via #647, independent of the four merges). - :2119 -- dropped the irreproducible pylint "364 messages" figure; kept mypy's 112, which does reproduce. - :2119 -- "326 registry writes" -> 327 (``R1CounterParameter``'s second code, from #690). Also fixed six false claims in the PR body (separate from the .rst): hunk/line counts, six-commits -> 46, the 5-row table's implied total, "not trimmed", main's red/green state, and the now-unreachable cherry-pick target. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests.
5 tasks
JarryShaw
added a commit
that referenced
this pull request
Sep 24, 2026
…arning count Round 8: ran the merge-base/changed-files/cited-path intersection to completion (0c7f2b7..origin/main: 43 commits/124 files; 54 cited paths, 34 of them touched by that range) instead of trusting a tense grep. - :1540/:1542 -- "93 of the 95 sites"/"48 of the 49 record lengths" -> 94 of 95 / 49 of 49; zero old-expression sites remain on main. - :1544-1546, :1566 -- "LOCATOR_SET keeps the old expression ... is currently right" / "is unchanged in both respects" -> past tense; #679 fixed both LOCATOR_SET sites. - :1566 -- "HIP_COPIES stays at two" -> past tense; #679 fixed LOCATOR_SET's Length unit, #689 then dropped HIP_COPIES to one. - :1976-1977 -- "pcapng.txt ... wants a separate refresh" -> past tense; #685 removed it from the index instead of regenerating it. - :2327-2328 -- "55 warnings on main before this change, 56 after" (-b html) -> 53/54; the raw `grep -c WARNING:` double-counts two Scapy import lines as Sphinx warnings, confirmed live on this head with both -b dummy and -b html (real 36/raw 38 with const/reg.rst excluded, same +2 gap either way). - :834 -- stale ``protocol.py:1016`` -> ``:1413`` (the actual ``self._file.read()`` call inside ``_read_fileng``). - :1969 -- the #646 entry's coverage renumbering was wrong twice over (first ``1153 to 1265``, then ``1153 to 1443``, the latter being main's ``def`` line, which always executes and can never be the single miss). Corrected to ``1248 to 1360``, the ``warn(...)`` statement's line before/after #646's own diff. Regenerated CHANGELOG.md from the edited entries. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Follow-up: origin/main advanced through #726/#740/#741/#742 (to 0a3abff) and then #747/#748 (to 074c53e) while this sat at good-to-go; #726 moved three more claims anchored on files it touched. - :834 -- ``protocol.py:1413`` -> ``:1411``; #726 shifted the ``self._file.read()`` call in ``_read_fileng`` by -2 lines. - :2382-83 -- traceflow.py "Line 406" -> "Line 424"; #742 inserted 18 lines above the ``#: Type[Dumper]: Dumper class.`` comment. - :2099-2104, :2187-89 -- the "seven code-keyed parser registrars" and ``Option.register`` are no longer presence-only. #726, fixing #718, gave all seven -- and ``Option.register`` itself -- the same identity guard ``register_protocol`` already had; reworded both passages to say so, confirmed against the guards' own current docstrings. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at 948ac49 came back NEEDS CHANGES: the round-12 edit fixed two sites of the harmonisation claim and left its twin, plus its own reasoning, asserting the opposite; and four numbers anchored on files the merges touched had drifted independently of #726. - :1877-1878, :1883-1884 (#675) -- "carries the guarded ``if code in cls.__xxx__: warn(...)``" / "every sibling warns on mere presence" -> past tense, noting #726 later gave all seven the identity guard this entry's own comparison assumes they lack. - :2100-2109 -- dropped the retained "yields two keys and never reaches one key twice" (false: ``Internet.register(TransType.TCP, TCP)`` warns once, incumbent.klass is TCP) and "leaves a different-class test undecidable" (contradicted by :2404-2406's own ``incumbent is not protocol`` definition); replaced with the actual false positive the guard has -- pre-seeded ``ModuleDescriptor`` incumbents never compare equal to the resolved class. - :2192 -- reflowed the ``Option.register`` paragraph (orphan lines fixed alongside). - :2387-2388 -- the ``Type[Dumper]`` quote now matches what is actually at line 424 (post-#709-fix), rather than the pre-fix bare form. - :945 -- ``README.md`` (103) -> (102). - :1136 -- "75 of the 117 modules" -> "77 ... after #647 below adds the same ending to three more" (drifted via #647, independent of the four merges). - :2119 -- dropped the irreproducible pylint "364 messages" figure; kept mypy's 112, which does reproduce. - :2119 -- "326 registry writes" -> 327 (``R1CounterParameter``'s second code, from #690). Also fixed six false claims in the PR body (separate from the .rst): hunk/line counts, six-commits -> 46, the 5-row table's implied total, "not trimmed", main's red/green state, and the now-unreachable cherry-pick target. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at 1749cc0 came back NEEDS CHANGES: round 13 fixed five of the nine sites and introduced four new false claims doing it, including two inside the flagship rewrite -- swapping one inaccuracy for another is this document's recurring failure mode. - :1136-37 -- "75 ... 77 now, after #647 ... adds ... three more" was internally inconsistent (75+3=78, not 77). Traced #647's own diff (fc32d1b): it adds ``_missing_`` to three IntFlag classes across only two *new* files -- ``tcp/flags.py`` and ``ftp/command.py`` -- since the third, ``TransportProtocol``, shares ``reg/apptype.py`` with the already-counted ``AppType``. Module delta is +2, matching 75+2=77; reworded to say so. - :1879-88 -- dropped "the comparison below assumes they still lack" it, which was false about text 8 lines below in the same diff (already past-tensed). Also reflowed three orphan lines this introduced (`passes, whereas`, `it twice with nothing`, `none of the`). - :2107-19 -- "These tables also ship pre-seeded" over-generalised: verified live (``ProtocolBase.__proto__`` is 0 entries, ``Transport.__proto__ is ProtocolBase.__proto__`` -- True) that 2 of 7 have nothing pre-seeded. Scoped to the five that do (Link 7, Internet 16, Frame 3, PCAPNG 3, SCTP 2). Also fixed "the guard resolves only the incoming class", which contradicts the guard's own docstring ("the comparison itself resolves nothing") -- resolution is the earlier ``isinstance(protocol, ModuleDescriptor)`` step, three lines above the guard, not something the guard does. - :2129-30 -- dropped the invented "327th" ordinal (327 total stays; traced-write instrumentation via ``sys`` hooks found the seeding is literal dict construction, not ``.register()`` calls, so I could not reproduce an ordinal with confidence -- said "one of them" instead of guessing). - :7-8 -- "between #326 and #509" now says the programme continued past it (verified: 193 distinct #nnn refs, max #726, 103 above 509). - PR body -- "7 hunks, 1168+/11-" was the previous head's figure, not this one's; replaced with the actual command (``git diff --shortstat da697fa -- docs/source/changelog/1.5.0.rst``) and today's figure (9 hunks, 1152+/16-), since a hardcoded count here has now gone stale twice. Left alone per this round's scope: :1969/:1974 (before/after claim, not falsified by #726's later +1), mypy "112" (correct, re-ran with the project's own flags), ":2122" 13-to-14 (correct at its delta scope), and the other 121 cited paths (unaffected by main's one new commit, #745, confirmed test-only). Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests.
JarryShaw
added a commit
that referenced
this pull request
Sep 24, 2026
…arning count Round 8: ran the merge-base/changed-files/cited-path intersection to completion (0c7f2b7..origin/main: 43 commits/124 files; 54 cited paths, 34 of them touched by that range) instead of trusting a tense grep. - :1540/:1542 -- "93 of the 95 sites"/"48 of the 49 record lengths" -> 94 of 95 / 49 of 49; zero old-expression sites remain on main. - :1544-1546, :1566 -- "LOCATOR_SET keeps the old expression ... is currently right" / "is unchanged in both respects" -> past tense; #679 fixed both LOCATOR_SET sites. - :1566 -- "HIP_COPIES stays at two" -> past tense; #679 fixed LOCATOR_SET's Length unit, #689 then dropped HIP_COPIES to one. - :1976-1977 -- "pcapng.txt ... wants a separate refresh" -> past tense; #685 removed it from the index instead of regenerating it. - :2327-2328 -- "55 warnings on main before this change, 56 after" (-b html) -> 53/54; the raw `grep -c WARNING:` double-counts two Scapy import lines as Sphinx warnings, confirmed live on this head with both -b dummy and -b html (real 36/raw 38 with const/reg.rst excluded, same +2 gap either way). - :834 -- stale ``protocol.py:1016`` -> ``:1413`` (the actual ``self._file.read()`` call inside ``_read_fileng``). - :1969 -- the #646 entry's coverage renumbering was wrong twice over (first ``1153 to 1265``, then ``1153 to 1443``, the latter being main's ``def`` line, which always executes and can never be the single miss). Corrected to ``1248 to 1360``, the ``warn(...)`` statement's line before/after #646's own diff. Regenerated CHANGELOG.md from the edited entries. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Follow-up: origin/main advanced through #726/#740/#741/#742 (to 0a3abff) and then #747/#748 (to 074c53e) while this sat at good-to-go; #726 moved three more claims anchored on files it touched. - :834 -- ``protocol.py:1413`` -> ``:1411``; #726 shifted the ``self._file.read()`` call in ``_read_fileng`` by -2 lines. - :2382-83 -- traceflow.py "Line 406" -> "Line 424"; #742 inserted 18 lines above the ``#: Type[Dumper]: Dumper class.`` comment. - :2099-2104, :2187-89 -- the "seven code-keyed parser registrars" and ``Option.register`` are no longer presence-only. #726, fixing #718, gave all seven -- and ``Option.register`` itself -- the same identity guard ``register_protocol`` already had; reworded both passages to say so, confirmed against the guards' own current docstrings. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at 948ac49 came back NEEDS CHANGES: the round-12 edit fixed two sites of the harmonisation claim and left its twin, plus its own reasoning, asserting the opposite; and four numbers anchored on files the merges touched had drifted independently of #726. - :1877-1878, :1883-1884 (#675) -- "carries the guarded ``if code in cls.__xxx__: warn(...)``" / "every sibling warns on mere presence" -> past tense, noting #726 later gave all seven the identity guard this entry's own comparison assumes they lack. - :2100-2109 -- dropped the retained "yields two keys and never reaches one key twice" (false: ``Internet.register(TransType.TCP, TCP)`` warns once, incumbent.klass is TCP) and "leaves a different-class test undecidable" (contradicted by :2404-2406's own ``incumbent is not protocol`` definition); replaced with the actual false positive the guard has -- pre-seeded ``ModuleDescriptor`` incumbents never compare equal to the resolved class. - :2192 -- reflowed the ``Option.register`` paragraph (orphan lines fixed alongside). - :2387-2388 -- the ``Type[Dumper]`` quote now matches what is actually at line 424 (post-#709-fix), rather than the pre-fix bare form. - :945 -- ``README.md`` (103) -> (102). - :1136 -- "75 of the 117 modules" -> "77 ... after #647 below adds the same ending to three more" (drifted via #647, independent of the four merges). - :2119 -- dropped the irreproducible pylint "364 messages" figure; kept mypy's 112, which does reproduce. - :2119 -- "326 registry writes" -> 327 (``R1CounterParameter``'s second code, from #690). Also fixed six false claims in the PR body (separate from the .rst): hunk/line counts, six-commits -> 46, the 5-row table's implied total, "not trimmed", main's red/green state, and the now-unreachable cherry-pick target. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at 1749cc0 came back NEEDS CHANGES: round 13 fixed five of the nine sites and introduced four new false claims doing it, including two inside the flagship rewrite -- swapping one inaccuracy for another is this document's recurring failure mode. - :1136-37 -- "75 ... 77 now, after #647 ... adds ... three more" was internally inconsistent (75+3=78, not 77). Traced #647's own diff (fc32d1b): it adds ``_missing_`` to three IntFlag classes across only two *new* files -- ``tcp/flags.py`` and ``ftp/command.py`` -- since the third, ``TransportProtocol``, shares ``reg/apptype.py`` with the already-counted ``AppType``. Module delta is +2, matching 75+2=77; reworded to say so. - :1879-88 -- dropped "the comparison below assumes they still lack" it, which was false about text 8 lines below in the same diff (already past-tensed). Also reflowed three orphan lines this introduced (`passes, whereas`, `it twice with nothing`, `none of the`). - :2107-19 -- "These tables also ship pre-seeded" over-generalised: verified live (``ProtocolBase.__proto__`` is 0 entries, ``Transport.__proto__ is ProtocolBase.__proto__`` -- True) that 2 of 7 have nothing pre-seeded. Scoped to the five that do (Link 7, Internet 16, Frame 3, PCAPNG 3, SCTP 2). Also fixed "the guard resolves only the incoming class", which contradicts the guard's own docstring ("the comparison itself resolves nothing") -- resolution is the earlier ``isinstance(protocol, ModuleDescriptor)`` step, three lines above the guard, not something the guard does. - :2129-30 -- dropped the invented "327th" ordinal (327 total stays; traced-write instrumentation via ``sys`` hooks found the seeding is literal dict construction, not ``.register()`` calls, so I could not reproduce an ordinal with confidence -- said "one of them" instead of guessing). - :7-8 -- "between #326 and #509" now says the programme continued past it (verified: 193 distinct #nnn refs, max #726, 103 above 509). - PR body -- "7 hunks, 1168+/11-" was the previous head's figure, not this one's; replaced with the actual command (``git diff --shortstat da697fa -- docs/source/changelog/1.5.0.rst``) and today's figure (9 hunks, 1152+/16-), since a hardcoded count here has now gone stale twice. Left alone per this round's scope: :1969/:1974 (before/after claim, not falsified by #726's later +1), mypy "112" (correct, re-ran with the project's own flags), ":2122" 13-to-14 (correct at its delta scope), and the other 121 cited paths (unaffected by main's one new commit, #745, confirmed test-only). Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at b2ac58b came back NEEDS CHANGES: round 15 fixed four sites clean but swapped in two new inaccuracies, and left one round-fourteen defect (body :26) unfixed. - :7 -- "past #726" was wrong direction: #726 is the max ref in the document (193 distinct, min #251, max #726), not one exceeded -> "reaching #726". - :2110-19 -- "ProtocolBase and Transport share one dict, starting and staying empty until a subclass registers" was false three ways, verified live against origin/main (pcapkit.__file__ asserted): Transport.register() itself raises UnsupportedCall (abstract); TCP and UDP keep their own separate __proto__ (4 and 3 entries), not the shared one, so registering on them leaves the shared dict at 0; only a direct ProtocolBase.register() call fills it. Narrowing to "five of these seven" also hid that TCP/UDP are pre-seeded too, which is exactly where the false positive bites in the transport family -- restored that. - body :26 -- "26 entry commits" -> 27 (commits whose subject starts "docs(changelog): the 1.5.0 entry/entries for", verified by grep), 28 bullets added and 0 removed (verified via the .rst diff against da697fa; one commit, 6a956c4, adds two bullets for #648/#649). - body :41 -- dropped the hardcoded "9 hunks, 1152+/16-" figure entirely (it had already drifted to 1154+ by the time of this commit) and named the second command needed for the hunk count, since --shortstat cannot print one. On the ordinal question raised last round: dropping it was still right (the asserted "327th" was wrong), but "no ordinal is derivable" does not hold either -- the writes are at pcapkit/protocols/schema/schema.py, not the 8 dict-literal registrar sites my instrumentation covered, and they are traceable. Left the text as "one of them being R1CounterParameter's second code" (no ordinal asserted, no false derivability claim either) rather than reopen a site outside this round's scope. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests.
JarryShaw
added a commit
that referenced
this pull request
Sep 24, 2026
…st 54 sites (#752) Completes #514 part (c). #750 renamed 28 of 82 `ProtocolBase as Protocol` alias imports and deferred the remaining 54 -- all `ProtocolBase` -- to paths #726 and #742 owned. Both have merged, so the deferral is over. * renamed the alias import at all 54 sites (51 under pcapkit/protocols/, 3 under pcapkit/foundation/) and every in-file reference that used the local alias: class headers, annotations, cast(), isinstance/issubclass checks, # type: comments, and the bracketed part of a handful of Sphinx #: doc comments -- 191 lines changed, no statements added * merged two now-unaliased same-module imports per isort in 4 files (application.py, internet.py, link.py, transport.py), removing 4 statements * left descriptive prose, protocol-name string literals, error-message text, and fully-qualified :class:/:meth:/:rtype: cross-references to the real public Protocol class untouched, matching #750's own precedent * emptied tests/test_base_class_contract.py's PENDING_ALIAS_PATHS, its documented end state, and updated the stale docstring narrative No behaviour change: __mro__/__bases__/__module__ identical across 18 classes spanning every family, __proto__ registry 38 keys before and after, descendants(Protocol) 0 in both. mypy stays at the 112-error baseline.
JarryShaw
removed
the
review: good-to-go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
make pylint,make mypy,make isort) — ran targeted against the changed files only, see belowmake testpasses, and a test case covers the change — ran the targeted suite only (see below), not the fullmake testdocs/source/changelog/and regeneratedCHANGELOG.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) #657What is the purpose of your pull request?
fix— corrects a defectDescription of your pull request and other information
Five registrars warned on mere key presence, so re-registering the exact same class/descriptor read as "overwriting X" even though nothing was displaced. Issue #718 gave ten code-keyed registrars an identity guard (
incumbent is not None and incumbent is not new, landed in PR #726); these five sit outside its wording ("names only the key") but share the defect.register_engineextraction.pyif name in cls.__engine__:incumbent = cls.__engine__.get(name); if incumbent is not None and incumbent is not engine:register_reassemblyextraction.pyif protocol in cls.__reassembly__:cls.__reassembly__register_traceflowextraction.pyif protocol in cls.__traceflow__:cls.__traceflow__register_dumperextraction.pyif format in cls.__output__:0of the stored(dumper, ext)pairregister_dumpertraceflow/traceflow.py__output__Matched against PR #726's converted
ProtocolBase.register(the diff, notmain, which doesn't have it yet). Message text unchanged throughout.Measured, not assumed: at import time
__engine__(6),__reassembly__(3),__traceflow__(1), and both__output__s (8 each) hold onlyModuleDescriptors — zero classes. So the first real registration still legitimately warns; only a repeat is now silent — narrower than #718's win, as the issue predicted.__engine__/__reassembly__/__traceflow__are plaindicts. Both__output__s arecollections.defaultdict(format -> (dumper, ext)): confirmed.get()/inon a miss leavelen()unchanged, whiled[missing]inserts the default-factory tuple (+1)..get()stays non-inserting; the identity check compares the incumbent dumper only, so a re-registration that changes justextis identity-equal on the dumper and stays silent too — judged defensible rather than switching to a full-pair comparison.Judged non-breaking: a correct caller sees strictly fewer warnings, never a different return value or exception; not ticking
breaking.5 new test methods (one per registrar) pin: same-object re-registration silent, different-object still warns with unchanged message, registry write happens both ways (plus a same-dumper/different-
extcase for the two dumper sites). Reverted each guard hunk alone and confirmed all five fail withAssertionError: Expected 'warn' to not have been called. Called 1 times.Ran
tests/foundation/test_extraction.py+tests/foundation/traceflow/test_traceflow_base.py: 16 → 21 passed (28 subtests, unchanged). Coverage:extraction.py367 → 372 stmts (miss unchanged at 7, 96%),traceflow.py121 → 123 stmts (0 miss, 100%) — all new statements exercised.Fixes #739.