Skip to content

docs(conventions): 16 unresolved cross-references render as plain text, CI stays green #934

Closed

Description

@JarryShaw

docs/source/contributing/conventions.rst has 16 unresolved Sphinx cross-references. They render as plain literal text instead of links, and CI never notices because make html passes neither -W nor nitpicky — the build succeeds and the reference silently degrades.

Measured with an explicit nitpicky build (python -m sphinx -n -b html -j 4 docs/source/. <out>) plus a read of the rendered HTML, on af2324522:

role as written lines why it fails
:mod:pcapkit.corekit.sentinels`` 165, 168, 171, 174, 261 no API page registers that module
:class:NoValueType`` 197 unqualified, no target
:mod:aenum`` 371, 388, 536 aenum is not in intersphinx_mapping
:class:~aenum.Enum, `:class:`~aenum.StrEnum 340, 536, 701 same
:meth:~pcapkit.const.http.method.Method.get`` 444 no target
:class:~pcapkit.const.ftp.command.FEATCode`` 575, 716 ×2 no target

These split into two kinds, and the fix differs:

  • The five pcapkit.corekit.sentinels references are a genuine gap. refactor(corekit): house all four sentinels in one shared module (#911) #922 created that module as the single home for all four sentinels, and no docs/source/ page documents it — so the page that defines the sentinel convention cannot link to where the sentinels live. Adding the API page fixes five of the 16 and is the highest-value part.
  • The six aenum ones may be working as intended — aenum is deliberately absent from intersphinx_mapping. If that exclusion is deliberate, these should stop using :mod:/:class: roles and use plain literals instead, so the page stops claiming a link it cannot make. Worth a decision rather than an assumption.
  • NoValueType, Method.get and FEATCode ×3 look like plain misses — either the target needs qualifying or the API page needs the entry.

Surfaced while verifying #932, which introduces none of them: the nitpicky warning sets for af2324522 and #932's head are byte-identical. Filed separately rather than widening that PR.

Worth considering alongside the fix: adding -n to the docs build so a future unresolved reference fails CI instead of degrading silently. That is a bigger change than this issue and may want its own decision, given there are 1275 warnings on main today.

Activity

  1. added
    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)
    docsPull requests that change documentation only (docs: subject prefix)
    on Sep 29, 2026
  2. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Splitting this three ways, because one part has file contention and one needs your call.

    Part A — the pcapkit.corekit.sentinels API page. In flight now. A worker is adding the missing page under docs/source/pcapkit/corekit/ plus its index entry. This resolves five of the 16 (:165, :168, :171, :174, :261) without editing conventions.rst at all — the references fail because no page documents the module, so creating the page is the whole fix. That is deliberate: conventions.rst is owned by #932's branch right now and two agents editing one file lose each other's work silently.

    Part B — the remaining edits inside conventions.rst. Blocked on #932. NoValueType at :197 is unqualified and Method.get at :444 plus FEATCode at :575/:716×2 need either qualifying or an API entry; either way the fixes land in that file. Blocked until #932 merges, at which point the contention is gone.

    Part C — the six aenum roles. Needs your call. :mod:aenum at `:371`, `:388`, `:536` and `:class:`~aenum.Enum/~aenum.StrEnum`` at :340, :536, :701fail becauseaenumis absent fromintersphinx_mapping`. Two readings:

    1. The exclusion is deliberate — then these should stop using :mod:/:class: roles and become plain literals, so the page stops promising a link it cannot make.
    2. The exclusion is an oversight — then adding aenum to intersphinx_mapping fixes all six at once.

    I lean on (1) without knowing the history: aenum has no published objects.inv on its docs host that I have verified, and the other intersphinx_mapping entries are all projects that do. But you may have excluded it for a reason I cannot see from the config, so I am not guessing.

    Also worth a separate decision, not filed here: adding -n to the docs build so a future unresolved reference fails CI rather than degrading silently. There are 1275 warnings on main today, so that is a project in itself.

  3. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Sep 29, 2026
  4. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Part A is up as #936, and it turned up more than the five references — plus a trade-off worth recording.

    Four additional :mod:pcapkit.corekit.sentinels`` misses resolve as a side effect, inside the docstrings of pcapkit/corekit/enum.py, `pcapkit/corekit/module.py` and `sentinels.py` itself. So nine of the tree's unresolved references to that module go away, not five.

    But the total warning count rises, 1275 → 1287. Each sentinel type is now documented in two places — its old re-export path and the new canonical page:

    docs/source/pcapkit/corekit/module.rst:30        autoclass:: pcapkit.corekit.module.NullType
    docs/source/pcapkit/corekit/enum.rst:28          autoclass:: pcapkit.corekit.enum.NoDefaultType
    docs/source/pcapkit/corekit/fields/field.rst:25  autoclass:: pcapkit.corekit.fields.field.NoValueType
    docs/source/pcapkit/corekit/sentinels.rst:18,21,25   the same three, canonically
    

    so pre-existing broken references inside those docstrings fire twice. Those three autoclass lines also predate this work and cut against the house rule that docs name the real defining module rather than the re-export.

    A cross-review is measuring the one thing that decides whether this is acceptable: whether the duplication produces ambiguous cross-reference targets. An ambiguous :class:NullType`` can silently resolve to the wrong page, which would be worse than the plain-text rendering #936 fixes. If it does, the fix is to make one documented home per class and drop or redirect the three shim directives.

    Separately, a new defect this surfaced — broken cross-references inside pcapkit/corekit/sentinels.py's own docstrings: an unqualified __new__, a bare NullType, and a ProtocolBase._lookup_next_layer attributed to the wrong class. Out of scope for #936, which was told not to touch that file. Recording them here rather than filing a fourth issue; say if you would rather they had their own.

  5. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    My plan is for (2) but i dont thinkaenum published any objects.inv so we cannot cross ref them anyways unfortunately.

  6. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Ruling recorded, verbatim:

    My plan is for (2) but i dont thinkaenum published any objects.inv so we cannot cross ref them anyways unfortunately.

    Your conclusion is right; the mechanism is slightly different, and the project already decided this. Measured rather than assumed:

    aenum does publish an objects.inv — https://aenum.readthedocs.io/en/latest/objects.inv returns 200 (309 bytes, # Project: aenum). But parsed with Sphinx's own loader it holds 5 objects total: 2 std:doc, 3 std:label, and zero py: objects. So no :mod:, :class: or :func: reference into aenum can ever resolve, exactly as you expected — adding it to intersphinx_mapping would fix none of the six and would cost an inventory fetch per build.

    And this was already settled and written down: docs/source/conf.py:95-106 deliberately excludes aenum with that reasoning recorded — "do serve a real objects.inv, but each contains only std:doc/std:label entries and zero py: objects … so no :mod:, :class: or :func: cross-reference into them could ever resolve", alongside pyshark, pypcap and emoji, with pypcapfile excluded for 404ing outright. The note ends by saying those packages are referenced as plain external links instead.

    So option (2) is not available, and option (1) is already project policy — the six aenum roles in conventions.rst are violating a rule the config states explicitly. The fix is to convert them to plain literals or external links, matching conf.py's own instruction.

    needs: decision removed — part C is decided. It joins part B as blocked on the same thing: every remaining fix lands in conventions.rst, which #932 owns until it merges. Part A (#936) is already review: good-to-go and independent of that file.

  7. added
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    and removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Sep 29, 2026
  8. removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 30, 2026
  9. JarryShaw commented on Sep 30, 2026

    @JarryShaw
    OwnerAuthor

    Unblocked — #932 merged as 382375811, so this can start. Holding it behind one other item, deliberately.

    docs/source/contributing/conventions.rst is wanted by three issues at once: #937 (adding the sentinel object-naming rule the SCREAMING_SNAKE ruling needs), #918 (harvesting the settled design rulings), and #934 parts B and C (the remaining unresolved-reference fixes). Two agents editing one file lose each other's work with no error — the later write simply wins — so these go one at a time rather than in parallel.

    #937 has the file first and exclusively, scoped to the .. _sentinel-convention: section and the sentinel table only, and briefed not to reflow or tidy anything else so the next edit does not collide with unrelated churn. This issue starts the moment #937's PR is up.

    Nothing else blocks it. Recording the order so it is checkable rather than remembered.

  10. added
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 30, 2026
  11. JarryShaw commented on Sep 30, 2026

    @JarryShaw
    OwnerAuthor

    Correcting my own blocker condition. I wrote that this starts "the moment #937's PR is up". That PR is now up as #939 (8c5feda9c, +343/-214 across 24 files) — but the condition I stated was wrong.

    docs/source/contributing/conventions.rst stays contended until #939 merges, not until it opens. A worker starting here now would branch from 382375811 and edit the same file #939 already rewrites, producing a merge conflict on a 24-file rename rather than a clean apply. Nothing is lost either way, but unpicking that costs more than waiting.

    So the real condition is: starts when #939 merges. Checkable with gh pr view 939 -R JarryShaw/PyPCAPKit --json state,mergedAt. #939 is review: pending with a cross-review in flight on a different model from its author.

    Recording the correction rather than quietly keeping this blocked past a condition I had already declared satisfied.

  12. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 30, 2026
  13. JarryShaw commented on Sep 30, 2026

    @JarryShaw
    OwnerAuthor

    Closing: all 16 are resolved on main, re-counted rather than assumed. #941 merged as 9ea0d6a5a (squashed), on top of #936 which landed the sentinels API page.

    Re-measured at 9ea0d6a5a against this issue's own table:

    kind was now how
    :mod:/:class: naming aenum 6 0 demoted to plain literals — aenum's objects.inv carries zero py: objects, so no role could ever have resolved
    bare :class:NoValueType`` 1 0 qualified to ~pcapkit.corekit.sentinels.NoValueType
    :meth:…Method.get`` 1 0 demoted; autodoc emits no method entries for these enum classes
    :class:…FEATCode`` 3 3 roles, but resolving docs/source/pcapkit/const/ftp.rst now carries autoclass:: pcapkit.const.ftp.command.FEATCode
    :mod:pcapkit.corekit.sentinels`` 5 5 roles, but resolving docs/source/pcapkit/corekit/sentinels.rst exists (#936)

    A seventeenth appeared mid-flight and is also fixed: #940 deleted FastBindingAcknowledgmentStatus.get outright, so its :meth: role stopped resolving between the filing and the fix. Now a plain literal at conventions.rst:459, role count 0.

    Two things this issue raised that are deliberately not closed with it, so they are not lost:

    • -n in the docs build, so a future unresolved reference fails CI rather than degrading silently. Still worth doing and still its own decision — there are ~1275 warnings on main, so it cannot simply be switched on.
    • The aenum exclusion is now pinned by a test rather than resting on prose: tests/project/test_conventions_doc_claims.py::AenumRoleExclusionTests fails if any aenum role is reintroduced.
  14. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 30, 2026
  15. added this to the 1.5 milestone on Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)docsPull requests that change documentation only (docs: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions