Skip to content

docs(const): state what _missing_ actually does instead of claiming it mints - #1018

Merged
JarryShaw merged 1 commit into
mainfrom
docs/1013-const-minting-prose
Oct 5, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/1013-const-minting-prose

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — N/A, no code changed
  • make test passes, and a test case covers the change — I ran tests/project only (268 passed, 1 skipped, 864 subtests), not the full suite
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A, prose only

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

Description of your pull request and other information

Closes #1013. One file, +13/−10.

The pcapkit.const landing page is the first thing a reader of those registries meets, and it described the behaviour backwards. It said an unregistered value inside a registry's valid range is minted into a permanent member via aenum.extend_enum, and that a minted member's identity is stable for the life of the process. Both are true of exactly one registry out of 121.

Measured at runtime, in a worktree with pcapkit.__file__ asserted:

EnumRegistry subclasses under pcapkit.const 127
defining their own _missing_ 121
that mint (reach extend_enum) 1 — CGAType
that unmint (_unregistered_member, no install) 114
that only range-check and defer to super()._missing_ 6

The identity claim is the inverse of the truth for those 114. Two lookups of the same unassigned value:

  • EtherType(0x0) twice — compare equal, are not the same object, member count unchanged at 160, and the value is absent from both __members__ and iteration.
  • CGAType(5) twice — same object, and __members__ grows 7 → 8.

So "stable for the life of the process" described CGAType and nothing else, while the reader was told it applied to all of them. Out-of-range and wrong-type values (-1, 0x10000, 'x', None, 1.5) all raise ValueError, which the page had right.

What changed. The opening passage now says the object is not installed, that a second lookup returns an equal but distinct object, and names CGAType as the single exception with a :ref: to mint-criterion for the split. The closed-set sentence now states the consequence that actually bites — compare such members by value, not identity. And "for every registry" is dropped from the claim about the generated _missing_ override, because six registries have none.

mint-criterion.rst is not touched: it was already correct, and it belongs to another slice. This page now agrees with it rather than contradicting it.

The page carries no issue or PR citations, before or after. No Sphinx build was run, so the new :ref: and :class: targets are checked by reading the label and by import, not by Sphinx's own resolver.

@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 labels Oct 4, 2026
…t mints

`docs/source/pcapkit/const/index.rst` told the reader that an unregistered value
inside a registry's valid range is minted into a permanent member via
``aenum.extend_enum``, and that a minted member's identity is stable for the life
of the process. Measured, there are **four** behaviours across the 127
`EnumRegistry` subclasses, not one:

- **1 installs a permanent member** -- `CGAType`, so `__members__` and iteration
  grow.
- **5 cache a composed object in the value table only** -- `tcp.flags.Flags` and
  the four Mobility Header flag registries. They are `aenum.IntFlag` subclasses,
  so `super()._missing_` reaches `aenum.Flag._create_pseudo_member_`, which ends
  in `cls._value2member_map_.setdefault(value, pseudo_member)`. A second lookup
  therefore returns the **same** object, while `__members__` and iteration do not
  change.
- **117 return an uninstalled object** -- nothing grows, and a second lookup gives
  an equal but distinct object. This is the common case the page now describes.
- **4 raise for any unknown value** -- `Transport`, `ExtensionHeader`,
  `TLSKeyLabel`, and the memberless `AppType` base.

So the old identity claim was true of six registries rather than one, and false of
the other 121. The page now leads with the warning that a lookup does not raise,
states the common case, and lists the exceptions, deferring the full split to
`mint-criterion` by `:ref:`. "The mechanism is uniform because it is generated"
is corrected: only the common form of the override is generated.

Closes #1013. `tests/project`: 268 passed, 1 skipped, 864 subtests passed.
@JarryShaw
JarryShaw force-pushed the docs/1013-const-minting-prose branch from a992025 to f134b2d Compare October 4, 2026 23:14
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES at a992025ca, fixed at f134b2d0f — opus cross-review, a different model from the sonnet that drafted it. It confirmed all the counts and the identity measurement, then found the rewrite was wrong in the opposite direction from the bug it fixed.

My replacement prose was false for five registries. It said the resolved object is not installed and that a second lookup returns an equal but distinct object. Measured: tcp.flags.Flags and the four Mobility Header flag registries return the same object, and _value2member_map_ grows by one, while __members__ and iteration stay unchanged. So stable identity applied to six registries, not one — the old text was true of six and false of 121, which is a different error from the one #1013 described.

The mechanism, established rather than guessed. They are aenum.IntFlag subclasses, so super()._missing_ reaches aenum.Flag._create_pseudo_member_, which ends in cls._value2member_map_.setdefault(value, pseudo_member) — a thread-safety guard that also makes the composite permanent in the value table.

A correction I owe the reviewer. I pushed back on its IntFlag attribution, having measured issubclass(cls, enum.IntFlag) as False. That check used the stdlib enum; against aenum it is True. Its attribution was right and my correction of it was the error.

The full partition is four behaviours over 127 registries, and it also corrects the issue's own framing: 1 installs a permanent member (CGAType); 5 cache in the value table only; 117 return an uninstalled object; 4 raise — Transport, ExtensionHeader, TLSKeyLabel and the memberless AppType base. The four AppType transport subclasses inherit AppType._missing_ and do unmint, so only two registries define no _missing_ at all, not six.

The page now leads with the warning that a lookup does not raise, states the common case, lists the three exceptions, and defers the full split to mint-criterion by :ref:. "The mechanism is uniform because it is generated" is corrected — only the common form is generated.

This means mint-criterion.rst is itself wrong, which is how this PR inherited the error by deferring to it. Filed separately rather than touching a page that sits in an already-approved PR. Delta re-review dispatched.

@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at f134b2d0f — opus delta re-review. It confirmed the four-way partition two independent ways and closed the one number I was unsure of.

1 installs / 5 cache / 117 uninstalled / 4 raise = 127. The 117 arrives as 114 − 1 + 4, and the −1 is the part worth recording: AppType does call _unregistered_member, across a very large IANA span table, yet raises for everything because of a guard above those spans — if cls.__registry__ is None: raise ValueError(...), whose NOTE says extending the base would give it a member and aenum would then refuse to subclass it permanently, for every registry not yet imported. Verified: AppType.__registry__ is None is True while TCP.__registry__ is None is False, and the transports share the base's _missing_ by identity.

It withdrew its own earlier bucketing — its first pass had measured __dict__ membership and labelled it behaviour, and its one-shot probe stopped at the first interior gap, which for the transports is 0 and does raise. It also flagged two probe artifacts worth knowing about: TCP.__members__ values are str ('reserved [0 - tcp]'), not int, and pcapng.option_type.OptionType's likewise, so an int-assuming scan builds an empty search space and reports false raises.

One precision note it raised and I agree is not a defect. Thirteen further registries never reach _unregistered_member because their declared range exactly equals their fully-assigned span — the five ToS registries, OptionClass, OptionAction, SeedID, SMFDPDMode, l2tp.type.Type, DHCPSupportMode, PriorityLevel. Every undefined value for them is out of range, and the page already says out-of-range raises, so its rule predicts their behaviour correctly. The four raisers differ in kind: they raise for a value inside the assigned span.

All five rewrite checks confirmed, and the warning test came out better than before rather than merely no worse — the opening paragraph is now 6 lines where it was 10, with the mechanism moved into a scannable list, and "nearly every" is well-earned at 123/127. The aenum sentence deliberately stops short of naming _create_pseudo_member_ or the setdefault, which is right for a landing page.

Residual, non-blocking: nothing automated gates this page — test_conventions_doc_claims.py scopes to the conventions directory — so the partition asserted here has no test behind it, unlike mint-criterion's figures. If #1019's fix touches that test, adding the 1/5/117/4 split is cheap insurance. And the page now says "installs a permanent member" rather than the repo's own verb "mint", so a reader crossing to mint-criterion meets different vocabulary for one thing; not worth a revision alone.

@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 4, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Correcting my own number above, and the verdict is unchanged — still GOOD TO GO at f134b2d0f.

I wrote "thirteen further registries" never reach _unregistered_member and then listed twelve. Twelve is right: the five ToS registries, OptionClass, OptionAction, SeedID, SMFDPDMode, l2tp.type.Type, DHCPSupportMode, PriorityLevel. The thirteenth was pcapng.block_type.BlockType, and it does not belong on that list — it unmints and sits in the 117. Measured:

BlockType(0x12345678) -> <BlockType.Reserved: 305419896>
  equal=True  same_object=False  __members__ 29->29  _value2member_map_ 29->29
BlockType(0x11171)    -> ValueError
BlockType(0x100000)   -> <BlockType.Reserved: 1048576>

The cause is worth recording, because it will catch the next person probing these registries. BlockType's unassigned bands are 0x0a0d0a00, 0x000a0d0a, 0x000a0d0d, 0x0d0d0a00 and 0x80000000-0xffffffff, under a guard of 0 <= value <= 0xFFFFFFFF. A bounded integer scan over 0..70000 never reaches any of them, so it reports a false ValueError and the registry looks like a raiser. BlockType(0x11171) raising while BlockType(0x100000) resolves is the scattered-band signature.

That is the third probe artifact in this one area, and they share a shape: assuming the value space is small, contiguous and integer-keyed. The earlier two were TCP.__members__ and pcapng.option_type.OptionType holding str values rather than ints, which makes an int-assuming scan build an empty search space and report false raises across the whole registry.

None of this touches the partition. 1 / 5 / 117 / 4 = 127 rests on source inspection — 114 registries owning a _missing_ that calls _unregistered_member, minus AppType which raises via its __registry__ is None guard, plus the four transports that inherit it — not on any probe. Every disputed point was independently corroborated: one minter, five cachers with _value2member_map_ growing on all five, TCP(48130) resolving with a distinct second object and no growth, AppType memberless and raising, all four transports resolving _missing_ to AppType._missing_, and issubclass(Flags, aenum.IntFlag) True against the stdlib's False.

One item stays UNVERIFIED and changes nothing downstream: of the twelve, the range guard was read directly on five; the other seven were inferred from member counts matching fully-assigned 1-, 2- and 3-bit fields.

@JarryShaw
JarryShaw merged commit dd64913 into main Oct 5, 2026
73 checks passed
@JarryShaw
JarryShaw deleted the docs/1013-const-minting-prose branch October 5, 2026 00:36
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 5, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 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)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

docs: the pcapkit.const landing page describes _missing_ as universally minting, contradicting mint-criterion

1 participant