fix(corekit): stop EnumRegistry.get's default from minting

[JarryShaw]

• Searched for similar pull requests
• Followed the coding style (isort/pylint clean on pcapkit/corekit/enum.py; mypy shows one pre-existing, unrelated error there -- confirmed identical on unmodified main)
• make test passes, and a test case covers the change
• 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?

• fix — corrects a defect

---

Description of your pull request and other information

Fixes #864. EnumRegistry.get's docstring claimed "It never mints", but both
cls(default) sites reached _missing_ for a default that fell inside a
still-minting registry's own range, growing the registry as a side effect of
resolving default rather than key.

Owner's rulings, verbatim:

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

I think 1 is correct mechanism we'd like.

("1" of three proposed implementation options: default resolves through a
plain _value2member_map_ lookup only, never cls(default).)

Both cls(default) sites are replaced accordingly. key resolution is
unchanged: EtherType.get(0x0888) still mints Xyplex_0x0888 through its
own _missing_, exactly as the ruling permits -- landing in both lookup
tables, exactly as register would leave it, since that is #775's own
deliberately-kept-minting exception rather than something get itself
does. A default naming no registered member now falls through to the
same lookup error key itself would have raised, rather than a fresh error
about the default -- the accepted cost the ruling names explicitly.

Docstring updated to match: "It never mints" is now qualified (true for
default; key may still mint on the three still-minting registries), the
paragraph claiming an unregistered default "can mint there just the same"
is corrected, and Args/Raises reflect the new propagation.

New GetDefaultNoMintTests pins the issue's own repro on the real shipped
EtherType registry (with member count asserted unchanged), the preserved
key-path mint as a no-change guard, default resolving to a registered value
on both an int and a str registry, and the exception type per path. Two
NoDefaultSentinelTests assertions that encoded the old cls(default)
failure mode for an unregistered -1/-1.0 default are retargeted to the
new contract, with docstrings noting they now coincidentally pass on the
pre-#857 tree too and pointing at the test that still discriminates that
defect (test_no_default_is_not_equal_to_any_plausible_caller_value).

Round 2: the fix also broke four pre-existing assertions in
tests/const/test_const_enum_get.py and
tests/const/test_const_enum_builtin_parity.py that encoded the old
contract (an unresolvable/unregistered default failing with its own named
error, or resolving into a declared-but-unassigned range/an unregistered
IntFlag composite). Each is retargeted rather than weakened -- a
registered-default check is added back wherever the old assertion's only
remaining job was proving "a default is genuinely consulted".

Round 3: three review findings, all addressed. (1) Two
NoDefaultSentinelTests docstrings claimed to fail on the pre-#857 tree
when they now coincidentally pass there too -- fixed, and each now names
the test that still discriminates #857 itself. (2) The renamed
test_filter_type_default_never_resolves_but_key_path_still_uses_an_uncached_member
restores the three resolve/identity assertions dropped in round 2 by
demonstrating them through FilterType's key path (FilterType.get(0)),
which #864 does not touch, alongside the default-never-resolves assertion
already added. (3) enum.py's docstring is corrected: "It never mints" is
qualified to apply to default only (key may still mint on EtherType/
Socket/CGAType, #775's deliberate exception), and the claim that a
declared-but-unassigned value is "deliberately absent from the lookup
tables" is qualified to exclude those same three registries, where it lands
in both tables -- measured: EtherType.get(0x0888) grows _value2member_map_,
_member_map_ and _member_names_ alike.

tests/const/test_const_registry_protocol.py: 74/74 under plain
unittest (methods, not subTest records). tests/const/test_const_enum_get.py:
8/8. tests/const/test_const_enum_builtin_parity.py: 33/33. tests/vendor/:
92/92 (grown from 86 by #867's own new test module, unrelated to this
change). No regressions. Rebased onto current main (3173c4fb0, #867)
after the remote branch picked up a merge commit from a branch-sync action;
replaced with a clean rebase, still one commit.

Not in scope, reported rather than fixed: tests/const/test_const_enum_no_mint.py::test_unresolvable_string_key_with_default_in_unassigned_range
also fails for the same reason (Hardware.get('Definitely-Not-A-Member', 40)
used to resolve via the declared-but-unassigned-range pseudo-member path;
40 is not a registered Hardware value, so it now raises KeyError: 'Definitely-Not-A-Member' instead, member count unchanged) -- left alone
because that file is contended with #865.

Round 4: tests/const/test_const_enum_no_mint.py is no longer contended
(#865 merged). Applied the deferred change from round 2 to
GetNoLongerMintsTests.test_unresolvable_string_key_with_default_in_unassigned_range:
Hardware.get('Definitely-Not-A-Member', 40) (40 is in Hardware's
declared-but-unassigned 39-255 range, not a registered member) now asserts
KeyError naming the original key, member count unchanged -- re-measured
fresh (before/after both 42) after #866/#867 and #862/#865 both
landed. Checked for overlap with #865's own additions to that file
(ETHERTYPE_UNASSIGNED_PROBES, EtherTypeMixedMintTests, the retired
test_masked_old_xerox_row_converts_by_source): none -- those are all
EtherType key-path minting-order probes (#862), unrelated to Hardware's
default-path resolution this test covers. GetNoLongerMintsTests remains
the right class. Added as a new commit on top of the existing merge commit
rather than amending, since the round-4 target commit was no longer the
branch tip. tests/const/ as a whole: 203/203, zero failures -- the one
deferred failure from round 3 is gone.

[JarryShaw]

NEEDS CHANGES on 7aff71fd7 — not for the fix, which is right and implements the ruling faithfully, but because the PR knowingly leaves 5 test methods failing in files it did not own. Verified independently, tree asserted:

test_const_registry_protocol:      RUN=74 FAIL=0 ERR=0
test_const_enum_builtin_parity:    RUN=33 FAIL=0 ERR=1
   X ConstEnumRegisterFallbackTests.test_the_guard_leaves_the_other_lookup_paths_alone
test_const_enum_get:               RUN=8  FAIL=3 ERR=1
   X ConstEnumGetDefaultTests.test_omitting_the_default_still_raises_for_the_original_key  (Hardware, Operation)
   X ConstEnumGetDefaultTests.test_the_reported_case_returns_the_default
   X ConstEnumGetDefaultTests.test_filter_type_default_is_consulted_without_a_cached_fallback

test_the_reported_case_returns_the_default is #584's own repro, so I measured it rather than assuming:

0 registered? True | 1 registered? True
get(99999, 0)      -> <Hardware.Reserved_0: 0>
get(99999, 1)      -> <Hardware.Ethernet: 1>
get(99999, 'X')    !! ValueError: 99999 is not a valid Hardware
get(99999, 40)     !! ValueError: 99999 is not a valid Hardware

#584's promise survives intact — a registered default still resolves. What changes is exactly the two things the ruling named as its cost: an unresolvable default now raises the original key's error rather than one naming the default, and a default in a declared-but-unassigned range (40 in Hardware's 39–255) stops resolving instead of returning an unregistered pseudo-member. So these tests encode the superseded contract and are the PR's to update, not evidence against it.

Two NoDefaultSentinelTests assertions were already retargeted in-PR from ValueError/-1 to KeyError/the original name, which is correct and no weaker — the #857 purpose (that -1 is a real default, not confused with NO_DEFAULT) is preserved.

Scope extended to tests/const/test_const_enum_get.py and tests/const/test_const_enum_builtin_parity.py. The remaining failure in tests/const/test_const_enum_no_mint.py (test_unresolvable_string_key_with_default_in_unassigned_range) is contended — #865's author is editing that file right now for the ethertype ordering prose — so I will apply that one change myself once #865 lands, rather than have two branches fight over it.

Note the 5 other tests/const/ failures the author saw are #866's recursion regression inherited from main, not this PR — #867 fixes them.

[JarryShaw]

The 6 red marks here are two causes, not one — recording the split so the author's remaining work is not confused with inherited breakage.

#868's own, the 5 already under review: needs-changes:

FAILED    test_const_enum_builtin_parity.py::ConstEnumRegisterFallbackTests::test_the_guard_leaves_the_other_lookup_paths_alone
FAILED    test_const_enum_get.py::ConstEnumGetDefaultTests::test_filter_type_default_is_consulted_without_a_cached_fallback
SUBFAILED test_const_enum_get.py::ConstEnumGetDefaultTests::test_omitting_the_default_still_raises_for_the_original_key  (Hardware, Operation)
FAILED    test_const_enum_get.py::ConstEnumGetDefaultTests::test_the_reported_case_returns_the_default
FAILED    test_const_enum_no_mint.py::GetNoLongerMintsTests::test_unresolvable_string_key_with_default_in_unassigned_range

#866's, inherited from main and nothing to do with this PR:

SUBFAILED test_const_enum_lookup.py::ConstEnumZeroLookupTests::test_zero_lookup_matches_the_registry            (SecretsType)
SUBFAILED test_const_enum_no_mint.py::RulingConversionDoesNotMintTests::test_converted_value_does_not_mint       (RecordType, SecretsType)
SUBFAILED test_const_enum_no_mint.py::RulingConversionDoesNotMintTests::test_repeated_lookup_does_not_grow_members (RecordType, SecretsType)
FAILED    test_pcapng_unit.py  ×several, all RecursionError

So blocked applies as well: even with its own five fixed, this PR cannot read green until #867 merges. Both labels are correct simultaneously — review: needs-changes for the work it owes, blocked for the part it cannot influence.

Note the last of #868's five is in tests/const/test_const_enum_no_mint.py, which #865 is editing concurrently. That one change is mine to apply once #865 lands, as said earlier — the author is not touching that file.

[JarryShaw]

NEEDS CHANGES on 1607126d2 — cross-review (opus; author sonnet). The enum.py fix is correct, faithful to both rulings, and the review could not break it. Both changes are in tests the PR itself rewrote.

1. test_missing_name_with_default_negative_one_is_now_a_real_default and its _float_ sibling now pass on the pre-const: make EnumRegistry.get's NO_DEFAULT a sentinel object, not -1 #857 tree while the docstring claims they fail there. The docstring itself concedes pre-const: make EnumRegistry.get's NO_DEFAULT a sentinel object, not -1 #857 raised KeyError "coincidentally the same exception type this asserts" — and the new body asserts exactly that. Fix the lead sentence; the retarget is fine. The mitigation is verified sound: test_no_default_is_not_equal_to_any_plausible_caller_value pins the load-bearing fact, never touches get, and is unaffected by EnumRegistry.get() mints through its default, contradicting its own "It never mints" contract #864.
2. test_filter_type_default_is_consulted_without_a_cached_fallback is named for the opposite of what it now asserts, and three assertions went missing — int(resolved) == 0, assertEqual(resolved, FilterType(0)), assertIsNot(resolved, FilterType(0)). The review checked where they might have landed: test_converted_value_does_not_mint covers FilterType but not non-identity; test_repeated_lookup_does_not_grow_members pins assertEqual, not assertIsNot; test_the_always_resolving_registries_have_nothing_to_fall_back_to:378 pins assertIsNot for ProtectionAuthority, not FilterType. So that property is now unpinned. Restorable at zero risk via the key path, which fix(corekit): stop EnumRegistry.get's default from minting #868 does not touch: keep the assertRaises, add FilterType.get(0) carrying the three, and rename the method to match its body.

A prediction of mine was wrong, and it matters here. On #864 I wrote that get's bare "It never mints." would "finally become true". It does not. Measured on this head:

EtherType.get(0x0888) -> <EtherType.Xyplex_0x0888: 2184> | members 160 -> 161
0x0888 in _value2member_map_ after? True

It becomes true of the default path only — which is all #864 was about. #868 removed the -- see #864 for the default path hedge, so the bare claim is now unqualified and load-bearing, and the next sentence ("a member that is deliberately absent from the lookup tables") is falsified for the three KEEP-set registries. One clause fixes it: never mints while resolving default; key may still mint through a _missing_ #775 kept minting.

Also inapplicable, and my earlier verification was of the wrong proposition: the hazard paragraph cites FEATCode as the registry where a mere get() could mint. class FEATCode(StrEnum) does not mix in EnumRegistry — hasattr(FEATCode, 'get') is False. I confirmed the extend_enum mechanism earlier and never checked that the class has the method the design defends. Inherited from #863, not charged to this PR.

Confirmed alongside: option 1 implemented and nothing more (both cls(default) sites replaced by guarded _value2member_map_ lookups, key resolution byte-identical, no path reaches _missing_ via default); #584 intact; the Flags probe swap good (0 in _value2member_map_ False, min(m.value) = 16, the #647 guard still exercised); fail-without-the-fix 2 of 5 as methods plus all four retargeted assertions; tests/const RUN=195, 6 records fully reconciled as 3 methods inherited from #866 plus the 1 deliberately deferred — no seventh failure; pylint 10.00/10, isort clean, and the one mypy "Self" has no attribute "name" confirmed pre-existing by line-shift against main.

breaking confirmed on firmer ground than I gave: the exception type changes — ExtensionHeader.get('<missing>', -1) went ValueError → KeyError, so any caller with except ValueError around get(name, default) now sees an uncaught KeyError.

One non-blocking nit: default not in cls._value2member_map_ makes an unhashable default raise TypeError (Hardware.get(99999, [1])), which escapes a Raises: naming only ValueError and KeyError.

[JarryShaw]

Unblocked — #867 merged at 00:10:35Z as 3173c4fb0, and it was the only gate on this PR's CI.

Verified on merged main rather than assumed, tree asserted in a clean worktree:

SecretsType(0)      -> <SecretsType.Unassigned: 0>
RecordType(0xFFFF)  -> <RecordType.Unassigned: 65535>

So the RecursionError that produced every red mark here is gone at source. blocked removed. This PR's own CI needs a fresh run against the new base before it can be read — updating the branch is the next step, and its review: good-to-go verdict stands on the code, which has not moved.

[JarryShaw]

Round 3 pushed — head 8cb69d27b, back to one commit, rebased onto 3173c4fb0 (post-#867). 4 files: enum.py 35/19 plus the three test files.

All three items applied, verified by me at the pushed sha:

1. Both NoDefaultSentinelTests docstrings now say they coincidentally pass on the pre-const: make EnumRegistry.get's NO_DEFAULT a sentinel object, not -1 #857 tree too, and name test_no_default_is_not_equal_to_any_plausible_caller_value as the test that still discriminates const: make EnumRegistry.get's NO_DEFAULT a sentinel object, not -1 #857.
2. test_filter_type_default_is_consulted_without_a_cached_fallback → test_filter_type_default_never_resolves_but_key_path_still_uses_an_uncached_member, with the three dropped assertions restored through the untouched key path (:430 carries the assertIsNot), the default-never-resolves assertion kept, and the one cross-reference in EXPECTED_WITHOUT_A_CACHEABLE_FALLBACK updated.
3. enum.py:267-270 now reads "It never mints while resolving default; key may still mint through a _missing_ that GitHub issue Registries mint a permanent member for every unrecognised value: 1,169 extend_enum sites across 113 registries #775's ruling deliberately kept minting, on three registries (EtherType, Socket, CGAType)", and the following sentence is qualified to say such a value lands in both tables on those three. The author measured more than I asked — r.name in _member_map_ and in _member_names_ both True, so it lands in every table register would. The closed #864 pointer was correctly not restored.

One side effect to flag, because it is the opposite of what I did on #865. The remote branch had picked up a merge commit (61a68eddc, main merged in via the Update-branch button). The rebase replaced it, so this PR is one commit again while #865 still carries its merge commit. That follows the house one-commit-per-PR rule and it followed my brief, but it did discard that branch update — flagging it rather than letting the two PRs quietly differ in shape. If you would rather #865 match, say so and I will squash it; if you would rather this one had kept the merge, that is also recoverable.

Counts, methods not subTest records: test_const_registry_protocol.py 74, test_const_enum_get.py 8, test_const_enum_builtin_parity.py 33, tests/vendor/ 92 (up from 86 — #867 added test_vendor_missing_body_unit.py, all passing).

Label back to review: pending; re-review dispatched on this head.

[JarryShaw]

GOOD TO GO on 8cb69d27b — round-3 cross-review (opus; author sonnet). Label granted.

The round-3 enum.py delta is docstring-only — 34 changed lines, no code tokens, and the method body is byte-identical to the round-2 text already cleared. That is what lets rounds 1 and 2 carry over rather than needing re-derivation.

The exhaustiveness claim I asked about holds, and I reproduced it independently on merged main:

EnumRegistry subclasses: 111
MINTS (3):  ipx.socket.Socket | mh.cga_type.CGAType | reg.ethertype.EtherType
no own _missing_ (1):  ipv6.extension_header.ExtensionHeader   (inherits plain Enum -> cannot mint)

Same three by a second method — measured __members__ growth from an unassigned probe: Socket 15→17, CGAType 7→13, EtherType 160→163. So the new sentence is not false the way the old one was. Worth recording why a raw grep looks alarming and isn't: grep -rln extend_enum pcapkit/const/ hits nine modules, six with genuinely minting _missing_ bodies — OptionType, ReturnCode, ResponseKind, GroupingInformation, Method, StatusCode, Command, FEATCode, AppType — but not one mixes in EnumRegistry, so none has get() at all. They are the still-bespoke registries and sit outside the universe this docstring describes.

Both round-2 findings properly taken. The pre-#857 docstrings now lead with the true fact and state plainly "This test no longer discriminates that defect", naming the pin — verified accurate in all three parts: test_const_registry_protocol.py:1231 asserts both assertNotEqual and assertIsNot over (-1, -1.0, 0, '', None, False), contains no get call, and is untouched by this diff. And since NoDefaultType defines no __eq__, that inequality is genuinely load-bearing rather than a consolation pin.

The FilterType restoration is at test_const_enum_get.py:427-430 on the key path, and the review confirmed the part that mattered: FilterType.get(0) passes no default, so it returns from cls(key) before the default guard — the addition depends on nothing #864 changed. The method still fails pre-fix at :416, the default-resolves assertion, i.e. discrimination rests on the changed behaviour and not on the restoration. grep -rn over tests/, pcapkit/ and docs/: zero occurrences of the old method name.

Counts as methods: 74 / 8 / 33 / 92 (tests/vendor up from 86, #867's new file). Pre-fix still discriminates: 3 + 2F/2E + 1 methods, with the FAIL_RECORDS=4 → FAIL_METHODS=3 gap being test_omitting_the_default_still_raises_for_the_original_key's two subTests.

breaking stands, strongest on the ValueError → KeyError type change for get(name, unregistered_default).

[JarryShaw]

review: good-to-go re-confirmed on f23e9ed4d — the last deferred piece has landed, so this PR is complete.

Round 4 added the one test that #865's merge un-contended: tests/const/test_const_enum_no_mint.py::GetNoLongerMintsTests::test_unresolvable_string_key_with_default_in_unassigned_range, retargeted from

result = Hardware.get('Definitely-Not-A-Member', 40)
self.assertEqual(result.value, 40)

to asserting KeyError naming the key, with the member-count and absence checks retained. Its docstring quotes the ruling's accepted cost verbatim.

Why the verdict carries without a fifth review round, verified by me:

• git diff 8cb69d27b f23e9ed4d -- pcapkit/corekit/enum.py is empty — the code is byte-identical to the sha the cross-review cleared.
• The merge-base diff is the expected five files and nothing else: enum.py 35/19, builtin_parity 25/1, enum_get 96/28, enum_no_mint 12/6, registry_protocol 171/15.

The author measured Hardware fresh rather than trusting the round-2 handover — 42 before, 42 after, KeyError, key absent from __members__, 40 not in _value2member_map_. Identical to the earlier numbers; the count did not shift across #867 and #865.

On the redundancy question I asked: no collision with #865's work. GetNoLongerMintsTests is the get()-string-path sibling to UnassignedRangeDoesNotMintTests's int path, whereas ETHERTYPE_UNASSIGNED_PROBES, EtherTypeMixedMintTests and the retired test_masked_old_xerox_row_converts_by_source are all about EtherType's key-path ordering. Different registry, different axis — the test is not redundant.

tests/const/ is now 203/203 with zero failures, which retires the single red Python 3.13 leg that was this deferred test. Counts as methods: enum_no_mint 20, registry_protocol 74, enum_get 8, builtin_parity 33, tests/vendor 92.

Note the branch carries three commits (the fix, a main merge, and this test commit) rather than one. Deliberate — I told the author not to rebase after the earlier churn, and #865 landed by squash with its own merge commit, so history flattens on merge anyway.