refactor(const): convert AppType's six %-style raises to f-strings (#792)

[JarryShaw]

Please follow the guide below

• You will be asked some questions, please read them carefully and answer honestly

• Put an x into all the boxes [ ] relevant to your pull request (like that [x])

• Use Preview tab to see how your pull request will actually look like

• 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

What is the purpose of your pull request?

Tick the commit type your subject line carries.

• 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

  Converts the six remaining %-style raises in the generated AppType const module to f-strings (#783's ruling). Each message was proven old == new byte-identical, and re-derived through its real call path (AppType.get('http', proto=tcp), AppType.get(80, proto=undefined), extend_enum(AppType, ...), out-of-range AppType(...)) with matching live exception text before/after.

  The edit is in the BASE template of pcapkit/vendor/reg/apptype/apptype.py (doubled braces) and applied by hand to the const file to avoid an IANA crawl rewriting member rows. Rendering BASE(...) and diffing against the const file gives exactly 2 non-equal regions — the TABLE/MISS-dependent docstring table and _missing_ range tail — and 0 lines differing elsewhere.

  Left unconverted: the three %-formatted dunders (__new__/__repr__/__str__) — out of the issue's raise-only scope, and __new__'s format sets every one of ~12,391 real members' StrEnum value, a far bigger blast radius than an error path. The consider-using-f-string disable stays earned by those and by extend_enum's '%d' calls.

  19+6 tests pass under pytest and unittest alike; mypy clean before/after; pylint 0 new findings, R0801 unchanged at 575.

Closes #792

[JarryShaw]

7 FAILED legs — Python 3.10/3.11/3.12/3.13/3.14 and Engines 3.12/3.13 — and they are all one defect. Root cause, from the 3.10 job log:

self.assertIn(
    "raise ValueError('%r is not a valid %s' % (value, cls.__name__))",
E   AssertionError: "raise ValueError('%r is not a valid %s' % (value, cls.__name__))" not found in '...'

tests/const/test_const_enum_builtin_parity.py:588, test_every_bespoke_template_carries_the_guard. It exists for #647 — "a regeneration must not undo the guard" — and it checks presence by searching each template's source for the literal %-style string. This PR reformatted that raise, so the literal no longer matches.

The guard was not removed; the test pins its formatting rather than its presence. But it is not simply over-specified, because of this:

BESPOKE_TEMPLATES = ('pcapkit.vendor.tcp.flags', 'pcapkit.vendor.ftp.command',
                     'pcapkit.vendor.http.method', 'pcapkit.vendor.reg.apptype.apptype')

all four carry the %-style guard on origin/main

The test loops over four templates with one expected string. This PR converted one of them, so that string can no longer serve all four — it now fails for reg.apptype.apptype and passes for the other three.

So there is a scope decision, and I am taking the narrow one:

• Not converting the other three templates' guards. const(apptype): six pre-existing %-style raises remain where the tree convention is f-strings #792's scope is the six raises in the AppType const module; sweeping three unrelated vendor templates is a different change and would make this const diff much harder to review.
• Instead, the test's expectation becomes per-template, so each template's actual guard form is pinned. That keeps what the test is for — catching a removed guard, which is the generated enum guards raise a bare ValueError in 113 of 117 const modules, and tcp.flags.Flags has no guard at all #647's whole point — without pinning a formatting style the maintainer's convention is actively moving away from. Asserting "either form" would weaken it; asserting per-template does not.

The remaining three templates' %-style guards get folded into #796, which is already the vendor-side % cleanup and already blocked on this PR.

Worth noting the worker's pre-flight check was not wrong, just aimed one target over: it reported "found no test pinning any of the three dunders' exact format", which was true. A test pinning one of the six raises it converted is what it missed. review: needs-changes; routed.

[JarryShaw]

Revised to ecfb1cddd. The test change is the shape I asked for, and I verified it still bites rather than taking the claim:

baseline                                  -> Ran 1 test  OK
guard -> pass in pcapkit/vendor/tcp/flags.py
  AssertionError: "raise ValueError('%r is not a valid %s' % ...)" not found in '...'
  : pcapkit.vendor.tcp.flags no longer emits its guard; see GitHub issue #647
  FAILED (failures=1)
restored; git status: 0 modified

So a genuinely deleted guard still fails, on the right subTest, with #647 named. That was the risk in relaxing a shared literal and it is closed.

BESPOKE_TEMPLATES became a dict[module, guard_literal] — each of the four pinned to its own actual text, three still %-style and reg.apptype.apptype on the f-string form with doubled braces as it appears on disk. No or-of-both-forms, for the reason I gave: a template that lost its guard and gained an unrelated raise ValueError would slip through. The #: comment explains why one shared literal can no longer pin all four and points at #796 for the remaining three.

The tests/ grep it ran is the part worth keeping. Searching for all six converted raises' literal text, old and new, single- and double-brace, found exactly one file asserting on them — this one. Two other hits in tests/const/test_const_enum_get.py and tests/corekit/test_fields_numbers_unassigned_enum.py are docstring prose quoting aenum's own exception text for different registries (Hardware, BlockType), not assertions against our code. That is the check whose absence let this reach CI, and it is now the standing instruction for any source-text change.

It also confirmed requests 2.34.2 is importable in its environment, so both skipUnless guard tests actually ran rather than skipping — which is how the original failure got past a green local run.

Unchanged and re-derived: the template-equivalence diff still shows exactly 2 non-equal regions (the TABLE docstring block and the MISS-dependent _missing_ tail), all six byte-identity pairs still MATCH, and main's move to 2d1c8675d touches only examples/generators/pcapng.py and tests/integration/test_pcapng_end_to_end.py — no overlap.

tests/const/test_const_enum_builtin_parity.py 20/20 with 565 subtests under pytest and unittest alike; the apptype and vendor-generator modules 25/25 under both.

CI is re-running on the new head — 5✅ / 3⏭ / 0❌ with 21 in flight as I write. The cross-review on this PR died on a rate limit (429 · Too many tokens, model fable) before producing a verdict, so I am dispatching a fresh one on a different model now that the head is stable. Label stays review: needs-changes until both CI and that verdict are in.

[JarryShaw]

Cross-review at ecfb1cddd (sonnet — the fable attempt died on 429 · Too many tokens having produced nothing, so this is the substitution): GOOD TO GO, with one fix I am making rather than waving through, because the defect it found is mine.

The test's comment cites the wrong issue. It says "#796 tracks sweeping them" — but #796 is vendor: thirteen re.sub sites pass a regex flag as the positional count, entirely unrelated. I wrote that mis-citation into the worker's brief. The reviewer went further and searched the tracker: no issue existed for sweeping the other three bespoke templates' % raises. Filed now as #798, and the comment is being corrected to point there.

And it caught that Closes #792 would have closed #792 with its own criteria unmet. #792's "Expected behavior" read "f-strings throughout, and the disable dropped once nothing needs it" — neither of which this PR does, correctly, since __new__'s format sets the StrEnum value for ~12,391 members. I have narrowed #792's body to match its own title (the six named raises), with the over-reach recorded and the remainder pointed at #798. So Closes #792 is now accurate rather than premature.

What it verified independently, all five starred claims:

• Byte-identity through live call paths on two real worktrees — base f41e5f880 and head ecfb1cddd, diff of the two output files empty on every line. It also checked the %r/!r and %s/{} matrix specifically on TransportProtocol as an aenum.IntFlag, for undefined (0), a single bit, and a composite.
• Template equivalence re-derived from scratch with two unique sentinels for TABLE/MISS: the 118-line prefix and 284-line middle matched byte-for-byte, only the docstring table and _missing_ tail differed — exactly the two claimed regions, with all six raises inside the matched span.
• No seventh % raise missed — it grepped every raise in both files: exactly 6 ValueError (all converted) plus 1 ProtocolError that was already an f-string from const(apptype): per-registry __canonical__ divergence makes get(port, proto=<multi-bit>) return a different service #759/fix(const): refuse a multi-bit proto in AppType._dispatch rather than resolving it LSB-first (#759) #783.
• CI green on ecfb1cddd: 27 pass, 3 skip, 0 fail.

One pre-existing test weakness it surfaced by mutation, not introduced here: moving the guard text into a dead comment (pass # raise ValueError(...)) still passes, because assertIn checks substring presence rather than liveness. It is caught in practice by the sibling test_the_tcp_flags_template_renders_the_committed_module, which full-file-diffs and does fail on that mutation. Noting it rather than expanding this PR.

It marked the "pylint R0801 unchanged at 575" claim UNVERIFIABLE — it did not run a whole-tree scan, and said so instead of implying it had.

[JarryShaw]

Revised to ae6d22f2a. The citation is corrected and #796 is gone from the tree and the commit message both — verified:

$ git grep -c "#796" ae6d22f2a -- tests pcapkit   ->  none
$ git log -1 --format=%B ae6d22f2a | grep -c "#796" ->  0

The comment now reads "#798 tracks sweeping them, along with the %-formatted dunders and dropping the f-string disable" — the worker widened my wording on its own, correctly: "sweeping them" alone understated #798's scope, which carries the three templates, the dunders and the disable.

To be clear about whose error this was: I put #796 in the brief, and #796 is the unrelated re.sub positional-count issue. The worker also found it in its own commit message, which I had not thought to check, and fixed both.

tests/const/test_const_enum_builtin_parity.py 20 passed / 565 subtests under pytest, 20 OK under unittest, no skips — a comment-only change, but confirmed rather than assumed.

CI is re-running on the new head: 16✅ / 3⏭ / 0❌ with 11 in flight. Setting review: good-to-go on the review verdict — the cross-review passed at ecfb1cddd and this amend touches one comment line, which cannot affect any of its five starred claims. I will say so if CI turns red rather than letting the label imply green.

Unpublished and unmerged — yours to take. Closes #792, whose acceptance criteria I narrowed to match its own title, with the remainder tracked at #798 (blocked on this PR merging, along with #796 and #775). Landing this unblocks three issues.