Skip to content

docs: repair the top-level Sphinx pages and cut their timed context - #1014

Merged
JarryShaw merged 1 commit into
mainfrom
docs/719-docs-toplevel
Oct 5, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/719-docs-toplevel

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — N/A, no Python changed
  • make test passes, and a test case covers the change — I ran tests/project only (268 passed, 1 skipped, 864 subtests), not the full suite
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A, prose only, no user-visible behaviour

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

The four depth-1 pages under docs/source/ — changelog.rst, demo.rst, ext.rst, index.rst — as a slice of #719. −37 lines, 102 insertions / 139 deletions.

This slice turned out to be mostly accuracy, not concision, and two of the fixes are broken sample code rather than stale prose:

ext.rst imported and called pack2chain, which has never existed. The function is packet2chain (pcapkit/toolkit/scapy.py:70, exported at :64), so the documented sample raised ImportError — while a NOTE three lines above spelled it correctly. A second sample had return self indented 12 spaces under an 11-space with.

The *Base split is undocumented in three places. ext.rst and index.rst said engines, reassembly and flow tracing are written as Engine, Reassembly and TraceFlow subclasses. All eight shipped engines subclass EngineBase (engines/engine.py:81, with Engine(EngineBase[_T]) at :213), and reassembly/ip.py:33, reassembly/tcp.py:28, traceflow/tcp.py:35 all sit on their *Base. Registration is on the public class's __init_subclass__. This is the same conflation #513 hit from the other side.

Two counts were wrong and I re-derived both myself rather than taking them on trust. pcapkit/__init__.py exports 39 protocol classes against a table of 38 — the absentee is IPv6_Ext; the universal claim is dropped rather than a 255-column grid table rewritten. And index.rst's "the 12 such parameters in pcapkit.corekit.io" matches nothing measurable: an AST count gives 13 methods declaring positional-only parameters, all on SeekableReader, 27 such parameters counting self, 14 without, and 0 module-level functions. It now names the 13 methods, which is what bpc-poseur actually trips on.

Also fixed: a pyshark-on-3.13 prerequisite that contradicted the page's own measured timing table, two genuine nested-inline-markup defects (a literal inside bold, which renders the backticks visibly with no Sphinx warning), four typos, and an unused hyperlink target.

Citations are frozen per the ruling on #719 — changelog.rst carries 19 bare #nnn and all are byte-identical. Timed context is cut, including the 2020 sphinx-quickstart generator banner; .. deprecated:: 0.8.0 and the changelog's version-bounded narrative stay.

Not done: no Sphinx build. A docutils structural parse of all four pages with roles stubbed came back with zero errors or warnings, and the new cross-reference targets (EngineBase, ReassemblyBase, TraceFlowBase, SeekableReader) were each confirmed autoclass'd — but the project builds without -n/-W, so a build would not have checked them anyway. The 38 :class: targets inside ext.rst's grid table are not individually audited; only the membership claim was. The pyshark-3.13 fix rests on the page's own measured table, not on a re-measurement.

@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 labels Oct 4, 2026
@JarryShaw
JarryShaw force-pushed the docs/719-docs-toplevel branch from a5dcdf8 to 6d781c0 Compare October 4, 2026 22:45
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at a5dcdf8f3 — sonnet cross-review, a different model from the opus that drafted the slice. It confirmed all eight load-bearing claims independently, including the ones I cared most about: pack2chain has zero hits tree-wide; all eight engines subclass EngineBase, with engine= consumed only in Engine.__init_subclass__; BufferID carries no SPI anywhere under reassembly/ or traceflow/; 39 exports against 38 table rows with IPv6_Ext the absentee; and no reading of corekit/io.py reproduces the original "12".

It also closed the gap I had flagged UNVERIFIED: all 140 pcapkit.* targets in ext.rst resolve by import, 139 of them documented. And it confirmed no grid-table damage — the one cell edit, Extensable → Extensible, is 10 characters either way.

I took two of its three optional suggestions, now at 6d781c0cf:

  • ext.rst — the rewritten warning had traded main's reason ("may cause unexpected errors") for a concrete pointer to the registry module. That is a rationale cut, which docs: sweep all prose for concision and accuracy, and drop timed context outside the changelog #719 protects, so it now carries both the reason and the pointer rather than one at the expense of the other.
  • index.rst — "the 13 such methods on SeekableReader" read as though they were the only ones. Measured: 13 in corekit/io.py and 12 more across 8 files (schema/schema.py 4, protocol.py 2, then one each in infoclass, dumpkit/common, engine, reassembly, traceflow, pcapng schema). Reworded so the 13 are an example, not the set.

I left the third (the dropped EngineWarning name) — the fall-back behaviour is still described under Engine Comparison.

One pre-existing cross-document contradiction it surfaced is fixed in the sibling PR, not here. README.md said the module-structure page covers "the eight subpackages"; there are nine with an __init__.py — const, corekit, dumpkit, foundation, interface, protocols, toolkit, utilities, vendor. Since this PR's index.rst now states nine, shipping both unchanged would have put two files in one wave in direct contradiction. Corrected in #1011.

Delta re-review dispatched. Citations stay frozen at the new head and no file is wider than main.

- `ext.rst` imported and called `pack2chain`, which does not exist. The real name
  is `packet2chain` (`pcapkit/toolkit/scapy.py:70`), so the sample raised
  `ImportError` as written; a NOTE three lines above already had it right.
- `ext.rst` and `index.rst` said engines, reassembly and flow tracing are
  implemented as `Engine`, `Reassembly` and `TraceFlow` subclasses. All eight
  shipped engines and all three concrete reassembly and trace-flow classes derive
  from the `*Base` classes; registration lives on the public class's
  `__init_subclass__`. Both halves are now stated.
- `ext.rst`'s `BufferID` comment described separate IPv4 and IPv6 shapes with an
  SPI. `foundation/reassembly/data/ip.py:26` is one 4-tuple shared through `_AT`,
  and no reassembly `BufferID` carries an SPI.
- `ext.rst` claimed its table lists all protocol classes; `pcapkit/__init__.py`
  exports 39 and the table has 38, missing `IPv6_Ext`. The universal is dropped
  rather than the 255-column grid table edited.
- `index.rst` cited "the 12 such parameters in `pcapkit.corekit.io`". An AST count
  gives 13 methods declaring positional-only parameters, all on `SeekableReader`,
  and 0 module-level functions; 12 matches no reading, so it now names the methods.
- Fixes invalid indentation in a dumper sample, a pyshark-on-3.13 contradiction
  against the page's own measured table, two nested-markup defects, four typos and
  an unused hyperlink target.
- Cuts timed context per #719, including a 2020 `sphinx-quickstart` banner.
  `.. deprecated:: 0.8.0` and the changelog's version-bounded prose stay.

Citations are frozen: `changelog.rst`'s 19 bare `#nnn` are byte-identical.

`tests/project`: 268 passed, 1 skipped, 864 subtests passed. Part of #719.
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 6d781c0cf, carried to 75ef52b08 — sonnet delta re-review, a different model from the opus that drafted the slice.

It confirmed both fixes. The restored warning reads naturally and matches main's reason with the registry pointer kept. And it independently re-derived the positional-only spread: 13 in corekit/io.py, 12 more across 8 files — protocols/schema/schema.py 4, protocols/protocol.py 2, then one each in corekit/infoclass.py, dumpkit/common.py, protocols/schema/misc/pcapng.py, foundation/reassembly/reassembly.py, foundation/engines/engine.py, foundation/traceflow/traceflow.py.

It then caught that my own summary phrase was imprecise: "across the schema, protocol and foundation modules" covers only 10 of those 12, since infoclass.py and dumpkit/common.py are none of the three, and the schema files sit under protocols/ so two of the names overlap. 75ef52b08 takes its exact suggested wording — "across the protocols, foundation, dumpkit and corekit modules" — so the only delta from the reviewed head is the reviewer's own proposed sentence, which is why I am carrying the verdict rather than opening a fourth round.

On the vague reason in the restored warning: it did not derive the real mechanism and declined to invent one, which is the right call. It stays as main had it until someone can name the mechanism.

Re-verified at the reviewed head: citations unchanged (changelog.rst 19 against 19, lists equal; the other three pages 0), docutils structural parse with zero errors or warnings, grid-table geometry intact with every row matching its border width, longest lines still 255 in ext.rst and 114 in index.rst, and tests/project 268 passed / 1 skipped / 864 subtests.

It also agreed with leaving the third suggestion alone — the dropped EngineWarning name — since Engine Comparison still describes the warning and the fall-back.

Merge-order note: this PR and #1011 should land in the same wave, or this one first. index.rst here fixes both the count and the missing Vendor bullet, while #1011 fixes the README's count; #1011 alone would leave the README saying nine and this page saying eight.

@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 4, 2026
@JarryShaw
JarryShaw merged commit 439227b into main Oct 5, 2026
73 checks passed
@JarryShaw
JarryShaw deleted the docs/719-docs-toplevel branch October 5, 2026 00:35
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 5, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 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

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant