docs: retarget every dead :rfc:959 sub-section anchor (#944)

[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 #944.

RFC 959's rendered HTML carries only section-1 through section-8, so the six :rfc:959#section-4.1 citations named an anchor that does not exist — a live link to nothing. Per the ruling on #944 (*"Sure `section-4` is good."*) all six now cite `:rfc:`959#section-4.

Three template/generated pairs, each string a literal in the vendor/ template, so both halves are hand-edited; I verified the cited lines end up byte-identical across all three pairs. No crawl, no regeneration.

A finding this change deliberately does not act on. One of the six is different in kind, and the ruling was given before it was known. pcapkit/{const,vendor}/http/method.py says "case-insensitive because :rfc:959#section-4 says FTP command codes are not" — but §4 FILE TRANSFER FUNCTIONS contains no case-sensitivity language at all. The statement lives in §5.3 COMMANDS: "The command codes are four or fewer alphabetic characters. Upper and lower case alphabetic characters are to be treated identically." That is the only such sentence in the whole document, and it sits between id="section-5" and id="section-6", so it is outside §4. The two ftp/command.py pairs are fine — §4 does contain §4.1 FTP COMMANDS, which supports "type of kind of command".

So this PR fixes the dead anchor at all six sites as ruled, and leaves that one site citing a section that does not say what the docstring says it says. Raised separately rather than widened into here, since picking #section-5 for it is a second editorial call.

Test. #943's shape invariant accepts section-4.1 as well-formed and by construction cannot see a dead anchor, so this adds a narrower second check: a KNOWN_DEAD_ANCHORS denylist and test_no_known_dead_anchor_citations. Offline by design — CI has no network — and the docstring states plainly that a denylist proves nothing about a fragment it has never heard of.

tests/project: 207 passed, 1 skipped, 562 subtests. Without the fix the new test fails with DeadAnchorFinding(path='pcapkit/vendor/ftp/command.py', rfc=959, fragment='section-4.1').

[JarryShaw]

NEEDS CHANGES at 2f5463872 — Fable cross-review (author sonnet), and it is right: the PR fixed the six pcapkit/ sites while five more live :rfc:959#section-4.1`` roles survived elsewhere, four of them Sphinx-rendered. Verified myself before accepting it:

docs/source/contributing/conventions/registry-protocol.rst:224,281,337,443
tests/const/test_const_enum_no_mint.py:2111

Two corrections to its detail, neither weakening the finding. It attributed those four to #945/d7489c61c; git log -S puts them at #913 (1f4337e12) — #945 only moved them into the split file. And it said three of the four carry the §4.1-vs-§5.3 misattribution; it is all four — line 281 cites 959#section-4.1 directly above the quote "Upper and lower case alphabetic characters are to be treated identically", which is §5.3's own sentence.

Two further dead roles the sweep then turned up, same class, cited for the case rule: tests/protocols/application/test_ftp_unit.py:90 and docs/source/changelog/1.5.0.rst:759 both say :rfc:`959#section-5.3. RFC 959 renders anchors only for its eight top-level sections, so every sub-numbered 959 fragment is dead — not just section-4.1.

That last file already knew it: its docstring says "The citation is section 5.3 … not section 4.1 … #582 cited 4.1 and a cross-review caught it." So §4.1-for-the-case-rule was caught once before and the wrong-anchor habit outlived the correction. #947 is therefore not a new finding and I have said so there.

Fixing in this PR rather than deferring, since its title promises exactly this: all thirteen roles retargeted, section-5.3 added to KNOWN_DEAD_ANCHORS, and the denylist scan widened from pcapkit/ to docs/source and tests/ — its docstring's claim that the fragment "cannot come back unnoticed" was false while it looked at one directory. A lookbehind keeps ``:rfc:`... ``` literals scannable so the defect stays discussable. New revision shortly.

[JarryShaw]

NEEDS CHANGES at 05805fc98 — sonnet round 2 (Fable's round 2 died on a rate limit; substitution noted below). Two findings, both verified before acting.

1. My lookbehind was not a literal-vs-role classifier. (?<!)` asks only whether one backtick precedes the role. Measured on four cases:

0  live role after a closing inline literal   <- FALSE NEGATIVE
0  live role after a title reference          <- FALSE NEGATIVE
1  inert literal padded with spaces           <- FALSE POSITIVE
1  citation inside a :: literal block         <- FALSE POSITIVE

So an ordinary way of writing a code literal beside a citation defeated the check entirely — the "cannot come back unnoticed" claim was false for a second reason. Replaced with masking inline literal spans before matching. Now:

1  live role after a closing inline literal
1  live role after a title reference
0  inert literal padded with spaces
0  plain inert literal
1  citation inside a :: literal block         <- still wrong, documented

Two residual gaps stated rather than claimed away: the :: block, and a triple-backtick wrapper around a padded literal. I found the second by tripping my own test — my explanatory comment embedded an example and the masker mis-spanned it. Both fail in the safe direction: a false positive fails loudly, a false negative is a dead link nobody sees.

2. Other RFCs have the same defect. It asked the right question — which other cited RFCs lack sub-anchors:

RFC 1122: id="section-1".."section-5" only, zero sub-numbered  -> 1122#section-3.3.2 cited at 9 sites
RFC 719:  zero id= attributes in the entire page              -> 719#section-3.1 cited at 1 site

Out of #944's scope, so filed as #950 rather than folded in. Older RFCs are the pattern: 959 (1985), 1122 (1989), 719 (1976) all predate per-subsection anchors.

It independently confirmed the 13-site inventory, the scan roots, the one-line CHANGELOG.md regeneration, byte-identity of all three template pairs with no interpolation, the counterfactuals in both docs/ and tests/, and §5.3's location.

Model substitution, reported rather than silent: this PR's round 1 ran on Fable, which found the five missing sites. Its round 2 was killed by a bare 429 Too many tokens after partial work, so round 2 ran on sonnet instead. Both rounds differ from the authoring model, but not from each other as intended.

Also in this revision: the module docstring no longer quotes the maintainer, per his request on #918, and its "six sites" count is corrected to eleven.

New revision fbe2b2af6. tests/project: 207 passed, 1 skipped, 562 subtests.

[JarryShaw]

One question before this merges, raised by the #950 ruling rather than by the review.

On #950 the maintainer ruled that a dead anchor is only worth dropping if the anchored ref would generate no hyperlink, and that where it still links to the RFC itself the anchor stays as a notation to the reader. Measured through this repo's own Sphinx, a hyperlink is always generated — sphinx.roles.RFC.build_uri appends the fragment to the document URL, so a dead fragment only means the browser stays at the top of the page:

:rfc:`1122#section-3.3.2` -> href="https://datatracker.ietf.org/doc/html/rfc1122.html#section-3.3.2"
:rfc:`959#section-4`      -> href="https://datatracker.ietf.org/doc/html/rfc959.html#section-4"

Under that principle none of this PR's thirteen retargets was strictly necessary — every one of the original citations already produced a working link to RFC 959. The #944 question was which replacement to use, which presupposed replacing; it was never whether to replace.

So, explicitly:

1. Keep the retargeting — a fragment that resolves to the right section beats one that resolves to nothing, and it was ruled twice for RFC 959 (docs(const,vendor): six :rfc:959#section-4.1 citations name an anchor RFC 959 does not have #944, docs(const,vendor): Method.get cites RFC 959 §4 for a case-insensitivity rule stated in §5.3 #947). docs: ten :rfc: citations name sub-section anchors RFC 1122 and RFC 719 do not render #950's principle then covers only the cases where no better anchor exists, RFC 719 being the clean one since it renders no anchors at all.
2. Revert it — restore #section-4.1 and #section-5.3 and keep them as notation, consistent with docs: ten :rfc: citations name sub-section anchors RFC 1122 and RFC 719 do not render #950, leaving only the test and its denylist.

My lean is (1), and this PR currently implements it.

If (1), one consequence worth naming: KNOWN_DEAD_ANCHORS in tests/project/test_rfc_anchor_fragments.py forbids 959#section-4.1 and 959#section-5.3 outright, which is stricter than #950's principle. It would stay justified for RFC 959 specifically — a better anchor exists there, so citing the dead one is a mistake rather than a notation — but it should not grow entries for RFCs like 719 where no better anchor exists. I have written that limit into the denylist's own comment either way.

Marking needs: decision and holding the merge. The round-3 cross-review continues in parallel.

[JarryShaw]

NEEDS CHANGES at fbe2b2af6 — haiku round 3 (Fable died on a 429 twice on this PR, so the three rounds ran on three different models). It found the hole the masking fix left, and it is a false negative, already live in the tree.

INLINE_LITERAL used .+? with re.DOTALL, so a span could cross a paragraph break. An inline literal cannot, so any such span is a mis-pairing of two unrelated literals — and because masking blanks whatever it matches, it silently deleted real text from what the guard then searched. Measured on this tree:

mask spans crossing a BLANK line: 28
roles hidden inside them: [('tests/project/test_changelog_md.py', [('4303', 'section-2.1')])]

A live :rfc:4303#section-2.1`` role was already invisible to the guard. Root cause is a three-backtick run — which this repo writes routinely, since a literal whose content ends in a backtick closes with three — flipping the pairing parity so a span opened at the wrong delimiter.

Fixed by forbidding a blank line inside a span and consuming backtick runs atomically:

INLINE_LITERAL = re.compile(r'``(?:[^\n`]|`(?!`)|\n(?!\s*\n))+?``+')

All 28 mis-spans gone, and the six role-vs-literal cases still classify correctly. re.DOTALL removed — newlines are handled by the alternation, and the flag would only re-admit the case the pattern excludes.

It also falsified my own safety claim. The comment said both residual gaps "fail in the safe direction"; it demonstrated each producing a false negative — the direction I asserted was impossible. The comment now records that rather than the reassurance, and the remaining ::-block gap is stated plainly as a false positive.

Three prose defects fixed alongside: a dangling :data:RFC_ROLE_NOT_LITERAL`` reference to a symbol this PR renamed, an explanation that still described the lookbehind the PR replaced, and four statements scoping the dead-anchor sweep to pcapkit when it covers three roots — including a failure message that misattributed a `docs/source` finding to `pcapkit/`.

New test, because the defect was invisible to every existing one: test_no_mask_span_crosses_a_blank_line asserts the invariant over the real tree rather than hand-written cases — which is where the 28 were, and where hand-written cases had all passed. Verified it fires: restoring the old regex fails it, naming the hidden 4303#section-2.1 role.

New revision a18532df9. tests/project: 208 passed, 1 skipped, 562 subtests. Still needs: decision on the keep-versus-revert question above.

[JarryShaw]

NEEDS CHANGES at a18532df9 — sonnet round 4. A fourth false-negative path in the same masking pattern, and my round-3 invariant was blind to it.

The close was + ``, greedy. Two literals written back to back with no separator make a four-backtick boundary, so the first literal's close swallows the second's opener:

input : '``a````b`` :rfc:`959#section-4` ``c``'
spans : ['``a````', '`` :rfc:`959#section-4` ``']
roles visible: 0        <- the live role is gone

The orphaned tail pairs with the next literal downstream on the same line, blanking everything between. That needs a third literal to complete — a two-literal probe does not reproduce it, which is worth stating since it is why this survived three rounds.

Fixed by closing on exactly two backticks:

spans : ['``a``', '``b``', '``c``']
roles visible: 1

All five role-vs-literal cases still classify correctly, including this repo's own idiom where a literal quotes a role, and the tree still has zero blank-line spans.

The round-3 invariant could not see this, exactly as the review says: it checks for \n\n in a span, and nothing here crosses a paragraph. So there is a second, mechanism-independent invariant — test_no_mask_span_merges_two_literals — asserting no span contains an inner ``. A correctly-paired span cannot: the close is the first one reached. That catches a merged span however it arose, rather than being keyed to a known cause, which is the fix for the pattern of me guarding the last defect instead of the class.

Scope, so this is not overstated: grep -rlP '{4,}'` over the scanned roots finds no such run today, so the defect was latent rather than live — unlike round 3's, which was already hiding a real role.

Its other checks all held, several by independent re-derivation: the 28 mis-spans and the hidden 4303#section-2.1 role re-measured by reconstructing the old pattern; CRLF ruled out as a hole because read_text normalises before matching; whitespace-only separator lines correctly refused by \s*; the three-root guard confirmed by probe files in each; and it noted that a probe at docs/ rather than docs/source/ is correctly not scanned.

New revision 8f3953589. tests/project: 209 passed, 1 skipped, 562 subtests. Still needs: decision on keep-versus-revert.

[JarryShaw]

take (1)

[JarryShaw]

Ruling: take (1) — the retargeting stands. So all thirteen RFC 959 citations keep their corrected anchors, and #950's principle covers only the cases where no better anchor exists, RFC 719 being the clean one since it renders none at all.

That settles the question posted above. needs: decision off for it.

Round 5's verdict was missing from this PR, which is my error — it went into a status report and never here. Recording it now, because a cross-review's verdict exists nowhere else.

NEEDS CHANGES at 8f3953589 — sonnet round 5. A fifth false-negative mechanism in INLINE_LITERAL, distinct from the four already fixed: when a role's own closing backtick sits flush against a literal's opener, the masker fuses across the boundary.

"cite :rfc:`959#section-4.1```next`` after"   -> span '```next``'  -> 0 roles      (swallowed)
"cite :rfc:`959#section-4.1`` ``next`` after" -> fragment 'section-4.1     next'   (corrupted)

Verified both myself. Neither invariant fires — nothing crosses a blank line, and the inner content holds no complete `` pair. Latent, not live: no such adjacency exists on the tree.

The review's substantive answer, which I asked for and agree with: a hand-rolled regex is the wrong instrument here. Five mechanisms in five rounds is not converging, and the pattern's own comment already concedes a permanent ::-block gap. Two corrections to my assumptions came with it: docutils is not a test dependency — it reaches the tree only transitively through Sphinx in the docs extra, which unit-tests.yml never installs — so the parser route means a new hard dependency. And the bounded alternative is small: searching for the denylisted fragment string and classifying each hit against an exact allowlist faces 2 occurrences tree-wide, against the 23,869 literal spans the masker has to get right.

So the second question, which has not been asked here before:

1. Swap in the bounded design — string search plus a small reviewed allowlist of quoting forms, replacing INLINE_LITERAL, _mask_literals and both masker invariants.
2. Keep the current masker, accept the ::-block and flush-adjacency gaps as documented, and stop hardening it.
3. Drop the dead-anchor guard entirely. Per your docs: ten :rfc: citations name sub-section anchors RFC 1122 and RFC 719 do not render #950 ruling a dead anchor still renders a working link and only loses the in-page jump, so this guard protects a cosmetic property — arguably not worth five rounds of subtle regex.

My lean is (1): it removes the bug class rather than the bug, and it is smaller than what it replaces. I have not implemented it, so the branch still carries the masker.

needs: decision back on for this one only.

[JarryShaw]

#947's ruling applied, now that keep-versus-revert is settled. Six of the seven sites I listed there move from #section-4 to #section-5:

pcapkit/const/http/method.py:268        pcapkit/vendor/http/method.py:173
tests/const/test_const_enum_no_mint.py:2111
docs/source/contributing/conventions/registry-protocol.rst:224, 281, 443

The http/method.py pair stays byte-identical on the cited line, checked.

The seventh I deliberately left at #section-4, and I would rather flag it than decide it silently either way. registry-protocol.rst:337 is the row for CommandType/ConformanceRequirement, and its claim is not the case-insensitivity rule:

Limb 2 holds on measurement: the RFC and registry pages present the kind and conformance letters upper case (A/P/S, M/O/H) …

That is a claim about how §4.1's command list presents the kind letters, so §4 encloses the right prose and §5.3 does not mention them. I listed it among the seven when I put the question to you, so moving it would have followed your ruling literally while making the citation worse. Say the word if you want it at #section-5 for consistency instead.

Also unchanged, as #947 said: the two ftp/command.py pairs keep #section-4, carrying the command-kind claim §4 does support.

Revision 0d025d24d. tests/project 209 passed / 1 skipped / 562 subtests; test_ftp_unit.py + test_const_enum_no_mint.py 79 passed / 602 subtests.

The design question above — bounded string-search, keep the documented-gap masker, or drop the guard — is still open and is the only thing holding this PR now.

[JarryShaw]

(1) as well

[JarryShaw]

Ruling: (1) as well — swap in the bounded design. So INLINE_LITERAL, _mask_literals and both masker invariants come out, replaced by a direct search for each denylisted fragment string plus a small reviewed allowlist of the quoting forms this repository actually uses.

Recording the scale, since it is the argument for the change: the masker had to classify 23,869 inline-literal spans across 926 files correctly, and got it wrong five times in five rounds. The bounded check faces 2 occurrences tree-wide, both currently the documentation idiom:

tests/project/test_rfc_anchor_fragments.py  959#section-4.1  quoted=True
tests/project/test_rfc_anchor_fragments.py  959#section-4.1  quoted=True

classified by text[i-2:i] == '``' and text[i+len(needle)] == ''— exact, local, and reviewable one site at a time, the same wayKNOWN_DEAD_ANCHORS` entries already are.

needs: decision off. Implementing next; this PR stays review: pending until the rewrite has had a cross-review, since replacing the guard is a bigger change than any of the five fixes it supersedes.

One thing I will carry over rather than drop: the honest statement of what the check does not prove. The denylist is not an existence oracle — it catches only anchors someone has already confirmed dead and added, and its absence from the table is not evidence a fragment is live. That limitation survives the redesign unchanged.

[JarryShaw]

NEEDS CHANGES at 71dcf6945 — haiku, reviewing the bounded rewrite. It found a sixth mechanism, and it was inside the very line I had labelled "measured, not reasoned about".

The boundary check allowed ([{"' before the opener, on the reasoning that a bracket or quote may precede a literal. Those characters are equally valid as a literal's last content character — in which case the delimiter is a closer and the role after it is live:

found   literal content ends with a letter   ``foo``:rfc:`959#…``
EXEMPT  literal content ends with '          ``x'``:rfc:`959#…``      <- false negative
EXEMPT  literal content ends with (          ``foo(``:rfc:`959#…``    <- false negative
EXEMPT  the real ``'base'`` literal          ``'base'``:rfc:`959#…``  <- false negative

'base' is not invented — it is an inline literal in pcapkit/const/ftp/command.py today. Measured across the scanned roots: 764 literals in 188 files end in one of those five characters, so the exemption was one adjacency from hiding a live dead link.

That comment was the real defect. It had been measured only against foo , a literal ending in a letter, and never against the characters in its own allowed set — which is exactly where it failed. A measurement that skips the interesting inputs is reasoning wearing a measurement's clothes, and the comment now says so.

Fixed: the boundary is whitespace or start-of-file only, which RST's grammar makes safe since an inline literal's end-string cannot follow whitespace. The suffix is now the real two-backtick close rather than one character of it.

Its sharpest finding was about my test, not my code. The mechanism table missed two regressions — dropping the suffix check, and widening the boundary set back — so the (prefix, suffix) pairing the comment calls the reason the list holds pairs was unasserted. Seven rows added, and re-running its mutation matrix:

CAUGHT   M3 drop the suffix check
CAUGHT   M6 widen the boundary set back
CAUGHT   M1 one-character lookbehind

All three now caught. The first attempt at those rows still missed M6 — the suffix condition masked the boundary condition, so the rows needed the full idiom suffix for the boundary to be the deciding factor. I only found that by re-running the matrix rather than trusting that adding rows was enough.

It also confirmed, against a docutils oracle: every existing row's liveness claim, the three template/generated pairs byte-identical, the guard catching planted sites in all three roots, the citation arithmetic reconciling with the commit message, and 5/5 under python3.10. One correction to my framing it made fairly: INLINE_LITERAL and _mask_literals never existed on main, only in earlier pushes of this branch, so "nothing else referenced them" was vacuously true rather than swept.

New revision 95f11b34b. tests/project: 208 passed, 1 skipped, 577 subtests, and 5/5 on 3.10.

[JarryShaw]

GOOD TO GO at 95f11b34b — sonnet, and carried forward to 8df0feedc; the delta is two test rows and no production code, stated below.

The first clean verdict on this PR after seven rounds. No seventh mechanism. It built a docutils oracle — a local :rfc: role mimicking sphinx.roles.RFC.build_uri, classifying "live" by whether a reference node was produced — and ran the fifteen table rows plus roughly thirty synthetic probes: every whitespace kind before the opener including NBSP, U+3000, U+2028; start-of-file and start-of-line; a preceding literal, title reference, substitution reference and footnote, flush and spaced; table-cell adjacency; four and five trailing backticks; :: blocks, .. code-block::, comments, doctests, definition lists, escaped backticks. Zero disagreements in the dangerous direction.

It also verified the whitespace-safety claim rather than accepting it: docutils' Inliner.non_whitespace_before = r'(?<!\s)' (states.py:779) is what a literal's closing scan uses, and str.isspace() agrees with re's \s on every character tested. The only divergence, U+200B, is whitespace by neither definition and cannot reach the branch.

Its one non-blocking finding, verified and acted on. Narrowing isspace() to == ' ' passed all fifteen rows — only one row reached that branch and it used a plain space, so the boundary condition the comment relies on was pinned for one whitespace character out of the set it names. Measured myself:

before:  narrowing isspace() to == " " : MISSED
after:   narrowing isspace() to == " " : CAUGHT

Two rows added — the idiom preceded by a tab, and by a newline — which is what a reader hits writing it after a line wrap or first on an indented line. Full matrix now: dropping the suffix check, widening the boundary back, the one-character lookbehind, and the space-narrowing are all caught. Three of those four escaped an earlier version of the table.

That is the whole delta from 95f11b34b: two rows in test_the_documentation_exemption_holds_on_every_known_mechanism, fifteen → seventeen, subtests 577 → 579. No change to _is_documentation, QUOTED_FORMS, _dead_anchor_citations or any citation. I am carrying the verdict rather than spending an eighth round on it, and saying so plainly so the provenance is not overstated.

Also confirmed by the review: the citation arithmetic reconciles (6+5=11 from #section-4.1, plus 2 from #section-5.3 = thirteen), both template/generated pairs byte-identical on the cited lines, the guard catching planted sites in all three roots, and 5/5 under python3.10. It correctly noted CHANGELOG.md's edit is a markdown link outside DEAD_ANCHOR_SCAN and outside :rfc: syntax, so a generated sync rather than a counted site.

tests/project: 208 passed, 1 skipped, 579 subtests, 5/5 on 3.10. test_ftp_unit.py + test_const_enum_no_mint.py: 79 passed, 602 subtests.

Unpublished and yours to merge.