Skip to content

feat(reg): expose AppType aliases in repr/str and allow registering them per transport (#807) - #837

Merged
JarryShaw merged 1 commit into
mainfrom
feat/807-apptype-aliases
Sep 26, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
feat/807-apptype-aliases

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

  • Searched for similar pull requests
  • Followed the coding style (make mypy, make isort; ran both directly against the changed files)
  • make test passes, and a test case covers the change (scoped to tests/const/, tests/foundation/registry/, tests/vendor/ per the affected trees; tests/dumpkit/ also run and shows six pre-existing, unrelated failures from missing generated sample captures)
  • Added a changelog entry under docs/source/changelog/ — N/A, changelog centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

  • feat — adds a feature

Description of your pull request and other information

Implements #807, step 2 of #801's three: a public .aliases property, repr/str exposing aliases when there are any, and a way to register an alias on a per-transport registry.

.aliases. No new storage: AppType.__registry__.getlist(self.port) minus self is exactly the bucket get_all already walks to answer with the aliases after the canonical member, so .aliases is that same bucket. Returns an empty tuple — never None — when nothing else collides. TCP.http.aliases gives (TCP.www, TCP.www_http); SCTP.get(80).aliases is (), because IANA never registered www on SCTP.

repr/str. Grow an (aliases: ...) suffix, computed fresh on every call, only when .aliases is non-empty:

>>> repr(TCP.finger)      # no alias: byte-identical to before #807
'<TCP.finger: 79 [tcp]>'
>>> repr(TCP.http)
'<TCP.http: 80 [tcp] (aliases: www, www-http)>'
>>> str(TCP.http)
'http [80 - tcp] (aliases: www, www-http)'

_value_ is deliberately untouched — it is the live key in _value2member_map_, built once at class creation, and aliases are registerable at runtime, so baking the list into _value_ would go stale the moment one is registered. str(member) == member.value holds when a member is minted and stops holding the moment a sibling is later registered as its alias.

Registering an alias: register_alias(port, name), a classmethod scoped to one per-transport registry. The maintainer's own signature sketches were explicitly "just POC not how it should actually look like," and his final ruling was the operative one: "alias should thus be per-transport, and we should probably allow registering aliases for a given enum in the transport apptype enum." So the mechanism lives directly on TCP/UDP/SCTP/DCCP rather than on a shared entry point that takes a transport argument — TCP.register_alias(...) cannot touch UDP's or SCTP's members by construction, which is a stronger guarantee than a proto= keyword would give and matches how .aliases itself is already per-registry. It requires port to already carry a member of that registry (aliasing names a second service on a port that already has one, rather than declaring a fresh one out of nothing — that stays the generator's job, or get's own mint-on-miss), and it refuses a name already registered on that port. It mints through the same extend_enum call get's mint-on-miss fallback already uses, with the Python attribute name derived from the port and a counter rather than from name — sanitising arbitrary caller text into an identifier would duplicate the generator's own curated dedup logic for a case nothing in the library needs, since no lookup path reaches an alias by its attribute name.

pcapkit/foundation/registry/protocols.py is deliberately left untouched. Its register_apptype only ever writes to {tcp, udp} protocol-class registries (pcapkit.protocols.transport.{tcp,udp}, a different pair of registries entirely from pcapkit.const.reg.apptype), so an aliases= keyword there would be strictly weaker than the general, all-four-transport register_alias above, and would conflate registering a protocol handler with registering an enum member.

Generator agreement, verified rather than asserted. Applied identically to the vendor template's BASE lambda. Verified by rendering BASE(NAME, DOCS, FLAG, TABLE, MISS, MODL) with the real TABLE/MISS blobs extracted from the previously-committed const file (by template-matching the old BASE against it) and diffing the result against the committed file end to end — the only differences are the new aliases property, the new register_alias classmethod, and the __repr__/__str__ bodies, with the 59,366-character docstring table and the 134,846-character _missing_ branch list byte-identical either side. The four per-transport const files (tcp.py, udp.py, sctp.py, dccp.py) need no changes: none of them override __repr__, __str__, or declare their own aliases/register_alias, so all three are inherited unchanged from the base.

Two existing sweeps over all 12,391 members (test_the_new_dunders_are_byte_identical_to_the_percent_form, test_every_member_renders_its_own_registrys_transport_protocol) pinned the pre-#807 repr/str text for every member, aliased or not, and would otherwise fail for the ~94 members on the 44 colliding ports. Both are updated to append the identical suffix through member.aliases before comparing, while keeping the _value_/lookup round-trip checks on the un-suffixed form.

Six new tests cover .aliases, the repr/str suffix (with and without aliases), and every register_alias path — success, called on the base registry, called on a port with no canonical member yet, and a duplicate name. Each was confirmed to fail against the pre-fix code with the expected AttributeError/AssertionError before the fix landed, and to pass after.

Coverage of pcapkit/const/reg/apptype/apptype.py, measured with a standalone coveragerc pointing source at this worktree's absolute pcapkit path and a distinct COVERAGE_FILE per run (coverage run under tests/const/test_const_apptype_split_unit.py, filtered at coverage report --include=... time): 52.65% (855/1624 statements) before this change's tests, 53.59% (888/1657) after — all 33 new statements covered, the pre-existing 769 misses (unrelated dunder comparisons and _missing_ branches never exercised) unchanged in count.

tests/const/, tests/foundation/registry/ and tests/vendor/ pass in full: 187 tests, 39,468 subtests. tests/dumpkit/ was also run per the affected-trees list; six failures there are pre-existing and unrelated — FileNotFoundError for generated sample captures (test.pcapng etc.) that this checkout never generated, in a test file with no reference to AppType or TransportProtocol.

@JarryShaw JarryShaw added enhancement Issues requesting a new capability (set by the feature request template) feat Pull requests that add a new capability (feat: subject prefix) test Pull requests that add or correct tests (test: subject prefix) const Regenerated IANA or vendor constant tables; members keep their numeric values review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 26, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES — one finding. The three requirements are met and the per-transport semantics are exactly as ruled, but a registered alias cannot be looked up by the name it was registered under.

Verified on fd2b9b983:

TCP['http']            <TCP.http: 80 [tcp] (aliases: www, www-http)>
TCP['http'].aliases    (<TCP.www: …>, <TCP.www-http: …>)
SCTP['http'].aliases   ()                      <- per-transport, as ruled
unaliased member       <TCP.reserved: 0 [tcp]>  <- byte-identical, no suffix
register_alias(80,…)   TCP 6147 -> 6148,  UDP 6143 -> 6143 unchanged
duplicate name         ValueError: 'http' is already registered on port 80 of TCP
unknown port           ValueError: 65432 is not yet a member of TCP…

The finding. register_alias(80, 'http-alt-test') mints a member named ALIAS_80_tcp_3, keeping the requested name only in svc. So:

TCP['http-alt-test']      KeyError
TCP.get('http-alt-test')  ValueError: 'http-alt-test' is not a valid port number for TCP

An alias whose name cannot resolve is most of the point of an alias. And it departs from this file's own convention — pcapkit/const/reg/apptype/tcp.py:1264 is www_http = 80, 'www-http', TransportProtocol.tcp, i.e. the generator identifier-normalises the service name into the member name and keeps the real string as the second field. Following that, register_alias(80, 'http-alt-test') should be reachable as TCP['http_alt_test']. Sent back to do that and to pin the lookup in a test.

The deliberate divergence from my brief is correct, and I checked it rather than taking it on trust. I had pointed at register_apptype for requirement 3; the worker declined, and it is right. That function writes to pcapkit.protocols.transport.tcp.TCP.__proto__ and …udp.UDP.__proto__ — the protocol-class registries, and only two of the four transports — which is a different registry family from pcapkit.const.reg.apptype.{TCP,UDP,SCTP,DCCP}. An aliases= kwarg there would have been strictly weaker and would have conflated the two. register_alias as a classmethod on each const enum covers all four and cannot touch a sibling by construction.

Also confirmed: the four per-transport const files need no changes; .aliases derives from the bucket get_all already walks rather than adding storage; the 6 tests/dumpkit/ failures are the known missing-fixture ones, identical on main.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 26, 2026
@JarryShaw
JarryShaw force-pushed the feat/807-apptype-aliases branch from fd2b9b9 to 976c949 Compare September 26, 2026 15:11
@JarryShaw

Copy link
Copy Markdown
Owner Author

Finding fixed in 976c949f9 (one commit, parent 14d3d3dc7). Verified every branch myself:

TCP['http_alt_test']   <TCP.http-alt-test: 80 [tcp] (aliases: …)>   member normalised, svc kept verbatim
TCP['class_']          resolves                                      keyword guard
'finger' collision     ValueError: 'finger' already names a member of TCP…  and TCP.finger still port 79
'...'  /  '80s-club'   ValueError: … has no valid Python identifier …
ALIAS_ counter names   []                                            scheme gone entirely

The normalisation matches Vendor.safe_name at pcapkit/vendor/default.py:260, reproduced rather than imported — pcapkit.const importing pcapkit.vendor would invert the layering, since const is generated from vendor, and safe_name is a self-contained regex transform that touches no crawler state. I agree with that call. rename()'s further _{port} dedup was deliberately not reproduced, which is right: that is a curated full-corpus decision at generation time, and synthesising it here would be the silent rename the collision refusal exists to prevent.

One trap it caught and fixed on its own, worth recording because #775 will hit it across 113 registries. Its first draft put r'\(.*\)' and r'\W' directly into the generator's non-raw f-string template, which Python flags as an invalid escape sequence at vendor-file compile time — not in the generated output. Confirmed fixed and confirmed by me:

const  (emitted)    re.sub(r'\(.*\)', …)      re.sub(r'\W', …)      single backslash
vendor (template)   re.sub(r'\\(.*\\)', …)    re.sub(r'\\W', …)     doubled
py_compile(..., doraise=True) under -W error::SyntaxWarning: both files clean

Generator/const agreement re-verified by rendering BASE(...) and diffing — only the import keyword / import re lines and the register_alias body differ. Coverage on the const file 52.65% → 53.59%, all new statements covered; 191 passed / 39,468 subtests across tests/const/, tests/foundation/registry/, tests/vendor/; mypy clean on both touched files; the six tests/dumpkit/ failures are the known missing-fixture ones, identical on main.

Head is new so no verdict carries: review: pending, cross-review dispatched on a different model from the author.

@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Sep 26, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES — cross-review (opus, a different model from the author) on 976c949f9. Two findings, both prose-only; the code is correct and tests/const/ is 87 run / 0 fail. I re-derived both numbers myself before posting.

F1 — the inline safe_name copy in register_alias drops Vendor.safe_name's NAME_ fallback, and the NOTE claims it mirrors it "exactly". pcapkit/vendor/default.py:260 ends if temp.isidentifier(): return temp / return f'{self.NAME}_{temp}'; the copy raises instead. Measured: the generator takes that branch 58 times in the shipped const — 28 TCP, 29 UDP, 1 SCTP, 0 DCCP — e.g. pcapkit/const/reg/apptype/tcp.py:1360 TCP_3com_tsmux = 106, '3com-tsmux'. So TCP.register_alias(80, '3com-tsmux') raises where the generator succeeds. Not a split-brain — it is a loud ValueError the Raises: section describes — but "exactly", and the docstring's "the same way the generator derives every statically declared member's", are false. Either strike both claims and flag the omission the way the same comment already flags rename's dedup, or adopt the fallback using cls.__name__ (the shipped prefix is TCP_/UDP_/SCTP_, per-registry, not AppType_). Keeping the raise is defensible — an empty residue would yield a bare TCP_ — and test_register_alias_refuses_an_unsanitisable_name pins the current choice, so a behaviour change needs that test updated too.

F2 — __str__'s docstring mis-states when str(m) == m.value breaks. It says the invariant "holds when a member is minted and stops holding the moment a sibling is later registered as its alias". Measured at import with zero runtime registration: 94 of 12,391 members already have str(m) != m.value (TCP 49, UDP 45, SCTP 0, DCCP 0), because their alias siblings are statically declared — str(TCP.http) is 'http [80 - tcp] (aliases: www, www-http)' against .value 'http [80 - tcp]'. It breaks at class creation, not at first register_alias.

Confirmed, and it settles the sequencing question: register_alias's extend_enum is not a site #775 must undo. #775 exempts caller-named members in its own text — "unless user/caller explicitly created them" — and targets the mint-on-miss paths; its unregistered-member mechanism cannot satisfy TCP(t.value) is t or TCP['name'] is t, and would make .aliases blind to the member. #775 stays sequenced behind this PR. Per-transport isolation confirmed adversarially (same name on two transports gives distinct members, no leak into SCTP/DCCP/AppType; __registry__ and __canonical__ are 4 distinct objects each). Generator/const agreement confirmed by rendering the template and comparing ast source segments — aliases, __repr__, __str__, register_alias, __new__, get, get_all all byte-identical. .aliases invariants hold across all 12,391 members, 0 violations. No doctest, writer or doc pins repr/str — 5 grep hits, all in the test file under test.

The 52.65% → 53.59% coverage figures are UNVERIFIED (budget spent on F1); direction is not in doubt.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 26, 2026
@JarryShaw
JarryShaw force-pushed the feat/807-apptype-aliases branch 2 times, most recently from fe543c3 to 7898476 Compare September 26, 2026 16:03
@JarryShaw

Copy link
Copy Markdown
Owner Author

Both findings fixed in 7898476aa (one commit, parent 14d3d3dc7), and I verified every claim on the pushed head myself rather than taking the report:

  • F1 — "mirrors Vendor.safe_name … exactly" is gone; the comment now says "mirrors … Vendor.safe_name's sanitising steps" and flags the omission in the same form as the existing rename-dedup sentence: "safe_name's own fallback — minting {cls.__name__}_{residue} … — is not reproduced either: this method raises instead (see Raises below), so the 58 members the generator mints that way in the shipped const (28 TCP, 29 UDP, 1 SCTP, 0 DCCP) are names this runtime path refuses rather than accepts." The docstring's "the same way the generator derives every statically declared member's" is now "through the same sanitising steps … reproduced below (minus its fallback; see Raises below)". Raises: untouched, behaviour untouched, no test changed.
  • F2 — __str__'s comment now reads "already breaks at class-creation time for every port that statically carries more than one service (94 such members as shipped); register_alias merely adds to that set at runtime."
  • One thing I fixed on top of the worker's revision: it wrote "the ~58 members" while its own parenthetical gives the exact 28/29/1/0. The tilde contradicted the breakdown, so it is now "the 58 members" — that is the whole of fe543c3d1 → 7898476aa.

Verified on the pushed tree (scratch worktree at the head, PYTHONSAFEPATH=1, pcapkit.__file__ asserted):

  • Render-vs-const agreement by AST source segment, not grep: aliases, __repr__, __str__, register_alias, __new__, get, get_all all IDENTICAL; class body 23 members both sides.
  • python -W error::SyntaxWarning -m py_compile clean on both the vendor template and the const file — no mis-doubled brace or backslash.
  • tests/const/ under plain unittest: 87 run, OK, 0 failures / 0 errors / 0 skipped.
  • Both of my measurements unchanged, confirming no behaviour moved: 58 NAME_-prefixed const members (28/29/1/0) and 94 of 12,391 members with str(m) != m.value at import (TCP 49, UDP 45, SCTP 0, DCCP 0).
  • Still one commit, author and committer Jarry Shaw <jarryshaw@icloud.com>, and the diff from 976c949f9 touches only pcapkit/vendor/reg/apptype/apptype.py and pcapkit/const/reg/apptype/apptype.py.

Head is new, so the previous verdict does not carry: back to review: pending and a fresh cross-review dispatched on a different model from the author. Awaiting CI on 7898476aa.

@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Sep 26, 2026
…hem per transport (#807)

- Add a public `.aliases` property on every `AppType` member. No new
  storage: derived as `__registry__.getlist(self.port)` minus `self`,
  which is exactly the bucket `get_all` already walks -- an empty tuple
  when nothing collides, never `None`.
- Grow `__repr__`/`__str__` an `(aliases: ...)` suffix when `.aliases` is
  non-empty; unaliased members (the vast majority) render byte-identical
  to before. `_value_` deliberately stays untouched, since it is the live
  key in `_value2member_map_` and aliases are registerable at runtime.
- Add `register_alias(port, name)`, a classmethod scoped to one
  per-transport registry at a time -- `TCP.register_alias(...)` never
  touches `UDP`'s or `SCTP`'s members, matching the maintainer's ruling
  that "alias should ... be per-transport". Requires a canonical member
  already on `port`; mints via the same `extend_enum` call `get`'s own
  mint-on-miss already uses.
- The Python attribute name is sanitised from `name` the same way
  `Vendor.safe_name` (pcapkit/vendor/default.py) sanitises every
  statically declared member's -- `www-http` becomes `www_http` -- so
  `TCP.register_alias(80, 'http-alt-test')` lands as `TCP['http_alt_test']`
  with `svc` still reading the unsanitised string. Refuses explicitly,
  never with a synthetic fallback, when the name has no valid identifier
  left after sanitising (empty, or leading with a digit) or when the
  identifier collides with an unrelated existing member.
- Applied to the vendor generator template identically, verified by
  rendering `BASE(...)` with the real TABLE/MISS blobs extracted from the
  committed const file and diffing -- the only differences are the
  intended new code. The four per-transport const files need no changes:
  none of them override `__repr__`/`__str__`, and `.aliases`/
  `register_alias` are inherited unchanged from the base.
- Updated the two existing sweeps over all 12,391 members
  (`test_the_new_dunders_are_byte_identical_to_the_percent_form`,
  `test_every_member_renders_its_own_registrys_transport_protocol`) to
  account for the new suffix, and added tests for `.aliases`, the repr/str
  suffix, and every `register_alias` success/error path, including the
  sanitised-lookup, collision, and invalid-identifier cases.

Build/test: `tests/const/`, `tests/foundation/registry/` and `tests/vendor/`
pass (187 tests, 39,468+ subtests). Coverage of the touched const file rose
from 52.65% to 53.59% (855/1624 -> 888/1657 lines), with 0 of the new
statements missed.
@JarryShaw
JarryShaw force-pushed the feat/807-apptype-aliases branch from 7898476 to c98ac3c Compare September 26, 2026 16:14
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO — delta cross-review (fable, a model distinct from both the sonnet author and the earlier opus reviewer) on 7898476aa, scoped to 976c949f9..7898476aa. It raised one real nit, which I fixed; head is now c98ac3ced.

What it derived independently, and I re-derived myself before posting:

  • 58 confirmed two ways. It re-implemented safe_name minus its fallback and counted members whose residue fails isidentifier(): TCP 28, UDP 29, SCTP 1 (SCTP_3gpp_w1ap), DCCP 0. My prefix-count cross-check agrees exactly, with 0 mismatches between the two methods. (Precise citation, since I was loose about it earlier: safe_name is defined at pcapkit/vendor/default.py:260; the fallback is at :286-288.)
  • 94 confirmed, and the member/port conflation I asked it to look for does not exist. TCP 49 / UDP 45 / SCTP 0 / DCCP 0 out of 12,391 members. It went further than my check: the broken set is set-equal to the set of members sitting on multi-member ports in all four classes (broken == on_multi → True each), so the invariant breaks for all members of such ports and no others. Distinct multi-service ports are 44 (TCP 23, UDP 21) — had the sentence counted ports it would have been wrong, but it says "94 such members", which is exact.
  • Prose-only proven by AST, not by eyeballing the diff. Const file: docstrings neutralised, ast.dump identical across the two shas. Vendor file: the BASE string literal's value neutralised, ast.dump identical, and every diff hunk falls inside that literal's span. No executable statement changed.
  • Seven methods render byte-identical between template and const (register_alias 6088 chars, including the new NOTE's doubled {{cls.__name__}}_{{residue}}); both files py_compile clean under -W error::SyntaxWarning; tests/const/ 87 run, OK.

The nit, which was correct and is now fixed. The body NOTE said "see Raises below" while Raises: is above it in source order — I measured Raises: at line 2634 against the NOTE at 2668. The docstring's own "see Raises below" at 2610 is correct and I left it alone. Changed the body NOTE only, in both files, to "see Raises above". 7898476aa → c98ac3ced is that one word, twice; I re-ran py_compile on both files and tests/const/ (87 run, OK) on the result.

Head moved, so the verdict does not carry yet: the same reviewer is re-checking the one-word delta on c98ac3ced, and CI is running on it.

@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO on c98ac3ced — the reviewer (fable) re-checked the one-word delta with its context intact, and I verified its three claims myself.

  • Delta is exactly "see Raises below" → "see Raises above", twice, one hunk per file, one commit on top of 14d3d3dc7. git diff 7898476aa c98ac3ced shows those two lines and nothing else.
  • The right copy was flipped. Vendor geometry at the head: line 565 "below" → Raises: at 589 → line 623 "above". Const geometry: 2610 "below" → Raises: at 2634 → 2668 "above". Both cross-refs now point at their real target and neither was inverted the wrong way.
  • Prose-only re-proven by ast — a # comment change never reaches the AST at all, so the const dump is identical without even neutralising docstrings; the vendor file is identical outside the BASE literal. All seven methods still render identical between template and const, which also re-proves the {{cls.__name__}}_{{residue}} doubling survived the edit. Both files py_compile clean under -W error::SyntaxWarning, and I ran tests/const/ myself on this head: 87 run, OK.

Setting review: good-to-go. Verdict history on this PR: opus NEEDS CHANGES on 976c949f9 (two false prose claims) → fixed → fable GOOD TO GO on 7898476aa with one wayfinding nit → fixed → fable GOOD TO GO on c98ac3ced. Not reporting it ready to merge until CI on this head reaches zero incomplete legs — it is still running.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 26, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge. CI on c98ac3ced is complete and clean — 27 SUCCESS / 0 FAILURE / 3 SKIPPED / 0 incomplete, 30 checks total, counted with the __typename=="CheckRun" filter and the empty-string incomplete test rather than conclusion==null. mergeable=MERGEABLE, mergeStateStatus=CLEAN on both the REST and GraphQL reads.

No merge-into-main rehearsal was needed: the PR's parent is current origin/main (14d3d3dc7 both), so there is no drift to test against — it is one commit sitting directly on the tip.

Gates, all closed: single commit authored Jarry Shaw <jarryshaw@icloud.com>; cross-review GOOD TO GO on this exact head from a model distinct from the author, with its findings and my re-derivations posted above; tests/const/ 87 run / OK verified by me on this head; both the generator template and the generated const file py_compile clean under -W error::SyntaxWarning; all seven touched methods render byte-identical between the two.

One thing still open and deliberately not claimed: the PR body's coverage figures 52.65% → 53.59% were never reproduced by either reviewer. The direction is not in doubt — 156 lines added to the const file with 10 new tests exercising them — but the two exact percentages are unverified, so treat them as approximate.

Unmerged and awaiting you. #657 remains last by design.

@JarryShaw
JarryShaw merged commit add2a82 into main Sep 26, 2026
31 checks passed
@JarryShaw

Copy link
Copy Markdown
Owner Author

Coverage claim now verified, and it exposed a stale figure in the PR body, which I have corrected.

Measured on c98ac3ced, statement-only, in one worktree with the three changed paths swapped between 14d3d3dc7 and the head so the collector resolved to the same absolute file both times:

Stmts Miss Cover
before 14d3d3dc7 1624 769 52.6478% → 52.65%
after c98ac3ced 1657 769 53.5908% → 53.59%

Tests 25 → 35 in tests/const/test_const_apptype_split_unit.py; misses flat at 769, so all 33 new statements are covered. So the commit message's 52.65% → 53.59% (855/1624 → 888/1657) is exact.

The PR body was wrong and is now fixed. It read 53.28% (877/1646) after — all 22 new statements covered, which was measured before the alias-name gap fix added 11 statements; it now reads 53.59% (888/1657) … all 33 new statements covered. Nothing else in the body changed and the head is untouched — still c98ac3ced, still one commit, so the verdict and CI both stand.

Two methodology notes, since a coverage number in this repo is easy to get wrong:

  • [tool.coverage.run] source = ["pcapkit"] in pyproject.toml overrides a command-line --include and resolves through the editable install, which points at the main checkout rather than a worktree. A standalone rcfile with an absolute source path avoids both.
  • coverage report re-reads the source file from disk at report time. My first attempt reported after swapping the file back to the parent version and produced 1624 stmts / 864 miss / 46.80% — a plausible-looking number that is pure artefact. Report before swapping, or the comparison is worthless.
  • The figures only reproduce with branch = False. With the repo default branch = true the same runs give 51.82% → 52.55%, so the claim is a statement-coverage claim.

#837 remains ready to merge and unmerged.

@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

const Regenerated IANA or vendor constant tables; members keep their numeric values enhancement Issues requesting a new capability (set by the feature request template) feat Pull requests that add a new capability (feat: subject prefix) test Pull requests that add or correct tests (test: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant