schema: let ListField.pack accept the tuple its own data models declare

[JarryShaw]

Closes #476

Summary

• Schema.pack's ListField branch (pcapkit/protocols/schema/schema.py:685-694) accepted list and bytes but raised ProtocolUnbound on a tuple, even though several data models declare the corresponding attribute as tuple[...] -- HIP's group_id, MH's prefixes/fid/bid/addresses/requests, and others -- and _read_* hands one straight back to _make_* on a parse-then-reconstruct cycle.
• Widened the check to isinstance(data, (list, tuple)), converting via list(data) before calling field.pack so ListField.pack's own Optional[list[_TL]] signature stays accurate for mypy. ListField.pack only ever iterates its argument, so the list/tuple distinction was never load-bearing -- it just broke every such round trip.
• Deleted the 16 EXPECTED_FAILURES entries in tests/protocols/test_option_roundtrip_unit.py this closes: hip-parameter/ACK, ACK_DATA, DH_GROUP_LIST, ESP_TRANSFORM, HIP_CIPHER, HIP_TRANSPORT_MODE, HIT_SUITE_LIST, NAT_TRAVERSAL_MODE, REG_FAILED, REG_INFO, REG_REQUEST, REG_RESPONSE, ROUTE_DST, ROUTE_VIA, TRANSPORT_FORMAT_LIST, VIA_RVS. Each was individually re-enumerated from examples/generators/options.py's cases() and confirmed roundtrip() now returns 'OK'.
• Added a direct unit test in tests/protocols/schema/test_schema_unit.py that packs a ListField with a tuple (a schema built directly with a list can never see this defect) and confirms a str -- also a sequence -- is still rejected.

The unverified tail

Checked whether the same list-only assumption exists in the unpack direction or in PCAP-NG/MH, which also declare tuple[...] on ListField/OptionField-backed attributes:

• unpack: no assumption to widen. ListField.unpack always builds a fresh list itself off the wire; it never inspects what a previous cycle produced.
• PCAP-NG / MH: same defect, same function -- not a separate issue. A sweep of all 322 examples/generators/options.py cases shows zero RECONSTRUCT failures, before or after this change, outside the 16 HIP ones. Reading why: _make_ext_multiprefix, _make_opt_context_request, and the flow-id fid/bid helpers in pcapkit/protocols/internet/mh.py already work around the identical ProtocolUnbound: unsupported type <class 'tuple'> at each call site with an explicit list(...) conversion (one of them even has a comment quoting that exact error), and PCAP-NG's NRB records never goes through ListField at all -- it's joined into a NUL-separated string and packed as a plain byte string. So nothing else needed filing.

Test plan

• pytest tests/ -q (PYTHONPATH pointed at this worktree, pcapkit.__file__ verified to resolve there first): 1032 passed, 17 skipped, 1631 subtests passed, 0 failed (896s)
• mypy pcapkit/protocols/schema/schema.py --config-file mypy.ini: 2 pre-existing errors (__prepare__ override, unpack "no self" false positive), 0 new
• Enumerated the 16 labels individually via options.cases() + roundtrip(): all 'OK'
• Revert-proof: reverting just the schema.py hunk reproduces exactly 16 subtest failures in test_option_roundtrip_unit.py and 1 failure in the new test_schema_unit.py test, all ProtocolUnbound: unsupported type <class 'tuple'>

[JarryShaw]

Setup and scope check

• Worktree started at main (e80c42217) rather than the PR head — caught it with git rev-parse HEAD before measuring anything, then git fetch origin pull/480/head:pr-480 && git checkout pr-480, landing at 94eb9f149, which matches the stated head.
• git merge-base main pr-480 → e80c42217ccd55926f3ea1e1032fd135a2cdff9e, i.e. main itself, so two-dot and three-dot diffs agree here. git diff --stat main...pr-480:

  pcapkit/protocols/schema/schema.py            | 15 +++++++++--
  tests/protocols/schema/test_schema_unit.py    | 37 ++++++++++++++++++++++++++
  tests/protocols/test_option_roundtrip_unit.py | 38 +++------------------------
  3 files changed, 54 insertions(+), 36 deletions(-)
  

  No mass-revert shape.
• Read gh issue view 476 --comments: the comment's corrected location, pcapkit/protocols/schema/schema.py:685-694, matches the code at 94eb9f149 — the if isinstance(field, ListField): block starts at line 685.
• CI: waited it out rather than judging a partial rollup. Final statusCheckRollup: 21 SUCCESS + 2 SKIPPED (Docs test gate, Gate (full suite, Python 3.14) — both skip-by-design on pull_request). The one StatusContext (pyup.io/safety-ci) is also SUCCESS. Fully green.

The fix itself (schema.py:685-702)

elif isinstance(data, (list, tuple)):
    ...
    self.__buffer__[field.name] = field.pack(list(data), packet)

ListField.pack (pcapkit/corekit/fields/collections.py:90-117) only ever does for item in value: ... — it never indexes or re-iterates, so it genuinely does not care whether it receives a list or a tuple. The list(data) wrapper exists purely to keep pack's own Optional[list[_TL]] signature honest for mypy, not for any runtime behavior — confirmed by running mypy --config-file mypy.ini pcapkit/protocols/schema/schema.py: 98 pre-existing errors in the package (arp.py, ipv6.py, esp.py, etc., all unrelated to this diff and present identically on main), and exactly the same two pre-existing schema.py errors as main (:194 __prepare__ return type, :749 missing self) — nothing new at the changed lines.

Is list(data) safe / does it hide anything? Empirically yes. I built the exact FeatureSchema/repeated: ListField fixture from test_schema_unit.py and probed every type that could plausibly reach this branch (script run with sys.path.insert(0, ROOT) + assert pcapkit.__file__.startswith(ROOT), printed: pcapkit.__file__ = .../agent-a387d14916949cfca/pcapkit/__init__.py):

value	result
list [0x10, 0x11]	OK, b'\x01\x10\x113\x00\x00'
tuple (0x10, 0x11)	OK, identical bytes
tuple subclass	OK, identical bytes
namedtuple	OK, identical bytes
generator (fresh)	ProtocolUnbound: unsupported type <class 'generator'>
generator (reused/exhausted)	same — never reaches list(data) either way
str 'xy'	ProtocolUnbound: unsupported type <class 'str'>
bytes b'\x10\x11'	OK (handled by the earlier elif isinstance(data, bytes) branch, unchanged)
bytearray	ProtocolUnbound: unsupported type <class 'bytearray'>
memoryview	ProtocolUnbound: unsupported type <class 'memoryview'>
object() / None	unchanged (None→b'', object()→ProtocolUnbound)

The isinstance(data, (list, tuple)) guard filters everything before list(data) ever runs, so a generator/iterator is never silently consumed, and a tuple subclass or namedtuple is flattened to a plain list harmlessly (pack never inspected the container's identity, only its elements). str still raises ProtocolUnbound and bytes is still caught by its own earlier branch — both unchanged from main. bytearray and memoryview behave identically before and after this PR (both still refused), so no regression there either. Given this, I agree the narrow (list, tuple) form is the right call over a collections.abc.Sequence test: every currently-declared data-model type is exactly list[...] or tuple[...] (verified by grep across pcapkit/protocols/data/), so Sequence would only add scope (and would need an explicit str/bytes/bytearray exclusion to avoid quietly admitting types nothing currently produces) without fixing anything live today.

The 16 deletions, verified by direct enumeration (not suite-green inference)

Loaded examples/generators/options.py the same way the test suite does and called cases() / roundtrip() directly:

pcapkit.__file__ = .../agent-a387d14916949cfca/pcapkit/__init__.py
total cases: 322
all 16 target labels present: missing=[]

Per-label result, calling roundtrip() myself on each:

label	result
hip-parameter/ACK	OK
hip-parameter/ACK_DATA	OK
hip-parameter/DH_GROUP_LIST	OK
hip-parameter/ESP_TRANSFORM	OK
hip-parameter/HIP_CIPHER	OK
hip-parameter/HIP_TRANSPORT_MODE	OK
hip-parameter/HIT_SUITE_LIST	OK
hip-parameter/NAT_TRAVERSAL_MODE	OK
hip-parameter/REG_FAILED	OK
hip-parameter/REG_INFO	OK
hip-parameter/REG_REQUEST	OK
hip-parameter/REG_RESPONSE	OK
hip-parameter/ROUTE_DST	OK
hip-parameter/ROUTE_VIA	OK
hip-parameter/TRANSPORT_FORMAT_LIST	OK
hip-parameter/VIA_RVS	OK

And across the full 322-case set: RECONSTRUCT count: 0 — nothing anywhere still fails the way this defect used to.

EXPECTED_FAILURES, compared by importing both main's and the PR's copy of tests/protocols/test_option_roundtrip_unit.py (not grep/ast — the dict uses ** unpacking):

main EXPECTED_FAILURES count: 77
pr   EXPECTED_FAILURES count: 61
removed labels (16): <exactly the 16 above>
added labels (0)
changed common entries (0)   # none of the other 61 were touched

And: every one of main's 16 removed entries names schema.py:624 in its defect string (e.g. Gap(status='RECONSTRUCT', fragment="unsupported type <class 'tuple'>", defect='pcapkit/protocols/schema/schema.py:624 -- _read_param_* returns a tuple where _make_param_* needs a list')); zero entries in the PR's 61 cite it.

Stale-entry check on the remaining 61 (mirror-defect concern): re-ran roundtrip() against every surviving EXPECTED_FAILURES entry.

entries naming a case that no longer exists in cases(): []
entries that now return OK (stale): 0
entries whose recorded status/fragment no longer matches actual outcome: 0

Also ran the real suite rather than only my scripts: pytest tests/protocols/schema/test_schema_unit.py tests/protocols/test_option_roundtrip_unit.py -v → 22 passed, 358 subtests passed (includes test_expected_failures_name_real_cases and test_round_trip_is_identity_or_a_recorded_gap), run with PYTHONPATH pointed at this worktree and pcapkit.__file__ confirmed to resolve here first.

The new test (test_schema_unit.py:102-137)

test_list_field_pack_accepts_a_tuple_like_it_accepts_a_list builds the module's own FeatureSchema directly with repeated=(0x10, 0x11) and calls bytes(as_tuple) — that goes through the real Schema.__bytes__ → pack() → the exact modified isinstance(field, ListField) branch, not a hand-rolled stand-in. It is revert-sensitive by construction: under the pre-fix elif isinstance(data, list): / else: raise ProtocolUnbound(...), bytes(as_tuple) would raise ProtocolUnbound instead of returning bytes, so self.assertEqual(bytes(as_tuple), bytes(as_list)) would error rather than pass. I could not execute this revert directly — that needs editing tracked schema.py, which is out of scope for a review, and an in-memory monkeypatch would be defeated by this file's own purge_modules(['pcapkit']) in setUp. Judged by reading instead, per the brief. The second half (repeated='xy' still raising ProtocolUnbound) I did verify empirically above.

Point 5: the mh.py "already worked around this" claim — three confirmed, one does not hold

• _make_ext_multiprefix (mh.py:10452-10460): prefixes = list(option.prefixes), with a comment literally quoting unsupported type <class 'tuple'>. Data_MultiPrefixExtension.prefixes is tuple[int, ...] (mh.py data model :839), Schema_MultiPrefixExtension.prefixes is a real ListField (mh.py schema :1462). Confirmed — genuine pre-existing workaround for this identical bug, now redundant but harmless.
• _make_opt_fs (mh.py:8980-8981): fid = list(option.fid). Data_FlowSummaryOption.fid is tuple[int, ...] (mh.py data :1328), Schema_FlowSummaryOption.fid is a ListField (mh.py schema :1462-1465). Confirmed.
• _make_fid_suboption, BID_Reference case (mh.py:9054-9059): bid = list(cast(..., option).bid). Data_BIDReferenceSuboption.bid is tuple[int, ...] (mh.py data :1369), Schema_BIDReferenceSuboption.bid is a ListField (mh.py schema :1536). Confirmed.
• The function named in the brief as _make_opt_context_request is actually _make_opt_cr (mh.py:8803). Its requests field is not a ListField at all — ContextRequestOption's own docstring (schema/internet/mh.py:1373-1385) says the request entries are "carried as one opaque run" and packed as requests=bytes(buffer) in _make_opt_cr (mh.py:8841-8844), walked manually rather than through ListField. So the list(option.requests) at mh.py:8821 is not a workaround for the ProtocolUnbound/tuple bug this PR fixes — it never went through the branch this PR touches, before or after. Minor inaccuracy in the investigation note, not a code defect (mh.py isn't part of this diff), and it doesn't change the "not a defect, don't block" conclusion for the other three.

Bonus, unprompted: the identical tuple-vs-ListField gap also exists in ipv6_route.py:605-611's _make_data_type_src (Data_SourceRoute.ip is tuple[IPv6Address, ...], Schema_SourceRoute.ip is a real ListField, passed through unconverted). It's currently masked by an unrelated, earlier CONSTRUCT-step defect already recorded as ipv6-route-type/Source_Route in EXPECTED_FAILURES on both main and this PR (ipv6_route.py:276, length-units bug) — so nothing exercises the tuple path today. This PR's general fix in schema.py happens to future-proof that case for free; it's outside this PR's stated scope (correctly not touched or claimed), not a defect, and not something this PR needs to address.

• PCAP-NG NRB records claim: confirmed accurate. IPv4Record.records / IPv6Record.records are declared tuple[str, ...] (data/misc/pcapng.py:546,560), but the schema side only has a plain StringField (resol); post_process splits a NUL-joined string into self.names, and _make_record_ipv4/_make_record_ipv6 (pcapng.py:5444-5506) join names back into '\x00'.join(names) + '\x00' before it ever reaches a field — it never touches ListField.

Verdict

No code defects found in the diff. The fix is minimal, matches ListField.pack's actual (iteration-only) contract, and is proven safe against every adjacent type by direct probing rather than inference. All 16 EXPECTED_FAILURES deletions are individually justified by enumeration (roundtrip() on each, not suite-green inference), no other entry was touched, and none of the remaining 61 are stale. The new unit test exercises the real code path and is revert-sensitive by reading. CI is fully green (21 SUCCESS + 2 SKIPPED-by-design), and my own pytest run of both touched files passes (22 tests / 358 subtests). The one inaccuracy found — the mh.py "context request" workaround claim — is about an investigation note, not about code in this diff, and doesn't affect the merge decision.

GOOD TO MERGE at 94eb9f149