Skip to content

docs(sphinx): add opt-in issue, pr and discussion roles and convert the conventions pages - #998

Merged
JarryShaw merged 1 commit into
mainfrom
docs/989-issue-citation-roles
Oct 3, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/989-issue-citation-roles

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

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

First tranche of #989.


Description of your pull request and other information

A bare #NNN renders as literal text, so every tracker citation in the built documentation is unclickable. This adds sphinx.ext.extlinks with :issue:, :pr: and :discussion: roles, and converts the seven conventions pages.

The roles are opt-in per site, not an automatic #NNN rule, because no digit-keyed pattern separates a citation from a packet-diagram label. #\d{3} already misses 17 two-digit citations that are real, and #\d+ would catch the 45 one-digit RFC diagram labels in pcapkit/protocols/internet/hip.py and pcapkit/protocols/transport/sctp.py — DH GROUP ID #1, Gap Ack Block #1 — which reference nothing. A role nobody writes cannot corrupt them.

:issue: and :pr: both caption #%s, so converting a bare citation or an explicit link changes the markup and not the rendered text. Six sites were inline literals and are the exception — #934, #949 and #911 in documentation.rst, #759, #805 and #775 in process.rst — which rendered as monospace and now render as links in body font. They are separate roles because the issue-versus-pull-request distinction is itself a documented convention and one role would flatten it in the source. :discussion: exists because the repository has five GitHub Discussions — 105, 106, 127, 251 and 274 — for which the issues API returns 404, so :issue: on one would render a dead link.

Scope, measured rather than taken from the issue. 895 bare citations sit on the surface Sphinx actually renders: docs/**/*.rst plus pcapkit/ docstrings and #: comments. All 1,580 autodoc directives target pcapkit.* and there is no literalinclude, so tests/, util/ and examples/ never reach a page and their citations cannot fail to resolve — which is why the issue's earlier figure of ~1,858 overstated the rendered surface about 4.4×, 1,563 of it being tests/. 420 of the 895 are actionable; the other 475 are under docs/source/changelog, which #657 owns and util/changelog_md.py generates.

This tranche converts 78 sites — 45 explicit links collapsed, 6 inline literals, 27 bare. All 33 distinct numbers resolved in one GraphQL issueOrPullRequest call and every one is an issue, so :issue: is correct at each site; the collapsed links asserted label == URL number, so every rendered URL is byte-identical to before.

Two tests pinned the bare #NNN source form and this change reddens both. test_conventions_doc_claims.py's floor now counts explicit links and role citations together, keeping the pairwise label-versus-URL comparison over whatever explicit links remain. test_sentinel_exports_unit.py accepts the citation in either markup form. Neither is vacuous: run the pre-change assertions against the new pages and they fail.

Verified — Sphinx 9.1.0 exits 0 with the documented root confirmed inside the worktree, 78 rendered /issues/NNN anchors matching the 78 conversions, no warning naming any conventions page; tests/project/ gives 258 passed, 1 skipped, 859 subtests; the page-reading corekit tests give 61 passed, 129 subtests. All exit codes read from the process.

Left for the follow-up: 387 pcapkit/ sites across 69 files, the 475 changelog sites (which contain the only two bare Discussion citations, #106 and #251 — a blind :issue: pass there would create dead links), and the :iana: role, which turns out never to have been committed.

@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one test Pull requests that add or correct tests (test: subject prefix) labels Oct 2, 2026
…the conventions pages

Closes nothing yet; the first tranche of #989. A bare ``#NNN`` renders as literal
text, so every tracker citation in the built documentation is unclickable.

The roles are deliberately opt-in per site rather than an automatic ``#NNN``
rule, because no digit-keyed pattern separates a citation from a packet-diagram
label. ``#\d{3}`` already misses 17 two-digit citations that are real, and
``#\d+`` would catch the 45 one-digit RFC diagram labels in
pcapkit/protocols/internet/hip.py and pcapkit/protocols/transport/sctp.py
(``DH GROUP ID #1``, ``Gap Ack Block #1``), which reference nothing. A role
nobody writes cannot corrupt them.

``:issue:`` and ``:pr:`` both caption ``#%s``, so converting a bare citation or
an explicit link changes the markup and not the rendered text. Six sites were
inline literals and are the exception: ``#934``, ``#949`` and ``#911`` in
documentation.rst and ``#759``, ``#805`` and ``#775`` in process.rst rendered as
monospace and now render as links in body font. They are separate roles because the
issue-versus-pull-request distinction is itself a documented convention and one
role would flatten it in the source. ``:discussion:`` exists because the
repository has five GitHub Discussions -- 105, 106, 127, 251 and 274 -- for
which the issues API returns 404, so ``:issue:`` would render a dead link.

Scope, measured rather than taken from the issue: 895 bare citations sit on the
surface Sphinx actually renders, which is docs/**/*.rst plus pcapkit/
docstrings and ``#:`` comments. All 1,580 autodoc directives target pcapkit.*
and there is no literalinclude, so tests/, util/ and examples/ never reach a
page and their citations cannot fail to resolve. 420 of the 895 are actionable;
the other 475 are under docs/source/changelog, which #657 owns and
util/changelog_md.py generates.

This tranche converts 78 sites across the seven conventions pages -- 45 explicit
links collapsed, 6 inline literals, 27 bare. All 33 distinct numbers were
resolved in one GraphQL issueOrPullRequest call and every one is an issue, so
:issue: is correct at each site; the collapsed links asserted label == URL
number, so every rendered URL is byte-identical to before.

Two tests pinned the bare ``#NNN`` source form and this change reddens both.
test_conventions_doc_claims.py's floor now counts explicit links and role
citations together, keeping the pairwise label-versus-URL comparison over
whatever explicit links remain. test_sentinel_exports_unit.py accepts the
citation in either markup form. Both were checked against the old page, where
the pre-change assertions fail, so neither is vacuous.

Verified: Sphinx 9.1.0 builds with exit code 0, the documented root printed and
confirmed inside the worktree, 78 rendered /issues/NNN anchors matching the 78
conversions, and no warning naming any conventions page. tests/project/ gives
258 passed, 1 skipped, 859 subtests; the page-reading corekit tests give 61
passed, 129 subtests. All exit codes read from the process.

The conf.py comment also had three inaccuracies of its own, which a change about
citation accuracy should not ship: it named two files for the 45 one-digit
diagram labels where they live in three (internet/hip.py 36, transport/sctp.py 7,
schema/internet/hip.py 2); it said ``#\d{3}`` misses four-digit numbers "now
arriving" when there are none yet, and omitted the 17 two-digit ones it does
miss; and it claimed rendering was unchanged without the inline-literal
exception.

The Sphinx build is unchanged against main: 61 warnings and 2 errors on both,
no warning or error naming any conventions page, and no warning kind new to the
branch. Both errors are pre-existing, in pcapkit/corekit/infoclass.py and
pcapkit/protocols/schema/schema.py docstrings, neither of which this change
touches.
@JarryShaw
JarryShaw force-pushed the docs/989-issue-citation-roles branch from 5b99328 to 0681926 Compare October 2, 2026 22:21
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 5b9932870 — sonnet cross-review, a different model from the author. No blocking finding. It raised four non-blocking nits; two were accuracy defects in a change about accuracy, so I fixed them rather than shipping them. New head 068192625.

The description claimed rendering was unchanged, and that was false for six sites. #934, #949 and #911 in documentation.rst and #759, #805 and #775 in process.rst were inline literals — monospace before, links in body font now. Exactly six, verified against main. Both the commit message and the description now carry the exception.

The conf.py comment named two files where the labels live in three. The 45 one-digit diagram labels are internet/hip.py 36, transport/sctp.py 7 and schema/internet/hip.py 2 — so two of the 45 sat in a file the comment did not mention. While correcting it I found two more of my own: it said #\d{3} misses four-digit numbers "now arriving" when there are none yet, and it omitted the 17 two-digit ones it does miss. Reflowing that comment split Gap Ack Block #1 across lines on the first attempt — the same wrapped-literal defect this sweep has been removing elsewhere — so it was redone with inline literals protected.

On the build, the reviewer was right and my own check was the wrong instrument. I grepped the log for "conventions" and got 14 hits; all 14 are reading sources/writing output progress lines and none is a warning. I then built origin/main for a like-for-like comparison: 61 warnings and 2 errors on both, no warning or error naming any conventions page, and no warning kind new to the branch. Both errors are pre-existing, in pcapkit/corekit/infoclass.py and pcapkit/protocols/schema/schema.py docstrings, neither touched here.

Re-verified at the new head: Sphinx exit 0 with the documented root confirmed inside the worktree, 78 rendered /issues/NNN anchors, tests/project/ plus the page-reading corekit test at 277 passed, 1 skipped, 887 subtests, exit 0 read from the process.

Two nits I am deliberately not acting on. The floor's pairwise label-versus-URL comparison now runs over an empty list, since no explicit links remain — so half that test is inert. The floor itself still catches regressions (reverting all 78 roles gives 0, reverting 38 gives 40, both failing), and raising its strictness from > 40 toward 70 is a test-policy call I would rather surface than make unilaterally. And documentation.rst went 88 to 90 columns on four lines with sentinel-convention.rst 88 to 89 on three; both files already run past 88 elsewhere, so that is cosmetic.

Label stays review: pending until a verdict lands on 068192625.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Delta round: GOOD TO GO at 068192625. The comment-only correction holds, and the reviewer re-derived every figure in it rather than accepting mine: the three file counts (internet/hip.py 36, transport/sctp.py 7, schema/internet/hip.py 2, summing to 45), zero four-digit citations, and 17 two-digit ones. It also confirmed the reflow split no inline literal — all 36 double-backtick marks in that block are paired on their own line — and that the two short lines are forced rather than orphans, the 49-column one having no room for a 44-column path literal.

The build is identical to main, now measured with fresh -E -a builds of both revisions. 61 warnings and 2 errors each, exit 0 each, identical warning kinds (the tally diff is empty), the two errors pre-existing in pcapkit/corekit/infoclass.py and pcapkit/protocols/schema/schema.py docstrings, and the only log lines naming "conventions" being progress lines. 78 /issues/NNN anchors in the same per-page split, no caption differing from #<number>, and extlinks plus all three roles unchanged — git diff 5b9932870 068192625 touches comment lines only.

One wording nit I am deliberately leaving, with the measurement on the record. The comment says #\d{3} "already misses 17 two-digit ones". That is literally true, and I enumerated them: 16 sit under changelog paths — 8 in docs/source/changelog.rst and 8 across the per-version files — and the 17th is docs/source/contributing/pep.rst:351, which is already an explicit link. So none is actionable today, and the reviewer suggested narrowing the phrasing to the changelog figure.

I am keeping it because the sentence is doing a different job from what the narrowing would imply. The point of that paragraph is that a digit-keyed pattern is the wrong instrument in both directions — #\d+ produces false positives on the 45 diagram labels, and #\d{3} produces false negatives on two-digit citations. Those 17 are real citations whether or not this pull request may touch their files, so the count supports the argument as written. Narrowing it to "16 changelog citations" would tie a general claim about the pattern to one directory's ownership, which is a weaker statement and would go stale the moment a two-digit citation appears elsewhere.

Label is now review: good-to-go. Not yet ready to merge — CI at this head is 54 passed, 3 skipped, 14 in flight, zero failures.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge at 068192625 — closing the "not yet" on my last comment. CI finished clean: 69 success, 3 skipped, 0 in flight, 0 failures across 72 CheckRuns, and mergeStateStatus is CLEAN. All six required contexts are green — Required checks passed and Compat Python 3.10 through 3.14, with Compat Python 3.15 (scheduled) skipped. Two cross-review rounds on a model other than the author's, both GOOD TO GO, the second after I corrected three inaccuracies the first round exposed in my own prose.

One commit on main at 3c59e2917. First tranche of #989; the issue stays open for the rest.

Unpublished decisions are yours, and I have not merged. What is queued behind this, and why it is queued rather than in flight: the remaining 387 pcapkit/ citation sites cannot be converted until this lands, because the roles live in docs/source/conf.py here — a converted docstring built against main would error on an unknown role. A combined tranche for pcapkit/corekit/ is already prepared against this head, folding in #719's separate item that a pull request must not be described as an issue, since both land on the same docstrings and two branches there would mean the later push silently winning. The 475 changelog sites stay with #657, and they contain the only two bare Discussion citations in the tree — #106 and #251 — which a blind :issue: pass would turn into dead links.

@JarryShaw
JarryShaw merged commit 7cf0976 into main Oct 3, 2026
73 checks passed
@JarryShaw
JarryShaw deleted the docs/989-issue-citation-roles branch October 3, 2026 02:53
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 3, 2026
JarryShaw added a commit that referenced this pull request Oct 3, 2026
…irst use

Adds two entries alongside the three added in #998, so that the citations
these pages already carry as bare URLs have a role to convert to when that
sweep reaches them.

- `:wikipedia:` expands `https://en.wikipedia.org/wiki/%s`. 28 of the 34
  distinct Wikipedia and Wikimedia URLs cited today are that shape, over 23
  distinct articles -- `IPv6_packet` appears four times, differing only by
  fragment. Three cited URLs the template cannot express must stay
  hardcoded: a `/w/index.php?...&oldid=...` permalink, a
  `foundation.wikimedia.org` policy page in `pcapkit/vendor/default.py`, and
  an `http://` ARP link that a role would silently upgrade to `https`.
- `:iana:` expands `https://www.iana.org/assignments/%s`, the argument being
  the path after `/assignments/`. All 179 distinct IANA URLs are that shape
  across 21 registries, so both a registry index and a specific table
  resolve through one role.

A two-`%s` IANA template would be wrong: over the 108 distinct
fragment-stripped paths, the file stem equals the registry name in only 20
-- the rest are per-table `.csv` files the vendor crawlers read -- and
Python's `%` takes one argument, so the second placeholder raises at
role-expansion time rather than degrading.

Both captions are `%s`, not `#%s`, because the argument is a slug or path
rather than a number; prose should use the explicit-title form, since a bare
caption renders the raw path as visible text. Neither role is cited yet, by
design, per the ruling on #989.

Verified: both roles render to the expected hrefs with the fragment and both
IANA shapes intact; docs build exit 0 with 61 warnings / 2 errors, unchanged
from the baseline, and 0 unknown-role errors; `tests/project` 258 passed,
1 skipped, 859 subtests.
JarryShaw added a commit that referenced this pull request Oct 3, 2026
…irst use

Adds two entries alongside the three added in #998, so that the citations
these pages already carry as bare URLs have a role to convert to when that
sweep reaches them.

- `:wikipedia:` expands `https://en.wikipedia.org/wiki/%s`. 28 of the 31
  distinct `wikipedia.org`/`wikimedia.org` URLs cited today are that shape,
  over 23 distinct articles -- `IPv6_packet` appears four times, differing
  only by fragment. The other three must stay hardcoded: a
  `/w/index.php?...&oldid=...` permalink, a `foundation.wikimedia.org` policy
  page in `pcapkit/vendor/default.py`, and an `http://` ARP link that a role
  would silently upgrade to `https`.
- `:iana:` expands `https://www.iana.org/assignments/%s`, the argument being
  the path after `/assignments/`. All 179 distinct IANA URLs are that shape
  across 21 registries, so both a registry index and a specific table
  resolve through one role.

A two-`%s` IANA template would be wrong: over the 108 distinct
fragment-stripped paths, the file stem equals the registry name in only 20
-- the rest are per-table `.csv` files the vendor crawlers read -- and
Python's `%` takes one argument, so the second placeholder raises at
role-expansion time rather than degrading.

Both captions are `%s`, not `#%s`, because the argument is a slug or path
rather than a number; prose should use the explicit-title form, since a bare
caption renders the raw path as visible text. Neither role is cited yet, by
design, per the ruling on #989.

Verified: both roles render to the expected hrefs with the fragment and both
IANA shapes intact; docs build exit 0 with 61 warnings / 2 errors, unchanged
from the baseline, and 0 unknown-role errors; `tests/project` 258 passed,
1 skipped, 859 subtests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Pull requests that change documentation only (docs: subject prefix) test Pull requests that add or correct tests (test: subject prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant