reassembly: resolve conflicting TCP overlaps first-write-wins, per RFC 9293

[JarryShaw]

Closes #443

Summary

Two segments claiming the same TCP sequence range but carrying different
bytes used to resolve last-write-wins, silently, in the two overlap
branches of pcapkit/foundation/reassembly/tcp.py (:194 and its mirrored
reach-back case at :204). The returned Datagram still reported
completed=complete, with nothing recording that two segments had
disagreed.

RFC 9293 (which obsoletes RFC 793) says the resolution should be the
opposite:

§3.10: When a segment overlaps other already received segments, we
reconstruct the segment to contain just the new data and adjust the header
fields to be consistent.

"Just the new data" means the already-received bytes are kept and the
overlapping portion of the arriving segment is discarded -- first-write-wins.
§3.10.7.4 agrees: trim any portion outside the window off the incoming
segment, never overwrite what's already buffered.

This is an accepted, intended behaviour break. The reproduction from the
issue now yields payload=b'AAAAAAAACCCC' instead of b'BBBBBBBBCCCC' --
the current (pre-fix) output is not RFC-conformant, and a conforming sender
retransmits identical bytes, so nothing changes for a well-behaved stream.

What changed

• pcapkit/foundation/reassembly/tcp.py: both overlap branches now
  route through a new _merge_overlap static helper. It compares the
  arriving segment against buffered bytes only where the hole descriptor
  list says the range was genuinely already received; a position still
  marked as a hole gets the arriving bytes outright (an ordinary gap fill,
  not a conflict), and a received position that disagrees keeps the
  buffered byte and records the disagreement. completed is untouched, per
  the design decision on the issue -- it is no longer the only signal.
• pcapkit/foundation/reassembly/data/tcp.py: additive conflict
  field -- a list[tuple[int, int]] accumulator on Fragment, exposed as
  an immutable tuple[tuple[int, int], ...] of absolute, inclusive
  (first, last) sequence ranges on Datagram. Existing callers are
  unaffected; a caller can now ask a datagram after the fact whether it was
  contested (precedent: Zeek's rexmit_inconsistency event).
• docs/source/pcapkit/foundation/reassembly/tcp.rst: documents the new
  field on both the datagram and buffer ASCII diagrams.
• tests/foundation/reassembly/test_tcp.py: new
  TCPReassemblyConflictTests covering identical retransmission
  (uncontested, unchanged), full and partial overlap on both the forward and
  reach-back branches, a three-way conflict, and a conflict that survives to
  a later completion. Updated the existing overlap-merge test
  (test_fragment_merging_covers_gaps_overlaps_new_acks_and_holes) for the
  new first-write-wins arithmetic and hole-aware gap-fill.
• tests/foundation/reassembly/data/test_models.py: updated for the new
  constructor argument.

Explicitly out of scope

• strict is not used to gate anything here -- it means "return all
  datagrams, including those not implemented" (self._flag_s), unrelated to
  this path, and repurposing it would change the meaning of a public
  keyword.
• UDP has no reassembler in this codebase (no udp.py under
  foundation/reassembly/).
• IP fragment reassembly (pcapkit/foundation/reassembly/ip.py) has a
  structurally similar unconditional overwrite on an overlapping fragment,
  but it is a separate implementation governed by a different RFC
  (791/815 IP fragment reassembly, not TCP's RFC 9293 §3.10) -- filed
  separately as IP fragment reassembly overwrites already-received bytes on an overlapping fragment, unconditionally #477 rather than widening this PR.

Test plan

• Reproduced the issue exactly as described (with timestamp=0.0 added
  for the now-required argument): completed=complete,
  payload=b'BBBBBBBBCCCC', index=(1, 2, 3), on 5182ad0ce before any
  change, and unaffected by strict.
• After the fix, the same reproduction yields
  payload=b'AAAAAAAACCCC', conflict=((100, 107),).
• New regression tests exercise: identical retransmission, full overlap,
  partial overlap on the tail side (forward branch extending past the
  buffered end), partial overlap on the head side (reach-back branch), a
  three-way conflict, and a conflict recorded while the datagram was still
  partial that survives its later completion.
• Revert-proof: reverting the fix (via a temporary stash, not
  touching the shared stash stack across other sessions) and rerunning fails
  8 tests (6 new + 2 updated), confirming the tests catch the regression.
• Reassembly suite: 62 passed.
• Full suite: 1026 passed, 17 skipped, no failures.
• mypy/pylint against the two touched source files: no new findings
  (one pre-existing, unrelated Deferred arg-type mypy error was confirmed
  present before this change too).

[JarryShaw]

Review of PR #478 (fix-443-tcp-overlap-conflict) at 274df0651

Read the issue and both comments first. Comment 5725766109 corrected three stale details (line numbers, the Packet.timestamp requirement, and that strict makes no difference). Comment 5732234538 retracted the "roughly what Linux does" claim in the body — unverified, and I have not relied on it — and records the actual design decision: record conflicts on Datagram, resolve first-write-wins per RFC 9293 §3.10, leave completed alone. That decision is not reopened here; this review is about whether the implementation delivers it correctly.

What I checked

Worktree hygiene. git rev-parse HEAD in my assigned worktree read e80c42217 (main), not the PR head — confirmed the warning in the brief is real. Fetched refs/pull/478/head as pr-478-review (274df0651b563a82008343e8032e4fc2270d5e14, one commit, base main). Confirmed git merge-base origin/main pr-478-review = 5182ad0ce, and git rev-list --count origin/main ^5182ad0ce = 20 — matches the brief, so I used 5182ad0ce...pr-478-review throughout rather than a two-dot diff.

RFC 9293. Fetched the actual text (curl -sS https://www.rfc-editor.org/rfc/rfc9293.txt). §3.10 (line 2948) reads: "When a segment overlaps other already received segments, we reconstruct the segment to contain just the new data and adjust the header fields to be consistent." That supports first-write-wins as implemented. §3.10.7.4's "if a segment's contents straddle the boundary between old and new, only the new parts are processed" (line 3481) is anchored to RCV.NXT/window trimming rather than to disagreements among not-yet-delivered out-of-order segments specifically, so it's a weaker anchor than §3.10 alone — but the PR's own code comments, docstrings, and the .rst additions cite only :rfc:9293#section-3.10`` (verified via grep -rn "section-3.10" pcapkit/foundation/reassembly/tcp.py pcapkit/foundation/reassembly/data/tcp.py docs/source/pcapkit/foundation/reassembly/tcp.rst), never over-reaching to §3.10.7.4. So this is accurate as written, not a defect.

Hole-descriptor interaction (single fragment). Built a throwaway worktree at the PR head (git worktree add --detach /tmp/pr478-verify pr-478-review), confirmed pcapkit.__file__ resolved there before importing, and drove TCP.reassembly directly through six hand-built scenarios: a hole partially overlapped by an arriving segment, a hole exactly abutted by one, a hole bracketed by two received islands (both an exact fill and a fill that also disagrees with both flanks), a single-byte conflict, and two touching single-byte conflicts from separate segments. All six matched hand-computed expected payload/conflict values exactly, byte for byte and range for range — the core _merge_overlap distinction between "still a hole" (gap-fill, not a conflict) and "already received and disagreeing" (conflict, first write kept) holds correctly within a single fragment.

Hole-descriptor interaction (multiple ack buckets) — this is where it breaks. Buffer.hdl is shared per BUFID across every ack bucket, documented as deliberate at the top of the file, and the "update hole descriptor list" step that mutates it is ack-agnostic by construction (it only ever looks at info.first/info.last, never which bucket the segment landed in) — confirmed by reading the step-by-step loop. _merge_overlap, added by this PR, assumes the opposite: that a position no longer marked as a hole in that shared list was actually received by the fragment whose old bytes it was just handed. When a different ack bucket is what closed the hole, that assumption is false, and I reproduced concrete data loss from it — see the inline comment on tcp.py:291-299 for the full repro. Summary: fragment A has an internal zero-gap; fragment B (a different ack value under the same BUFID) fills the same absolute range, which removes the hole from the shared list without touching fragment A's own buffer; a later segment under fragment A's own ack that legitimately fills its own gap gets that data silently discarded and replaced with the never-received zero-padding, and the discard gets recorded as a resolved conflict — a real disagreement never happened. Multiple ack buckets per BUFID is not a contrived scenario: the PR's own updated test file already builds one directly (test_submit_incomplete_strict_complete_strict_false_and_empty_buffers's mixed buffer), and in real captures it's what a retransmission carrying an updated ACK value produces. Before this PR the unconditional slice assignment didn't consult hdl at all, so the same sequence would have kept all the arriving bytes (last-write-wins, no loss) — this is a regression, not merely a pre-existing gap.

The "cannot both happen at once" comment. Checked with a direct repro (see inline comment on tcp.py:230): a reach-back segment (PSN < ISN, guaranteeing a head prepend) that is also long enough to run past the buffered tail produces both a head prepend and a tail append from the same call. The code handles it correctly via the final concatenation; only the comment's claim is wrong.

conflict boundaries. Off-by-one at both ends, single-byte ranges, and two touching conflicts from independent segments all matched expectations exactly (see repro cases above). Two independent overlap events that happen to touch are recorded as two separate (first, last) tuples rather than coalesced into one — the field is documented as a log of individual disagreements, not a minimal interval set, and the behaviour is internally consistent with that; worth being aware of but not a defect.

Additivity. greped the whole PR worktree for every Fragment(/Datagram( construction site outside tcp.py itself — none exist; the ip.py construction sites are the unrelated reassembly.data.ip.Datagram class. All internal call sites (both submit() branches, both buffer-init branches in reassembly(), and every test construction site) pass the new conflict field. Confirmed via pcapkit/corekit/infoclass.py that Info's generated __init__ makes every annotated field a required positional/keyword argument with no default — so this is additive in the sense that every internal caller was updated, not in the sense that external code positionally constructing Fragment/Datagram stays source-compatible; that is consistent with how every other field on these Info subclasses already works, not something new to this PR. Round-trip: tests/foundation/reassembly/test_tcp_runtime.py's test_sample_capture_reassembles_every_stream_byte_exactly dumps through format='tree' (dictdumper) successfully with a real reassembled Datagram in play, so the new tuple-of-tuples field serializes fine through the one dumper path the suite actually exercises.

completed/conflict pairing. test_conflict_persists_once_a_later_segment_completes_the_datagram (already in the PR) directly exercises the intended pairing — a conflict recorded while partial survives to Completion.COMPLETE once the real gap closes — and I reran it in isolation, it passes.

Docs. The two extended ASCII tree diagrams in docs/source/pcapkit/foundation/reassembly/tcp.rst and the new prose paragraph describe conflict as absolute/inclusive and independent of completed; nothing there contradicts what I measured.

Test/lint claims — reproduced independently, with my own selections

• PYTHONPATH=/tmp/pr478-verify … pytest tests/foundation/reassembly/ → 62 passed, 0 failed (61.0s). Matches the claimed reassembly-suite figure.
• pytest tests/integration/test_reassembly_end_to_end.py tests/integration/test_reassembly_engine_parity.py → 20 passed (6.5s).
• pytest tests/ (after python examples/generators/make_samples.py to generate fixtures) → 1026 passed, 17 skipped in 901.99s. Matches the claimed full-suite figure exactly.
• mypy pcapkit/foundation/reassembly/tcp.py pcapkit/foundation/reassembly/data/tcp.py → one finding, Argument 2 to "Deferred" has incompatible type "tuple[Any, Any]"; expected "TransType" at (post-PR) line 440. Reran the identical command against the merge-base 5182ad0ce: same error, same code, at line 344 (shifted only by the lines this PR added). Confirmed pre-existing, not introduced here.
• pylint pcapkit/foundation/reassembly/tcp.py pcapkit/foundation/reassembly/data/tcp.py with the default config (no .pylintrc exists in this repo, and no CI job runs pylint) → 8.34/10, not 10.00/10. Ran the same command against 5182ad0ce for comparison: 8.39/10. The score barely moves (-0.05) and every finding is either a pre-existing ALL_CAPS-local-variable pattern already used throughout this exact method (BUFID, PSN, RAW, LEN, GAP, HDL, …) or a complexity-threshold count that was already over the default limit before this PR (too-many-branches 19→20, too-many-statements 58→69). I could not reproduce 10.00/10 with any config I could find in the repo, so I'm reporting what I actually ran rather than the second-hand number; this isn't a regression relative to the baseline either way.
• I did not attempt the revert-proof (6 new + 2 updated tests failing when the fix is set aside) — it requires editing tracked source, which is outside my mandate here, and I did not fabricate a result for it.

CI

Waited it out rather than judging a partial rollup. Final state: 21 SUCCESS + 2 SKIPPED (Docs test gate, Gate (full suite, Python 3.14) — both skip-by-design on pull_request) across all CheckRuns, plus the pyup.io/safety-ci StatusContext at SUCCESS. Fully green.

Verdict

The surfacing design (additive conflict field, completed untouched) is implemented correctly and is well tested for the single-fragment case, including exactly the boundary conditions this review was told to attack hardest (hole partially overlapped, hole exactly abutted, hole bracketed by two islands, single-byte and touching conflicts). But the crux question — does the hole-descriptor interaction hold in both directions — turns up a real one: _merge_overlap trusts the per-BUFID shared hole list as a proxy for "received by this specific fragment," and when a second ack bucket exists under the same BUFID (an existing, tested, realistic scenario, e.g. a retransmission carrying an updated ACK), that trust is misplaced. The result is silent data loss — genuinely new bytes discarded and replaced with never-received zero-padding — mislabeled as a resolved conflict, which is strictly worse than the last-write-wins behaviour this PR replaces. That is a regression the fix needs to close before this lands.

REQUEST CHANGES at 274df0651

[JarryShaw]

Fixed the data-loss regression in _merge_overlap, per review: it was using Buffer.hdl (shared across every ACK bucket under one BUFID) as a proxy for "has this fragment already received a byte here", when Buffer.ack is a dict of per-ACK Fragments with private raw buffers. A different bucket's segment closing a hole in the shared hdl said nothing about this fragment's own receipt, and consulting it there discarded a fragment's own real bytes whenever another bucket happened to cover the same absolute range first.

Fix (69e666fe8): Fragment now carries its own received mask (a bytearray aligned with raw), and _merge_overlap consults and updates that instead of hdl. Confirmed against the reviewer's own reproduction:

before (274df0651): b'AAAA\x00\x00\x00\x00\x00\x00DDDDEEEE'   conflict=((104, 109),)
after  (69e666fe8): b'AAAACCCCCCDDDDEEEE'                       conflict=()

Matches origin/main's lossless bytes exactly, with no spurious conflict.

Also added two tests (two-ACK-bucket cross-contamination, and a genuine gap-fill through the overlap-merge path asserting no conflict), fixed a wrong comment claiming a head-prepend and tail-append "cannot both happen at once" (they can), and re-ran the full suite: 1028 passed, 17 skipped.

[JarryShaw]

Re-review at 69e666fe8

Setup. Confirmed git rev-parse HEAD in my worktree before measuring anything -- it started at a stale sha (4c8ee6322, not the PR head), so I fetched fix-443-tcp-overlap-conflict and git checkout --detach 69e666fe8 before doing anything else. Verified git merge-base main 69e666fe8 = 5182ad0ce and used main...69e666fe8 (three-dot) for every diff. Generated fixtures with examples/generators/make_samples.py and, before every measurement, asserted pcapkit.__file__ started with this worktree's path -- printed each time, shown below.

CI. gh pr view 478 --json statusCheckRollup --jq [...] -> SKIPPED:2, SUCCESS:19 (final tally after waiting out the queue; Docs test gate and Gate (full suite, Python 3.14) skip by design on pull_request). All 19 real check runs (Python/Compat/Integration 3.10-3.15, Analyze, CodeQL, deploy-pages) plus pyup.io/safety-ci passed. Fully green.

Mechanism re-verified from scratch, not re-trusted from the prior review. Read pcapkit/foundation/reassembly/data/tcp.py and pcapkit/foundation/reassembly/tcp.py from this worktree at 69e666fe8 (caught and discarded one bad read from the shared canonical checkout at /home/jarryx/GitHub/PyPCAPKit, which turned out to be on a different, older commit -- missing received/conflict entirely -- before I'd used any of it). Fragment.received: bytearray is now a per-fragment, per-octet mask aligned with raw, maintained through every append/prepend/overlap branch in reassembly(), and _merge_overlap(rcvd, old, new, start) -> (merged, merged_rcvd, conflicts) answers "has this fragment received this byte" from that mask rather than from the shared Buffer.hdl.

1. Independently reproduced the cross-bucket fix (the exact scenario from the prior REQUEST CHANGES): bucket ack=1000 with a gap at 104-109, bucket ack=2000 filling that absolute range, then bucket 1000's own real bytes arriving for its own gap. Got raw=b'AAAACCCCCCDDDDEEEE', conflict=() -- matches the claimed fix exactly (script and full output below).
2. Alignment under append/prepend/overlap, by hand-proof and by fuzzing. Walked every branch in reassembly() algebraically: every place that concatenates onto RAW concatenates a matching-length piece onto RCVD (GAP-fill, tail-append, head-prepend, and both overlap branches), and the two "mutually exclusive tail" claims in the code hold algebraically (OVERLAP = min(...) forces exactly one of the two tails to be empty). Then verified this isn't just paper reasoning: built a case with simultaneous head-prepend and tail-extension in one call (old buffer isn=100 len=10, new segment dsn=90 len=30), both with and without a genuine mid-overlap disagreement, and ran a 200-trial fuzzer of random overlapping/out-of-order segments into one ACK bucket cross-checked against a first-write-wins reference model. All passed -- len(raw) == len(received) held throughout, no offset drift, byte values matched the reference. Scripts and full output below.
3. The corrected reach-back comment (lines ~234-248) is true. It replaces the prior, disproven claim that a head-prepend and tail-append "cannot both happen at once" with a claim that they can, and that what's actually mutually exclusive is which of the two tails is non-empty. My prepend+extend fuzz case above is a direct, positive test of exactly this, and it held.
4. Additivity. grep -rn "Fragment(" across pcapkit/ and tests/ finds exactly two production construction sites (both in tcp.py, both passing received and conflict) and the test-file constructions, all updated for the 6-arg signature. No other module constructs a pcapkit.foundation.reassembly.data.tcp.Fragment or Datagram -- everything else that imports from that module only imports Packet or uses Datagram/Fragment as a type reference. Datagram.conflict is threaded through both submit() branches (conflict=tuple(buffer.conflict)).
5. Full reassembly test suite: pytest tests/foundation/reassembly/ -- 64 passed, 18 subtests, including every new TCPReassemblyConflictTests case and the six-boundary-case coordinate tests from the prior review round.
6. mypy, three flag sets, all pointing at the same single pre-existing, unrelated error: --follow-imports=silent --ignore-missing-imports --show-column-numbers --show-error-codes (the Makefile's own mypy: target), --config-file mypy.ini alone, and no flags at all. All three report only tcp.py:467: error: Argument 2 to "Deferred" has incompatible type ... [arg-type] on these two files -- and a diff against the merge-base shows that exact line is byte-for-byte unchanged by this PR. Zero mypy errors introduced.
7. pylint -- checked, and partially refutes the "8.45 -> 8.19" claim as stated. Ran the project's actual make pylint flags (from the Makefile) at this commit and at the merge-base 5182ad0ce (exported via git archive to a scratch dir, no shared state touched). too-many-branches/too-many-statements cannot fire under those flags at all -- both "belong to the design checker" per pylint's own --help-msg, and the Makefile's --enable=...,R,... is followed later by --disable=...,design,..., which wins. Confirmed empirically: zero matches for either check, or for either changed file, anywhere in the full-package run. Score under those flags: merge-base 8.57/10, head 8.56/10 (-0.01, not -0.26). Under default pylint (no flags), the two checks do fire, but they are not new: at the merge-base reassembly() was already over both thresholds (19/12 branches, 58/50 statements); this PR moves it to 20/12 and 76/50. Real, but pre-existing and off the project's own gate.
8. Memory/cost judgement call (not blocking). received doubles the per-fragment footprint -- noted inline, with reasoning for why I think the bytearray tradeoff is the right call here rather than an interval list, given TCP reassembly here has no timeout by default.

Every command run and its real output is in the session transcript; the key ones: git rev-parse HEAD, git merge-base main 69e666fe8, gh pr checks 478 --watch, the fixture-generation and pytest tests/foundation/reassembly/ run (64 passed), the three mypy invocations, the two pylint invocations (head and merge-base under Makefile flags, plus default-flag runs at both), and two ad hoc scripts (/tmp/probe_reasm.py, /tmp/probe_crossbucket.py) exercising the mechanism directly against this worktree's pcapkit.

I did not attempt the revert-proof described in the review brief -- it requires editing tracked source, which is outside a review-only mandate; I did not check whether it still holds.

Three inline comments posted: a memory/cost tradeoff note on Fragment.received (judgement call, not a defect), a correction of the pylint claim anchored near reassembly(), and a confirmation that the corrected reach-back comment is verifiably true. None of the three are blocking.

GOOD TO MERGE at 69e666fe8

[JarryShaw]

Re-reviewed at 583a5d1a7 (confirmed via git rev-parse HEAD after git checkout 583a5d1a7), specifically the two commits that voided the prior GOOD TO MERGE: 2a795938c (Fragment.received: bytearray → Fragment.gap: list[tuple[int, int]]) and 7ce6c200d (splitting TCP.reassembly() into _reassemble_append/_reassemble_prepend/_update_hole_descriptors).

1. Is the split a pure extraction? Read reassembly() and the three new helpers line by line against a1c50d4df's single function. Both _reassemble_append/_reassemble_prepend capture ISN/RAW/GAPS as locals with exactly the same scoping the inline code had -- in particular, _reassemble_prepend captures the old ISN before calling fragment.__update__(isn=PSN), matching the original's local-variable capture-before-mutate order exactly. The mutual exclusivity of "old tail survives" vs "new tail survives" in the reach-back overlap branch is unchanged (OVERLAP = min(len(RAW), LEN - OFFSET) caps it identically). No state is read after being mutated out of order, no early return changes control flow, and _update_hole_descriptors is a verbatim lift of the HDL loop with no logic touched. Found no divergence from a pure extraction.

2. conflict divergence hunt on cases I built myself (two detached worktrees, a1c50d4df vs 583a5d1a7, identical driver against both):

• A segment spanning two separate existing gaps plus the two real-data ranges between them, in one _merge_overlap call (AAAAA real, gap, BBBBB real, gap, CCCCC real, then one 20-byte segment covering all of it with conflicting bytes) → identical on both trees: raw=AAAAAXXXXXBBBBBXXXXXCCCCC, conflict=[(base+10,base+14),(base+20,base+24)]; new tree's gap=[] matches the old tree's received-mask-derived holes ([]).
• A gap closed by one segment, then retouched by a later segment with different bytes → identical conflict=[(base+5,base+9)] on both -- confirms the closed gap entry is actually retired rather than merely masked, since a live gap entry there would have suppressed the conflict.
• GAP == 0 on both the forward-append and reach-back-prepend paths (exact abutment, no hole) → conflict=[], gap=[] on both trees. Also confirmed by code reading: both helpers guard GAPS.append(...) with if GAP > 0:, so a GAP == 0 call never appends an inverted (x, x-1) entry.
• A conflict + a partial gap-fill + a new-head-prepend, all in the same _merge_overlap call (prepend side: old fragment isn+20..24, prepend creates a gap at isn+15..19, then a reach-back segment supplies a new head, disagrees with the old real data, and partially fills the gap, all at once) → byte-identical raw, identical single conflict range, and the new tree's surviving trimmed gap entry ((isn+18, isn+19)) matches the old tree's received-mask-derived holes exactly.

All 6 cases matched to the byte; none diverged. Ran a 5000-trial randomized stress test on top of that (biased toward GAP==0 and negative/overlapping offsets, single bucket) checking every gap entry is genuinely all-zero in raw: 0 violations.

3. Interval hygiene. 5000 randomized trials, checked for (a) any zero-length or inverted gap entry ever stored, (b) any two gap entries adjacent or overlapping, (c) every gap entry genuinely all-zero in raw at the time it's checked. Zero violations. This confirms the reasoning: a new gap entry is only ever created at one of the two current edges of the buffer (the far end in _reassemble_append, the near end in _reassemble_prepend), and both of those edges are always real payload by construction (the buffer only ever grows by splicing real payload at its extremities), so a fresh entry never lands adjacent to another gap entry, and _merge_overlap's trim-or-remove logic never produces a zero-length remnant (the first < lo / last > hi guards ensure a surviving slice is always non-empty).

4. Test adequacy -- enumerated. Exactly one test in test_tcp.py uses two or more ACK buckets: test_one_ack_buckets_hole_closing_does_not_leak_receipt_into_another (ack=1000 and ack=2000), and that test predates this PR's two reviewed commits -- it's part of the earlier 274df0651/69e666fe8 work and asserts only .conflict/.payload, never .gap. Both tests added by the two commits under review here are single-ACK-bucket only: test_a_simultaneous_head_prepend_and_tail_extension_in_one_overlap_call and test_gap_list_matches_the_zero_filled_positions_across_200_random_trials (the latter's own docstring says "segments fed into a single ACK bucket"). So there is no committed test that drives the new gap mechanism across multiple ACK buckets. I did not build a dedicated multi-bucket probe of .gap itself -- what I have is a code-reading argument: _reassemble_append, _reassemble_prepend, and _merge_overlap only ever read or write fragment.gap, the specific ACK bucket's own Fragment, never self._buffer[BUFID].hdl or any other bucket's Fragment, so the isolation the existing multi-bucket test proved for the old received/conflict fields extends to gap by the same code path, not by a fresh test I ran. Worth a follow-up test; not something I found broken.

5. test_models.py. Diffed the PR head against origin/main's tip directly. test_ip_data_models_and_package_aliases already carries main's conflict field (from #482, unrelated to this PR) untouched by this PR. test_tcp_data_models_and_package_aliases is the only section this PR touched, updating Fragment(...)'s 5th positional arg from a received bytearray to gap=[]. No corruption, no cross-contamination between the IP and TCP sections.

6. Docs. Read the full 374-line tcp.rst. The ASCII .. code-block:: text block (lines 250-291) is intact and correctly indented; the two gap-explanation paragraphs sit outside the block, not wedged into it at the wrong indent. automethod:: lists only the two public methods (reassembly, submit); the new private helpers aren't autodoc'd, consistent with the pre-existing _merge_overlap convention. All autoclass::/.. module::/.. currentmodule:: directives name the real defining modules (pcapkit.foundation.reassembly.data.tcp, pcapkit.foundation.reassembly.tcp), not a re-export. RFC 815's RCVBT and the one English use of "received" in prose are the only surviving occurrences of the word, both legitimate.

Measurements I ran myself (fixtures generated via PYTHONPATH=<worktree> .../make_samples.py, confirmed pcapkit.__file__ resolved into the worktree before every run):

• pytest tests/foundation/reassembly/test_tcp.py -v: 25 passed, 205 subtests passed, 0 failures.
• pytest tests/foundation/reassembly/ -v: 75 passed, 218 subtests passed, 0 failures, 0 skips (the subtest count matches the quoted baseline of 218; the top-level pass count I measured is 75 rather than 73, but there are no failures either way so it doesn't change the picture).
• pylint pcapkit/foundation/reassembly/tcp.py --disable=all --enable=R0912,R0915: 10.00/10 -- neither too-many-branches nor too-many-statements fires post-split (previous run 7.73/10 on the pre-split file).

CI. The PR moved twice during this review (the owner pushed follow-ups): 583a5d1a7 -> b248e3651 (test-only commit touching only tests/toolkit/test_scapy_unit.py, unrelated to this PR's files) -> 4f7063f4f (a comment-alignment-only commit touching pcapkit/foundation/reassembly/tcp.py, verified by diff to contain zero code changes -- only trailing-comment columns shift). Each push cancelled the in-flight run for the prior sha via GitHub's concurrency group, which is why 583a5d1a7 itself never shows a completed run of its own. I polled the now-stable head 4f7063f4f to full settlement: 21 success + 2 skipped-by-design (Gate (full suite, Python 3.14), Docs test gate) = 23/23 checks, 0 failures, 0 pending, 0 cancelled.

Verdict. No code defect found in the split or the gap-list rewrite -- every adversarial case built to try to break it (spanning two gaps in one call, retouching a closed gap, both GAP==0 paths, conflict-plus-gap-fill-plus-new-head-prepend in one call, and 5000 randomized hygiene trials) matched the pre-rewrite behavior exactly, and CI is fully green on the current head. The one real, actionable gap is test coverage, not correctness: no committed test drives the gap mechanism through two-or-more ACK buckets. Worth a follow-up test; not a blocker.

GOOD TO MERGE 583a5d1a7 (the two commits that followed during this review -- 23bddffd5/b248e3651 and 4f7063f4f -- touch only an unrelated test file and comment formatting respectively, change no reassembly logic, and CI is green on the resulting head 4f7063f4f).