corekit: let a nested schema's field callbacks reach the enclosing schema

[JarryShaw]

Summary

Closes #445.

A nested schema's field callbacks got a packet dict holding the enclosing
schema only under __packet__, so a callback written the ordinary way --
length=lambda pkt: pkt['length'], exactly as every top-level schema writes
it -- raised KeyError the moment the schema was nested. The measured
casualty is CGA Parameters: CGAParameter.extensions
(pcapkit/protocols/schema/internet/mh.py:515-521) sizes itself from
pkt['length'], which belongs to the enclosing CGAParametersOption, and a
well-formed 40-octet option raised KeyError: 'length' in
SchemaField.unpack (pcapkit/corekit/fields/misc.py:619, the
{'__packet__': packet} literal).

Design chosen: chained lookup (option 1 of the three in the issue)

nested_packet_context() (pcapkit/corekit/fields/misc.py) replaces the
literal with a NestedPacketContext, a dict subclass (see "A Python
3.10-only regression" below for why it is a dict subclass rather than
collections.ChainMap, which is where this started). A name the nested
schema does not declare falls through to the enclosing schema; a name it
does declare, or __packet__ itself, is found locally first. This design
was chosen over the other two because:

• Option 2 (a documented helper) would have required touching every
  callback site, including ones outside this PR's scope, to get the same
  fix mh.py's CGA Parameters needed. The chained lookup needed zero
  changes to mh.py: CGAParameter.extensions's existing
  pkt['length'] just resolves once the mapping falls through. Verified
  by reproducing the issue's exact 40-octet packet before and after.
• Option 3 (merge the parent in) was rejected for the reason the issue
  names: it lets an inner field silently shadow an outer one. The mapping
  keeps them apart -- see the shadowing test below.
• The __packet__ contract had exactly one piece of documentation (a
  docstring on an unrelated pcapng.py helper) and zero mentions in
  Schema.unpack's own reserved-key list. Both are fixed: the contract is
  now documented on nested_packet_context itself, and Schema.unpack's
  docstring names __packet__ alongside __length__ and
  __option_padding__.

Write path (explicitly checked, since a mapping that writes through or
drops writes would be worse than today): plain dict assignment and
deletion (pkt[key] = value, del pkt[key]) always act on this instance's
own storage -- that is what a dict subclass gives for free, with no
override needed. So a field a nested schema sets -- including one that
shadows a parent field name -- is never visible to the parent, and never
silently dropped either. in and .get() are overridden (the dict
built-ins for both bypass __missing__ and would otherwise miss the
fallback entirely), and so are __iter__/__len__/keys/values/items,
for the union-of-both-levels iteration the design promises. All of this is
covered by
test_nested_schema_reads_enclosing_field_by_name_and_does_not_leak_writes.

pcapkit/protocols/schema/misc/pcapng.py's two existing __packet__
consumers (packet_byteorder, BlockType.post_process) are untouched.
Both are called directly, in tests and in
pcapkit/foundation/engines/pcapng.py:192, with a plain
{'__packet__': {...}} dict rather than through SchemaField, so their
hand-rolled fallback has to keep handling that shape regardless of this
design -- simplifying them to rely on the chain would have broken those
direct callers. Covered by
test_pcapng_byteorder_consumer_still_works_with_both_shapes and
test_pcapng_block_type_mismatch_consumer_still_works_with_both_shapes,
each exercising both the hand-built dict and NestedPacketContext.

A Python 3.10-only regression, and why NestedPacketContext is a dict subclass

The first pushed version of this PR used collections.ChainMap({'__packet__': packet}, packet) directly, and CI went red on exactly two legs: Python 3.10 and Integration Python 3.10, both on
tests/protocols/misc/test_pcapng_unit.py::PCAPNGUnitTests::test_pcapng_remaining_constructor_branches_and_custom_dispatch,
with AttributeError: 'dict' object has no attribute 'to_dict' from
Schema.to_dict (schema.py:520, isinstance(value, Schema) wrongly true
for a plain dict).

That test has nothing to do with CGA Parameters, HTTP/2, or MH -- it
exercises fifteen unrelated PCAP-NG option constructors. The actual cause,
confirmed with a local Python 3.10 venv:

• The test passes in isolation on the unmodified tree, and fails in
  isolation (no other test file loaded, so none of this PR's new dynamic
  Schema subclasses are even created) as soon as this PR's misc.py
  change alone is applied.
• Reverting only the collections.ChainMap(...) call back to a plain
  {'__packet__': packet} literal -- nothing else changed -- makes it pass
  again. Restoring the ChainMap call reproduces the failure.

Diagnosis (full version posted on #439): Schema inherits
collections.abc.Mapping (schema.py:245), and on CPython <= 3.10
SchemaMeta.__new__ bypasses ABCMeta.__new__, so no Schema subclass
ever gets its own _abc_impl -- every one of them shares Schema's.
Asking Mapping a question before asking Schema one therefore poisons
the cache for the whole family. collections.ChainMap is itself a
collections.abc.MutableMapping, and constructing one pulled Mapping
into the question order, flipping the unrelated, later
isinstance(some_dict, Schema) check in the PCAP-NG test from False to
True. This is not a fault in the chained-lookup design; it is #439 (not
this PR's to fix) surfacing through an implementation detail of this PR's
own code.

Because the failing test is a plain unittest method, not one of
test_option_roundtrip_unit.py's per-code cases, INTERPRETER_GAPS cannot
express it -- that table only overrides a case.label lookup, and there is
no table to add this to. Rather than leave a real, reproducible CI failure
in place, NestedPacketContext now subclasses dict directly instead of
ChainMap/Mapping. dict's own metaclass is plain type, not
ABCMeta -- constructing or using a dict subclass never asks Mapping
anything, so the shared cache is never touched. Being a real dict also
nominally satisfies every existing packet: 'dict[str, Any]' annotation on
the rest of the field classes, so no other file needed to change (see the
mypy section below for why that mattered).

• __missing__ gives the enclosing-schema fallback for free: dict's own
  __getitem__ calls it automatically the moment a key is not found
  locally.
• __contains__ and get are overridden, since the dict built-ins for
  both bypass __missing__ entirely and would otherwise never fall
  through.
• __iter__/__len__/keys/values/items are overridden for the
  union-of-both-levels iteration the design promises; dict's own versions
  would only see this instance's local keys.
• copy is overridden because dict.copy() always returns a plain dict,
  even for a subclass, which would silently drop the fallback.
• Plain assignment and deletion need no override -- dict's own
  __setitem__/__delitem__ already only touch this instance's own
  storage, which is exactly the write isolation the design requires.

Same fallback, same __packet__ reachability, same write isolation as the
ChainMap version -- confirmed by the unchanged test suite (all
pre-existing tests pass unmodified). Verified after the change: the Python
3.10 venv runs tests/protocols/misc/test_pcapng_unit.py clean (37 passed,
139 subtests, 0 failed), and the previously-failing test passes both alone
and as part of that module.

mypy: two errors, from the first NestedPacketContext, now zero net

An intermediate version of this fix (after moving off ChainMap, before
settling on dict) implemented the same two-level mapping from a bare
object deriving from nothing at all. That is a legitimate way to avoid
collections.abc entirely, but it does not satisfy the existing
packet: 'dict[str, Any]' annotation Schema.pack/unpack declare, so
mypy flagged Argument 1 to "pack" of "Schema" has incompatible type "NestedPacketContext"; expected "dict[str, Any] | None", plus an
SchemaField.length type: ignore[has-type] that the restructuring made
newly unused.

Widening Schema.pack/unpack's annotation to admit the new type directly
was the obvious fix and the wrong one: packet flows from there into
pre_unpack/pre_pack/post_process, FieldBase.__call__,
ListField.pack, ConditionalField.test and more, each with its own
narrowly-typed packet: 'dict[str, Any]' signature. Widening only the two
Schema methods produced seven new errors at those call sites; doing it
properly would mean widening every field class's own signature, well
outside this PR's file list.

Making NestedPacketContext a dict subclass (previous section) sidesteps
this too: it satisfies the existing annotation nominally, everywhere,
because it is one. The type: ignore[has-type] was simply deleted rather
than replaced -- and checked: it is already flagged unused on a clean
da2422728 checkout (no changes at all), so it is a pre-existing,
unrelated mypy hygiene gap this restructuring happened to touch, not
something this PR introduced. mypy pcapkit on this PR's head: 123
errors in 40 files (main: 124) -- one fewer than baseline, and no new
error anywhere.

One stale citation

The httpv2-frame/{DATA,HEADERS,CONTINUATION} Gap entries' defect string
pointed at pcapkit/utilities/decorators.py:222; the actual raise EOFError is at :228 (prepare itself starts at :177). Corrected.

#437's three Exp_FFFD/Exp_FFFE/Exp_FFFF failures

Yes, same defect -- and #437 has since merged (489eef651), which let this
be checked directly rather than by reading its diff. ExperimentalExtension
declares data: 'bytes' = BytesField(length=lambda pkt: pkt['length']), but
length there is CGAExtension's own field (parsed locally, no nesting
problem). The KeyError those three cases hit happened before
ExperimentalExtension was even selected: CGAParameter.extensions's
OptionField has to size the whole extensions area first, via the same
pkt['length'] this PR fixes, regardless of which extension type ends up
inside it. #437 itself registered all three codes and attributed all four
mh-extension/* entries (including Multi_Prefix) to #445 with exactly
that reasoning. See "Second merge" below for what happens to those four
entries once this branch also carries #437 and #446/#456.

CGA Parameters: #446 merged too, so it now parses end to end

This PR alone does not unblock CGA Parameters: with only #445 fixed, it
reaches FieldValueError: Field parameters has invalid length. -- the
separate, already-filed ForwardMatchField/Schema.__len__ defect (#446)
-- instead of the KeyError. That was true when this PR was first opened.
#446 has since merged as #456 (0283a6d59), and with both fixes on the
same tree, the issue's own 40-octet reproduction parses completely: no
exception, a populated CGAParametersOption with one CGAParameter. See
"Second merge" below for the test updates this required.

EXPECTED_FAILURES fallout in the round-trip harness

Fixing the KeyError unblocks the same latent defect in six HTTP/2 frame
schemas (they read the header's flags the same way, on the pack side).
Running tests/protocols/test_option_roundtrip_unit.py after the fix
turned seven cases red against the old table, each hitting a distinct,
unrelated, previously-unreachable defect underneath:

• httpv2-frame/PUSH_PROMISE, httpv2-frame/PING: now round-trip cleanly
  -- entries deleted.
• httpv2-frame/DATA, HEADERS, CONTINUATION: now EOFError --
  pcapkit/utilities/decorators.py:222's prepare unconditionally treats
  a zero-length nested unpack as end-of-file, which is wrong for a frame
  that legitimately has no payload. Not fixed here (that file is owned by
  open PR utilities: let @prepare read length/packet by keyword, not position #450).
• httpv2-frame/SETTINGS: now AttributeError: 'SettingPair' object has no attribute 'length' -- SettingsFrame.settings
  (pcapkit/protocols/schema/application/httpv2.py:283-285) passes the
  SettingPair class to ListField's item_type instead of wrapping it
  in a SchemaField, silenced by a # type: ignore[arg-type]. Filed as
  SettingsFrame.settings passes a Schema class to ListField's item_type instead of a SchemaField #459; not fixed here.
• mh-extension/Multi_Prefix: at the time, now FieldValueError: Field prefixes has invalid length -- _make_ext_multiprefix
  (pcapkit/protocols/internet/mh.py:3864) wrote
  length=1 + len(prefixes) * 16 (17 octets for one prefix) instead of
  4 + len(prefixes) * 8 (12). Not a new defect: open PR protocols: complete the Mobility Header registry -- all 24 message types, 70 of 71 options, all 4 CGA extensions #437 already
  fixed exactly this. protocols: complete the Mobility Header registry -- all 24 message types, 70 of 71 options, all 4 CGA extensions #437 has since merged -- see "Second merge" below for
  where this entry ends up.

Two more defects surfaced the same way, filed rather than fixed here:
#458 (@prepare raises a bare EOFError for any zero-length schema,
decorators.py:227-228 -- the mechanism behind the DATA/HEADERS/
CONTINUATION entries above) and #459 (above). Both were open when
this was written and have since merged -- see "Third merge" below.

All entries re-attributed with the new status, fragment and file:line,
verified against the actual exception text.

Second merge: main gained #437 and #446/#456 mid-review

main moved again while this PR was in review -- #437 (MH registry
completion) and #456 (issue #446, the ForwardMatchField double-count)
both merged. Merged origin/main (0283a6d59) a second time, one
conflict in the same mh-extension region of EXPECTED_FAILURES
(resolved by re-deriving each entry from the actual post-merge behaviour,
not by picking a side):

• protocols: complete the Mobility Header registry -- all 24 message types, 70 of 71 options, all 4 CGA extensions #437 itself had added three sibling entries --
  mh-extension/{Exp_FFFD,Exp_FFFE,Exp_FFFF} -- alongside Multi_Prefix,
  all four attributed to A nested schema cannot reach the enclosing packet's fields by name, so CGA Parameters raises KeyError: 'length' #445 with the identical KeyError: 'length'.
  Ran each of the four individually against the merged tree rather than
  assuming they behave alike: all four now return 'OK'. protocols: complete the Mobility Header registry -- all 24 message types, 70 of 71 options, all 4 CGA extensions #437 both
  registered the three experimental codes and fixed
  _make_ext_multiprefix's bogus arithmetic; schema: a forward match consumes nothing, so bill it nothing in len(schema) #456 fixed the
  ForwardMatchField double-count that stopped
  CGAParametersOption.parameters from sizing correctly. Combined with
  this PR's own fix, nothing blocks any of the four any more. All four
  entries deleted.
• The CGA Parameters 40-octet reproduction now parses end to end (see
  above). test_cga_parameters_option_reaches_the_446_boundary_not_a_keyerror,
  which asserted the now-stale FieldValueError boundary, is rewritten as
  test_cga_parameters_option_now_parses_end_to_end, asserting the parsed
  fields directly.
• protocols: complete the Mobility Header registry -- all 24 message types, 70 of 71 options, all 4 CGA extensions #437 had itself pinned the pre-A nested schema cannot reach the enclosing packet's fields by name, so CGA Parameters raises KeyError: 'length' #445 KeyError as
  tests/protocols/internet/test_mh_unit.py::test_mh_cga_parameters_option_is_unparsable_upstream,
  explicitly so that "whoever fixes it finds out here" -- and it did: that
  test went red the moment this merge landed, for exactly the reason its
  own docstring named. Replaced it with
  test_mh_cga_parameters_option_now_parses, asserting the option parses
  with the fields the wire says it should have, and corrected the now-false
  claim in test_mh_pmipv6_options_round_trip_byte_for_byte's docstring
  (CGA_Parameters is still excluded from that test's round-trip cases, but
  no longer because it cannot be parsed -- that is a separate, deliberate
  scope decision for whoever adds it, not implied by parsing alone).

Re-verified after this second merge: mypy pcapkit -> 123 errors/40
files (a fresh origin/main at 0283a6d59: 124, unchanged from the
first merge's baseline). Round-trip harness: 7 passed, 363 subtests
passed, 0 failed (up from 299 before #437 grew the mh-extension family
to four codes). tests/protocols/internet/test_mh_unit.py: 35 passed, 266
subtests passed, 0 failed. Full suite (Python 3.14,
PYTHONSAFEPATH=1 python -m pytest tests -q): 1001 passed, 17 skipped,
1547 subtests passed, 0 failed (883s).

NestedPacketContext: from a hand-written mapping to a dict subclass, and back to semantics

The ChainMap-vs-dict diagnosis above turned out to be only half right on
review. #462 merged mid-review and, independently, an owner review thread
asked why nested_packet_context() doesn't just reuse Info
(pcapkit/corekit/infoclass.py) instead of a dedicated class. Rather than
argue from the (now-corrected) ABC-cache mechanism, I built an actual
Info-based nested context on disk and ran it: on a local Python 3.10.21
venv (matching CI's exact patch), tests/protocols/misc/test_pcapng_unit.py
-- the module that broke under ChainMap -- came back clean, 37 passed,
139 subtests, 0 failed. So Info is not unsafe here; the real question is
semantics, not the ABC cache:

• __missing__-style fall-through: no saving either way. Info.__getitem__
  is a from-scratch self.__dict__[...] lookup with no such hook (that's a
  dict C-level feature), so it needs the same custom code whichever base
  is used.
• __contains__/.get(): a genuine point for Info -- Mapping supplies
  mixins for both that delegate to __getitem__, so fixing __getitem__
  once gets both for free, where dict's own versions bypass __missing__
  and need separate overrides.
• Writes landing on the nested instance only: the actual blocker. Info is
  deliberately immutable (__setattr__ raises, no __setitem__ at all),
  but field callbacks write into the packet dict throughout parsing
  (Schema.unpack's per-field loop: packet[field.name] = value, once per
  field). Supporting that on an Info subclass means writing into
  self.__dict__ directly from a custom __setitem__, bypassing rather
  than extending the immutability Info's own docstring promises.
• Extra bookkeeping to filter: Info instances carry __map__/
  __map_reverse__ in self.__dict__ (for its builtin-name-collision
  handling), which then show up in naive iteration alongside the real
  packet keys -- measured directly, not assumed.

Full reasoning posted on the owner's thread
(#457 (comment));
left unresolved for the owner to close.

Third merge: main gained #461 (closing #458) and #462 (closing #459)

Two more of the newly-surfaced defects this PR had filed (#458, #459) were
fixed and merged while this was in review, as #461 and #462. Merged
origin/main (fa128959e, then e7004191a after reconciling with a
duplicate parallel merge already pushed to this branch) -- no conflicts;
#464's changes to tests/protocols/internet/test_mh_unit.py land in a
different region and were confirmed non-interacting by running that file
(62 passed, 272 subtests, 0 failed).

• #461 makes @prepare distinguish a declared zero length (a nested
  schema legitimately sized to zero) from a derived one (genuine
  end-of-stream), raising only for the latter. Verified directly:
  httpv2-frame/{DATA,HEADERS,CONTINUATION} all now return 'OK'.
• #462 wraps SettingsFrame.settings's item type in
  SchemaField(schema=SettingPair). Verified directly:
  httpv2-frame/SETTINGS now returns 'OK' too.

All six httpv2-frame entries this PR's own fix had exposed are now gone:
PUSH_PROMISE/PING closed by #445 itself, DATA/HEADERS/CONTINUATION
by #461, SETTINGS by #462. Rewrote the section comment to summarise all
six rather than describe five stale gaps.

Final numbers, at this branch's current head: mypy pcapkit -> 124
errors/40 files (a fresh origin/main at fa128959e: 125 -- one more
than its own earlier count, unrelated to this branch, and this branch
stays one fewer than whatever main's own count is, from the same
pre-existing type: ignore cleanup as before). Round-trip harness: 7
passed, 363 subtests, 0 failed. Full suite (Python 3.14): 1010 passed,
17 skipped, 1553 subtests passed, 0 failed (906s). CI is green: all
22 checks pass (2 skip by design -- the docs gate and the single-version
full-suite gate).

Testing

New file tests/corekit/test_fields_misc_packet_context.py:

• test_nested_schema_reads_enclosing_field_by_name_and_does_not_leak_writes
  -- the load-bearing case. Before: KeyError: 'length' (matches the
  issue exactly). After: passes, and also checks shadowing, the write
  path, in, .get() and iteration in one pass.
• test_pcapng_byteorder_consumer_still_works_with_both_shapes,
  test_pcapng_block_type_mismatch_consumer_still_works_with_both_shapes
  -- the two existing consumers, with a hand-built dict and with the new
  chain. Before: ImportError (the helper does not exist yet).
  After: pass.
• test_cga_parameters_option_reaches_the_446_boundary_not_a_keyerror --
  the issue's own 40-octet reproduction. Before: KeyError: 'length'
  at mh.py:516. After: FieldValueError, confirming the fix worked
  and CGA Parameters correctly still does not parse.

Baseline at e2d8ed6d1 (this branch's original merge-base, confirmed by
temporarily reverting the changed files back to that commit's content in
this worktree, not quoted second-hand): PYTHONSAFEPATH=1 python -m pytest tests -q -> 960 passed, 17 skipped, 1264 subtests passed (617s). After
this PR's original commit, same command: 964 passed, 17 skipped, 1264
subtests passed, 0 failed (605s).

main then moved four commits (#449, #450, #451, #453) while this PR was in
review, so it was merged (git merge --no-ff origin/main, one clean
auto-merge in the EXPECTED_FAILURES table -- see the heads-up above) and
re-baselined the same way, at the new merge-base da2422728: 977 passed,
17 skipped, 1267 subtests passed (609s). After this PR's changes on top
(including the NestedPacketContext fix described above): 981 passed, 17
skipped, 1267 subtests passed, 0 failed (638s) -- the same 4 new tests
join the passing count; the subtest total is unchanged from the new
baseline (main's own commits added tests of their own, which is why 1267
differs from the original 1264).

Also verified on a local Python 3.10 venv (this repo's CI runs 3.10-3.15;
only 3.10 carries #439's risk): 943 passed, 55 skipped, 1237 subtests
passed, 0 failed -- the higher skip count is only this venv missing some
optional runtime deps (dpkt/pyshark extras), not a difference in outcome.

Test plan

• New test fails with the exact recorded error before the fix, passes after (verified both directions for all 4 new tests)
• Both existing pcapng.py __packet__ consumers verified unaffected
• CGA Parameters reproduction confirmed to reach FieldValueError (ForwardMatchField's non-consuming bytes count toward Schema.__len__, so correct input fails a declared-length check #446), not the KeyError
• #437's three Exp_FFF* failures confirmed to be this same defect
• Round-trip harness (tests/protocols/test_option_roundtrip_unit.py) green, EXPECTED_FAILURES updated for all 7 cases whose status changed
• Full suite green before and after at the stated baseline sha
• Python 3.10 (isolated test, full test_pcapng_unit.py, and a local full-suite run) confirmed clean after moving off ChainMap
• mypy pcapkit confirmed at 123 errors/40 files on this PR's head, against 124 on main -- no new error, one pre-existing one incidentally cleaned up

---

Revision at 82dbf9416 — the dedicated class is gone

Appended rather than edited in, because the review comments above quote the original text.

At the repo owner's request, NestedPacketContext is deleted. nested_packet_context() now returns a bare collections.ChainMap({'__packet__': packet}, packet) — no dedicated class, which is where this design started before ChainMap was suspected of disturbing #439.

That suspicion was wrong, and the retired class's docstring asserted it at length, so it is worth stating plainly: the ABCMeta caches key on the exact type queried, not on ancestry. Asking about a ChainMap instance caches ChainMap, not dict. The actual poisoner was ordinary code asking isinstance about a plain dict — pcapkit/corekit/infoclass.py:270 does isinstance(dict_, (dict, collections.abc.Mapping)) — which a ChainMap-based context never did either way. #471 has since fixed #439 directly by giving every Schema subclass its own _abc_impl, removing the mechanism regardless.

What dropping the class buys, with no code to maintain — each measured on a fresh context, since these mutate and a reused one contaminates the next reading:

operation	retired dict subclass	ChainMap
setdefault('length', 999), parent has 3	999 — bypassed __missing__, inserted locally	3
ctx == dict(ctx)	False — dict.__eq__ saw only __packet__	True
pop('length')	bare KeyError	KeyError "Key not found in the first mapping: 'length'" — an explicit refusal
copy()	needed an override or silently dropped the fallback	native; keeps the parent, writes stay local

The first three rows are the substance of #474, which this closes out.

Write isolation is unchanged: ctx['length'] = 99 leaves the enclosing dict at 3, ctx['newname'] = 5 never appears in it, and del ctx['length'] falls back to 3 rather than deleting the parent's.

One thing needed a local fix rather than a class. A ChainMap is not nominally a dict, so it does not satisfy Schema.pack's dict[str, Any] annotation. Widening that annotation cascades into every other field class's pack/unpack, which forward the same argument onward with their own dict[str, Any] annotations (measured: +7 new mypy errors). There is a single explicit cast('dict[str, Any]', …) at the one call site instead, asserting that ChainMap supports every operation the annotation promises — subscript, in, .get(), iteration.

Still required alongside #471, which is the obvious question and now measurable rather than arguable, since main contains #471 as 5182ad0ce: #471's merge touches no line of pcapkit/corekit/fields/misc.py, and reverting only that file to origin/main's version — leaving all of #471 in place — fails 4 tests, including CGAParametersRegressionTests::test_cga_parameters_option_now_parses_end_to_end, the #445 symptom itself. Restore it and the same selection is 101 passed, 416 subtests, 0 failed.

The ten EXPECTED_FAILURES deletions were re-verified individually, not inferred from a green suite — "suite green" and "case covered" are different claims. cases() from examples/generators/options.py enumerates 322 cases; all ten labels are present in that enumeration (six httpv2-frame: CONTINUATION, DATA, HEADERS, PING, PUSH_PROMISE, SETTINGS; four mh-extension: Exp_FFFD, Exp_FFFE, Exp_FFFF, Multi_Prefix), and roundtrip() returns OK for each. 87 entries on main, 77 here.

Two corrections at b647e65e0

The revert-proof figure above is wrong: it fails 5 tests, not 4. My original run reverted misc.py but then executed only tests/corekit/test_fields_misc_packet_context.py, so it never reached the fifth — tests/protocols/internet/test_mh_unit.py::MHUnitTests::test_mh_cga_parameters_option_now_parses, a second and independent witness to the #445 symptom in a different file. Across the full targeted selection it is 5 failed, 96 passed, 416 subtests; restoring the file gives 101 passed, 416 subtests, 0 failed. The error understated the evidence that this fix is load-bearing.

The nested_packet_context docstring overclaimed the #439 history, and no longer does. It stated the "ChainMap never poisoned anything" reversal as measured fact, which does not reconcile with commit 86370d7d5's own directly-measured result — on a real CPython 3.10 venv at the time, toggling only the ChainMap call moved test_pcapng_remaining_constructor_branches_and_custom_dispatch between passing and failing, both ways. What is genuinely measured is narrower: the ABCMeta cache keys on the exact type queried, and #439's poisoning came from ordinary code asking isinstance about a plain dict (Info.__update__). The likeliest reconciliation — the ChainMap changed which concrete types flowed through unrelated isinstance calls and so changed when the pre-existing corruption fired — is plausible rather than demonstrated, and is now untestable, since #439's direct fix removed the mechanism. The docstring records that as two measurements that do not fully reconcile, and says it does not affect correctness either way.

Also NestedPacketContextTests → NestedPacketContextSemanticsTests: the old name referred to a class deleted earlier in this same branch.

Both changes are prose plus one class name. Verified mechanically rather than by eye — parsing misc.py before and after and comparing ASTs with docstrings stripped gives an exact match, so no executable line moved, and the GOOD TO MERGE at 82dbf9416 stands by content.

[JarryShaw]

Reviewing on behalf of Copilot (out of tokens). Head 8a7b4b906, branch fix-445-nested-packet-context. Fetched refs/pull/457/head, confirmed it equals 8a7b4b906. All code read via git show <ref>:<path>, never the ambient working tree; tests run against a real checkout of that exact sha with pcapkit.__file__ printed/asserted before trusting output.

CI

Settled at 19 SUCCESS / 2 FAILURE / 2 SKIPPED:

• FAILURE: Python 3.10, Integration Python 3.10 — both on the same root cause, detailed below.
• SUCCESS: Analyze, Compat Python 3.10-3.15 (all 6), Python 3.11-3.15 (all 5), Integration Python 3.11-3.15 (all 5), deploy-pages, CodeQL.
• SKIPPED: Docs test gate, Gate (full suite, Python 3.14).

git rev-list --count pr457..origin/main = 1 as of this review (origin/main has advanced by one commit, 94a93e721, since the da2422728 snapshot the task was dispatched against — that commit is docs-only, "record the delivery sequence", and doesn't touch pcapkit/, so it doesn't affect this review). At da2422728/dispatch time the PR was 0 behind.

The design: chained lookup via collections.ChainMap

Confirmed by diff (pcapkit/corekit/fields/misc.py): nested_packet_context() returns exactly collections.ChainMap({'__packet__': packet}, packet), used by both SchemaField.pack and SchemaField.unpack in place of the old {'__packet__': packet} literal. mh.py is untouched — confirmed absent from the diff stat (git diff origin/main..pr457 --stat touches only pcapkit/corekit/fields/misc.py, pcapkit/protocols/schema/schema.py, and two test files) — so CGAParameter.extensions's existing pkt['length'] resolves purely through the new fallback, matching the "zero changes to mh.py" claim.

Write path, checked directly rather than taken on faith: ran the new tests/corekit/test_fields_misc_packet_context.py (4 tests) against the PR head — all pass. test_nested_schema_reads_enclosing_field_by_name_and_does_not_leak_writes is the load-bearing one: it exercises __getitem__ fallthrough, the shadowing case (Inner.tag vs Outer.tag staying distinct), in, .get(), iteration (union of both levels), and confirms nothing Inner sets lands in Outer's own packet dict after the fact. This matches ChainMap.__setitem__/__delitem__ semantics (always act on maps[0]) exactly as claimed.

The two pcapng.py __packet__ consumers (packet_byteorder, BlockType.post_process) are confirmed untouched, and the reasoning holds: pcapkit/foundation/engines/pcapng.py:192 really does call with a hand-built plain {'snaplen': ...} dict via the __packet__= protocol-level kwarg (a different mechanism entirely from SchemaField's packet-context dict — that layer is untouched by this PR and out of scope). The new test file's test_pcapng_byteorder_consumer_still_works_with_both_shapes and test_pcapng_block_type_mismatch_consumer_still_works_with_both_shapes both pass, confirming compatibility with what SchemaField now actually builds.

Documentation: Schema.unpack's docstring now names __packet__ alongside __length__/__option_padding__, and nested_packet_context's own docstring documents the contract. Confirmed by diff.

CGA Parameters reaches the #446 boundary, not the old KeyError

Ran test_cga_parameters_option_reaches_the_446_boundary_not_a_keyerror directly against the PR head: passes, and the issue's exact 40-octet reproduction now raises FieldValueError: Field parameters has invalid length (the ForwardMatchField/Schema.__len__ defect, #446, addressed separately in open PR #456) instead of KeyError: 'length'. Confirmed Schema.__len__ and ForwardMatchField are both untouched by this PR (no hits in the diff for either).

HTTP/2 and Multi_Prefix re-attribution

Checked each cited defect against the actual code at 8a7b4b906:

• pcapkit/protocols/internet/mh.py:3864 really does write length=1 + len(prefixes) * 16, and open PR protocols: complete the Mobility Header registry -- all 24 message types, 70 of 71 options, all 4 CGA extensions #437 really does change that exact line to length=4 + len(prefixes) * 8 (confirmed via gh pr diff 437) — so citing it as "fixed on open PR protocols: complete the Mobility Header registry -- all 24 message types, 70 of 71 options, all 4 CGA extensions #437, not yet merged" rather than a new untracked defect is accurate.
• pcapkit/protocols/schema/application/httpv2.py:283-285: SettingsFrame.settings really does declare item_type=SettingPair (the class, not SchemaField(schema=SettingPair)), with the # type: ignore[arg-type] the PR mentions. Filed as SettingsFrame.settings passes a Schema class to ListField's item_type instead of a SchemaField #459 per the PR description.
• pcapkit/utilities/decorators.py: prepare's inner unpack closure really does raise EOFError unconditionally on length == 0 — confirmed, though the exact line is 228, not the cited 222 (inline comment posted; six lines off, same function, doesn't misdirect).
• Ran the full tests/protocols/test_option_roundtrip_unit.py: 7 passed, 299 subtests passed, at 8a7b4b906. This suite asserts each EXPECTED_FAILURES entry both fails and fails in the recorded way, and asserts every case absent from the table comes back 'OK' — so PUSH_PROMISE/PING coming back clean (entries deleted) and DATA/HEADERS/CONTINUATION/SETTINGS/Multi_Prefix failing exactly as newly recorded are both mechanically re-verified by the suite itself, not just asserted in prose.

The one real problem: this PR flips a latent #439 case on Python 3.10

Full detail and reproduction is in the inline comment on pcapkit/corekit/fields/misc.py:536. Summary: tests/protocols/misc/test_pcapng_unit.py::PCAPNGUnitTests::test_pcapng_remaining_constructor_branches_and_custom_dispatch fails deterministically on 8a7b4b906 under Python 3.10.20 (isinstance(value, Schema) wrongly returns True for a plain dict, so to_dict() at schema.py:520 calls .to_dict() on it and raises AttributeError), and passes on origin/main under the identical interpreter and test invocation. Reproduced 3/3 times on the PR head, 1/1 on main, both via a driver script that inserts the target tree at sys.path[0] and asserts pcapkit.__file__ before running (the 3.10 venv's own site-packages have no pcapkit installed at all, but the bash tool's cwd was still shadowing plain PYTHONPATH since PYTHONSAFEPATH is a 3.11+-only flag and is silently a no-op on this 3.10.20 interpreter — worth knowing for anyone else testing this repo under 3.10). Also ran the entire Python 3.10 CI job's own command (pytest -q --ignore=tests/integration --ignore-glob='*_runtime.py' --ignore-glob='*_regression.py') locally against 8a7b4b906: 1 failed, 773 passed, 71 skipped, ... 1043 subtests passed — the one failure is this same test, matching CI exactly. The equivalent run under Python 3.14 on this same tree: 840 passed, 5 skipped (0 failures), matching CI's green Python 3.14.

This matches the task's second hypothesis, not the first: INTERPRETER_GAPS is unrelated here (that table only guards test_option_roundtrip_unit.py, and is untouched by this PR's diff), but the mechanism is the same one #439 names — SchemaMeta's shared, order-sensitive ABCMeta cache on CPython <=3.10 — and nested_packet_context introducing collections.ChainMap for the first time against Schema packet dicts is exactly the kind of change that reorders which schema class gets touched first. Since main doesn't have this failure and this PR's head does, it is a real regression this PR introduces, not one it merely inherits. Per the guidance for this situation, it is not this PR's job to fix #439 itself — the right fix is an INTERPRETER_GAPS-equivalent guard on this specific test, citing #439, mirroring the mechanism test_option_roundtrip_unit.py already has. As submitted, Python 3.10 and Integration Python 3.10 are red with no acknowledgement of why.

mypy

Ran mypy against both trees directly: 124 errors in 40 files on origin/main (matches the calibration baseline exactly) vs 125 errors in 40 files on 8a7b4b906. Diffed the two full error lists (normalizing path prefixes) and confirmed the only substantive difference is one new error at pcapkit/corekit/fields/misc.py:640:27 (ChainMap[str, Any] passed where dict[str, Any] | None is expected) — inline comment posted. Every other line-number difference between the two runs is the same pre-existing error shifted by the new function's line count, not a new error.

Verdict

Everything the PR claims about the mechanism, the write path, the pcapng.py scoping decision, the #446 boundary, and the HTTP/2 and Multi_Prefix re-attributions checks out against the code and against test runs I executed myself at 8a7b4b906. The one thing not addressed is real: this PR turns Python 3.10 and Integration Python 3.10 red by flipping a latent #439 case, and ships with no INTERPRETER_GAPS-style acknowledgement of it, plus one small new mypy error and one citation off by six lines.

REQUEST CHANGES — not because the core fix is wrong (it isn't), but because it currently regresses two CI jobs from green to red with nothing in the PR acknowledging why. Add an interpreter-gap-style guard citing #439 for test_pcapng_remaining_constructor_branches_and_custom_dispatch (or otherwise make Python 3.10 green for a stated reason), and optionally clean up the new mypy error, before this merges.

[JarryShaw]

Closing and reopening to re-trigger CI -- the last three pushes to this branch (86370d7, 6ac029a) have not produced any check-runs at all, which looks like a missed webhook delivery rather than anything about the commits themselves.

[JarryShaw]

what about use an Info subclass? and im not sure why must we use a dedicated class for this.

[JarryShaw]

Measured rather than argued from the abc angle (that angle turned out to be a
dead end -- see below), by actually building an Info-based nested context
and running it: it does not break Python 3.10. Swapped NestedPacketContext
for an Info subclass on disk, ran it on a local Python 3.10.21 venv (matching
CI's exact patch), and tests/protocols/misc/test_pcapng_unit.py -- the module
that broke under the ChainMap version -- came back clean: 37 passed, 139
subtests passed, 0 failed, both for the one previously-failing test alone and
for the whole module.

So the earlier "avoids a Mapping tie" reasoning I gave for the dict choice
doesn't hold up, and I'm not using it any more: Info is itself Mapping-based
and gets its own _abc_impl regardless (its metaclass has no CPython-version
bypass), so a Mapping tie alone was never the risk. I'm not citing a
mechanism I haven't personally re-verified, so I'll leave the actual cache
mechanics to whoever measured that -- what I can say directly is that Info
passes the test that mattered.

That still leaves the real question: not "is it safe" but "does it fit". I
built the Info version to answer this honestly rather than guess, and here
is what it would and wouldn't give for free:

• __missing__ fallback: no saving. Info.__getitem__ is
  self.__dict__[self.__map__.get(name, name)] -- a from-scratch
  implementation with no __missing__-style hook (that's a dict C-level
  feature, not a general Mapping one). Whether I subclass dict or Info,
  I have to write the fallback lookup myself; there's no version where Info
  saves me this code.
• __contains__/get: a genuine point in Info's favour. Mapping
  supplies mixin implementations of both that delegate to __getitem__, so
  once __getitem__ has the fallback, in and .get() inherit it for free.
  dict's own __contains__/get are C-level and bypass __missing__
  entirely, so my NestedPacketContext(dict) has to override both by hand.
  Info would save that code.
• Writes must land on the nested instance only, never on the enclosing
  schema -- this is where it breaks down. Info is deliberately immutable:
  __setattr__ raises UnsupportedCall, and it has no __setitem__ at all
  (it inherits read-only Mapping, not MutableMapping). Field callbacks
  write into the packet dict constantly during parsing --
  Schema.unpack's own per-field loop does packet[field.name] = value once
  per field, for every field of every nested schema. My Info scratch class
  only supports this by writing into self.__dict__ directly from a custom
  __setitem__, bypassing the immutability Info's own docstring promises
  ("Info objects are immutable, thus cannot set or delete attributes after
  initialisation"). It works, but it's a subclass that quietly defeats the
  guarantee the class exists to provide -- not an extension of Info's
  contract, a contradiction of it.
• Extra bookkeeping to filter out: Info instances carry __map__ and
  __map_reverse__ in self.__dict__ for its builtin-name-collision
  handling, which then show up in naive iteration alongside the real packet
  keys (I hit this directly -- set(packet.keys()) came back with _parent,
  __map__ and __map_reverse__ mixed in with the actual field names, and
  I'd have needed to filter them the way Info.__iter__ filters
  self.__excluded__). A plain dict subclass has no such baggage: its own
  storage holds only what's explicitly put there.
• Purpose mismatch, not just mechanics: every real Info subclass in this
  codebase declares a fixed, type-annotated field set and is built once as a
  stable snapshot (that's what info_final/__new__ are for). A nested
  packet context is the opposite: one shared, generic type instantiated fresh
  per parse, holding whatever field names that schema happens to declare,
  mutated field-by-field as parsing proceeds. Reusing the bare Info class
  works because it happens to accept arbitrary kwargs, not because that's
  what it's for.

So: two of the four things this needs (__contains__/get) Info gives for
free; one (__missing__) is a wash; and the write-isolation requirement is a
real contract conflict, not just extra code, because Info advertises
immutability as a feature and a nested context needs exactly the opposite.
That's why I kept the small dedicated class rather than switching -- "least
code that gets the contract right without fighting another class's
guarantees," not "because that's how I already did it." If the immutability
conflict is judged acceptable to paper over (my scratch class shows it's
mechanically possible), I'll switch; I don't think it should be, given the
class's own docstring says the opposite of what the subclass would then do.

[JarryShaw]

Is this still required with #471?

[JarryShaw]

Yes, still required — and that is now measurable directly rather than by argument, because main already contains #471 (merged as 5182ad0ce).

Two separate facts:

#471 does not touch this file at all. git diff origin/main~1..origin/main --stat for #471's merge lists no change to pcapkit/corekit/fields/misc.py. It fixed #439 by giving every Schema subclass its own _abc_impl; it never addressed a nested schema's inability to reach the enclosing packet's fields by name.

#445's own symptom persists on a tree that has #471. Reverting only pcapkit/corekit/fields/misc.py to origin/main's version — leaving everything else on this branch, including all of #471 — fails 4 tests:

FAILED tests/corekit/test_fields_misc_packet_context.py::CGAParametersRegressionTests::test_cga_parameters_option_now_parses_end_to_end
FAILED tests/corekit/test_fields_misc_packet_context.py::NestedPacketContextTests::test_pcapng_block_type_mismatch_consumer_still_works_with_both_shapes
FAILED tests/corekit/test_fields_misc_packet_context.py::NestedPacketContextTests::test_pcapng_byteorder_consumer_still_works_with_both_shapes
4 failed, 5 warnings in 2.99s

Restore the file and the same selection is 101 passed, 416 subtests, 0 failed. Different defects.

---

Separately: the dict subclass is gone, as of 82dbf9416. nested_packet_context() now returns a bare collections.ChainMap({'__packet__': packet}, packet) — no dedicated class at all, which is where this started before ChainMap was (wrongly) suspected of disturbing #439.

That suspicion was wrong, and it is worth stating plainly since it is what drove the detour: the ABCMeta caches key on the exact type queried, not on ancestry. Asking about a ChainMap instance caches ChainMap, not dict. The actual poisoner was ordinary code asking isinstance about a plain dict — pcapkit/corekit/infoclass.py:270 does isinstance(dict_, (dict, collections.abc.Mapping)) — which a ChainMap-based context never did either way. And #471 has since removed the mechanism regardless.

What dropping the class buys, with no code to maintain:

operation	retired dict subclass	ChainMap
setdefault('length', 999) where the parent has 3	999 — bypassed __missing__ and inserted locally	3
ctx == dict(ctx)	False — dict.__eq__ saw only __packet__	True
pop('length')	bare KeyError	KeyError "Key not found in the first mapping: 'length'" — an explicit refusal
copy()	needed an override or it silently dropped the fallback	native, keeps the parent, writes stay local

Each measured on a fresh context, because these mutate and a reused one contaminates the next reading. The first three rows are the substance of #474, which this therefore closes out.

Write isolation is unchanged and still holds: ctx['length'] = 99 leaves the enclosing dict at 3, ctx['newname'] = 5 never appears in it, and del ctx['length'] falls back to 3 rather than deleting the parent's.

One thing needed a local fix rather than a class: a ChainMap is not nominally a dict, so it does not satisfy Schema.pack's dict[str, Any] annotation. Widening that annotation cascades into every other field class's pack/unpack (measured: +7 new mypy errors), so there is a single explicit cast('dict[str, Any]', ...) at the one call site instead, asserting that ChainMap supports every operation the annotation promises.

The ten EXPECTED_FAILURES entries this branch deletes were re-verified individually rather than inferred from a green suite — cases() enumerates 322 cases, all ten labels are present in it (six httpv2-frame, four mh-extension), and each returns OK: 87 entries on main down to 77 here.

[JarryShaw]

A correction to my own earlier reply in this thread, since the review measured it more carefully than I did.

I wrote that reverting only pcapkit/corekit/fields/misc.py to origin/main's version fails 4 tests. It fails 5. My run had reverted the file but then executed only tests/corekit/test_fields_misc_packet_context.py, so it never reached the fifth. Re-run across the whole targeted selection:

FAILED tests/corekit/test_fields_misc_packet_context.py::NestedPacketContextSemanticsTests::test_nested_schema_reads_enclosing_field_by_name_and_does_not_leak_writes
FAILED tests/corekit/test_fields_misc_packet_context.py::NestedPacketContextSemanticsTests::test_pcapng_block_type_mismatch_consumer_still_works_with_both_shapes
FAILED tests/corekit/test_fields_misc_packet_context.py::NestedPacketContextSemanticsTests::test_pcapng_byteorder_consumer_still_works_with_both_shapes
FAILED tests/corekit/test_fields_misc_packet_context.py::CGAParametersRegressionTests::test_cga_parameters_option_now_parses_end_to_end
FAILED tests/protocols/internet/test_mh_unit.py::MHUnitTests::test_mh_cga_parameters_option_now_parses
5 failed, 96 passed, 23 warnings, 416 subtests passed

The fifth is test_mh_unit.py::MHUnitTests::test_mh_cga_parameters_option_now_parses — a second, independent witness to the #445 symptom, in a different file from the rest. The error was in my favour, which is the more insidious direction: it understated the evidence that this fix is load-bearing.

Restoring the file gives 101 passed, 416 subtests, 0 failed on the same selection. The corrected figure is in the PR body as well.

[JarryShaw]

Reviewing on behalf of Copilot (out of tokens). This supersedes the stale REQUEST CHANGES left at 8a7b4b906 — that review's one real objection (this PR flipped a latent #439 case red on Python 3.10) is fixed on the current head, confirmed below. Head aab958b1d, branch fix-445-nested-packet-context, base main at fa128959e (git rev-list --count pr457..origin/main = 0, still 0 behind). All code read via git show <ref>:<path> or a checkout of that exact sha in an isolated worktree, never a stale working tree; pcapkit.__file__ asserted to start with the worktree root before trusting any test/mypy output.

CI

gh api repos/JarryShaw/PyPCAPKit/commits/aab958b1dd743259e71003a056455507d338c848/status reports state: success. gh pr checks 457: 21 pass, 2 skipping (Docs test gate, Gate (full suite, Python 3.14) — both intentionally gated, not failures), pyup.io/safety-ci pass (this is the StatusContext with .state, not a GitHub Actions run — not evidence of a pending Actions check). Python 3.10 and Integration Python 3.10 — the two jobs the stale review flagged red — both pass (9m1s, 11m19s). That confirms the fix described in the PR body: moving NestedPacketContext off collections.ChainMap and onto a plain dict subclass (commits 86370d7d5, 6ac029a36) stopped disturbing the shared _abc_impl cache from #439.

Full suite and mypy, run myself, not taken from the PR body

Generated fixtures with PYTHONPATH=<worktree> python examples/generators/make_samples.py (required in a fresh worktree; without it ~130 tests raise spurious FileNotFoundError). Then, with the shared repo venv (Python 3.14.7) and pcapkit.__file__ asserted against the worktree root:

• PYTHONSAFEPATH=1 python -m pytest tests -q → 1010 passed, 17 skipped, 1553 subtests passed, 0 failed in 840.25s. Exact match to the number claimed for this head. All 4 RUNTIME_DEPS (tbtrim, aenum, chardet, dictdumper) are installed in this venv, which is consistent with the skip count landing at the low end (17) of the stated 17–20 range — noting the correlation, not asserting it as the mechanism.
• mypy pcapkit on the PR head: 124 errors in 40 files. Swapped only the two touched source files (pcapkit/corekit/fields/misc.py, pcapkit/protocols/schema/schema.py) back to origin/main's content via a /tmp copy (never touched the shared stash) and reran: 125 errors in 40 files. Diffed both full error lists: the only difference is pcapkit/corekit/fields/misc.py:510: error: Unused "type: ignore" comment [unused-ignore], present on main, absent on the PR head. Traced it in the diff: SchemaField.length's getter went from return self._length # type: ignore[has-type] to return self._length — a pre-existing stale ignore comment the PR incidentally dropped while editing that class, unrelated to NestedPacketContext itself. Zero new mypy errors.

The six httpv2-frame and four mh-extension deletions, verified individually, not by trusting the aggregate

Loaded tests/protocols/test_option_roundtrip_unit.py's own harness (OptionRoundTripTests) directly and called self._run(case) on each of the ten cases by name, at the PR head:

httpv2-frame/DATA              status='OK'
httpv2-frame/HEADERS           status='OK'
httpv2-frame/CONTINUATION      status='OK'
httpv2-frame/PUSH_PROMISE      status='OK'
httpv2-frame/PING              status='OK'
httpv2-frame/SETTINGS          status='OK'
mh-extension/Multi_Prefix      status='OK'
mh-extension/Exp_FFFD          status='OK'
mh-extension/Exp_FFFE          status='OK'
mh-extension/Exp_FFFF          status='OK'

Then, to isolate what #445 alone contributes (main has since merged #437/#446/#456/#461/#462, any of which could independently explain a case flipping to 'OK'), I swapped just the two touched files back to origin/main's content and reran the same ten cases against the same harness:

httpv2-frame/DATA              status='CONSTRUCT' detail="KeyError: 'flags'"
httpv2-frame/HEADERS           status='CONSTRUCT' detail="KeyError: 'flags'"
httpv2-frame/CONTINUATION      status='CONSTRUCT' detail="KeyError: 'flags'"
httpv2-frame/PUSH_PROMISE      status='CONSTRUCT' detail="KeyError: 'flags'"
httpv2-frame/PING              status='CONSTRUCT' detail="KeyError: 'flags'"
httpv2-frame/SETTINGS          status='CONSTRUCT' detail="KeyError: 'flags'"
mh-extension/Multi_Prefix      status='PARSE' detail="KeyError: 'length'"
mh-extension/Exp_FFFD          status='PARSE' detail="KeyError: 'length'"
mh-extension/Exp_FFFE          status='PARSE' detail="KeyError: 'length'"
mh-extension/Exp_FFFF          status='PARSE' detail="KeyError: 'length'"

All ten fail, exactly as the deleted EXPECTED_FAILURES entries recorded, with every other merged fix still in the tree. Flipping only misc.py/schema.py is what flips all ten. This holds up: the accounting in the PR body (PUSH_PROMISE/PING clean as soon as #445 landed; DATA/HEADERS/CONTINUATION needing #458→#461; SETTINGS needing #459→#462; the four mh-extension cases needing #437 and #446/#456 alongside #445) is correct, not just plausible.

Independently re-verified the rest of the accounting too: t.options.cases() returns 322 cases; EXPECTED_FAILURES has 77 entries with zero mh-extension/* and exactly one httpv2-frame/* (PRIORITY, a separate, still-open, unrelated defect — the length = payload + 9 arithmetic — correctly left in place); INTERPRETER_GAPS is untouched at 7 entries. pytest tests/protocols/test_option_roundtrip_unit.py -q → 7 passed, 363 subtests passed.

NestedPacketContext (pcapkit/corekit/fields/misc.py:493), probed directly against the live class

__missing__, __contains__, get, __iter__, __len__, keys/values/items, and copy are all correctly overridden, and I confirmed why copy in particular is load-bearing rather than incidental: Schema.unpack (pcapkit/protocols/schema/schema.py:751, value = field.unpack(byte, packet.copy())) calls .copy() on whatever packet currently is — and for a doubly-nested schema, packet is itself a NestedPacketContext. An unoverridden dict.copy() would silently hand back a plain dict, dropping the fallback for anything nested one level further. The override (misc.py:601-609) avoids that by rebuilding a NestedPacketContext sharing _parent — checked and correct.

Two real gaps in the override set, both reproduced directly against pcapkit.corekit.fields.misc.nested_packet_context:

• setdefault doesn't consult the fallback. dict.setdefault is C-level and isn't overridden, so it only checks local storage. Given parent = {'length': 40}; ctx = nested_packet_context(parent): 'length' in ctx is True and ctx['length'] is 40, yet ctx.setdefault('length', 999) returns 999 and leaves ctx['length'] == 999 afterward (parent['length'] stays 40, untouched) — it silently shadows a name the mapping itself just reported as present, rather than returning the existing (fallthrough) value the way dict.setdefault is documented to behave for any key already visible on the mapping.
• pop raises where __getitem__/in/get all succeed. Same setup, fresh context: ctx['length'] → 40, 'length' in ctx → True, but ctx.pop('length') raises KeyError: 'length'. dict.pop is likewise C-level and unoverridden.
• Related but lower severity: __eq__ isn't overridden either, so it's dict's own C-level equality — which compares only local storage, not the union __len__/__iter__ present. nested_packet_context({'length': 40, 'other': 'x'}) with ctx['tag']='y' set: len(ctx) == 4 and sorted(ctx) == ['__packet__', 'length', 'other', 'tag'], but ctx == dict(dict.items(ctx)) (i.e., {'__packet__': {...}, 'tag': 'y'}, local storage only) is True. A mapping that presents a 4-key union everywhere else agrees with a 2-key dict under ==.
• I checked ** unpacking too, expecting the same class of gap (CPython's dict-merge fast path can bypass keys()), but it is actually fine: {**ctx} produces the full 4-key union. NestedPacketContext overriding __iter__ changes its tp_iter slot away from dict's own, which is precisely the condition that makes CPython's dict_merge take the generic keys()/__getitem__ path instead of the direct-hash-table fast path — confirmed by reading the result, not assumed.

I searched the current tree (grep -rn across pcapkit/) for .setdefault(, .pop(, **pkt/**packet, and ==/pkt == on a packet dict, in any field callback, post_process, or condition — there are none. So both setdefault and pop gaps, and the __eq__ inconsistency, are real but currently latent: nothing in this codebase calls them on a packet context today, and none of the shipped tests exercise them. One more, cosmetic: keys() returns a one-shot generator rather than a dict_keys-style view (len(ctx.keys()) raises TypeError, and a second list(...) over the same returned object comes back empty) — acknowledged by the # type: ignore[override] comments already on those methods, and nothing calls len() on it today either.

Write isolation (item 2) and the __packet__ escape hatch (item 3)

Ran the shipped test_nested_schema_reads_enclosing_field_by_name_and_does_not_leak_writes and the two pcapng.py-consumer tests directly (pytest tests/corekit/test_fields_misc_packet_context.py -v): 4/4 pass. Beyond that test, I set a shadowing key directly (ctx2 = nested_packet_context({'length': 40}); ctx2.update({'length': 55}); ctx2['length'] == 55 while parent2['length'] stays 40) — plain dict assignment/update never touches _parent, confirmed, and nothing is silently dropped either.

pcapkit/protocols/schema/misc/pcapng.py's two consumers are untouched by this PR (absent from the diff stat) and stay coherent with the new class: packet_byteorder (:168-169) checks 'byteorder' not in packet and '__packet__' in packet and BlockType.post_process (:354) does packet.get('__packet__', {}) — both only ever reach for __packet__ explicitly, which NestedPacketContext.__init__ always seeds into local storage (super().__init__({'__packet__': packet}), misc.py:558), so both the hand-built {'__packet__': ...} shape and the new class satisfy them identically. I traced the reverse concern too: SchemaField.unpack's nested_packet_context(packet) wraps whatever packet currently is, so for a doubly-nested schema packet['__packet__'] names the immediate enclosing schema, and that schema's own NestedPacketContext (if it is itself nested) carries the chain one level further via its own __missing__ — so a name absent from every intervening level still resolves all the way up, and __packet__ at each level names the schema one level up rather than the topmost one, matching what the docstring promises ("the enclosing schema is also reachable unconditionally").

Also confirmed the two mechanisms sharing the __packet__ name are genuinely distinct and this PR only touches one of them: pcapkit/protocols/protocol.py, ipv4.py, ipv6.py, misc/pcap/frame.py, misc/pcapng.py (protocol-level, not schema-level) and foundation/engines/pcapng.py:192 all use __packet__ as a protocol/Data_*-construction keyword, unrelated to SchemaField's packet-context dict — none of those sites appear in this PR's diff.

The Info-subclass question (comment 4043754294/4043784827) — every checkable claim verified against source

Read pcapkit/corekit/infoclass.py directly rather than taking the reply's characterization on faith:

• No __missing__ analog: Info.__getitem__ (:332-334) is key = self.__map__.get(name, name); return self.__dict__[key] — a flat lookup with no hook comparable to dict.__missing__. Confirmed as claimed.
• __contains__/get free via Mapping: Info (:196, class Info(Mapping[str, VT], ...)) defines neither; both come from the Mapping ABC's mixins, which delegate to __getitem__. Confirmed.
• Immutability is real and structural, not incidental: __setattr__ (:336-337) unconditionally raises UnsupportedCall, there is no __setitem__/__delitem__ anywhere in the file, and the base class list is Mapping, not MutableMapping. Confirmed exactly as described.
• Schema.unpack's write loop: packet[field.name] = value appears at schema.py:767 (the common case) and the equivalent packet[field.name] = ... appears three more times for PayloadField (:734), PaddingField (:745), and the ConditionalField-false branch (:755). "Once per field" is accurate.
• __map__/__map_reverse__ leaking into naive iteration: confirmed this is real for exactly the shape the reply describes. The exclusion bookkeeping (cls.__excluded__.extend(cls.__builtin__), adding '__map__', '__map_reverse__', '__builtin__', '__finalised__' to __excluded__) happens only inside the @info_final decorator (infoclass.py:71-77), not automatically for every Info subclass. A bare class Scratch(Info): ... used without that decorator — which is what a generic, instantiated-fresh-per-parse nested context would have to be — has no such filtering, so __map__/__map_reverse__ would show up in set(packet.keys()) exactly as reported. And @info_final itself doesn't fit here anyway: it's built to generate a fixed __init__ from a stable, type-annotated field set (infoclass.py:78+), which is the "purpose mismatch" half of the reply's argument, not just the mechanical half.

Every specific, checkable claim in that reply holds up. The reasoning is sound.

schema.py's docstring addition (+10 lines inside Schema.unpack)

Matches the implementation precisely: "a name this schema does not itself declare falls through to the enclosing schema" (the __missing__ behavior), "the enclosing schema is also reachable unconditionally under a __packet__ key" (seeded unconditionally in __init__) — both accurate, and it correctly scopes itself to when "this schema is nested — unpacked through a SchemaField rather than directly."

One thing this PR's own diff got wrong: a stale ChainMap reference

tests/corekit/test_fields_misc_packet_context.py:24's class docstring still reads: "nested_packet_context replaces that literal with a two-level collections.ChainMap, so a name absent locally falls through to the enclosing schema." That's the design as of 46fe59ad4, before the Python 3.10 regression forced the pivot to a dict subclass in 86370d7d5/6ac029a36. pcapkit/corekit/fields/misc.py's own docstring on NestedPacketContext was correctly updated to explain why ChainMap was rejected (misc.py:538-544), but this test file's docstring, describing the current mechanism, was not updated to match and is simply incorrect about what the code now does. grep -rn "ChainMap" pcapkit/ tests/ shows this is the only stale reference; the two in misc.py are legitimately part of the rejected-design explanation. Doesn't affect what the tests assert or exercise — cosmetic, but worth fixing.

Verdict

CI is green on the actual head (aab958b1d), including the two jobs the superseded review was blocking on. The local full suite (1010 passed, 17 skipped, 1553 subtests, 0 failed) and mypy (124 vs 125, zero new errors, one incidental pre-existing cleanup) both match the PR's own claims exactly, reproduced independently rather than taken on faith. All six httpv2-frame and all four mh-extension EXPECTED_FAILURES deletions are individually verified against the harness's own cases, in both directions — passing at this head, and failing exactly as recorded when isolated back to main's two touched files. NestedPacketContext's override set has two real, currently-latent gaps (setdefault, pop) and one related inconsistency (__eq__), none exercised anywhere in this codebase today; one stale ChainMap reference in the new test file's docstring is a documentation-only leftover. Nothing here contradicts the design, the write-isolation guarantee, the __packet__ escape hatch, or the Info-subclass reasoning given to the owner — all of which check out against the actual source.

GOOD TO MERGE at aab958b1d.

[JarryShaw]

Non-blocking observation on this paragraph (and the commit message's matching claim).

The narrow technical point -- CPython's ABCMeta cache keys on the exact type queried, so probing a ChainMap instance and probing a dict instance land in different cache slots -- checks out. I confirmed independently on this tree (Python 3.14): Schema._abc_impl is not Mapping._abc_impl (the #439 fix gives every Schema subclass its own cache), and neither issubclass(dict, Schema) nor issubclass(collections.ChainMap, Schema) is True.

But "ChainMap never poisoned anything" doesn't fully reconcile with commit 86370d7d5's own empirical result: "reverting only the ChainMap call... makes the test pass again on Python 3.10; restoring it reproduces the failure... Confirmed both in an isolated single-test run and via a local Python 3.10 venv." That was a real, directly-measured result at the time (pre-#439-fix), and nothing here explains why toggling the ChainMap call moved that specific test's outcome if constructing one were truly inert to the shared cache. A plausible reconciling story exists -- swapping ChainMap for a plain-dict-literal (or the dict subclass) changes which concrete type flows through other, unrelated isinstance calls in the same run (e.g. Info.__update__'s isinstance(dict_, (dict, Mapping))), which could shift when the pre-existing #439 corruption got triggered rather than ChainMap being causal on its own -- but that reconciliation isn't spelled out, here or in the commit message.

I can't settle this myself: this venv only has Python 3.14, and 3.14 never hit the pre-#439 code path at all (the version guard that bypassed ABCMeta.__new__ only applied below 3.11), so a clean isinstance({}, Schema) here proves nothing about the Python-3.10 claim either way -- confirming that isn't the same as confirming the historical account.

Not a blocker: #439 is now fixed at the root (independent _abc_impl per Schema subclass, already merged into this branch), so the question is moot for correctness regardless of which account is right. Flagging only because the docstring states the reversal as settled fact ("Measured after the fact") rather than as "most likely, and now unfalsifiable."

[JarryShaw]

You are right, and this is the finding I most wanted someone to make — fixed in b647e65e0.

The docstring asserted the reversal as measured fact, which it was not. What is genuinely measured is narrower than what it claimed: the ABCMeta cache keys on the exact type queried, so probing a ChainMap caches ChainMap and not dict, and #439's poisoning came from ordinary code asking isinstance about a plain dict (Info.__update__, at pcapkit/corekit/infoclass.py:270). That does not by itself explain the 3.10 observation in 86370d7d5, which was a real, directly-measured, toggled-both-ways result at the time. Your reconciliation — the ChainMap was never causal but changed which concrete types flowed through unrelated isinstance calls, and so changed when the pre-existing corruption fired — is the likeliest one, and you are right that it is plausible rather than demonstrated.

So the docstring now records it as two measurements that do not fully reconcile, says the reconciliation is plausible rather than shown, says it is no longer testable because #439's direct fix removed the mechanism, and says plainly that it does not matter for correctness either way. Your phrasing — "most likely, and now unfalsifiable" — is what it should have said in the first place.

You also correctly noted the environment limit, and I agree it is decisive: only the 3.14 venv has dependencies installed, and 3.14 never took the pre-#439 code path at all, since the version guard that bypassed ABCMeta.__new__ only applied below 3.11. A clean isinstance({}, Schema) here proves nothing about the 3.10 claim. Building a 3.10.21 venv from scratch would be the only way to settle it, and I have deliberately not done that: with the mechanism removed at the root, the answer changes no code.

This one is worth more than a docstring tidy-up, which is why it got a commit of its own rather than being folded in: repeating an unverified conclusion as settled fact is a mistake this branch has already made twice, and a docstring is exactly where it would have outlived the PR and been cited later as evidence.

One thing your finding did not reach, which I fixed alongside: tests/corekit/test_fields_misc_packet_context.py's own class docstring carried the same "(wrongly) suspected" claim, so it would have been left contradicting the module it documents. Softened to match, with a pointer to the function's fuller account. There is no wrongly left in either file.

No executable line changed — verified by filtering the diff — and tests/corekit/test_fields_misc_packet_context.py plus tests/protocols/internet/test_mh_unit.py are 40 passed, 272 subtests, 0 failed.

[JarryShaw]

You were right, and this is fixed in b647e65e0 — the docstring now records the two measurements as not fully reconciling rather than as a settled reversal.

I went and read 86370d7d5 rather than arguing from memory, and it is as strong as you say:

Measured directly: with everything else unchanged, reverting only the ChainMap call back to a plain {'__packet__': packet} literal makes that test pass again on Python 3.10; restoring it reproduces the failure. Confirmed both in an isolated single-test run and via a local Python 3.10 venv.

So the docstring was asserting a reversal of a directly-measured result on the strength of a narrower measurement that does not explain it. Your reconciliation — the ChainMap was never causal but changed which concrete types flowed through unrelated isinstance calls, and so changed when the pre-existing corruption fired — is the one I find likeliest too, and the docstring now says exactly that, labelled as plausible rather than demonstrated.

I also agree it is no longer decidable, and for the reason you gave: #439's direct fix removed the mechanism, so the original conditions are gone. 3.10 interpreters do exist on this machine (/home/jarryx/.local/share/mise/installs/python/3.10.21, which is CI's exact patch level) and a venv against one was built earlier in this programme, so it is not the interpreter that is missing — it is that answering the question now would require also reverting #471 to restore the corruptible cache, i.e. reconstructing a state the codebase has deliberately left behind. Not worth it for a question that does not affect correctness either way.

The change is prose only. Verified mechanically rather than by eye: parsing misc.py before and after and comparing the ASTs with docstrings stripped gives an exact match, so no executable line moved.

Thank you for pushing on this one. Repeating an unverified conclusion as fact is a mistake this branch has already made twice, and a docstring is precisely where it would have outlived the PR.

[JarryShaw]

Non-blocking nit. NestedPacketContextTests names the deleted NestedPacketContext class, not the nested_packet_context() function that replaced it -- a dangling reference to a class that no longer exists in pcapkit/corekit/fields/misc.py.

The class's own docstring immediately below explains the history in detail ("An intermediate version of this fix used a hand-written dict subclass..."), so this reads as a deliberate, self-documented choice rather than an oversight, and I wouldn't block on it. Still, a name like NestedPacketContextSemanticsTests would describe what the class actually tests today (the ChainMap-based fallback semantics) rather than an implementation that was tried and retired within this same PR's history. Optional.

[JarryShaw]

Taken — renamed to NestedPacketContextSemanticsTests in b647e65e0, using your suggestion verbatim.

You read it more carefully than I did. I had looked at this earlier and talked myself out of it on the grounds that the name refers to the concept rather than the deleted class, so it was harmless. That was wrong for the reason you give: the class docstring immediately below spends a paragraph on the retired dict subclass, so the name does read as pointing at that class rather than at nested_packet_context(). A name that needs the docstring below it to disambiguate is not doing its job.

grep -rn NestedPacketContextTests pcapkit tests docs returns nothing after the rename, so there was no other reference to update.

[JarryShaw]

Agreed and done in b647e65e0 — renamed to NestedPacketContextSemanticsTests, which is the name you suggested and describes what the class actually covers.

You were right that it was a dangling reference rather than merely an odd name: NestedPacketContext was deleted earlier in this same branch, so nothing in pcapkit, tests or docs defines it any more. Confirmed with a grep across all three after the rename — the only remaining occurrences of the string are inside the new class name and the prose that explains the history.

While there I also softened that class's own docstring, which carried the same "(wrongly) suspected" overclaim as the module docstring you flagged in the other thread, so the two no longer contradict each other.

[JarryShaw]

Review of PR #457 (fix-445-nested-packet-context, 82dbf9416)

CI

gh pr view 457 --json statusCheckRollup ... was rechecked repeatedly as it ran during this review. Final state: 21 SUCCESS + 2 SKIPPED CheckRuns (Docs test gate and Gate (full suite, Python 3.14) skip by design on pull_request), plus the pyup.io/safety-ci StatusContext at SUCCESS. Fully green.

1. The cast at the SchemaField.pack call site (misc.py ~line 675)

Traced every operation the codebase performs on the packet argument downstream of Schema.pack/unpack, using both static reading and running code against the actual nested_packet_context():

• schema.py:640 packet.update(self.__dict__) and misc.py:665,699 packet.update(self._packet) -- ChainMap has no own update; it inherits MutableMapping.update(), which calls self[key] = value per item, routing through ChainMap.__setitem__ -> writes land in maps[0] (the nested schema's own map), never in the enclosing packet. Verified empirically.
• schema.py:833 field.unpack(byte, packet.copy()), and collections.py:374,412 (OptionField.unpack's new_packet = packet.copy() / field.unpack(byte, packet.copy())) -- ChainMap.copy() (stdlib) returns self.__class__(self.maps[0].copy(), *self.maps[1:]), i.e. a new ChainMap, not a plain dict. This matters: the dict-subclass this PR replaces did need (and had) a hand-written .copy() override for exactly this reason -- dict.copy() on a subclass instance silently returns a plain dict, which would have dropped the __missing__ fallback the moment any code copied the context. ChainMap gets the correct behaviour for free. Verified empirically (copy() returns ChainMap, still sees the parent, writes to the copy stay local to the copy).
• Subscript, in, .get(), iteration (.keys()/.values()/.items(), dict(**pkt)), setdefault, pop, __delitem__, equality against dict(...), isinstance(_, Mapping) -- all reproduced directly against a fresh nested_packet_context() and match every claim in the new docstring exactly, including the exact pop() message: KeyError("Key not found in the first mapping: 'length'").
• Ran mypy pcapkit myself on both trees:
  • origin/main: 125 errors in 40 files
  • PR tip (82dbf9416): 124 errors in 40 files
  • The one-fewer error is real and explained by the diff itself: the PR removes a now-dead # type: ignore[has-type] on SchemaField.length (misc.py), which was flagged as Unused "type: ignore" comment on main and is simply gone from the PR's log. No new errors anywhere, and specifically none on the cast(...) line or the unpack() call site. unpack() doesn't need its own explicit cast because that call already carries a pre-existing blanket # type: ignore[call-arg,misc] (for the unrelated @prepare-decorator signature change), which also happens to swallow the ChainMap/dict[str, Any] mismatch -- so the asymmetry between the pack() and `unpack()) call sites is real but explained, not an inconsistency.
• One aside, out of scope for this PR: collections.py's OptionField.unpack builds new_packet = packet.copy() and writes new_packet[self.name] = OrderedMultiDict() into it, but new_packet is never read again or returned -- looks like dead local state. This is unmodified by this PR (present identically on main) and unrelated to the ChainMap change, so it's not this PR's defect, just noting it since it's on the operation list point 1 asked about.

Conclusion: the cast is honest. Every operation the rest of the codebase performs on packet downstream of pack/unpack is something ChainMap genuinely supports with the same semantics dict[str, Any] promises, and nothing downstream needs a real dict.

2. The ABCMeta-cache-poisoning claim (see inline comment on misc.py)

Read the three relevant commits in this branch's own history (46fe59ad4 introducing the feature, 86370d7d5 swapping to a dict subclass over a directly measured Python-3.10 test failure, and 82dbf9416 swapping back with the claim that the suspicion was wrong) plus the independent #439 fix (6b5505fb2, merged as #471, already on main and merged into this branch).

• The #439 fix is real: SchemaMeta.__new__ now unconditionally calls abc.ABCMeta.__new__ instead of bypassing it via type.__new__ below Python 3.11, so every Schema subclass gets its own _abc_impl instead of inheriting Mapping's shared one. Confirmed on this tree: Schema._abc_impl is not Mapping._abc_impl, and neither dict nor ChainMap structurally satisfy Schema via issubclass.
• The cache-keys-on-exact-type claim is correct as a statement about CPython's ABC cache mechanics.
• Left an inline comment on misc.py's docstring (lines 544-557): the stronger claim, "ChainMap never poisoned anything," isn't fully reconciled against 86370d7d5's own directly-measured, reproducible Python-3.10 result (revert-the-ChainMap-call, test passes; restore it, test fails again -- confirmed on an actual 3.10 venv at the time). A plausible reconciling mechanism exists (ChainMap-vs-dict changes which type flows through unrelated isinstance calls elsewhere, shifting when the pre-existing #439 corruption surfaced, rather than ChainMap being causal on its own), but it isn't spelled out.
• I cannot verify either account myself: this environment's only interpreter with dependencies is Python 3.14, and the historical bug was version-gated below 3.11 -- on 3.14 the vulnerable code path never existed regardless of #439, so a clean isinstance({}, Schema) here proves nothing about 3.10. Said plainly rather than inventing a result, per instructions.
• Not a blocker either way: #439 is fixed at the root and already merged into this branch, so nested_packet_context's return type can no longer trigger this class of corruption regardless of which historical account is correct.

3. NestedPacketContextTests still named after the deleted class

Confirmed (tests/corekit/test_fields_misc_packet_context.py:14) -- flagged with a non-blocking inline nit. The class's own docstring immediately explains the naming history in detail, so this reads as deliberate rather than an oversight; a rename would be a cosmetic improvement, not a fix.

4. Docstring accuracy for setdefault/copy/__setitem__/__delitem__

Reproduced every specific claim directly against nested_packet_context() on a fresh context each time:

setdefault('length', 999) -> 3            (honours the fallback, doesn't shadow)
setdefault('brand_new', 7) -> 7, local only, not leaked to parent
copy() -> <class 'collections.ChainMap'>, still sees parent, writes stay local
ctx['length'] = 99 -> parent dict['length'] still 3
del ctx['length'] -> falls back to 3
ctx == dict(ctx) -> True
isinstance(ctx, Mapping) -> True
ctx.pop('length') -> KeyError("Key not found in the first mapping: 'length'")

All match. Nothing measurably false found in the docstring beyond the ABCMeta-history overreach noted above.

Regression proof (re-verified independently, not just accepted)

Reverted only pcapkit/corekit/fields/misc.py to origin/main's version (kept everything else at the PR tip) and reran tests/corekit/test_fields_misc_packet_context.py tests/protocols/schema/ tests/protocols/misc/test_pcapng_unit.py tests/protocols/internet/test_mh_unit.py:

5 failed, 96 passed, 23 warnings, 416 subtests passed
FAILED test_fields_misc_packet_context.py::NestedPacketContextTests::test_nested_schema_reads_enclosing_field_by_name_and_does_not_leak_writes
FAILED test_fields_misc_packet_context.py::NestedPacketContextTests::test_pcapng_block_type_mismatch_consumer_still_works_with_both_shapes
FAILED test_fields_misc_packet_context.py::NestedPacketContextTests::test_pcapng_byteorder_consumer_still_works_with_both_shapes
FAILED test_fields_misc_packet_context.py::CGAParametersRegressionTests::test_cga_parameters_option_now_parses_end_to_end
FAILED test_mh_unit.py::MHUnitTests::test_mh_cga_parameters_option_now_parses

That's 5, not 4 -- the extra one is in test_mh_unit.py (also a CGA/#445 regression test, in a file this PR also touches). This is a stronger regression signal than previously stated, not a discrepancy: misc.py's change is genuinely load-bearing. Restored misc.py to the PR version afterward; worktree diffs clean against origin/fix-445-nested-packet-context again.

With the PR's own misc.py in place, the same scope: 101 passed, 416 subtests passed, 0 failed (reproduced exactly).

Other independent re-measurements

• examples/generators/options.py cases(): 322 total cases (matches). All ten labels this PR removes from EXPECTED_FAILURES (httpv2-frame/{CONTINUATION,DATA,HEADERS,PING,PUSH_PROMISE,SETTINGS}, mh-extension/{Exp_FFFD,Exp_FFFE,Exp_FFFF,Multi_Prefix}) are present in that enumeration, and roundtrip() on each individually returns 'OK'.
• EXPECTED_FAILURES dict length via direct module introspection (not grep, which undercounts because of dict-comprehension entries): 87 on origin/main, 77 on the PR tip -- exactly the claimed 10 fewer.
• examples/generators/make_samples.py run with PYTHONPATH forced to this worktree (verified via pcapkit.__file__ before importing anything else) to generate fixtures before running the suite, per the setup instructions.

Verdict

No blocking defects found. The cast is justified by tracing every downstream consumer of packet; the regression it fixes is real (verified independently, and slightly stronger than claimed); the docstring's mapping semantics are all accurate; mypy has zero new errors (one fewer, explained); and CI is green. The one thing I can't fully settle -- the ABCMeta-poisoning historical claim -- is flagged inline but doesn't affect correctness today, since #439 is fixed at the root independently of this PR.

GOOD TO MERGE at 82dbf9416

[JarryShaw]

Re-review at b647e65e0

Scope: this is a follow-up review confirming that the two non-blocking findings from the prior 82dbf9416 review have been addressed without moving anything the prior review established. All code was read via git show/git diff against the actual refs, not the working tree (my worktree's checked-out branch was unrelated at the start of this review); I later checked out b647e65e0 inside my own isolated worktree only, to run tests locally.

1. The 82dbf9416..b647e65e0 diff is prose + one rename, no executable line

git diff 82dbf9416..origin/fix-445-nested-packet-context --stat
 pcapkit/corekit/fields/misc.py                   | 42 ++++++++++++++++--------
 tests/corekit/test_fields_misc_packet_context.py | 15 ++++++---
 2 files changed, 39 insertions(+), 18 deletions(-)

Full diff inspected line by line:

• pcapkit/corekit/fields/misc.py: every changed line is inside the nested_packet_context docstring's Notes: section. The function body (return collections.ChainMap({'__packet__': packet}, packet)) is untouched, appearing only as diff context.
• tests/corekit/test_fields_misc_packet_context.py: one code line changed — the class name NestedPacketContextTests -> NestedPacketContextSemanticsTests — plus its docstring prose.

No other lines, no other files. The claim holds exactly as stated.

2. misc.py docstring no longer overclaims

Checked the specific factual assertions against source, not just for internal consistency:

• "the cache keys on the exact type queried" — confirmed against #439's own fix commit 6b5505fb2, which added tests/protocols/schema/test_schema_metaclass_abc_cache_unit.py::test_probing_the_same_exact_type_reproduces_439s_shape and ::test_probing_a_different_exact_type_gives_a_false_negative, both built around exactly this exact-type-keying behavior. Not a new claim invented by this docstring; it matches the mechanism #439 itself measured and pinned.
• Info.__update__ / infoclass.py:270 — confirmed: git show origin/fix-445-nested-packet-context:pcapkit/corekit/infoclass.py has __update__ defined at line 259 inside class Info(Mapping[str, VT], Generic[VT], metaclass=InfoMeta) (line 196), and line 270 is exactly elif isinstance(dict_, (dict, collections.abc.Mapping)) or hasattr(dict_, 'items'):. The citation is accurate.
• "no longer testable" — confirmed via #439's fix commit message: the bypassed-ABCMeta.__new__ code path was gated by sys.version_info < (3, 11). The only venv with runtime deps here is 3.14, which never took that pre-#439 path even before the fix — and #439's fix deletes the version guard unconditionally, so no supported Python version can reproduce the original ChainMap/dict-swap experiment anymore. The docstring's claim is fair, not an excuse dressed as a fact.
• The new paragraph explicitly separates "directly measured" (exact-type cache keying, Info.__update__'s isinstance call) from "plausible rather than demonstrated" (the reconciliation theory) — it does not smuggle the latter back in as settled. No internal contradiction found.

3. Test class docstring now consistent with the module docstring

tests/corekit/test_fields_misc_packet_context.py's class docstring was softened in lockstep and now cross-references nested_packet_context for "why it is recorded as two measurements that do not fully reconcile." Confirmed no leftover overclaim:

git grep -n "wrongly" origin/fix-445-nested-packet-context -- pcapkit/corekit/fields/misc.py tests/corekit/test_fields_misc_packet_context.py
(no output)

4. Rename is clean and the class still runs

git grep -n "NestedPacketContextTests\b" origin/fix-445-nested-packet-context   -> no output
git grep -n "NestedPacketContext\b" origin/fix-445-nested-packet-context        -> no output (whole tree)

No stale reference anywhere in pcapkit, tests, or docs. Checked out b647e65e0 in my own isolated worktree, generated fixtures (examples/generators/make_samples.py), and ran the file directly:

pytest tests/corekit/test_fields_misc_packet_context.py \
       tests/protocols/internet/test_mh_unit.py::MHUnitTests::test_mh_cga_parameters_option_now_parses -v
=> 5 passed

That's all 4 tests in the renamed/rewritten file (3 in NestedPacketContextSemanticsTests, 1 in CGAParametersRegressionTests) plus the MH unit test, all collected and green under the new name.

5. RST correctness

Extracted both changed docstrings and classified every backtick span programmatically (after correcting for a script bug where `` double-backtick delimiters were mis-tokenized as empty single-backtick spans): every non-empty single-backtick span is immediately preceded by a role prefix (:class:, `:func:`, `:meth:`, `:exc:`); zero bare single-backtick (default-role) usages remain in either paragraph.

Spot-checked role targets resolve to real objects: Info.__update__ (infoclass.py:259), Schema (pcapkit/protocols/schema/schema.py:314), plus stdlib collections.ChainMap/abc.ABCMeta/dict/isinstance.

Two things I can't fully confirm as described:

• I could not find, within the 82dbf9416..b647e65e0 diff specifically, a "single-backtick `ChainMap` changed to double-backtick" edit — every ChainMap occurrence in that diff is already either a :class: role or already double-backtick on both sides. If that fix happened, it predates 82dbf9416; not a defect, just noting I can't corroborate that specific detail against this delta.
• Pre-existing (not introduced by this PR — same wording already present at 82dbf9416 and before): misc.py:559 uses :func:`~pcapkit.corekit.infoclass.Info.__update__` for what is an instance method; :meth: would be the more precise role. Cosmetic, non-blocking, out of scope for this delta.
• Sphinx build: attempted a targeted sphinx -b html -n -q <docs source> <out> <misc.rst> build via subprocess (to avoid a shell-sandbox false-positive in this environment). It did not complete inside a 170s budget — conf.py's intersphinx_mapping fetches inventories from docs.python.org, dictdumper.jarryshaw.me, chardet.readthedocs.io, dpkt's docs, etc. over the network at build start, and that's what was hanging. So: I did not obtain a completed Sphinx build, and I'm saying so rather than guessing. Role-target correctness above is from direct source inspection, not from a Sphinx nitpick run.

6. Spot-checks of what the 82dbf9416 review established (should be unmoved by a prose-only delta — verified, not re-derived)

All run against my own checkout of b647e65e0 in my own isolated worktree, fixtures regenerated first (PYTHONPATH=<worktree> examples/generators/make_samples.py), pcapkit.__file__ confirmed to resolve inside that worktree before each measurement.

• Reverted only pcapkit/corekit/fields/misc.py to origin/main (git checkout origin/main -- pcapkit/corekit/fields/misc.py) and re-ran the same 5-test scope: all 5 fail (NestedPacketContextSemanticsTests::test_nested_schema_reads_enclosing_field_by_name_and_does_not_leak_writes, ::test_pcapng_block_type_mismatch_consumer_still_works_with_both_shapes, ::test_pcapng_byteorder_consumer_still_works_with_both_shapes, CGAParametersRegressionTests::test_cga_parameters_option_now_parses_end_to_end, test_mh_unit.py::MHUnitTests::test_mh_cga_parameters_option_now_parses). Restored the PR's misc.py (git checkout b647e65e0 -- ...) and re-ran: all 5 pass.
• cases() (imported examples/generators/options.py directly, not parsed): 322, matches.
• EXPECTED_FAILURES (imported tests.protocols.test_option_roundtrip_unit directly — this dict uses ** unpacking, so ast/grep would undercount, per the module's own note): 77 on this branch, 87 on origin/main (verified by swapping the file content in my own worktree and re-importing). Diff: 10 removed, 0 added — httpv2-frame/{CONTINUATION,DATA,HEADERS,PING,PUSH_PROMISE,SETTINGS}, mh-extension/{Exp_FFFD,Exp_FFFE,Exp_FFFF,Multi_Prefix}.
• Each of those 10 labels: called options.roundtrip(case) directly for all 10 — every one returns status='OK'.
• I could not reproduce the "101 passed / 416 subtests" figure quoted in my brief with any file combination I tried (test_option_roundtrip_unit.py + test_fields_misc_packet_context.py + test_mh_unit.py together: 46 passed/630 subtests; test_option_roundtrip_unit.py alone: 6 passed/358 subtests). The exact scope that produces that number isn't specified, so per "no baseline may be trusted second-hand," I'm flagging it as not independently confirmed rather than assuming it. The substantive facts underneath it (revert causes failures, forward passes, cases()/EXPECTED_FAILURES counts, removed-label roundtrips) are all independently confirmed above regardless.

7. CI

Polled until every check left IN_PROGRESS/QUEUED:

gh pr view 457 --json statusCheckRollup --jq '[.statusCheckRollup[]|select(.__typename=="CheckRun")|.conclusion//.status]|group_by(.)|map("\(.[0]):\(length)")|join(", ")'
SKIPPED:2, SUCCESS:21

Docs test gate and Gate (full suite, Python 3.14) are the 2 SKIPPED (skip-by-design on pull_request, per the brief). pyup.io/safety-ci StatusContext: SUCCESS. Fully green, 23/23 accounted for.

---

GOOD TO MERGE at b647e65e0

[JarryShaw]

Both of the things you declined to confirm deserve an answer on the record, since you were right to flag them rather than assume.

The 101 passed / 416 subtests figure is real, and my failure was not writing down the selection. You could not reproduce it because I never stated which paths it covered. It is these four, and it reproduces exactly at b647e65e0:

$ pytest tests/corekit/test_fields_misc_packet_context.py \
         tests/protocols/schema/ \
         tests/protocols/misc/test_pcapng_unit.py \
         tests/protocols/internet/test_mh_unit.py -q
101 passed, 23 warnings, 416 subtests passed in 93.12s

A figure quoted without its selection is not a measurement anyone else can check, which is the whole point of quoting it. My fault, not a bad number.

On the backticks you are simply right, and my brief was wrong. I told you two single-backtick `ChainMap` occurrences had been changed to double-backtick in this delta. They had not:

$ git diff 82dbf9416..b647e65e0 -- pcapkit/corekit/fields/misc.py | grep -cE "^-.*[^`]`ChainMap`[^`]"
0

I did make that edit, but to an uncommitted intermediate state of the file that never existed at 82dbf9416 — so it was never part of the published delta, and describing it as though it were sent you looking for something that was not there. You reported it as uncorroborated instead of quietly agreeing, which is exactly right; a reviewer who cannot find the thing the brief describes should say so, because sometimes the brief is what is wrong.

Your third caveat — the Sphinx build timing out at 170s on intersphinx_mapping network fetches in conf.py — matches what this repo does generally; the docs build stalls for many minutes past pcapkit/const/reg before finishing. Saying you got no completed build was the right call over implying one.

Two things in your report I have adopted:

• Your independent re-derivation of the 5-failure revert proof confirms the corrected figure. My original 4 came from reverting misc.py but then running only tests/corekit/test_fields_misc_packet_context.py, so it never reached tests/protocols/internet/test_mh_unit.py::MHUnitTests::test_mh_cga_parameters_option_now_parses.
• Your check that #439's removed guard was sys.version_info < (3, 11), which is what makes the "no longer testable" claim fair rather than convenient: 3.14 is the only venv with dependencies and it never took the pre-fix path even before the fix.