fix(const): value-lookup in Method.get, so BASELINE-CONTROL/VERSION-CONTROL resolve (#908)

[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible -- 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

Closes #908. Method.get checked only _member_map_ (names), never _value2member_map_ (values), so the two methods whose name differs from their value -- BASELINE_CONTROL/'BASELINE-CONTROL' and VERSION_CONTROL/'VERSION-CONTROL' -- fell through to an unregistered member with the wrong safe/idempotent. httpv1.py's permissive method regex ([A-Z][A-Z-]*) lets a real capture reach this, so a successful parse silently reported wrong metadata.

Delegated to the base EnumLookup.get, keeping the override only for the unregistered-member fallback -- the issue's own suggested one-liner needed get to move from staticmethod to classmethod first, since zero-argument super() has no first argument to bind otherwise. Kept default's existing meaning (the unregistered member's value) rather than the base's NO_DEFAULT/registered-value-only semantics, since that would be caller-visible.

Case-sensitivity (#896/#907) and caller's-own-casing (#860) are unchanged, each pinned by its own test. Fixed in the crawler template (pcapkit/vendor/http/method.py) and the generated file together, verified byte-identical by rendering the template against the committed file. Added an end-to-end httpv1 test pinning the real parse path, plus a template/generated-file parity test so a regeneration can't drop the fix.

[JarryShaw]

Cross-review on opus (author sonnet) returned NEEDS CHANGES, and it was right. Fixed in d6663df38; the PR now stands at review: pending until CI clears, and the finding is worth recording because it is the kind CI cannot see.

except KeyError is narrower than what the base raises. Delegating routes a non-str key through cls(key) → _missing_ → ValueError, which the override does not catch. I re-measured on both trees rather than taking the report:

              origin/main e38d695ae            this PR
get(42)       AttributeError: no 'upper'   ->  ValueError: 42 is not a valid Method
get(None)     AttributeError: no 'upper'   ->  ValueError: None is not a valid Method
get(b'GET')   RETURNED name=b'GET' value=b'GET'  ->  ValueError
tables        (40, 40) -> (40, 40) on both — no minting either way

The bytes row is a return-to-raise change, not merely a different exception type, and bytes is the plausible mistake here: _RE_METHOD is a bytes pattern and httpv1 is bytes throughout, so the live call at httpv1.py:434 is safe only because it wraps the match in self.decode(...). Raising is the right behaviour — a bytes-valued Method is not something this registry should hand back — but it shipped with no Raises: clause, no test, and no mention, in the same commit that declined to widen default expressly because that would be caller-visible. Fixed with a Raises: clause and a new NonStrKeyTests, all four of which fail against unmodified main, in the crawler template as well as the generated module.

The reviewer also corrected me on the default divergence, and I had it backwards. I framed #915 as creating a divergence with #913. In fact #913 puts two contracts inside one file deliberately — FEATCode.get at NO_DEFAULT, Command.get left at default=None, with a test section headed "Contrast, pinned so the default is not quietly widened". The real axis is new override versus pre-existing override, and #915 makes the same conservative call #913 does. A third contract already predates both: pcapkit/const/pcapng/option_type.py:238 carries default: 'int' = -1. So after both land there are four distinct contracts across five overrides, none of them introduced here. A tree-wide default audit would be worth having, but it is not this PR's job.

Confirmed clean by the reviewer with its own derivation: the staticmethod→classmethod switch is caller-invisible (no bare-callable use, no subclass of Method, and the __func__ introspection at test_const_registry_protocol.py:177,902 walks only CONVERTED and GENERATED_SAMPLE, which exclude Method); name-vs-value precedence is inert — across all 40 members there is no string that is one member's name and a different member's value; template parity holds to one trailing newline the test's own normaliser re-adds; and the 8-of-12 pre-change failure count re-derives exactly.

Unverified and stated: the author's 100% coverage figure did not reproduce at the reviewer's narrower scope (94%, missing lines 57/312/316), and test_docstring_contract.py plus test_const_registry_protocol.py were cut at the time budget after ~9 minutes with no output.

[JarryShaw]

GOOD TO GO — the re-review closed its own finding on d6663df38, and I have made one further fix it flagged in my test, so the head is now c6695e019.

It attacked the hole I asked about — a non-str key where cls(key) might fail before _missing_ — and could not find one. aenum catches the unhashable TypeError internally, falls through to a linear search, then _missing_, so the isinstance guard always fires:

['GET']  {'a':1}  {1,2}  bytearray(b'GET')  memoryview(b'GET')  3.14  1+2j  object()
  -> ValueError every time;  final tables (40, 40)
Method overrides _validate_value: False   (so it is the base's no-op)

It also confirmed str subclasses and Method members themselves take the str path and resolve normally, which is correct and matches the base's isinstance(key, str) — so the clause is accurate as written. tests/test_docstring_contract.py passes, and not by luck: unreachable_exceptions() only flags a clause when the body is judged incapable of raising, and super().get(key) is not on its inert-call list, so a clause documenting a callee's exception is correctly exempt. Its two earlier UNVERIFIED suites now pass too — test_const_registry_protocol + test_const_enum_get, 82 tests, OK.

Coverage rose on exactly the line raised: pcapkit/const/http/method.py 94% → 96%, with line 312 — _missing_'s non-str guard, the line the fix newly makes reachable from get — now covered. The vendor file's misses shifted by exactly +14 (248→262, 262-264→276-278, 296→310), which independently confirms the only vendor change is the 14 docstring lines inside the LINE template.

And it was right about a weakness in a test I wrote, so I fixed it rather than accept "not worth a revision". My test_the_docstring_documents_the_value_error used Method.get.__func__, which a staticmethod does not carry — so against the pre-fix module it died with AttributeError before any assertion ran. It was pinning classmethod-ness, already covered by DefaultSignatureTests, rather than the clause it is named for. Now it uses inspect.getsource(Method.get), which works on both descriptor kinds, and asserts a distinctive sentence instead of the bare word ValueError, which was near-vacuous over 4 kB of docstring. Verified against a pristine origin/main worktree: it now fails with AssertionError: 'Raises:' not found. The pre-change tally moves from failures=1, errors=3 to failures=2, errors=2 — still four distinct method names.

Unverified and stated: the author's original 100% coverage figure at its own wider scope was never reproduced (96% at the reviewer's narrower scope, residual lines explained); the Sphinx build against the rewritten docstrings; and test_const_enum_builtin_parity.py / test_const_enum_no_mint.py, both of which call Method.get.