fix(const,vendor,docs): audit registry case sensitivity, fold FEATCode.get (#903)

[JarryShaw]

• 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
• 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 #903 — the registry-wide case-sensitivity audit, under the owner's lenient criterion.

The audit table is the deliverable, and it lives in docs/source/conventions.rst beside the ruling, as the issue asks. Population, with the counting convention stated: 127 EnumRegistry subclasses across 124 files (117 int-valued, 10 StrEnum-valued), plus 24 non-registry enumerations by a runtime walk over both enum and aenum including nested classes — 151 in total. The 127 is 125 plus the two NGAP registries #880 landed while this was in flight.

The int tier needs no per-class row, and the table says why. A registry whose values are numbers has nothing for case to apply to; the only string that reaches get there is a member name, which Vendor.safe_name derives from the registrar's own column with casing preserved. Measured across all 151: exactly one would collide if names were folded (hip.Parameter, R1_Counter vs R1_COUNTER), and none has two str values that collide.

One defect found and fixed: FEATCode.get. It folded nothing, so it ran the case-sensitive default — but RFC 5797 §2, defining the very registry the crawler reads, states "IANA maintains uniqueness of feature names (FEAT codes) based on case-insensitive comparison." Both limbs hold: that comparison rule, and a measured spelling disagreement — RFC 2389 §3.2 recommends upper case on the wire while the registry spells 5 of its 15 codes lower case (of 64 CSV rows: 11 upper-case, 52 lower-case, 1 blank, 0 mixed). get('BASE') raised KeyError before. The fold is a fallback: an exact hit still delegates to the base, so precedence and the non-minting str path are untouched, and members keep the registrar's casing.

Two rows left open deliberately, both wider than a case fix and recorded in the table rather than acted on: a service-name lookup on the AppType transport registries (RFC 6335 §5.1 does make service names case-insensitive, but no such lookup exists to fold — new public API on a 6,000-member registry), and CommandType/ConformanceRequirement (the disagreement is measured, but nothing looks them up by string and neither inherits EnumLookup yet — #877 phase 2).

[JarryShaw]

Verified the one behaviour change's justification myself, since the whole PR rests on it. The citation holds, with one wrinkle the table should mention.

RFC 5797 §2, lines 149–151 — quoted accurately, verbatim:

Future specifications are allowed to "upgrade" a placeholder to a true keyword unless it is specifically declared 'immutable' below, but otherwise IANA maintains uniqueness of feature names (FEAT codes) based on case-insensitive comparison.

Note for anyone re-checking this: grep -n 'case-insensitive' rfc5797.txt returns nothing, because the phrase wraps across the line break (case- / insensitive). My first pass read that as the claim failing. It isn't.

RFC 2389 §3.2 supports limb 2 as the table says — "implementations are recommended to use upper case letters when transmitting the feature response." But the same paragraph opens with:

The feature-label and feature-parms are nominally case sensitive, however the definitions of specific labels and parameters specify the precise interpretation, and it is to be expected that those definitions will usually specify the label and parameters in a case independent manner.

That does not overturn the verdict — RFC 5797 §2 is limb 1 and explicit, and §3.2's own next clause points the same way — but a contributor who follows the :rfc:2389#section-3.2`` link will hit "nominally case sensitive" first and think the row misread its source. Please add half a sentence to that cell noting §3.2 calls the label nominally case-sensitive and defers to each feature's own definition, which is what RFC 5797 then supplies.

Two other checks, both clean:

• RFC 6335 §5.1 (lines 499–502) is quoted verbatim and is scoped to service names, not the Transport Protocol field. So the table is right to carry TransportProtocol on limb 2 and the TCP/UDP/SCTP/DCCP service-name row on limb 1; they are genuinely different rules in the same RFC.
• FEATCode's 15 members hold no case-fold collision (15 distinct folded), and neither do CommandType (4) or ConformanceRequirement (3) in the same file. The fold is safe here — and semantically apt, since RFC 5797 uses case precisely to separate true keywords from pseudo-codes while forbidding a pair that differs only by case.

Cross-review is running on a different model from the author.

[JarryShaw]

GOOD TO GO on c0b88bbf5 — cross-review on sonnet (author opus). Two prose fixes applied by me since the verdict; the code is byte-identical to what was reviewed.

It re-derived the finding the whole audit rests on rather than accepting it: that the criterion is vacuous on the 117 int-valued registries, not merely unmet. It read Vendor.safe_name (pcapkit/vendor/default.py:239-267, no .upper()/.lower(), casing preserved verbatim) and traced every call site where wire data reaches an int-valued get — ftp.py:99-112 casts to int() first, ngap.py:452-461 passes pycrate's decoded ints. It also chased the one apparent counterexample, ngap.py's Criticality.get, which does take a string: Criticality is a plain IntEnum outside pcapkit/const, so it is not one of the 117, and the table already gives it separate and correct reasoning. The audit does not conflate the two.

Also independently confirmed: 127 registries / 124 files, 117 int (5 flags) + 10 str, 24 non-registry enumerations (17 top-level + 7 nested) = 151; exactly one name-fold collision (hip.Parameter's R1_Counter vs R1_COUNTER) and zero str-value collisions; the fold never mints, with both lookup tables still at 15 entries after exact, folded and missing probes in a fresh process; and VendorTemplateParityTests really does pin the generated get against the rendered template, so regeneration cannot drop it.

One discrepancy, which I re-derived myself: 7 test methods fail pre-change, not the 6 the commit message claimed. My list matches the reviewer's exactly. The cause is worth recording — reverting the three source files and running under plain unittest gives Ran 15 tests … failures=2, errors=36, because subTests inflate the outcome count, so the method tally has to come from distinct FAIL:/ERROR: names rather than the summary line. Amended to 7.

Also amended: the FEATCode cell now notes that :rfc:2389#section-3.2 opens by calling the feature-label "nominally case sensitive" before deferring to the specific definitions, and that the RFC 5797 §2 sentence wraps at case- / insensitive so a line grep misses it. Sphinx build after the edit: build succeeded, 58 warnings — the list-table cell is well-formed.

Deferrals all judged correct, including esp.py:282's _resolve, which the reviewer confirmed is a real case-insensitive fold living outside any get() — and found RFC 7296 states no case rule for it, so it needs its own decision rather than a copy of this one.

Unverified and stated as such: the exact coverage percentages, and mypy/pylint/isort were not re-run.

[JarryShaw]

Resolve conflicts. And I'm thinking we should probably consider splitting conventions.rst into several files per category as it grows longer.

[JarryShaw]

Rebased onto main after #912 merged (9806f16aa) — 50813f1df. The conflict was not a content clash: #912 moved docs/source/conventions.rst to docs/source/contributing/conventions.rst, and GitHub scores an edit against a deleted path as DIRTY even where git's rename detection resolves it. Cherry-picked with -X find-renames, so the audit section landed on the moved file and the old path was not recreated.

Verified blob-identical to the head you reviewed, file by file — git rev-parse on all seven paths gives the same object for c0b88bbf5 and 50813f1df, including conventions.rst across the rename. So nothing changed but where one file lives, and the review: good-to-go verdict carries rather than needing a re-review. tests/const/test_const_ftp_featcode_case_903_unit.py still passes 15/15 on the rebased tree.

The four pre-existing test references to the doc already point at contributing/conventions.rst — #912 updated those, so no fixup was needed here.

On splitting conventions.rst: agreed, and it is getting hard to argue against. Verbatim:

I'm thinking we should probably consider splitting conventions.rst into several files per category as it grows longer.

It is now ~440 lines carrying three unrelated rulings — the mint/unmint criterion, sentinel naming, and the registry protocol plus this case-sensitivity audit. A reader wanting the sentinel rule reads past two hundred lines about enum minting to reach it.

I have not folded that into this PR, because it would turn a reviewed and verified change into a reviewed change plus an unreviewed restructure, and the doc move has already cost three rebases today. I will file it as its own issue once this lands, and the natural split is one file per .. _label: anchor already in the file — mint-criterion, sentinel-convention, registry-protocol — under docs/source/contributing/conventions/, with the parent becoming a toctree. Say if you would rather have a different cut.