Skip to content

docs: mark the Type Stubs copies of four TypedDicts :no-index: - #532

Merged
JarryShaw merged 2 commits into
mainfrom
fix/520-duplicate-object-descriptions
Sep 20, 2026
Merged

JarryShaw merged 2 commits into
mainfrom
fix/520-duplicate-object-descriptions

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Follow-up to #520. The owner merged #520 earlier than intended and asked for the
duplicate-object warnings it surfaced to be fixed separately, which is what this does.

What #520 left behind

#520 moved os.environ['PCAPKIT_SPHINX'] = '1' above import pcapkit in
docs/source/conf.py. That took autodoc's failed to import warnings from 96 to 0, but
it also made the if SPHINX_TYPE_CHECKING: TypedDicts real for the first time — and
exposed that four of them are listed twice on the same page.

It is not a cross-module or const/vendor collision. Every warning's
other instance in … clause names the same document it came from. Each class appears
once under Header Schemas, next to the option it belongs to, and again in the
Type Stubs appendix. Before #520 both occurrences independently failed to import, so
there was no target to collide with.

Page Class Warnings
internet/hopopt.rst RPLFlags, MPLFlags, DFFFlags 12 (3 classes + 9 fields)
internet/ipv6_opts.rst RPLFlags, MPLFlags, DFFFlags 12 (3 classes + 9 fields)
transport/tcp.rst Flags 9 (1 class + 8 fields)

The change

Seven added lines. :no-index: on the Type Stubs occurrence in each case, keeping the
inline Header Schemas copy canonical — it sits beside its parent option
(RPLOption/RPLFlags, MPLOption/MPLFlags, IPDFFOption/DFFFlags, TCP/Flags),
which is where a reader looking the type up will be.

Only the class-level directive is marked; the :members: fields are not touched. One
:no-index: per class suppresses registration for the class and everything
:members: pulls in, because _document_members in sphinx/ext/autodoc/_generate.py
passes the same options object to each member's _generate_directives, and
_renderer.py emits :no-index: for any directive whose options.no_index is set. That
was verified by the build rather than assumed: if it covered classes only, 7 of the 33
would have gone and 26 would have remained.

Build comparison

Two full sphinx-build 9.1.0 runs off this worktree, before and after the seven lines,
with PYTHONSAFEPATH=1 and pcapkit.__file__ asserted to be this tree on each run (the
venv has pcapkit installed editable against a different checkout and would otherwise
shadow it).

before after
total warnings 83 50
duplicate object description 36 3
failed to import 0 0
more than one target found for cross-reference 31 31
other 16 16

Comparing warning sets with line numbers normalised, the after-set is a strict subset
of the before-set: no new warning of any kind appears, and the 33 removed are exactly
the 7 classes plus their 26 fields.

The 3 surviving duplicates pre-date #520, are unrelated to it, and are deliberately left
alone — pcapkit.corekit.infoclass.Info.__post_init__,
pcapkit.foundation.reassembly.data.tcp.BufferID, and
pcapkit.protocols.misc.pcap.header.Header.__post_init__ (that last one from two literal
back-to-back .. automethod:: __post_init__ lines in pcap.rst).

It also fixes cross-references

Worth calling out, since it makes this more than warning cleanup. A duplicate target
forced Sphinx to fall back to numbered anchors, and the inventory pointed at the wrong
copy
objects.inv resolved these seven names to #id0/#id5/#id9, the Type Stubs
appendix occurrence, rather than to the canonical Header Schemas one. After the change
each resolves to its proper name anchor:

before:  schema.internet.hopopt.RPLFlags -> pcapkit/protocols/internet/hopopt.html#id0
after:   schema.internet.hopopt.RPLFlags -> pcapkit/protocols/internet/hopopt.html#$

(#$ is the inventory's shorthand for "anchor equals the object name".) Confirmed in the
generated HTML too: hopopt.html carried both id="id0" and
id="pcapkit.protocols.schema.internet.hopopt.RPLFlags" before, and only the latter after.

No test

No test accompanies this, and inventing one would be theatre — it is a docs-only change
of seven directive options, with no importable behaviour to assert against. The two-build
warning-set comparison above is the evidence.

#520 made the `if SPHINX_TYPE_CHECKING:` TypedDicts importable during the
build, which exposed that four of them are documented twice on the same page
-- once under "Header Schemas" beside the option they belong to, and again in
the "Type Stubs" appendix. Before #520 both occurrences failed to import, so
nothing collided.

- add `:no-index:` to the appendix copy of `RPLFlags`, `MPLFlags` and
  `DFFFlags` in `hopopt.rst` and `ipv6_opts.rst`, and of `Flags` in `tcp.rst`,
  keeping the inline "Header Schemas" occurrence canonical

Measured with sphinx-build 9.1.0: 83 warnings -> 50, `duplicate object
description` 36 -> 3, `failed to import` stays 0, no new warning appears. The
3 remaining duplicates pre-date #520 and are untouched. Also repairs cross
references, which resolved to the appendix via `#id0` rather than to the
canonical name anchor.
@JarryShaw

Copy link
Copy Markdown
Owner Author

✅ GOOD TO MERGE — independently confirmed via objects.inv and generated HTML anchor inspection on both sides that this is a genuine navigation fix, not cosmetic warning suppression: before the fix, every :class: cross-reference to RPLFlags/MPLFlags/DFFFlags/Flags (and their 26 field members) resolved to the wrong #id0/#id5/#id9 Type-Stubs-appendix anchor; after, all resolve to the canonical Header-Schemas anchor. The build comparison (83→50 warnings, 36→3 duplicates, strict subset, zero new warnings) reproduces exactly.

@JarryShaw

Copy link
Copy Markdown
Owner Author

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

Reviewed at head 1dfe5bc29005f07b894555833b264d59311cd7f0 (main = c8fd97bcd) in an isolated worktree, with full Sphinx 9.1.0 builds run on both sides.

1. The diff. Confirmed all 7 :no-index: additions attach to the "Type Stubs" appendix copy of each autoclass::, never the canonical "Header Schemas" copy — checked by reading enough surrounding context in each of the 3 files to identify which section each directive sits in (e.g. hopopt.rst: Header Schemas 77–181 untouched at lines 146/154/178, Type Stubs 182+ patched at 193/198/203).

2. Build comparison, independently re-derived (not copied from the PR).

  • main: 83 warnings exactly — duplicate object description 36, cross-ref ambiguity 31, failed to import 0, other 16 (6 guarded-type-import + 10 forward-reference). 36+31+16=83.
  • PR head: 50 warnings exactly — duplicates down to 3, everything else unchanged (31/0/16).
  • The 3 surviving duplicates are exactly pcapkit.corekit.infoclass.Info.__post_init__, pcapkit.foundation.reassembly.data.tcp.BufferID, pcapkit.protocols.misc.pcap.header.Header.__post_init__ — verified directly in the PR-head warning list.
  • Per-class breakdown of the 33 removed: hopopt (3 classes + 9 fields = 12), ipv6_opts (12), tcp.Flags (1 class + 8 fields = 9); 12+12+9=33, matching 7 classes + 26 fields.
  • Strict-subset check, normalized: comm -13 main pr → empty (zero new warnings of any class). comm -23 main pr → exactly the 33 removed duplicate lines, nothing else.

3. The decisive claim — cross-reference resolution — inspected directly on both sides, not accepted from any summary. Loaded objects.inv via sphinx.util.inventory.InventoryFile.load and cross-checked against generated HTML anchor positions for all 4 classes across all 3 files:

Target main resolves to PR-head resolves to
hopopt.RPLFlags hopopt.html#id0 (Type Stubs, line 2345) hopopt.html#pcapkit...hopopt.RPLFlags (Header Schemas, line 2095)
hopopt.MPLFlags hopopt.html#id5 (Type Stubs) canonical name anchor (Header Schemas)
hopopt.DFFFlags hopopt.html#id9 (Type Stubs) canonical name anchor (Header Schemas)
ipv6_opts.{RPL,MPL,DFF}Flags same #id0/#id5/#id9 pattern same canonical-anchor pattern
tcp.Flags tcp.html#id0 tcp.html#pcapkit...tcp.Flags

All 26 field-level members showed the identical pattern. On main, "Header Schemas" starts at hopopt.html:1690 and "Type Stubs" at :2299 — the canonical name-anchors (2095/2168/2274) sit in Header Schemas while the numbered ids Sphinx actually indexed (2345/2370/2399) sit in Type Stubs, i.e. the inventory pointed at the wrong copy. On PR head, those numbered ids no longer appear anywhere in the file; the appendix content still renders (verified present, just unlinkable/de-indexed rather than deleted), and the canonical anchors are unchanged. This is a genuine, reproducible misresolution that the fix repairs — not anchor-ordering luck, not cosmetic.

4. Mechanism verified from Sphinx 9.1.0 source directly, not from the PR's assertion. sphinx/ext/autodoc/_generate.py's _document_members passes the same options object reference to every member's directive generation (no per-member copy); _renderer.py's _directive_header_lines emits :no-index: whenever options.no_index is set. So one class-level :no-index: propagates to every member generated from that shared options object — explaining exactly why 7 lines suppressed 33 warnings (7 classes + 26 fields) rather than needing one line per field, and why omitting any one class's line would leave that class's own duplicate plus all its field duplicates (29 remaining instead of 3).

5. Minor wording nitpick, non-blocking. The PR description calls the pre-existing Header.__post_init__ duplicate "two literal back-to-back automethod:: lines" — verified both lines exist in the same autoclass:: Header block but are not literally adjacent (16 other directive lines sit between them at lines 40 and 57). The underlying claim (duplicate directive within one class block, pre-existing and out of scope) is correct; "back-to-back" is just imprecise, not a factual error, and doesn't affect the diff or the numbers.

Judgment on "no test, deliberately"

Reasonable as scoped — a test merely asserting "these 7 lines contain :no-index:" would be theater. But there's a cheap, real follow-up opportunity: deploy-pages.yml already runs a full sphinx-build on every PR (that cost is already being paid), but only checks exit code — it never inspects objects.inv or warning counts, so this exact regression class would not move CI either way today or in the future. A lightweight follow-up test parsing the already-produced objects.inv after that existing build step, asserting these 4 class names resolve to their canonical dotted path rather than a bare #id\d+ fragment, would be cheap and would have caught both the original #520 regression and any recurrence. Worth a follow-up issue, not something this scoped fix needs to carry.

What could not be verified further

Nothing had to be abandoned — both full builds completed end-to-end, including the confirmed-benign ~13-14 minute const/reg CPU stall on each side, not killed. Host load was rechecked before each build with no concurrent Sphinx build running.

Verdict

The load-bearing claim is confirmed true by direct inspection: this fixes broken navigation, not just warning noise. Recommend merge.

@JarryShaw
JarryShaw merged commit e16a404 into main Sep 20, 2026
24 checks passed
@JarryShaw
JarryShaw deleted the fix/520-duplicate-object-descriptions branch September 20, 2026 05:24
@JarryShaw JarryShaw added the docs Pull requests that change documentation only (docs: subject prefix) label Sep 22, 2026
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)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant