refactor(const,vendor): f-string the three remaining %-formatted __repr__ methods

[JarryShaw]

Closes #818

• 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 — 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
• feat
• perf
• refactor
• test
• docs
• ci
• chore

---

Description

The three __repr__ methods #817 left out of scope: ftp.ReturnCode,
http.StatusCode, pcapng.OptionType — const module and vendor template each.
Derived via an ast walk scoped to __repr__, not #818's count of eight
(which folds in unrelated _missing_ guards and the already-converted
apptype, split out per the issue's last comment).

Const modules regenerated from their vendor LINE template (round-tripped
against the old template first) rather than hand-edited. repr() verified
byte-identical for all 161 members, before and after.

Extended test_const_enum_builtin_parity.py with an ISSUE_818_TRIPLE
mirroring ISSUE_804_PAIR. New tests fail on stock c287152d7.
coverage run -m pytest tests/const/ tests/vendor/test_ftp_return_code_unit.py:
39263 passed before and after; the three __repr__ lines move to covered.

[JarryShaw]

Cross-review verdict: GOOD TO GO (opus; author was sonnet). The conclusion holds, and the review refutes the per-file numbers I published — which I have now re-derived myself.

My %-site counts were wrong, and wrong in a way that mattered. I posted 5 / 10 / 5 / 5 / 4 / 5 from a crude grep. What pylint --enable=consider-using-f-string actually fires, with the module pragma stripped, is 5 / 0 / 10 / 0 / 6 / 0 — my values were transposed across the pairs, and the crude count was the wrong measure anyway.

The substantive error: the three vendor/ files have no module-level pragma of their own. The # pylint: disable=line-too-long,consider-using-f-string at vendor :32 / :28 / :31 sits inside the LINE = lambda template, which starts at :29 / :25 / :28 — it is template text destined for the generated const/ file. Verified two ways:

vendor/{ftp/return_code,http/status_code,pcapng/option_type}.py  pragmas BEFORE the template: 0
const/ftp/return_code.py (no template)                           pragmas BEFORE: 1
stripped vendor/ftp/return_code.py -> pylint fires 0     <- nothing to remove
stripped const/ftp/return_code.py  -> pylint fires 6     <- genuinely needed

So "the disables were correctly left in place on all six" is vacuous on the vendor half. The three const/ pragmas are live; the vendor ones cannot be removed because they are not pragmas in those files at all. Conclusion unchanged — no dead pragma on any of the six, so no code change needed — but the reasoning was wrong for half the files, and my own column-0 grep "confirming" a pragma in all six failed for the identical reason: the template's lines also begin at column 0 inside the triple-quoted string.

Verified independently and not in dispute: all 161 members' repr, str and value identical, with a known-positive control that injected a one-character change and correctly reported 94 mismatches; the templates' braces are correctly doubled, shown by rendering both versions and getting a diff of exactly one line per pair at render lines 103 / 40 / 51, matching the committed __repr__ line numbers; scope exactly seven files, 1 1 on each source; and "the three remaining" re-derived by an AST walk over every __repr__ in pcapkit/ — exactly 3 on stock, 0 on the head.

Two corrections to the test claims: 2 of the 4 new tests fail on stock, not all four (6 subtest failures across two), and one of the passers is tautological on stock since it compares repr() against the %-form that on stock is repr — the right shape for a regression guard, but not evidence of a fix. Coverage is UNVERIFIED; the reviewer spent its budget on the claims above, which was the right trade.

Four non-blocking findings recorded below.

[JarryShaw]

Four follow-up findings from the cross-review, none blocking this PR.

1. The vendor test does not test the property that can actually break. assertNotIn('%', repr_line) plus assertIn("f'<", repr_line) would both pass on a template with single braces. A single-brace regression would fail none of the four new tests — it would surface as a NameError on self at the next make vendor regeneration, far from the change. The check that closes it is to call LINE(...) with dummy args and assert the rendered __repr__ line equals the committed const/ line; the reviewer had to write exactly that to verify the braces, so it is known to work.

2. source.index("def __repr__(self) -> 'str':") is brittle in the same test — a first-occurrence text search that raises ValueError (a test error, not a clean failure) if the signature text changes, and would silently check the wrong method if the Vendor subclass ever gained a __repr__ above the template.

3. %d → {} on OptionType.opt_value is a strict loosening. %d raises TypeError on a non-int; {} renders whatever it gets. Measured: every current opt_value is a non-negative int and opt_name is str, so output is identical today — but the conversion removes a type guard that used to fail loudly. Worth a line in the PR body rather than a code change.

4. The AST helper's blind spots, all acceptable but worth knowing. percent_format_lines_in_repr() is a genuine ast.parse + ast.walk gated on FunctionDef named __repr__, not a heuristic dressed up as one. It would miss: AsyncFunctionDef; a non-literal left operand (fmt = '<%s>'; return fmt % x); .format(); an aliased __repr__ = _impl; and — by design — a % reintroduced just outside __repr__, in __str__ or _missing_. That last one is deliberate, since a whole-module check reads nonzero both before and after here.

Items 1 and 2 are test-quality improvements in a file this PR already touches; if they are wanted they are cheap to fold in now, otherwise they want their own issue. Item 3 is one sentence of PR prose. Item 4 is informational.

[JarryShaw]

Addendum, corroboration only: the full-flag pylint run finished and the verdict is unchanged.

Under the repo's exact PYLINT_FLAGS (Makefile:166) with the pragmas intact, the six files rate 9.80/10, with no C0209 consider-using-f-string and no E/W/F. Everything visible is R0801 duplicate-code between each const/ file and its vendor/ template — which is an artifact of linting a hand-picked six, since duplicate-code is computed across whatever is in one run and the repo lints pylint $(PYLINT_FLAGS) pcapkit as a whole. That duplication is inherent to the generator design: a template that emits a const file necessarily contains that file's text.

The reviewer flagged its own limitation, which is worth keeping visible: it piped that run through tail -30, so only the last 30 lines were captured and a finding in the truncated head cannot be ruled out; the worktree has since been removed, so it was not re-run.

That does not move the conclusion, because the decisive measurement was the other run — pragmas stripped, a single check enabled, full output captured — which gave 5 / 0 / 10 / 0 / 6 / 0 and is what establishes both that the three const/ pragmas are live and that the three vendor/ files have no pragma of their own. I re-derived that one independently. Treat the full-flag number as supporting evidence, not as the basis.