Skip to content

EnumRegistry.get() mints through its default, contradicting its own "It never mints" contract #864

Description

@JarryShaw

EnumRegistry.get's docstring says "It never mints. Registering a member is register's job and nobody else's, which is the ruling #775 exists to carry out" (pcapkit/corekit/enum.py:267-270). It does mint, today, on a shipped registry — both the str branch (:297) and the value branch (:303) end in return cls(default), which reaches _missing_.

Measured on main (6b333d7e4), throwaway process, pcapkit.__file__ asserted inside a clean worktree:

len before      : 160
get(0x1234, 0x0888) -> <EtherType.Xyplex_0x0888: 2184>
len after       : 161
MINTED          : ['Xyplex_0x0888']

0x1234 is in no _missing_ range, so the lookup falls to the default; 0x0888 is in the Xyplex range, which mints by the #775 ruling. So a failed lookup permanently grows the registry. This survives #861 for the three registries that still mint — EtherType (52 branches), Socket (1), CGAType (1).

Needs a ruling, because both readings are defensible. Either (a) a caller who names 0x0888 as a fallback has "explicitly" asked for that member, so minting is correct and the docstring is simply wrong; or (b) a fallback is a value to resolve, not a request to register, so get should resolve the default through the non-minting path (_value2member_map_, then _unregistered_member) and cls(default) is the defect.

My lean is (b): #775's wording is "so that we dont create registered enums out of unrecognised/unregistered values, unless user/caller explicitly created them", and passing a fallback is not creating one. But (b) changes behaviour on a shipped path, so it is not mine to decide.

Found by the #863 cross-review, which scoped it to str-valued registries; the int path above is the live case and is wider than that. Pre-existing — cls(default) is identical on main before #863.

Activity

  1. added
    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)
    designA design or decision issue: a pattern being decided rather than a defect or a request
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Sep 27, 2026
  2. JarryShaw commented on Sep 27, 2026

    @JarryShaw
    OwnerAuthor

    Take (b). Only register can mint. get should not mint unless it falls through the _missing_'s minted ranges.

  3. JarryShaw commented on Sep 27, 2026

    @JarryShaw
    OwnerAuthor

    Recorded, verbatim:

    Take (b). Only register can mint. get should not mint unless it falls through the _missing_'s minted ranges.

    Read as: minting is permitted only as the natural outcome of resolving key through _missing_ — so EtherType.get(0x0888) minting Xyplex_0x0888 stays correct, because #775 deliberately kept that range minting. It is not permitted as a side effect of resolving default, which is what :297/:303's cls(default) does today.

    One implementation question I do not want to answer by guessing, because every option costs something and it changes shipped behaviour:

    A default that is an already-registered value is easy — a _value2member_map_ lookup resolves it and cannot mint. The awkward case is a default inside a declared-but-unassigned range. Today cls(default) resolves it through _missing_; after #861 that is non-minting for every registry except the three in the KEEP set (EtherType 52 branches, Socket 1, CGAType 1), where it mints. There is no generic way to ask a _missing_ "would you mint this?" before calling it.

    So either:

    1. default must be an already-registered value — _value2member_map_ only. Never mints, one line, no inspection. Cost: a default in a declared-but-unassigned range stops resolving, where today it returns an unregistered member. Affects every registry, not just the three.
    2. Call cls(default) and refuse the result if it minted — snapshot members, and on growth raise instead of returning. Preserves unregistered-range defaults everywhere. Cost: it mints and then has to undo, and aenum has no clean removal, so the registry is briefly wrong.
    3. Only the three minting registries lose it — a class-level marker saying "my _missing_ mints", checked before the fallback. Most precise, most invasive, and the marker has to be kept in step with every future crawler change.

    My lean is 1: it is the only one that cannot mint by construction, and a caller who wants an unassigned-range member as a fallback can pass cls(value) themselves, which is explicit — which is the ruling's own test. But it is a real loss of behaviour on every registry, so it is your call.

    Keeping needs: decision on this for that question only; the direction itself is settled. Not folded into #863, which stays prose-only — though once this lands, #863's paragraph describing cls(default) as able to mint will need updating, and :267's bare "It never mints." finally becomes true.

  4. JarryShaw commented on Sep 27, 2026

    @JarryShaw
    OwnerAuthor

    I think 1 is correct mechanism we'd like.

  5. JarryShaw commented on Sep 27, 2026

    @JarryShaw
    OwnerAuthor

    Recorded, verbatim:

    I think 1 is correct mechanism we'd like.

    So option 1: default resolves through a _value2member_map_ lookup only. get then cannot mint by construction — no snapshot-and-undo, no per-class marker to keep in step with future crawlers.

    What that means concretely, so the scope is on the record before the work starts:

    • both cls(default) sites go — pcapkit/corekit/enum.py:297 in the str branch and :303 in the value branch.
    • key resolution is unchanged: EtherType.get(0x0888) still mints Xyplex_0x0888, because that is _missing_ falling through a range Registries mint a permanent member for every unrecognised value: 1,169 extend_enum sites across 113 registries #775 deliberately kept minting, which your first ruling permits explicitly.
    • a default naming a value with no registered member stops resolving, and the original lookup error propagates. That is a behaviour change on every registry, not just the three that mint — today such a default returns an unregistered member via _missing_. It is the price option 1 names, and a caller who wants that can pass cls(value) themselves, which is the explicit path your ruling asks for.
    • get's docstring loses the #864 caveat added in fix(corekit): fall EnumRegistry.get() back to a str key's value (#860) #863 and its bare "It never mints." finally becomes true.

    needs: decision removed, wip applied. Blocked on #863 merging first — both change pcapkit/corekit/enum.py, and #863 is review: good-to-go at 216369e28 awaiting your merge. Worker dispatched the moment it lands; I will not start it in parallel and hand you a conflict.

  6. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    and removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    wipWork in flight - a covering PR is open or an agent is actively on it
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 27, 2026
  7. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 28, 2026
  8. added this to the 1.5 milestone on Oct 6, 2026
  9. 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

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)designA design or decision issue: a pattern being decided rather than a defect or a request

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions