interface: dispatch follow_tcp_stream on the engine, and fix the dpkt crash

[JarryShaw]

Closes #399 — with a correction to that issue's diagnosis, which was mine and was wrong about half of it.

The dispatch bug

follow_tcp_stream chose its reassembly adapter with extraction.engine == 'dpkt', but Extractor.engine returns the Engine instance, so both branches were dead code and pcapkit.toolkit.pcap's adapter was always used — against frames from whichever engine actually ran.

The two # type: ignore[comparison-overlap] comments were mypy reporting precisely this and being silenced. Both are gone and mypy is clean on the file without them. Note it was only "clean" before because the always-false comparison made the count= call statically unreachable.

Dispatch is now on the engine type, chosen over .name deliberately: for a third-party engine registered as class Foo(Engine, name='foo') the registry key ('foo') and the .name property ('Foo') diverge, so a name comparison is unreliable — and a third-party engine that subclasses a built-in now routes to the right adapter. An engine matching none of the built-ins warns and returns () rather than silently using the wrong one, which is the failure mode this issue is about.

fallback became pass_count, set per adapter rather than per branch. The pcapkit-native adapters read the frame number off the dissected frame and take no count; the third-party ones have no intrinsic number and must be passed count=index. The old code had these backwards.

The second defect, and my mistake in the issue

I filed #399 claiming the dpkt AttributeError: 'dict' object has no attribute 'packet' came from the reassembly adapter. It does not. Traced on main:

File "pcapkit/dumpkit/pcap.py", line 97, in __call__
    self._append_value(value, file, name or '')
File "pcapkit/dumpkit/pcap.py", line 139, in _append_value
    packet=value.packet,
AttributeError: 'dict' object has no attribute 'packet'

It fires during Extractor construction, in the PCAP trace-flow dumper: dpkt and scapy hand the tracer plain dict frames, and follow_tcp_stream lets trace_format default to 'pcap', which that dumper cannot re-serialise.

This is a known, deliberately deferred defect — pcapkit/foundation/extraction.py:836-846 says so in a NOTE, and the guard on line 847 covers only 'pyshark' and 'pypcapfile':

The DPKT and Scapy engines report the frame as a mapping too and are deliberately not listed here: they are affected by the same defect on this revision, but they are outside the scope of this change.

Handled here inside interface/misc.py — upgrading to 'json' for dpkt and scapy, silently when unset and with a FormatWarning when 'pcap' was requested explicitly — rather than editing shared extraction.py, to respect that recorded deferral. Still open afterwards: direct Extractor use is untouched by this workaround, so extending the extraction.py:847 guard remains worth doing separately.

scapy returning 0 streams is not a bug

Also a correction to the issue. scapy does not recognise in.pcap's link-layer type (unknown LL type [1]) and yields all six frames as opaque Raw, so no frame carries TCP and there is genuinely no stream to follow. The test asserts that cause, so a future scapy that learns the link type fails loudly instead of silently returning 0.

Verification

engine	before	after
default	3 streams	3 streams
dpkt	crash	3 streams, byte-identical to default incl. per-stream packet counts
scapy	0 streams	0 streams (cause now asserted)

The old tests mocked engine as a string — which is exactly why they never caught this — and that mocking cannot survive type-based dispatch, so they are rewritten against real extraction of the committed in.pcap. Per-engine expected counts rather than a uniform number, since engines with genuine capability gaps legitimately differ. dpkt/scapy cases are gated on the optional extras so a fresh [test] clone skips rather than fails.

tests/interface: 12 passed. Fixture-free unit tier: 577 passed, 17 skipped against a 572/17 baseline measured on the unmodified worktree — the delta is exactly the new tests.

Also checked

grep -rn "comparison-overlap" finds two more, at protocols/internet/hopopt.py:1004 and ipv6_opts.py:1016. Those are a different, benign pattern — schema.seed is not NoValue, where the field is typed int but defaults to the sentinel at runtime, so the guard is correct and the ignore hides an annotation gap rather than a dead comparison. Left untouched. No other object-vs-string comparisons exist in pcapkit/.

[Copilot]

🔵 Needs a closer look

Public type hints for format/Formats are now observably inconsistent with supported runtime values (e.g., 'cap' alias), and should be corrected to avoid misleading API typing for callers.

Pull request overview

Fixes follow_tcp_stream engine dispatch by selecting the correct TCP reassembly adapter based on the running engine instance type (instead of comparing the engine object to a string), and avoids the dpkt/scapy “dict frame + PCAP trace dumper” crash by switching trace output to a dict-capable format.

Changes:

• Dispatch TCP reassembly adapter via isinstance(extraction.engine, ...) and align count= passing with adapter expectations.
• Work around dpkt/scapy traceflow dumping crash by upgrading/downgrading trace_format to json (warn only when the user explicitly requested an unusable PCAP format).
• Replace the previous mocked-engine tests with capture-based, per-engine regression tests (skipping when optional extras are not installed).

File summaries

File	Description
pcapkit/interface/misc.py	Fixes engine adapter selection and adds dpkt/scapy trace-format workaround to prevent dict→PCAP dumper crashes.
tests/interface/test_misc.py	Rewrites tests to validate real per-engine behavior against the committed in.pcap capture, including optional-engine gating.

Review details

Suppressed comments (1)

pcapkit/interface/misc.py:43

• The Formats Literal excludes 'cap', but both Extractor and follow_tcp_stream explicitly treat 'cap' as a PCAP alias (trace_format in ('pcap', 'cap', None)). This makes the public type hints inconsistent with supported runtime values and can lead to false-positive type errors for callers.

    Formats = Literal['pcap', 'json', 'tree', 'plist']
    # NOTE: this alias duplicates the one in ``pcapkit.foundation.extraction``;
    # both copies need updating when a new engine lands.
    Engines = Literal['default', 'pcapkit', 'dpkt', 'scapy', 'pyshark', 'pypcap', 'pypcapfile']

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[JarryShaw]

Copilot's finding is correct, and the gap is pre-existing rather than introduced — this PR merely made it observable, because the dpkt/scapy trace workaround has to compare trace_format against 'cap'. Fixed in ea800298d.

Formats declared four values:

Formats = Literal['pcap', 'json', 'tree', 'plist']

while both registries accept eight, checked against the live objects rather than by reading the source:

Extractor.__output__ : ['cap', 'json', 'pcap', 'plist', 'text', 'tree', 'txt', 'xml']
TraceFlow.__output__ : ['cap', 'json', 'pcap', 'plist', 'text', 'tree', 'txt', 'xml']
same set? True

So 'cap' and the 'text'/'txt'/'xml' aliases were accepted at runtime but unspellable for a type checker — a caller passing format='cap' got a spurious error against a value the library documents in its own registry. Both declarations (foundation/extraction.py and interface/misc.py) now match the registries exactly, and I asserted that programmatically against __output__ rather than trusting the literal.

One deliberate call worth recording: two of those eight keys are defective for output — 'pcap'/'cap' raise TypeError: PCAPIO.__init__() missing 1 required keyword-only argument: 'protocol', and 'text' raises AttributeError: module 'dictdumper' has no attribute 'Text'. I still included them, because they are registered, so they are part of the declared API surface; a type should describe what the API accepts, not which paths happen to work. Narrowing the type to hide broken formats would misreport the interface and silently change what a checker permits. Both defects are pre-existing and unfiled — worth their own issue.

tests/interface + tests/foundation/test_extraction.py: 19 passed, 2 subtests.