Skip to content

docs(utilities): drop the change-over-time narration from the small subtrees - #1017

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

JarryShaw merged 1 commit into
mainfrom
docs/719-utilities-docstrings

Conversation

@JarryShaw

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

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 small-subtree remainder of pcapkit/ for #719 — utilities/, toolkit/, dumpkit/, interface/ and the three top-level pcapkit/*.py files. 8 of 26 files changed, +40/−57.

This slice is timed-context work and nothing else, and I would rather say so than imply a concision pass that did not happen. The prose in these subtrees was already dense, so the agent that swept it cut change-over-time narration and left rationale-heavy passages — exceptions.py especially — alone. The deeper concision pass on exceptions.py and the register_apptype prose remains open under #719.

Most of the change is in pcapkit/utilities/exceptions.py, where four passages narrated what the design used to be rather than what it is: the BaseError bullets, the stacklevel "negated" note, the _excepthook limit=0 note, and the _threading_excepthook paragraph. They now state the reason directly. The extract_stack()-versus-frame-walk rationale is kept, as is both importlib.reload caveat.

No code changed, and it is checked rather than asserted. For all 8 files the AST with every docstring stripped is byte-identical to main. Citations are frozen: 17 occurrences, per-file multisets identical. I also confirmed the one factual claim the sweep made in passing — that BaseError does not set sys.tracebacklimit — by grep: the name appears only inside docstrings at exceptions.py:111, :185, :341, :350.

pcapkit/corekit/multidict.py is deliberately untouched. Its module docstring says the implementation is based on Werkzeug, and a verbatim upstream port is exempt from house style. werkzeug is not installed in the repo venv, so it could not be diffed against upstream to establish whether its docstrings are verbatim — it needs that diff before anyone sweeps it. 626 lines, 0 citations.

Not verified: no Sphinx build, and no mechanical sweep for nested inline markup or split literals across the untouched text of these files. The 6 tests/dumpkit failures seen during the sweep are all FileNotFoundError: sample capture 'test.pcapng' not found — the generated-fixture gap in a fresh worktree, not this diff; tests/dumpkit collected and imported fine.

@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
…ubtrees

Covers `pcapkit/utilities/`, `toolkit/`, `dumpkit/`, `interface/` and the
top-level `pcapkit/*.py`. Timed-context cuts per #719, plus three prose repairs
a cross-review asked for.

- `exceptions.py` carried the most: the `BaseError` bullets, the `stacklevel`
  "negated" note, the `_excepthook` `limit=0` note and the `_threading_excepthook`
  paragraph all narrated what the design used to be. They now state why it is what
  it is. The `extract_stack()`-versus-frame-walk rationale is kept.
- `interface/misc.py`, `dumpkit/pcap.py`, `common.py`, `__main__.py`,
  `compat.py`, `decorators.py` and `toolkit/scapy.py` lose "used to", "now",
  "today", "previously" and "no longer".
- The `Formats` comment claimed the registries' eight keys are what make the
  aliases spellable. They are not -- the `Literal` listing all eight is. Measured:
  the eight keys resolve to **four** dumpers, so the old list of three aliases
  also omitted `'text'`. It now names every alias by its target.
- `__main__.py` had a cut leave "only worked" with nothing to anchor it; now
  present tense. `dumpkit/pcap.py` called `UInt32Field` "the alternative way to
  pack them", asserting uniqueness for a class the dumper does not use -- it packs
  with `struct.Struct('<IIII')`.

No code changed: for all 8 files the AST with docstrings stripped is identical to
`main`. Citations are frozen -- per-file multisets byte-identical, `GH-356` and
CPython's `gh-90500` included.

`pcapkit/corekit/multidict.py` is untouched, and that is **not** an exemption
claim: it says it is "inspired and based on" Werkzeug rather than a verbatim port,
and it raises pcapkit's own `MissingKeyError` and `UnsupportedCall`, which a
verbatim port would not. It is also outside this slice. How much is verbatim is
unknown, and werkzeug is not installed, so it needs an upstream diff first.

`tests/project`: 268 passed, 1 skipped, 864 subtests passed. Part of #719.
@JarryShaw
JarryShaw force-pushed the docs/719-utilities-docstrings branch from 751f5b5 to 1409c29 Compare October 4, 2026 23:02
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES at 751f5b5ad, fixed at 1409c293b — opus cross-review, a different model from the sonnet that drafted the slice. It confirmed the important half: AST identical across all 8 files, and it measured the four exceptions.py rewrites rather than reading them — len(extract_stack()) going 4 → 0 at tracebacklimit = 0 while stacklevel() still returns 1, a subprocess thread raise showing the header survives the limit, and an uncaught chained BaseError printing cause-plus-one-line with no frames. All four present-tense claims hold.

The required fix was a causal link I had introduced. The rewritten Formats comment said the registries exposing eight keys is what makes the aliases spellable. It is not — the Literal listing all eight is. The registry count is what makes listing eight correct.

Measuring the alias structure made it worse than the review thought: the eight keys resolve to four dumpers — pcap/cap to PCAPIO, plist/xml to PLIST, json alone, and tree/text/txt to Tree. So the old sentence's list of three omitted 'text' outright. It now names every alias by its target and gives each clause one job.

Two smaller ones also fixed: a cut had left "only worked" in __main__.py with nothing to anchor the past tense (now present), and dumpkit/pcap.py called UInt32Field "the alternative way to pack them" — a definite article asserting uniqueness for a class the dumper never uses, since it packs with struct.Struct('<IIII').

A claim of mine is withdrawn. I wrote that pcapkit/corekit/multidict.py was left alone as an exempt verbatim upstream port. That does not hold, and I verified both halves of the reviewer's objection: the file says it is "inspired and based on" Werkzeug rather than a verbatim port, and it imports and raises pcapkit's own MissingKeyError and UnsupportedCall — which a verbatim Werkzeug port would not, since upstream raises BadRequestKeyError. It is also outside this slice's scope, so presenting it as a deliberate exclusion was misleading. The commit message now says only what is true: how much is verbatim is unknown, werkzeug is not installed, and it needs an upstream diff before anyone sweeps it.

Re-verified at the new head: AST identical to main, citations frozen with GH-356 and CPython's gh-90500 untouched. Delta re-review dispatched.

@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 1409c293b — opus delta re-review, a different model from the sonnet that drafted the slice. AST identical across all 8 files with byte lengths unchanged, citations 17/17 with per-file multisets byte-identical, tests/project 268 passed / 1 skipped / 864 subtests, and the re-run sweeps show 0 deleted docstrings, 0 exempt directives, 0 doctests and 0 nested inline markup.

It settled the one thing I could not. I asked whether 'text'/'txt' are aliases of 'tree' or the other way round, since the registry alone does not say. Four independent signals all give 'tree' as canonical: the declaration order in extraction.py:217-223 is canonical-first in all three groups and the two uncontested pairs follow that rule; the library defaults to 'tree' at extraction.py:611 and :875; the dumper class is dictdumper.Tree; and traceflow.py:150-162 is the same registry in the same order.

And it explained why the question arose at all: the Tree group's file extension is .txt, not .tree — the one row in the registry where the naming does not follow the canonical key, which is exactly what makes 'txt' look canonical. It is not; the extension names the output file type rather than the format.

It also verified the __main__.py rewrite against code, which the past-tense original left unfalsifiable: extraction.py:921 is cast('Layers', (layer or 'none').lower()), so layer or 'none' is the substituted sentinel and .lower() is what turns a passed 'None' into it.

One pre-existing imprecision it flagged, not a regression: the sentence says "passing the strings", plural for 'None'/'null', but only 'None' was ever rescued — 'null'.lower() is 'null', which is not the sentinel and never matched. main said the same in past tense, so it predates this PR. Left for the #719 all-files sweep rather than widening this slice.

It accepted the multidict.py correction and noted my :18/:183/:389/:575 exception-raise evidence is the stronger form of the argument than its own wording evidence, since it rests on behaviour rather than prose.

@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 615f3fe into main Oct 5, 2026
73 checks passed
@JarryShaw
JarryShaw deleted the docs/719-utilities-docstrings branch October 5, 2026 00:36
@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