Skip to content

fix(corekit): ProtoChain.__add__ returns stale protocols/aliases - #1107

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1099-protochain-add-stale-cache
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1099-protochain-add-stale-cache

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests

  • Followed the coding style (make pylint, make mypy, make isort). Ran isort, pylint and mypy directly on the touched file instead: isort is clean, mypy reports nothing in protochain.py, and pylint's two findings (:57, :194) are on existing lines.

  • make test passes, and a test case covers the change (only these modules: the new one, tests/corekit/test_protochain.py and tests/project)

  • Added a changelog entry, N/A: added centrally after the wave

  • fix — corrects a defect


Closes #1099. __add__ used copy.copy(self), which carried the left operand's cached protocols/aliases into the result. It now builds a fresh instance through __new__, as from_list does. __add__ is the only method that copies the chain, and a += b falls back to it.

Probe (a = ProtoChain(Ethernet); a.protocols; b = a + ProtoChain(IPv4)):

  • before: b.chain == 'Ethernet:IPv4', but b.protocols == (Ethernet,) and b.aliases == ('Ethernet',)
  • after: b.protocols == (Ethernet, IPv4), b.aliases == ('Ethernet', 'IPv4'), and a is unchanged

Tests in tests/corekit/test_protochain_add_stale_cache_1099_unit.py:

  • with the fix reverted: 2 failed, 1 passed. The warm-cache and += cases fail. The cold-cache case is a control and passes either way.
  • with the fix: 3 passed. test_protochain.py gives 4 passed, and tests/project gives 379 passed, 1 skipped, 1285 subtests passed.

__add__ built its result with copy.copy(self), which carried the left
operand's cached protocols/aliases into the merged chain. Build a fresh
instance through __new__ instead, as from_list does, and drop the now
unused copy import. __add__ is the only method that copies the chain;
a += b falls back to it and rebinds.

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

Copy link
Copy Markdown
Owner Author

Cross-review verdict on d0a7d7497: GOOD TO GO (reviewed on Sonnet; authored on Opus)

  • Fix: the result is now built with __class__.__new__ and __data__, as from_list already does. ProtoChain has no other instance state; its only other attributes are the two cached properties.
  • Subclasses: the result keeps the subclass, and the left operand is not mutated.
  • Error path: a non-ProtoChain right operand still raises AttributeError, as before.
  • Regression test: with protochain.py reverted, the new module gives 2 failed and 1 passed. The warm-cache and += cases fail; the cold-cache control passes.
  • Existing tests: test_protochain.py passes (4).
  • Other callers: pcapkit has no other + on a ProtoChain and no copy reliance on it.

Nit: a subclass whose __init__ sets extra state would lose it, as it already did under copy.copy. No such subclass exists.

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

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.74% (unit tier, Python 3.14, d0a7d7497, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1873 91 578 22 94.33%
pcapkit/dumpkit 136 0 40 0 100.00%
pcapkit/foundation 2422 143 842 34 92.62%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15653 187 3942 162 98.19%
pcapkit/toolkit 487 71 144 3 84.15%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

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

@JarryShaw
JarryShaw merged commit 7ae6fb6 into main Oct 6, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1099-protochain-add-stale-cache branch October 6, 2026 20:14
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(corekit): ProtoChain.__add__ returns stale protocols/aliases

1 participant