docs(hopopt): cite RFC 8200 section 4.2 for the Opt Data Len sentence (#530)

[JarryShaw]

Closes #530.

One commit on top of 122d32795.

The correction, verified against the RFC rather than the issue

pcapkit/protocols/internet/hopopt.py:456 cited RFC 8200 §4.3 while quoting a sentence verbatim. I checked the citation against rfc8200.txt rather than taking the issue's word, because the sibling #517 asked for a section that turned out to be wrong:

• The quoted sentence — "Length of the Option Data field of this option, in octets." — is the Opt Data Len definition, and it sits at rfc8200.txt lines 580-581, inside §4.2 (Options), which spans lines 567-691.
• §4.3 (Hop-by-Hop Options Header) spans lines 692-734. Grepping that range for Opt Data Len or Option Data field returns nothing: §4.3 defines only Next Header, Hdr Ext Len ("Length of the Hop-by-Hop Options header in 8-octet units, not including the first 8 octets") and Options, and it defers TLV encoding with "Contains one or more TLV-encoded options, as described in Section 4.2."

So the issue is right, and it is right for the reason it gives: Hdr Ext Len is the whole header in 8-octet units, which is a different quantity from one option's Opt Data Len in octets.

This is milder than #517 and the docstring now says why. For ipv6_opts.py, §4.3 was simply the wrong header — that class implements §4.6. For HOPOPT, §4.3 is the header this class implements, so the old citation was not wrong about the class, only about the sentence. A reviewer who learned to spot "cites a header it does not implement" catches #517 and reads straight past #530. The docstring now cites §4.2 for the quote, keeps §4.3 as the header reference, and points at the existing Note: further down that legitimately cites §4.3 for the multiple-of-8 rule — the same two-part treatment #528 landed, adapted to the milder case.

No behavioural effect, stated plainly

This is a docstring change. _hopopt_option_length still returns schema_len + 2 and no code path is touched. Nothing about the parsed output, the wire format, or the arithmetic changes, and the PR claims no coverage of behaviour it does not affect.

The sibling sweep found nothing else

Every RFC 8200 section citation in the repo was enumerated, and every docstring that quotes the RFC verbatim (*" sites) was checked against the section it names. Four verbatim-quote sites exist in total:

Site	Cites	Verdict
pcapkit/protocols/internet/hopopt.py:456	§4.3 → §4.2	the defect, fixed here
pcapkit/protocols/internet/ipv6_opts.py:467	§4.2	correct, landed by #528
pcapkit/protocols/internet/ipv6_route.py:223	§4.4	correct — §4.4 line 758 has that Hdr Ext Len wording verbatim
pcapkit/protocols/schema/internet/ipv6_route.py:43	§4.4	correct, same sentence

The other §4.3 citations in hopopt.py (lines 1241 and, post-change, the header reference) are right: §4.3's Options field really does require the complete header be "an integer multiple of 8 octets long". hopopt.py:446 and :1254 cite §4.2 for the unknown-option action bits and for Pad1, both of which §4.2 genuinely defines. No sibling miscitation was found, so none is fixed and none is left outstanding.

A test that can actually fail

The issue suggests any test here would only "pin a previously unpinned citation". That is true of the obvious test — assert 'section-4.2' in __doc__ is a snapshot with no derivation — so this does something else.

tests/test_docstring_contract.py gains a fourth property: a docstring that quotes one of these RFC 8200 sentences verbatim must be introduced by a citation naming the section the sentence is in. It is keyed on the quote rather than on the file, walks module, class and function docstrings under pcapkit/, and takes the nearest preceding citation — which is what gives it teeth, since #517 and #530 both cited the correct section elsewhere in the same paragraph while misattributing the quote. An "is §4.2 mentioned anywhere" test passes on both defects.

What it cannot prove, said in the code: RFC8200_QUOTED_SENTENCES maps four sentences to their sections, and those section numbers were read out of the RFC — the RFC is not vendored here and the suite does not fetch it. So the table is a recorded reading and the test does not verify the RFC. What it verifies is the pairing, at every site, which is the invariant that broke twice. A companion rot-guard asserts every table key still matches some docstring, so a reworded paragraph fails loudly instead of silently narrowing coverage — the same reasoning as the existing test_known_defects_are_still_defects, pointed the other way.

I dropped a fifth entry (§4.6's Hdr Ext Len) because nothing quotes it; carrying a key that matches no site is exactly what the rot-guard exists to prevent. Values are frozensets because the TLV-deferral sentence is worded identically in §4.3 and §4.6 (lines 722 and 1268), so demanding one section would fail a correct citation in the sibling header.

Fails-without evidence

Environment: .venv/bin/python 3.14.7, PYTHONSAFEPATH=1 PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=<worktree>, with pcapkit.__file__ and tests._tiers.ROOT both asserted to resolve inside the worktree rather than the editable install, and printed. pytest-subtests is not installed; pytest 9.1.1's native subtests print a failing subtest's parent as PASSED, so the exit code is the signal below, not the summary line.

The check run against hopopt.py as it stands on 122d32795, with ipv6_opts.py and ipv6_route.py at the same commit alongside it:

--- RED: unfixed hopopt.py at HEAD (122d32795) ---
    DEFECT  pcapkit/protocols/internet/hopopt.py  _hopopt_option_length():
            the length of the option data field of t attributed to 4.3, not 4.2

--- GREEN: fixed worktree, whole package ---
    no defects -> test_quoted_rfc_sentences_cite_their_own_section PASSES

RED   defect count : 1
GREEN defect count : 0

That the two already-correct siblings produce no defect in the red run is the other half of the result: the check does not false-positive on #528's landed fix.

Regression run, tests/test_docstring_contract.py + tests/protocols/internet/test_ipv6_extension_unit.py + tests/protocols/test_option_roundtrip_unit.py, with pytest's exit code persisted to a file rather than read through a pipe:

66 passed, 9 warnings, 449 subtests passed      EXIT=0

One unrelated deletion, which turns main green

This also deletes the KNOWN_DEFECTS entry naming pcapkit/vendor/ipx/packet.py's process, and that has nothing to do with #530. main has been red on test_known_defects_are_still_defects since the rot guard landed, through an ordering accident: a8912b46f (#524) renamed that parameter to data, and 48c6f07f8 (#535) added this list four commits later without re-running against the newer base. The guard was working exactly as designed — it fires when an entry describes a bug that is gone.

The defect really is fixed rather than merely undetected: pcapkit/vendor/ipx/packet.py:180 reads def process(self, data: 'dict[int, tuple[str, str]]') and its docstring documents data at :184, so signature and docstring agree and there is nothing left for the entry to describe. Confirmed pre-existing and independent of this change by running the unmodified HEAD:tests/test_docstring_contract.py against the same tree, which failed on that subtest and nothing else. So this PR turns main green rather than papering over a live defect, and every other KNOWN_DEFECTS entry is left untouched. The module docstring records the removal and its cause, next to the equivalent ipx/socket.py removal it already described.

[JarryShaw]

✅ GOOD TO MERGE — independently confirmed against the RFC 8200 text itself that Section 4.2 (not 4.3) defines Opt Data Len as "length of the Option Data field of this option, in octets", and that reverting the citation reproduces #530 and is caught by the new DocstringCitationTests::test_quoted_rfc_sentences_cite_their_own_section (exit code 1).

[JarryShaw]

Detailed review (independent verification, falsify-not-bless)

Head sha reviewed: 01eef2b9b17b4d98fa628485a4aa79d14919b62e

What changed

• pcapkit/protocols/internet/hopopt.py: one citation, :rfc:8200#section-4.3 → `:rfc:`8200#section-4.2 in _hopopt_option_length's docstring, plus ~18 lines of prose explaining why 4.3 was wrong and why 4.3 remains correct for the other citation later in the same docstring (the "multiple of 8 octets" note).
• tests/test_docstring_contract.py: a new DocstringCitationTests class with two tests, backed by a RFC8200_QUOTED_SENTENCES table mapping four verbatim RFC 8200 sentences to the section(s) that actually contain them.

Independent verification of the core claim

I did not take the PR's RFC reading on faith. I fetched https://www.rfc-editor.org/rfc/rfc8200.html directly and asked for the literal text of §4.2 and §4.3. Result, verbatim from the fetch:

• §4.2 defines Opt Data Len as "8-bit unsigned integer. Length of the Option Data field of this option, in octets." — matches the quoted sentence in the docstring.
• §4.3 defines Hdr Ext Len as "8-bit unsigned integer. Length of the Hop-by-Hop Options header in 8-octet units, not including the first 8 octets" — a different field, different unit (8-octet units, not octets), and §4.3 explicitly defers TLV option encoding to §4.2 ("one or more TLV-encoded options, as described in Section 4.2").

This independently confirms the PR's central claim: §4.2 is the right citation for the Opt Data Len sentence, §4.3 is not, and §4.3 is still correctly cited elsewhere in the file for the "whole header must be a multiple of 8 octets" note (line ~1259 on the PR branch), which the docstring's new prose also claims and I confirmed by grep.

I also spot-checked the PR's claim about sibling issue #517 (ipv6_opts.py): on the PR branch, ipv6_opts.py already cites 8200#section-4.2 for the same sentence (fixed previously) and its docstring narrates having cited 8200#section-4.3 "until #517" — consistent with the new test's own claim about that history.

Falsification attempts

Working in my own worktree, checked out to the PR head commit (detached), I made two targeted edits to hopopt.py only, leaving the new test file untouched, then ran tests/test_docstring_contract.py::DocstringCitationTests under PYTHONSAFEPATH=1 <repo>/.venv/bin/python -m pytest, reading the exit code from a file rather than a pipe:

1. Reintroduce the exact hopopt.py:456 cites RFC 8200 §4.3 for a sentence that lives in §4.2 #530 defect (revert the citation from 4.2 back to 4.3, quoted sentence unchanged): test_quoted_rfc_sentences_cite_their_own_section FAILED, exit code 1. The failure message correctly names hopopt.py, _hopopt_option_length, and reports "attributed to 4.3, not 4.2". This confirms the new test is a genuine regression guard for hopopt.py:456 cites RFC 8200 §4.3 for a sentence that lives in §4.2 #530, not decoration.
2. Keep the wrong citation (4.3) but paraphrase the quoted sentence (changed "the length of the Option Data field of this option, in octets" to "the size of the Option Data field for this option, in octets"): both new tests PASSED, exit code 0. The wrong citation survives undetected. This is a real gap in the check's coverage — but it is not a hidden one: the PR's own module-level docstring for RFC8200_QUOTED_SENTENCES and the test's own docstring both explicitly disclose it ("Prose that paraphrases the RFC, or cites a section without quoting it, is not matched and cannot be failed"). So this is a documented, accepted limitation rather than an undisclosed blind spot, and it does not bear on whether the shipped fix is correct — it only bears on how much protection the new test offers against a future copy-paste of this paragraph that also reworks the wording.

Both experiments were reverted (git checkout -- pcapkit/protocols/internet/hopopt.py) and the worktree returned to a clean state on the PR head before I moved on; diff against the PR head confirmed zero residual changes.

Pre-existing, unrelated observation

Running the full tests/test_docstring_contract.py module (not just the new class) on both the PR head and on main (122d327) produces the same one pre-existing subtest failure: DocstringParameterTests::test_known_defects_are_still_defects for Finding(module='pcapkit/vendor/ipx/packet.py', function='process', subject='data') — that KNOWN_DEFECTS entry no longer reproduces on either branch. This is unrelated to #530/#538 (present identically on main before this PR), not introduced or worsened by this change, and out of scope for this PR to fix. Flagging it only for visibility, not as a blocker.

Numbers re-derived

• RFC 8200 §4.2 vs §4.3 text: re-derived independently via direct fetch of the RFC, not taken from the PR description — matches.
• Test file diff: 172 additions / 3 deletions as reported by gh pr list --json files — confirmed by git diff --stat against main.
• New tests: 2 (test_quoted_rfc_sentences_cite_their_own_section, test_the_quoted_sentences_are_still_quoted_somewhere), both pass on the clean PR head (exit code 0).

What remains unverified

• The other two entries in RFC8200_QUOTED_SENTENCES (the Hdr Ext Len sentences for Hop-by-Hop and Routing headers, §4.3/§4.4) were not independently re-fetched against the RFC text by me beyond the general §4.3 confirmation above — I did not separately verify the §4.4 Routing header wording claim.
• I did not run the full repository test suite (only tests/test_docstring_contract.py), since the code change is a docstring-only string-literal edit with no behavioral surface; this seemed proportionate but is worth naming as a scope limit.

[JarryShaw]

✅ GOOD TO MERGE (re-review of amended head e24f17dc6) — the entry-deletion is clean (exactly one Known(Finding('pcapkit/vendor/ipx/packet.py', 'process', 'data')) removed, nothing else touched) and tests/test_docstring_contract.py now runs EXIT=0, 7 passed, 13 subtests passed on this head — the branch does turn the currently-red main green on test_known_defects_are_still_defects. Note: only verified locally — CI has not yet completed on this head.

[JarryShaw]

Detailed review (independent verification, falsify-not-bless) — re-review of amended head

Head sha reviewed: e24f17dc60c5cf7f01ba0d7a326b061a4a57a475 (supersedes my earlier review of 01eef2b9b, which no longer exists as this branch's head after a force-push).

What changed since my last review

git diff 01eef2b9b17b4d98fa628485a4aa79d14919b62e e24f17dc60c5cf7f01ba0d7a326b061a4a57a475 -- tests/test_docstring_contract.py shows exactly two things: a Known(Finding('pcapkit/vendor/ipx/packet.py', 'process', 'data'), ...) entry deleted from KNOWN_DEFECTS, and a module-docstring paragraph added explaining why (parameter renamed by a8912b46f #524, rot guard added four commits later by 48c6f07f8 #535, so main went red at the guard's own birth). pcapkit/protocols/internet/hopopt.py is byte-identical to the previous head — the RFC citation content is unchanged.

Deletion scope, independently confirmed: exactly one Known(Finding(...)) block removed, zero KNOWN_SWALLOWED changes, and I independently confirmed on disk that pcapkit/vendor/ipx/packet.py:180 reads def process(self, data: 'dict[int, tuple[str, str]]') with data documented at :184 — signature and docstring agree, so the defect really is gone rather than merely undetectable.

Green confirmation, independently run (not taken from the coordinator's numbers): on the clean amended head, PYTHONSAFEPATH=1 <repo>/.venv/bin/python -m pytest tests/test_docstring_contract.py, exit code read from a file: EXIT=0, 7 passed, 13 subtests passed. So yes — this branch turns the currently-red main (EXIT=1 at 122d32795, same subtest, independently reproduced in my first review) green.

The RFC §4.2 claim — carried forward, not re-verified this round

Nothing in the amend touches hopopt.py, so I am carrying forward my prior independent verification against the live RFC 8200 text (fetched directly from rfc-editor.org): §4.2 defines Opt Data Len with the exact quoted wording, §4.3 defines a different field (Hdr Ext Len, in 8-octet units) and explicitly defers TLV encoding to §4.2. That finding stands unchanged.

New falsification: naive "cited anywhere in the docstring" vs. the shipped "nearest preceding" check

This is the substantive new work this round, in response to a specific request to confirm the check depends on nearest preceding and not mere presence. I constructed a synthetic misattribution in my own worktree (checked out to e24f17dc6 detached, single edit to hopopt.py, reverted afterward — zero residual diff confirmed):

Changed the docstring to read See :rfc:8200#section-4.2 for background. Per :rfc:8200#section-4.3, an option's ``Opt Data Len`` field ... counts *"the length of the Option Data field of this option, in octets"* — i.e. the correct section (4.2) appears earlier in the same docstring, but the citation immediately introducing the quote is the wrong one (4.3).

• Shipped test (misattributed_quotes, nearest-preceding via flat[:start] + cited[-1]): FAILED, exit code 1, correctly reporting _hopopt_option_length ... attributed to 4.3, not 4.2.
• Hand-written naive check (same loop, but set(re.findall(...)) over the whole docstring instead of only the text before the quote, testing intersection with the allowed section set): zero defects found — it would have blessed this exact misattribution, because 4.2 is present somewhere in the docstring even though it isn't what introduces the quote.

This is a clean, decisive confirmation that "nearest preceding" is load-bearing, not cosmetic: a materially simpler and more obvious implementation of this same idea would have missed the defect class the test exists to catch.

I did not additionally reconstruct the historical pre-fix #517/#530 sentences byte-for-byte to check whether a naive check happens to have passed on those specific historical diffs — the synthetic case above is a direct, reproducible demonstration of the same principle and I judged it sufficient without re-deriving git archaeology on already-merged, already-fixed files.

Sibling false-positive check

ipv6_route.py:223 and schema/internet/ipv6_route.py:43 (both citing §4.4 for the routing-header Hdr Ext Len sentence) are exercised by the same full-suite run above (7 passed, 13 subtests passed, 0 failures) — no false positive on either site.

The dropped §4.6 table entry

Checked whether RFC8200_QUOTED_SENTENCES is missing an entry for the Destination Options header's own Hdr Ext Len sentence (§4.6, parallel to the §4.3 Hop-by-Hop and §4.4 Routing entries already in the table). Grepped ipv6_opts.py for Hdr Ext Len / Destination Options header / 8-octet units: every hit is either an ASCII wire-diagram field label or a paraphrase ("It is the whole extension header, fixed part included, that has to be a multiple of 8 octets") — no verbatim quotation-marked reproduction of the RFC's defining sentence for that field exists anywhere in the current tree. Given the test's own explicit design boundary ("prose that paraphrases the RFC ... is not matched and cannot be failed"), omitting a table entry for a sentence nobody quotes verbatim is correct, not merely convenient — an entry for it would fail test_the_quoted_sentences_are_still_quoted_somewhere immediately, which is exactly the rot-guard working as designed. I verified this by direct grep rather than taking the absence on trust.

CI status

Not verified in CI. Per the coordinator, the runner is backed up and this head has not been observed going green in CI, only locally by both the coordinator and me independently. Reporting that plainly: local test run only, CI pending.

What remains unverified

• CI has not completed on this head; the green result above is local-only (PYTHONSAFEPATH=1 + venv python + confirmed pcapkit.__file__ inside this worktree).
• I did not re-verify the §4.4 Routing-header RFC wording claim beyond my first review's pass (unchanged in this amend).
• I did not reconstruct the exact historical #509 follow-ups: one call site left open-coded, a dangling cross-reference, and a wrong RFC section #517 pre-fix docstring text to confirm a naive check would have passed on that specific historical diff; my falsification uses a synthetic but structurally equivalent construction instead.