Skip to content

fix(dumpkit): scale a PCAP trace record's fraction to the trace's resolution (#1500) - #1529

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1500-trace-fraction-scaling
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1500-trace-fraction-scaling

Conversation

@JarryShaw

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
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

PCAPIO wrote frame_info.ts_usec unscaled, so a trace at the other resolution was 1000× off. frame_info doesn't record its resolution, so the new PCAPIO._make_timestamp works it out from time_epoch: the fraction is in whichever resolution truncates the frame's offset into its second to exactly ts_usec.

  • Same resolution as the file, or neither fits (a hand-built frame): written as is, so the bytes don't change.
  • The other resolution: recomputed from time_epoch. µs → ns is ×1000. ns → µs divides by 1000 and truncates, like block2frame, Frame.make and libpcap's sf-pcap.c, so 999,999,999 ns stays in its second. A fraction of a second or more carries into ts_sec.
  • PCAP-NG had the same defect: block2frame converts at the interface's resolution. It also keeps the exact timestamp_epoch, though, so this fixes PCAP-NG too and leaves engines/pcapng.py alone.
  • Deletes the Gap(1500) rows: 32 unit cases and many_interfaces.pcapng.

The new tests/dumpkit/test_pcap_trace_resolution_unit.py fails on main: 12 failures, and every cross-resolution subtest fails. With the Gap rows gone, the traceflow modules fail on main too, with 33 failures. Ran one at a time, all green: tests/dumpkit, tests/foundation/traceflow, tests/project, tests/test_docstring_contract.py and tests/test_tier_guard.py.

Closes #1500

…olution (#1500)

- PCAPIO._make_timestamp: frame_info does not say which resolution its
  ts_usec is in, so take it from time_epoch -- the resolution at which the
  frame's offset into its second truncates to exactly ts_usec. Written as is
  at the file's own resolution (octets unchanged) or when neither fits;
  otherwise recomputed from time_epoch: microseconds x1000, nanoseconds
  /1000 truncated (as block2frame, Frame.make and libpcap do), with a
  fraction of a second or more carried into ts_sec.
- Covers PCAP-NG input too: block2frame converts at the interface's
  resolution but carries the exact block timestamp, so engines/pcapng.py
  is unchanged.
- Delete the Gap(1500) rows (32 unit cases, many_interfaces.pcapng).
- tests/dumpkit/test_pcap_trace_resolution_unit.py: PCAP and PCAP-NG
  frames at each resolution, written and re-read through pcapkit.

Closes #1500
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 7b0e244c4. Cross-reviewed on Sonnet; the author ran Opus.

Verified:

  • Inference is exact for every frame pcapkit builds. time_epoch is a Decimal there, and a fraction fits both resolutions only when it is 0. 24,000 boundary frames gave 0 mismatches.
  • Same-resolution traces are byte-identical; cross-resolution traces are corrected. Over 11 captures, 4,708 records and 132 synthetic PCAP-NG records across 11 interfaces were all 0 bad, against 2,354 and 58 bad on main.
  • ns→µs truncation matches upstream. It agrees with both editcap -F pcap and libpcap 1.11.0 read at MICRO precision.
  • No overflow. The carry and 32-bit bounds hold.
  • Tests. Fail-before reproduces at 12 + 33. Rounding, a dropped carry and a swapped resolution are each caught by the new tests.
  • Cost. About 1.5 µs per frame, which is within noise end to end.

Non-blocking:

  • A float time_epoch makes the inference heuristic. The library never produces one, but the annotation suggests it does.
  • A frame where neither resolution fits is written unchanged without a word; a logger.debug there would make it visible.

Merge-order note: #1511 gives the dpkt and scapy engines real PCAP traces. After the second of the two merges, I will re-run a cross-resolution dpkt/scapy trace against this inference.

@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 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Coverage: 90.09% (unit tier, Python 3.14, 7b0e244c4, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 843 2356 756 92.08%
pcapkit/corekit 2161 66 700 33 96.26%
pcapkit/dumpkit 258 2 90 2 98.85%
pcapkit/foundation 2665 98 934 42 95.00%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 17086 238 4628 190 97.98%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 431 4 124 4 98.56%
pcapkit/vendor 4409 2342 1006 157 43.25%

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

@JarryShaw
JarryShaw merged commit d40f799 into main Oct 9, 2026
40 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 9, 2026
@JarryShaw
JarryShaw deleted the fix/1500-trace-fraction-scaling branch October 9, 2026 23:46
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(dumpkit): a nanosecond trace of a microsecond PCAP keeps the fraction unscaled

1 participant