Skip to content

fix(corekit): pass the copy to ListField callbacks and size SchemaField templates from it (#2173) - #2190

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2173-field-call-new-self
Oct 25, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2173-field-call-new-self

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

Description

Closes #2173.

  • Defects:
    • ListField.__call__ copied itself and then ran new_self._callback(self, packet), so the callback mutated the shared template, not the copy.
    • SchemaField.__call__ chose the copy's template from self._length, which is always -1 when the length is a callable, so it always fell back to 1024s.
  • Fix: both use new_self.
  • Test: tests/corekit/test_fields_call_new_self_2173_unit.py. On main it fails 3 tests: the callback receives the template, the template is mutated, and '1024s' != '24s'.
  • Tests run: the nearby field tests and the tier guard pass (219), as do the HIP locator tests (43).
  • Overlap: docs(corekit): fifth whole-repo pass over corekit, const and vendor (#1103) #2156 also edits both files; the two merge cleanly.

…ld templates from it (#2173)

ListField.__call__ passed its callback the template instead of the copy it
returns, so a callback adjusting its field would mutate the shared template.
SchemaField.__call__ tested the template's -1 length when sizing the copy's
template, so a callable length always left it at '1024s'.

Both now read new_self, as Field.__call__ does.

Test: tests/corekit/test_fields_call_new_self_2173_unit.py checks callback
identity, an untouched template across two calls, and a '24s' template; three
of its four tests fail without the fix.
@JarryShaw JarryShaw added bug Issues reporting a defect (set by the bug report template; a default, not an assessment) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 25, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Review: GOOD TO GO at f3d335773 (Sonnet): 3 fail on main. The only shipped non-default ListField callback (HIP locator set) ignores its field argument, and no shipped SchemaField has a callable length, so parsing is unchanged. The other __call__ overrides already use the copy.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 25, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 25, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge at f3d335773. Merging closes #2173.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.55% (unit tier, Python 3.15, f3d335773, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19599 880 2618 866 92.12%
pcapkit/corekit 2589 74 816 37 96.51%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 2971 23 1036 25 98.80%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17418 219 4574 185 98.14%
pcapkit/toolkit 869 36 298 10 95.20%
pcapkit/utilities 504 7 146 5 98.15%
pcapkit/vendor 4432 1988 1018 167 51.87%

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

@JarryShaw
JarryShaw merged commit 29e82b0 into main Oct 25, 2026
67 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 25, 2026
@JarryShaw
JarryShaw deleted the fix/2173-field-call-new-self branch October 25, 2026 14:52
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 25, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issues reporting a defect (set by the bug report template; a default, not an assessment)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(corekit): ListField and SchemaField __call__ read the template instead of the copy

1 participant