docs: correct documentation that stated untrue things (#546)

[JarryShaw]

Closes #546.

Ten places where the documentation stated something untrue. Every item was
confirmed from its cited file:line before being touched; two turned out differently
from the issue's description and are called out as such, and two could not be fixed
here because their files are owned by other open work.

Rebased onto d656b09f1. One commit.

What was confirmed and fixed

1. no_eof documented backwards — extraction.py:787 said "if raise
EOFError when EOF"; interface/core.py:136 said "if not raise". The code
settles it: extraction.py:699-700 is if self._flag_n: continue, so the flag means
keep going. Corroborated by __main__.py:121, which sets no_eof = args.fin == '-'
so a stdin stream does not stop at a transient EOF. extraction.py now matches.
Tested.

2. register_extractor_engine documented a nonexistent parameter — engine
where the signature reads name, so the only documented spelling raised TypeError.
Fixed. I audited all 35 register_* functions in pcapkit/foundation/registry/
(99 documented parameter entries) and this was the only mismatch — contrary to
the reasonable expectation that a copy-paste defect would have spread.

No new test was needed: tests/test_docstring_contract.py already checks this
property project-wide and carried the defect as a KNOWN_DEFECTS entry reading
"owned by the registry docstring change". Deleting that entry is what turns the
existing check on. Tested, by an existing test.

4. pyproject.toml contradicted itself on the Python floor — the comment was
wrong, not the packaging. requires-python = ">=3.6, <4" is deliberate and
justified at length at :22-48 via the bpc-* source conversion, and the
classifiers list 3.6. So :187's "This project's own floor is now >=3.10" is the
false statement. Correcting it changes the meaning of the line around it: because the
project floor really is >=3.6, the python_version >= '3.10' marker on PCAP_CT is
load-bearing, not "belt-and-braces" — without it, pip install pypcapkit[PCAP_CT] on 3.6–3.9 would try to resolve two distributions that exclude
that interpreter and fail the whole install.

5. ext.rst omitted NGAP. Checked programmatically against every Protocol
subclass in the tree — 44 shipped, 37 tabled — and NGAP was the only concrete class
missing; the other six are abstract bases the table names in its "Protocol Type"
column instead. Added, and verified rendered inside the real table in the built HTML
rather than assumed.

6. The MH option count — the changelog was stale, pep.rst was right. Settled by
counting, not by picking: pcapkit.const.mh.option.Option has 71 members (0–70),
MH.__option__ has 71 entries, and CGA Parameters resolves to a dedicated
cga_param handler rather than a generic one. So "all 71 registered options" is
correct and "70 of the 71 … Only the CGA Parameters option remains" is stale.
CHANGELOG.md regenerated with util/changelog_md.py; --check passes.

7. deploy-pages.yml npm boilerplate over a Sphinx step — replaced with a comment
describing what the step does. I verified what PCAPKIT_DEVMODE actually does (DEBUG
logging on stderr plus untrimmed tracebacks) before writing it down, rather than
inferring from the name.

8. Two stacked #: comments on one field — reassembly/data/tcp.py:106-108. The
sibling reassembly/data/ip.py carries a single coherent comment for the same field,
so the stale first line is dropped and the wording brought into line with it.

10. Dead cross-reference targets — 31 fixed. Measured with a nitpicky
sphinx-build, and every replacement spelling was verified present in the baseline
build's objects.inv rather than inferred. That mattered: my first pass retargeted
Protocol._file and friends onto ProtocolBase, and the inventory showed the
opposite is true — Protocol.* is what Sphinx documents and ProtocolBase.* is
absent. That pass was reverted, and the 13 dead ProtocolBase.* references were
moved the other way.

Also fixed: engins→engines (twice, one of them in a type comment that a
role-based search cannot see), Secrets_Type→SecretsType,
Extraction→Extractor, StatusCode→DNSStatusCode,
:mod:pcapkit.exceptions``→pcapkit.utilities.exceptions, `Data_Raw`/`Schema_Raw`
onto their real modules, `PadOption.length`→`Option.length` where the field is
actually documented, `_read_ani_geo`→`_read_ani_suboptions`, and a stray `>` that
broke three `MH` roles in `registry/protocols.py`.

Three of these turned out to be real documentation gaps rather than typos:
Extractor._flag_r is set at extraction.py:824 and read by every engine but was the
one flag missing from the if TYPE_CHECKING: block documenting its eight siblings;
and Extractor._exctx, Protocol._data/_exctx were missing from the explicit
autoattribute lists their .rst files use under :no-members:. All now documented.
_flag_r is tested.

Two references had no addressable target at all and were demoted to literals
rather than papered over: dumpkit/common.py's default/_append_fallback are
methods of a class defined inside make_dumper(), so no dotted path can reach
them; and Frame._fnum/PCAPNG._fnum are instance-only attributes that
autoattribute cannot import — I tried adding them and it produced two new build
warnings, so I reverted that and demoted the references instead.

Measurements

Baseline is d656b09f1 — re-measured after the rebase rather than carried over,
since main moved by three commits while this was in progress.

	baseline	this PR
Sphinx warnings (plain)	50	50
of which duplicate-object	3	3
Sphinx warnings (nitpicky)	1035	985
distinct dead pcapkit.* targets	81	50
dead-target occurrences	195	144
unit suite	—	1077 passed, 8 skipped, 2604 subtests

The plain warning sets are identical — diffed as sets, not just counted: no
warning added, none removed. Dead cross-references do not warn outside nitpicky
mode, which is exactly why they accumulated. The 3 pre-existing duplicate-object
warnings (Info.__post_init__, reassembly.data.tcp.BufferID,
pcap.header.Header.__post_init__) are untouched.

All four builds ran with PYTHONPATH pinned to the tree under test, and each printed
the pcapkit.__file__ it imported. That is not ceremony: sphinx-build runs from
docs/, conf.py does not add the repository root to sys.path, and so without it
the build imports pcapkit from the editable install — a different checkout
entirely. Measured: with PYTHONPATH unset the import resolves outside the tree.

Testing

tests/project/test_documentation_claims.py is new (3 tests); the register_* check
reuses the existing contract test. Each assertion was proved to fail without its fix
by running the same files against a pristine snapshot — pytest's exit code read
from a file, because a wrapper's status is not pytest's and pytest 9.1.1 reports a
failing subtest's parent as PASSED:

• pristine tree: exit 1 — failing on the no_eof disagreement, the no_eof
  sense-vs-code check, the missing _flag_r declaration, and
  test_documented_parameters_exist naming register_extractor_engine documents 'engine'.
• fixed tree: exit 0.

The pristine run was confirmed to have imported the snapshot's own pcapkit, and its
failure text quotes the old docstring verbatim ('no_eof: if raise :exc:EOFError when EOF').

Checked on Python 3.10 as well, since #547 is currently red on that axis only:
both test files compile under 3.10.21, and the ast-only test class (the one needing
no pcapkit import) passes there. The new tests construct no classes and use no class
keywords, so the ABCMeta.__new__() got multiple values for argument 'name' failure
mode in #547 cannot arise from them.

Items 3, 4, 6, 7, 8, 9 are untested and cannot be — they are prose, a TOML
comment, a YAML comment and a deletion of unreachable code. Item 5 is verified by
inspecting the built HTML, not by a test. This change also cannot raise library
coverage, and I am not claiming it does: every source edit is a docstring or
comment, and the one added statement (_flag_r: 'bool') sits inside if TYPE_CHECKING: and never executes.

Three decisions I left to you

1. The SECURITY.md disclosure channel (item 3). The file was GitHub's unedited
template — 1.0.x supported, 0.x.x not, placeholder prose, and no disclosure
channel at all, which for a published PyPI package is the one item here with an
outside-facing consequence.

I did not invent an email address. Two verified facts made that unnecessary:
GitHub private vulnerability reporting is already enabled on this repository
(gh api repos/JarryShaw/PyPCAPKit/private-vulnerability-reporting →
{"enabled":true}), so security/advisories/new is a real channel you have already
turned on, and it is the primary route in the new file. The secondary route points at
the contact in CODE_OF_CONDUCT.md by reference rather than repeating the
address, so there is exactly one place to change it.

The open question is yours: if you would rather security reports did not arrive in
the same inbox as conduct reports, that is the one line to change — or delete, leaving
the GitHub advisory route as the only channel. Supported versions come from the
release list (1.4.x stable, latest v1.4.1.post2; 1.5.x pre-release), and I
deliberately did not pin the exact beta, because the automated Vendor Update job
bumps it — it moved from b3 to b4 while this PR was being prepared. The response
expectation ("best-effort", single maintainer) and the threat model section are my
drafting and are yours to soften or cut.

2. nitpicky — measured, and I recommend not enabling it. The issue calls it
"arguably the real fix". It is, but the cost is now known rather than speculative:
turning it on takes the build from 50 warnings to 1035, because nitpicky flags
every unresolvable reference including builtins and stdlib types with no intersphinx
coverage. Only ~150 of those are pcapkit.*. Enabling it as-is therefore buries the
signal it exists to surface, so conf.py is left unchanged. If you want it, the
route is nitpicky = True plus a nitpick_ignore_regex excluding the non-pcapkit
namespaces — its own change, with its own review, and a judgement about which
namespaces to trust.

3. conf.py (item 9) — I deleted the four dead functions rather than restoring the
hooks, and here is why. The deciding argument is that deletion is provably
output-neutral — they were never connected, so nothing rendered from them — whereas
restoring changes rendered output, which is your call and not mine. On the merits:
remove_module_docstring and process_docstring were two attempts at the same thing,
flipping typing.TYPE_CHECKING and reloading each module so autodoc could see guarded
names, and bind_type_checking_names now does that properly, per module and without
mutating a global mid-build. process_fields only ever printed — the lines.append
that was its point was itself commented out. source_read printed every document's
entire source, which here is thousands of pages of build log. The reasoning is left as
a comment where they were, so the next reader need not re-derive it. Their now-unused
sys, typing and List imports went with them. If you wanted any of them live,
this is the commit to object to.

Two things I could not fix, and why

• cga_type → cga_extension, the example Documentation states untrue things in ten places, including no_eof documented backwards and a nonexistent parameter name #546 names, lives in
  pcapkit/const/mh/__init__.py:92 and pcapkit/vendor/mh/__init__.py:92. Package
  __init__.py files under pcapkit/ are owned by concurrent work, so I left them
  alone rather than clobber it. The diagnosis is confirmed:
  cga_extension.CGAExtension is present in the inventory under both const and
  vendor, and cga_type defines CGAType. A one-word change whenever those files
  are free.
• data.internet.mh.PadOption.length is the same Option.length fix applied here
  to hopopt and ipv6_opts, but it lives in pcapkit/protocols/internet/mh.py, also
  owned by another change.

pcapkit/protocols/internet/ipv4.py:18-27 (the stale ip.dsfield.dscp /
ip.frag_offset / ip.proto docstring) was left alone as #546 directs, since #543
still owns that file. This PR touches no file owned by an open PR.

50 dead pcapkit.* targets remain, and they are not more copy-paste typos — they
need individual judgement (pcapkit.protocols.internet.AH,
const.ftp.command.CommandType, ReassemblyBase.timeout, several _AT/_PT/_ST
type variables, a truncated pcapkit.protocols.), and several sit in the off-limits
files above. Worth its own pass.

CI

CI has not run on this branch yet — it was pushed without waiting. Locally: the
unit tier is green (exit code read from a file), both Sphinx builds exit 0, and the
3.10 checks above pass. A red mark here should be treated as real rather than a queue
artefact.

[JarryShaw]

✅ GOOD TO MERGE — verified all three flagged owner decisions were handled correctly rather than left ambiguous: SECURITY.md references the real, pre-existing CODE_OF_CONDUCT.md contact (not an invented address — confirmed gh api repos/JarryShaw/PyPCAPKit/private-vulnerability-reporting returns {"enabled":true}, matching the PR's primary-channel claim), nitpicky is confirmed absent from conf.py (recommended, not enabled), and the four dead app.connect hooks are genuinely deleted (not restored) with the reasoning left as a comment. Independently confirmed the no_eof fix matches both the code (if self._flag_n: continue) and the sibling docstring exactly, and confirmed via the project's own existing parameter-name scanner that register_extractor_engine was the only mismatch across all foundation/registry/* files, on both main and this head.

[JarryShaw]

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

Head sha reviewed: d4a4fca442119dcbb7ee3e79625ea27fd37c6e9c, rebased onto main at d656b09f1.

The three owner decisions -- each checked directly, not read and trusted

SECURITY.md. Read the actual file rather than grepping for a placeholder marker that might not exist under that name. It does not invent an email address: the primary channel is GitHub's private vulnerability reporting (https://github.com/JarryShaw/PyPCAPKit/security/advisories/new), and I independently confirmed that channel is real and enabled on this repo:

gh api repos/JarryShaw/PyPCAPKit/private-vulnerability-reporting
{"enabled":true}

The secondary route says "email the project contact listed in CODE_OF_CONDUCT.md" -- a reference, not a repeated address. I confirmed CODE_OF_CONDUCT.md is untouched by this PR (git diff origin/main d4a4fca4 -- CODE_OF_CONDUCT.md is empty) and already contains a real address (jarryshaw@icloud.com, at line 37) that predates this PR entirely. So nothing was invented: the PR reused an already-public, already-real contact and flagged the actual open question honestly ("if you would rather security reports did not arrive in the same inbox as conduct reports, that is the one line to change"). This is the right resolution -- better than either a fake address or an unresolved TODO.

nitpicky. grep -n nitpick docs/source/conf.py returns nothing at all, before or after this PR. It was measured (via the sphinx-build -n command-line flag, not a conf.py setting) but not enabled, exactly as the PR claims -- recommended against, left as the owner's call.

The four dead app.connect hooks. Read the full diff on docs/source/conf.py: remove_module_docstring, process_docstring, process_fields, and source_read are all deleted (not commented back in, not left ambiguous), along with the four already-commented-out app.connect(...) lines that used to reference them. A multi-paragraph comment explaining why is left in their place. Ambiguity was not the outcome here -- a clear choice was made and documented.

no_eof -- the highest-value item, checked against the code directly

Read pcapkit/foundation/extraction.py:695-703 myself: the EOFError/StopIteration handler is if self._flag_n: continue -- the flag means keep going, i.e. suppress the stop. The corrected docstring now reads "no_eof: if not raise :exc:EOFError when reach EOF", which matches. Cross-checked pcapkit/interface/core.py:136, which already read "if not raise" before this PR -- confirming the fix corrected extraction.py to agree with the already-correct sibling, not the reverse. Also checked pcapkit/__main__.py:121 (no_eof = args.fin == '-'): a stdin stream sets no_eof=True so a transient EOF does not stop extraction -- consistent with "keep going," corroborating the corrected sense independently of the docstring comparison.

register_extractor_engine and the registry-wide audit -- checked with the project's own existing scanner, not a new one

Rather than trust "I audited all 35 register_* functions... this was the only mismatch," I ran tests/test_docstring_contract.py's own parameter_defects() function myself, filtered to foundation/registry/*:

• On main (before): exactly one defect -- Finding(module='pcapkit/foundation/registry/foundation.py', function='register_extractor_engine', subject='engine').
• On this PR's head (after): zero defects.

This independently confirms the claim using the project's own existing, general-purpose checker rather than anything bespoke to this review. Also read the three registry/protocols.py hunks directly: each removes a stray > inside :class:`~pcapkit.protocols.internet.mh.MH>` (malformed Sphinx role syntax, not a wrong parameter name) across register_mh_message, register_mh_option, register_mh_extension -- a different, correctly-identified defect class from the same audit pass.

Falsification of the fails-without claims

Reverted pcapkit/foundation/extraction.py to main alone, kept tests/project/test_documentation_claims.py: exit code 1, 3 failed -- test_documented_sense_matches_the_code, test_extractor_and_extract_agree, test_assigned_flags_are_declared (the last naming _flag_r specifically as assigned-but-undeclared). Matches the PR's claimed pristine-tree failure set exactly. Restored, then separately reverted pcapkit/foundation/registry/foundation.py to main with the KNOWN_DEFECTS entry already removed (as it is in this PR's tests/test_docstring_contract.py): exit code 1, 1 failed -- test_documented_parameters_exist, confirming the KNOWN_DEFECTS deletion genuinely re-arms the existing check rather than just looking tidy. Both reverts undone; final git diff --stat against the head empty. Clean re-run of both test files together: exit code 0, 10 passed, 12 subtests passed.

Read test_documented_sense_matches_the_code directly: it does not merely grep for the string "not raise" -- it also parses record_frames's AST and confirms the EOFError handler actually contains an if ... _flag_n ...: continue, so a future edit that changed the code's behaviour without updating the docstring (or vice versa) would still be caught.

MH option count (item 6) -- re-derived independently

len(list(pcapkit.const.mh.option.Option)) == 71
len(MH.__option__) == 71

Matches the PR's claim ("71 members, 71 entries") exactly, confirming "all 71 registered options" is the correct current statement and the changelog's old "70 of 71... only CGA Parameters remains" is genuinely stale.

Coverage honesty

The PR states items 3, 4, 6, 7, 8, 9 are untested and cannot be, and that item 5 is verified by HTML inspection rather than a test. Confirmed from the diff stat itself: the only test files touched are tests/project/test_documentation_claims.py (new) and tests/test_docstring_contract.py (one KNOWN_DEFECTS entry deletion plus its explanatory comment) -- no other test file in the 47-file diff. This matches the claim; the PR does not imply blanket coverage anywhere I could find.

pyproject.toml -- confirmed unchanged where it matters

requires-python = ">=3.6, <4" (line 49) and __version__ = '1.5.0b4' (pcapkit/__init__.py:125) both read as claimed -- the vendor-bot version bump is kept, and the floor is untouched; only the false comment claiming the floor had moved to >=3.10 was corrected, with a new explanation of why the python_version >= '3.10' marker on PCAP_CT is load-bearing rather than decorative.

Item 5 (NGAP in ext.rst) -- the omission and the fix both confirmed, the exact 44/37 split not fully pinned down

Confirmed pcapkit.protocols.application.ngap.NGAP is a real, concrete ProtocolBase subclass in the tree, and confirmed the diff adds it as a new table row. I could not exactly reproduce the PR's "44 shipped, 37 tabled" split with my own subclass walk (I get 43 total ProtocolBase descendants via __subclasses__(), and inspect.isabstract gives 34 "concrete" by Python's own definition, not 37) -- the discrepancy is very likely a difference in what each of us counts as "abstract" for the table's own "Protocol Type" column purposes (some layer bases may not register as inspect.isabstract even though the table treats them as non-tabled categories) rather than a defect in the PR's fix. The core fact -- NGAP was missing and is now present -- is independently confirmed either way.

CI status

Not run at review time, and the PR itself says so plainly ("CI has not run on this branch yet ... pushed without waiting"). Verdict on local evidence only, per standing instruction. Given the current board context (#547 red on an unrelated 3.10 ABCMeta issue, #543/#544/#549 green but behind main), this PR's own claim that both Sphinx builds and the unit suite are locally green was not independently re-run for the full ~13-minute Sphinx build or the ~15-minute unit suite, given the exhaustive targeted verification above already covers every falsifiable claim in the PR.

What remains unverified

• The exact "44 shipped / 37 tabled" count for item 5 was not reproduced precisely (see above); the underlying fix (NGAP added) is confirmed regardless.
• The full Sphinx build (plain and nitpicky) warning-count tables (50/50, 3/3, 1035/985, 81/50 distinct dead targets, 195/144 occurrences) were not independently re-run -- these take roughly 13 minutes each per the coordinator's own note, and I judged the exhaustive verification of the higher-priority items (the three owner decisions, no_eof, the registry audit, the fails-without proofs) to be the better use of that time within this review.
• The full unit-tier suite (1077 passed, 8 skipped, 2604 subtests claimed) was not independently re-run in full; only the two directly-relevant test files were run, both clean.
• The two deliberately-unfixed items (cga_type→cga_extension in const/mh/__init__.py and vendor/mh/__init__.py; data.internet.mh.PadOption.length) were read and confirmed as genuinely out-of-scope (owned by other open work) rather than independently re-diagnosed from scratch.