Skip to content

docs(corekit,tests): state what the enum tiers actually declare and dispatch - #997

Merged
JarryShaw merged 1 commit into
mainfrom
docs/719-enum-tier-claims
Oct 2, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/719-enum-tier-claims

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • chore — anything else

Part of #719.


Description of your pull request and other information

Four prose defects in pcapkit/corekit/enum.py and one test docstring, each verified against the code rather than read off the existing text. Prose only: both files are token-identical to main with STRING masked and comments dropped, and the over-95 line set is unchanged (one pre-existing line per file, at 96 and 121 columns).

The "inherit them from here" claim was transitively true at best. The module docstring said the pcapkit.const registries inherit all four of get/get_all/register/register_alias from here. Measured: vars(EnumLookup) owns get and get_all; vars(EnumRegistry) owns register and register_alias. The prose now names which tier declares which, and says a registry reaches all four from this module — which EtherType confirms, all four reporting __module__ == pcapkit.corekit.enum.

The _dispatch claim asserted the opposite of a documented design decision. The prose said AppType overrides all four "to route through its _dispatch". An AST walk of AppType's own methods finds a _dispatch call in get and get_all only — two of four. register merely mentions _dispatch in the docstring that explains why it deliberately does not dispatch, and pcapkit/const/reg/apptype/apptype.py:2766-2771 gives the reason: minting on one transport must never leak onto a transport IANA never assigned the service to. Worth noting that a naive '_dispatch' in source check reports three of four, because it matches that docstring reference — the AST walk is what separates a call from a mention.

Two docstring lines had drifted to 91 columns inside paragraphs otherwise wrapped at 74-82. Reflowed to their own neighbours' band; no word changed.

test_enum_lookup_reparent_930_unit.py:634 claimed a tree-wide negative that is not statically decidable — that no call site had ever passed get a key of a third type. A key arriving through a variable is invisible to any search, so the docstring now states what the change did establish and says why the stronger form cannot be.

No citation moved. The issue numbers left standing were each type-checked against the API: #842, #860, #877 and #935 are all issues.

tests/corekit/{test_enum_lookup_reparent_930,test_enum_lookup_base,test_enum_lookup_reparent_877,test_enum_get_exception_provenance_923}_unit.py and tests/project/test_conventions_doc_claims.py pass — 167 passed, 1 skipped, 252 subtests, exit code 0 read from the process rather than a summary line.

@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one test Pull requests that add or correct tests (test: subject prefix) labels Oct 2, 2026
…ispatch

Four prose defects under #719, each verified against the code rather than
read off the existing text. Prose only: both files are token-identical to
main with strings masked.

- enum.py said the const registries "inherit them from here", of all four
  of get/get_all/register/register_alias. Only two are declared here:
  vars(EnumLookup) owns get and get_all, vars(EnumRegistry) owns register
  and register_alias, so the claim was transitively true at best. The prose
  now says which tier declares which, and that a registry reaches all four
  from this module.
- enum.py said AppType overrides all four "to route through its _dispatch".
  Two of the four route through it. An AST walk of AppType's own methods
  finds a _dispatch call in get and get_all only; register mentions it in a
  docstring that explains why it deliberately does not dispatch, and
  apptype.py:2766-2771 gives the reason -- minting must never leak onto a
  transport IANA never assigned the service to. The old prose asserted the
  opposite of a documented design decision. The same claim one paragraph
  down is narrowed from "tier 2's methods" to "tier 2's lookups", since the
  premise is _dispatch's return value.
- The tier-2 sentence covered register and register_alias with "minting",
  which is exact for the first and loose for the second: register_alias
  registers an alias rather than minting a member, and its own docstring
  frames the concern as a registration. It now says "a write", naming both.
- Two docstring lines had drifted to 91 columns inside paragraphs otherwise
  wrapped at 74-82. Reflowed to their own neighbours' band; no word changed.
- test_enum_lookup_reparent_930_unit.py claimed no call site in the tree had
  ever passed get a key of a third type. That is not statically decidable --
  a key arriving through a variable is invisible to any search -- so the
  docstring now states what the change did establish and says why the
  stronger form cannot be.

No citation moved, and the issue numbers left standing were each checked
against the API: #842, #860, #877 and #935 are all issues.

tests/corekit/{test_enum_lookup_reparent_930,test_enum_lookup_base,
test_enum_lookup_reparent_877,test_enum_get_exception_provenance_923}_unit.py
and tests/project/test_conventions_doc_claims.py pass: 167 passed, 1 skipped,
252 subtests, exit code 0 read from the process.
@JarryShaw
JarryShaw force-pushed the docs/719-enum-tier-claims branch from ebe6183 to 84e5035 Compare October 2, 2026 21:24
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at ebe61839e — sonnet cross-review, a different model from the author. No blocking finding. It raised one wording nit that I have acted on, so the head is now 84e5035d3.

The nit was in a sentence this pull request wrote, which is why it earned a new head. The tier-2 bullet covered register and register_alias with the word "minting". That is exact for the first and loose for the second — register_alias registers an alias rather than minting a member, and its own docstring at pcapkit/const/reg/apptype/apptype.py:2815 frames the concern as keeping an alias registered on one transport from leaking. It now reads "a write", naming both outcomes. Shipping a loose word in the sentence that corrects a wrong one is the thing this issue exists to stop.

Reflowing that sentence also removed a pre-existing 19-column orphan tail: the paragraph now wraps 72-79 throughout.

What the review verified independently, each by its own measurement rather than by reading the diff: vars() on both classes, plus __qualname__ on a concrete registry's four attributes, giving EnumLookup.get, EnumLookup.get_all, EnumRegistry.register, EnumRegistry.register_alias; an AST walk finding _dispatch Call nodes in get and get_all only, with the substring check returning three as predicted; a word-level SequenceMatcher over all 11 docstrings in enum.py showing zero changes outside the module docstring; both files token-identical to main with strings and f-strings masked; the #NNN multiset unchanged, so nothing moved and nothing new was introduced; and 167 passed, 1 skipped, 252 subtests, exit 0 read from the process.

It also established something my record had wrong. The four defects this fixes are not logged on #719 at all — they were raised on #990 (comments at 18:29Z, 18:32Z and 18:46Z today) and #992 (18:03Z), searched across 2,129 issue comments and 334 inline review comments with --paginate. Review summary bodies and pull-request descriptions were not searched, so that is UNVERIFIED rather than exhaustive.

One observation it left for #719's accuracy half rather than acting on: _dispatch is also called from pcapkit/protocols/schema/transport/{tcp.py:439,udp.py:81,sctp.py:234} in post_process. Those are read paths, so "tier 2's lookups" still holds; it would need widening to "reads" only if a write ever joined them.

Label stays review: pending until a verdict lands on 84e5035d3.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Delta round: GOOD TO GO at 84e5035d3. The nit fix holds, and the reviewer pushed on it rather than accepting it — which is what I asked for, since the fix was mine.

"A write" is accurate for both methods, confirmed at the mechanism rather than the wording. AppType.register_alias ends in return extend_enum(cls, identifier, port, name, cls.__transport__), the same call register uses, so both genuinely write a new member. The distinction the sentence draws is one of precondition, not of mechanism: register requires the port to be unclaimed, register_alias requires it to be claimed and adds a further name to it. That is a true difference and the two docstrings use the same vocabulary for it, so the phrasing stays.

The rest, each re-derived at this head: a word-level diff of the module docstring against ebe61839e shows exactly two regions, both intended — "minting" to "a write", and the added clause naming the two outcomes — with _validate_value and get untouched; both files token-identical to main with strings and f-strings masked (693 and 3298 tokens); the over-95 set unchanged, one 96-column line in enum.py and the test file's 121-column line, neither of them new; the edited bullet wrapping 60-79 with no orphan tail; and 167 passed, 1 skipped, 252 subtests, exit 0 read from the process.

Label is now review: good-to-go. Not yet ready to merge — CI at this head is 2 passed, 2 skipped, 6 in flight, zero failures. The verdict is what the label tracks; the in-flight count is what gates calling it ready.

@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 Oct 2, 2026
@JarryShaw
JarryShaw merged commit 452c108 into main Oct 2, 2026
72 checks passed
@JarryShaw
JarryShaw deleted the docs/719-enum-tier-claims branch October 2, 2026 21:32
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Pull requests that change documentation only (docs: subject prefix) test Pull requests that add or correct tests (test: subject prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant