Skip to content

docs: mint-criterion understates which registries register, and miscounts the ones without _missing_ #1019

Closed

Description

@JarryShaw

docs/source/contributing/conventions/mint-criterion.rst is the authority the rest of the documentation defers to on what _missing_ does, and three of its claims are wrong. It is how #1018 inherited an error: that PR corrected the pcapkit.const landing page by pointing at this page, and this page is wrong in the same area.

All measured at runtime on main, in a worktree with pcapkit.__file__ asserted.

1. The "six without _missing_" is really two. Lines 6-11 name ExtensionHeader, TLSKeyLabel and AppType's four transport subclasses as carrying "no declared-but-unassigned range". The four transports inherit AppType._missing_ and do unmint:

TCP(48130) -> <TCP.unassigned: 48130 [undefined]>   __members__ 6147 -> 6147, same object on 2nd lookup = False
UDP(48130) -> <UDP.unassigned: 48130 [undefined]>   likewise

Only ExtensionHeader and TLSKeyLabel define no _missing_. The AppType base does define one, has 0 members, and raises for every value.

2. "rather than whether it registers" is false for five registries. Lines 26-31 say that outside CGAType the criterion decides only the name, not whether a value registers. But tcp.flags.Flags and the four Mobility Header flag registries do register:

Flags(17) -> <Flags.Reserved_4|1: 17>   __members__ 12 -> 12   _value2member_map_ 12 -> 13   same object on 2nd lookup = True

They are aenum.IntFlag subclasses, so super()._missing_ reaches aenum.Flag._create_pseudo_member_, which ends in cls._value2member_map_.setdefault(value, pseudo_member). The composite is permanent in the value table and the second lookup returns the first object. Note that issubclass(cls, enum.IntFlag) against the stdlib enum is False — only aenum sees it — which is how this stays hidden.

And hip.transport.Transport, grouped with those five, does not resolve to a name at all: Transport(4) raises ValueError, because it is an enum rather than a flag so Enum._missing_ returns None.

3. The warning at lines 143-148 names CGAType as the one registry a lookup mutates. _value2member_map_ also grows for those five, so a probe mutates six registries, not one. The advice to snapshot before probing is right; the scope is understated.

Two knock-on items in the same file: the 114 unmint figure counts AppType (which raises) and excludes the four transports (which unmint); and the AST snippet at lines 116-139 is per-file, so it prints nothing for the five flag registries or Transport and finds no _missing_ for the transports, mislabelling them.

The real partition over all 127 registries is 1 installs a permanent member / 5 cache in the value table only / 117 return an uninstalled object / 4 raise.

Not fixed here because this file sits in #1012, which is already approved and awaiting merge. It belongs to the docs/source/contributing/** slice of #719.

Activity

  1. added
    docsPull requests that change documentation only (docs: subject prefix)
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 4, 2026
  2. JarryShaw commented on Oct 5, 2026

    @JarryShaw
    OwnerAuthor

    Fixed on main in 3d8a0d476 (docs-only, direct push).

    All three defects were real and are corrected, with the measured four-way partition over all 127 registries replacing the old two-way split: 1 mints (CGAType) / 117 unmint / 5 cache in _value2member_map_ only (tcp.flags.Flags plus the four Mobility Header flag registries) / 4 raise. Every count re-derived against the tree with pcapkit.__file__ asserted, then cross-reviewed on a second model.

    Three things the cross-review caught that are worth recording, because two of them were corrections to the fix rather than to the original page:

    • The first draft gave Transport a false mechanism, saying it hands back to aenum like the flag registries and gets None because it is a plain enum. It never enters aenum.Enum._missing_ for any value: its guard declares 0..3 and its members are exactly [0, 1, 2, 3], so there is no in-range non-member and the hand-back on the next line is unreachable dead code. A sys.settrace frame count settles it — Transport(5) enters one _missing_ frame and raises from its own raise ValueError, while BlockType(11) enters two. The original page's "declaring no unassigned range at all" was the correct explanation, so the first draft replaced a right reason with a wrong one. The plain-enum path is real but is reached by an in-range hole in an unmint registry, which the page now cites as BlockType(11).
    • "113 resolve a value" overstated by at least 12. Twelve of those 113 declare a range their own members already cover, so the _unregistered_member call is dead code — ToSDelay declares 0..1 and has members [0, 1]. All twelve are now named, and three more are str-valued and unprobed, so the page states 12 as a floor and 15 as the ceiling rather than asserting a single figure.
    • The AST snippet's caveat undercounted what the walk mishandles: twelve registries, not ten. ExtensionHeader and TLSKeyLabel also print nothing, since they define no _missing_ of their own.

    Also corrected while landing it: the draft said "the remaining five ipv4.tos_*" where it is four, tos_del being the worked example.

    One method note for anyone repeating this. I first derived the twelve with a regex over the guard source and got 24, because (\d+)\s*<=\s*value\s*<=\s*(\d+) reads 0x0A0D0D0A as 0 and turns a 32-bit band into 0..0. An AST walk over the Compare nodes with ast.literal_eval on the bounds gives exactly 12. Do not parse these guards with a regex, and do not build set(range(lo, hi+1)) either — several registries declare 32-bit ranges and that gets the process OOM-killed.

  3. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 5, 2026
  4. added this to the 1.5 milestone on Oct 6, 2026
  5. moved this to Done in PyPCAPKiton Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    docsPull requests that change documentation only (docs: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions