Skip to content

docs(foundation): second-round pass over the foundation prose (#719) - #1084

Merged
JarryShaw merged 1 commit into
mainfrom
docs/719-foundation-round2
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/719-foundation-round2

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • 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

Second-round #719 pass over every file under pcapkit/foundation/ (32 read, 20 changed). It only touches prose the code contradicts, plus leftover timed context:

  • Reassembly strict. The docs said strict=False drops incomplete datagrams. A probe shows it still returns them, as one contiguous payload with the holes zero-filled. For IP, when the total length is unknown, it returns only the prefix up to the first hole. strict=True returns the received runs instead.
  • Extractor. Rewrote the make_name steps, because no output format is ever inferred from fout. Rewrote record_header, because tracing never reads the header byteorder. In _cleanup, replaced the dangling _read_frame with the real re-entry path (a probe confirmed it). Completed the run and __init__ Warns/Raises lists.
  • Flow tracing. The callback receives one Index when a flow is finalised, not a list of datagrams. make_fout raises only when fout exists but is not a directory. Index.packet holds one datagram per direction and acknowledgement number.
  • Opt-in registration comments. Engine uses engine=, not name=, and a mistyped keyword leaves the class unregistered rather than registered under its own name.
  • Smaller fixes. Removed "used to / older behaviour / all along / as of" phrasing. Corrected the TCP module class and the register_pcapng_block label. Fixed the PCAP-NG reST target, and changed the IPv4 timeout citation to RFC 1122. Where timed wording was cut, the rationale is kept in present tense: why register_engine checks Engine rather than EngineBase, why TransportProtocol['tcp|udp'] misses rather than composing into 3, and why the trace-format guard covers an engine that dissects nothing.

Sphinx -n, fresh output directories, foundation warnings only: main 144, branch 143. No new warnings; the dangling _read_frame reference is gone.

AST guard (every bare-string Expr stripped, branch vs origin/main):

File Identical
pcapkit/foundation/engines/__init__.py True
pcapkit/foundation/engines/engine.py True
pcapkit/foundation/engines/pcap.py True
pcapkit/foundation/engines/pcap_ct.py True
pcapkit/foundation/engines/pyshark.py True
pcapkit/foundation/extraction.py True
pcapkit/foundation/reassembly/data/data.py True
pcapkit/foundation/reassembly/data/ip.py True
pcapkit/foundation/reassembly/data/tcp.py True
pcapkit/foundation/reassembly/ip.py True
pcapkit/foundation/reassembly/ipv4.py True
pcapkit/foundation/reassembly/ipv6.py True
pcapkit/foundation/reassembly/reassembly.py True
pcapkit/foundation/reassembly/tcp.py True
pcapkit/foundation/registry/protocols.py True
pcapkit/foundation/traceflow/__init__.py True
pcapkit/foundation/traceflow/data/data.py True
pcapkit/foundation/traceflow/data/tcp.py True
pcapkit/foundation/traceflow/tcp.py True
pcapkit/foundation/traceflow/traceflow.py True

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 6, 2026
@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 88.74% (unit tier, Python 3.14, b2aab951a, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1874 91 578 22 94.33%
pcapkit/dumpkit 136 0 40 0 100.00%
pcapkit/foundation 2422 143 842 34 92.62%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15653 187 3942 162 98.19%
pcapkit/toolkit 487 71 144 3 84.15%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

Per-file detail: the coverage-html artifact of this run.

Correct docstrings and comments the code contradicts: the reassembly
strict flag, the Extractor make_name/record_header/_cleanup contracts,
the flow-tracing callback, make_fout and per-ack datagram counts, the
opt-in registration comments, and several Warns/Raises lists. Cut the
remaining timed context and fix a dangling reST target.

Docstrings and comments only; AST identical to main once docstrings are
stripped.
@JarryShaw
JarryShaw force-pushed the docs/719-foundation-round2 branch from b74e185 to b2aab95 Compare October 6, 2026 17:07
@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one review: running A cross-review is in flight against the current head - no verdict yet and removed review: running A cross-review is in flight against the current head - no verdict yet review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on b2aab951a: GOOD TO GO (ran on Sonnet; author Opus, two rounds)

Round 1, on b74e1851a:

  • All 20 files are AST-identical to main once bare strings are stripped.
  • strict, probed on gapped input:
    • IPv4 strict=True returns the received runs, and strict=False returns one zero-filled payload.
    • TCP behaves the same way.
  • Engine registers through engine=. name= and typo keywords raise UnsupportedCall.
  • The extraction, traceflow and engine claims were each probed and hold.
  • 25 tests/foundation modules plus test_docstring_contract pass.

Round 2, on the four requested fixes:

UNVERIFIED: the Sphinx build was not run by the reviewer; the PR body reports 144→143 warnings.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@JarryShaw
JarryShaw merged commit 16b3963 into main Oct 6, 2026
42 checks passed
@JarryShaw
JarryShaw deleted the docs/719-foundation-round2 branch October 6, 2026 17:38
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label 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