Skip to content

docs(protocols): second-round pass over the application-layer prose (#719) - #1082

Merged
JarryShaw merged 1 commit into
mainfrom
docs/719-r2-application
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/719-r2-application

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?

  • 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 read of pcapkit/protocols/application/. Fixes only where the code contradicts the prose:

  • ospf.py: header table offsets were wrong (type at octet 0, auth at octet 1); now 0/1/2/4/8/12/14/16, matching the schema. Dropped "as this did" history from the -1 dispatch comment.
  • http.py: _guess_version's Raises: claimed any non-ProtocolError failure falls through. A mock probe shows it escapes from both HTTP/1 arms, and only the final HTTP/2 trial absorbs struct.error.
  • httpv1.py: the obs-fold comment said a colon-less continuation raises IndexError, but the colon check below raises ProtocolError. Two past-tense lines made conditional.
  • httpv2.py: __frame__ comment now describes the str / (parser, constructor) lookup and the _none fallback. Added the missing pad_len / error_* args.
  • ftp.py, rarp.py, ngap.py: "contains X only" omitted FTP_DATA, DRARP and the NGAP enums. ftp.py gets a proper title.
  • __init__.py, application.py: mislabelled section comments, a wrong NoPayload path, "eg. … and etc."

Sphinx -n on fresh output: 12 warnings for these files on both branch and main, none new. Tests: every tests/protocols/application/test_*.py, test_tcp_http_dispatch_unit.py, tests/project, tests/test_docstring_contract.py, one module per process, all pass.

AST guard (docstrings stripped, vs origin/main) equal
__init__.py True
application.py True
ftp.py True
http.py True
httpv1.py True
httpv2.py True
ngap.py True
ospf.py True
rarp.py True

…719)

Correct claims the code contradicts: the OSPF header table's octet and bit offsets, the 'contains X only' module summaries in ftp, rarp and ngap, the HTTP._guess_version Raises section, the httpv1 obs-fold rationale, the httpv2 __frame__ mapping, and two mislabelled section comments in the package __init__. Drop leftover timed context in ospf and httpv1, and document the missing make-helper arguments in httpv2.
@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, 7c2c37f6f, 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.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 7c2c37f6f: GOOD TO GO (ran on Sonnet; author Opus)

  • AST guard: with every bare string stripped, all 9 files are identical to base.
  • http._guess_version Raises:: each arm was probed with mocked parsers:
    • In the HTTP/1 trial, ProtocolError falls through to "unknown HTTP version".
    • The final HTTP/2 trial absorbs ProtocolError and struct.error.
    • ValueError/KeyError escape.
    • The old "only exception" sentence was false and is gone.
  • OSPF: the offset table matches the schema field order and RFC 2328 A.3.1.
  • Comments: the HTTPv1 obs-fold, HTTPv2 __frame__ dispatch and the new pad_len/error_* parameter docs all match the code.
  • "contains X only" fixes match __all__: ftp, rarp, ngap.
  • Tests, one module per process: test_http_unit, ftp, rarp, ngap, ospf, httpv2_payload_length and test_docstring_contract all pass.

UNVERIFIED: the full Sphinx build was not run (docutils parse only, no new messages), and the httpv2_negative_length and tcp_http_dispatch tests were not run.

@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 9cc297d into main Oct 6, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the docs/719-r2-application branch October 6, 2026 17:35
@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