feat(corekit,schema,utilities)!: enforce @final at runtime on Info and Schema (#778)

[JarryShaw]

Please follow the guide below

• You will be asked some questions, please read them carefully and answer honestly

• Put an x into all the boxes [ ] relevant to your pull request (like that [x])

• Use Preview tab to see how your pull request will actually look like

• Searched for similar pull requests

• Followed the coding style (make pylint, make mypy, make isort)

• make test passes, and a test case covers the change

• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A — changelog centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

Tick the commit type your subject line carries.

• fix — corrects a defect
• feat — adds a feature
• perf — changes performance, not behaviour
• refactor — changes neither behaviour nor performance
• test — tests only
• docs — documentation only
• ci — workflows or build tooling
• chore — anything else

---

Description of your pull request and other information

Closes #778.

info_final/schema_final both end return final(cls), so every finalised class already carried typing.final's __final__ — and grep -rn '__final__' pcapkit/ found nothing reading it. #778 is the ruling that it should be read.

Four shapes, four answers (@schema_final / Schema / SchemaError / SchemaWarning identically):

shape	answer
@info_final	finalise, silent
@info_final @final and @final @info_final	finalise, silent — order does not matter
@info_final twice	finalise once, warn (InfoWarning), class returned usable
@final alone	raise InfoError at first construction
deriving from a finalised class	raise InfoError at declaration

• Re-entry check keys on __finalised__, not __final__. The two record different facts and only __finalised__ records the decorator having run. Decorators apply bottom-up, so @info_final over @final reaches the decorator with __final__ already set by something that generated nothing — a __final__ test there reads that as "already finalised", skips the generation, and returns a class with no __init__. That is what made row 2 order-dependent.
• @final alone is caught at first construction, in __new__. It cannot be caught in __init_subclass__: final is applied to the class object, after creation, so that hook has already returned. The check sits inside the existing one-shot FinalisedState.NONE branch, so a finalised class never reaches it — dis.dis(Info.__new__) shows the branch compiles to one POP_JUMP_IF_FALSE that skips clean past it for a FINAL class, zero of the added bytecode executes, a property of the compiled branch rather than of any one timing run. It raises before the auto-finalisation, so the refusal repeats rather than firing once.
• Every check reads its marker out of the class's own __dict__. Both markers are ordinary class attributes and so inherit; a class that merely descends from a finalised one has not been mismarked by anybody. That inheritance had a second edge: info_final/schema_final's own _finalised=False promotion wrote BASE onto Info/Schema themselves on a bare Info()/Schema() call, which every later subclass then inherited — silently defeating the bare-@final guard for the rest of the process. Fixed by skipping that promotion when cls is Info/cls is Schema, which on its own would re-enter the whole function on every bare call — closed with a second, own-__dict__ marker (__base_ready__) that short-circuits the re-entry outright, so it costs nothing repeated and never touches __finalised__ (two rounds of cross-review pinned this: an unbounded __excluded__ first, then an O(n²) dedup papering over the re-entry instead of stopping it).
• compat takes final from typing_extensions below 3.11, not 3.8 — typing.final only records __final__ from 3.11 (gh-90500), so on the 3.10 matrix leg every guard here was a silent no-op. typing-extensions is already a declared dependency for python_version < '3.11'.
• EnumSchema.__init_subclass__ calls the base hook first, so a refused declaration cannot leave __enum__ pointing at a discarded class — pcapkit/protocols/schema/misc/pcapng.py's Option.__init_subclass__ did not follow this (it registered into __enum__ before the base hook could raise) and is now reordered to match.

Two honest notes. Row 2 was fine on main and was broken only by an earlier revision of this branch, which had moved the re-entry check onto __final__; the fix restores main's marker with an own-__dict__ refinement. And the bare-@final guard can only see a marker that was actually recorded, so on 3.10 it fires for typing_extensions.final but not for a user's typing.final, which records nothing there. A @final class descending from a BASE-state ancestor also escapes the guard by inheriting BASE instead of NONE — documented as out of scope on Info.__new__/Schema.__new__ and pinned by a test, since closing it means re-finalising every BASE descendant on each subclassing.

Additive: Info 488 descendants / 455 carry __final__ / 0 subclassed; Schema 445 / 408 / 0; and no class in the tree carries __final__ without FinalisedState.FINAL, so the bare-@final guard cannot fire on the library either. Unit tier (serial, not -n auto) 1819 passed / 15 skipped / 0 failed in 28m07s; coverage of infoclass.py and pcapng.py 100%, schema.py 99% (one pre-existing miss), compat.py 100% via tests/utilities/test_compat.py's version-faking re-execution (unchanged from main); mypy and pylint unchanged (identical findings, same files/kinds, before and after). tests/test_final_enforcement.py is the only test file touched, plus a one-line reorder in pcapng.py — 31 tests, 12 shown failing against main.

[JarryShaw]

Labelled feat/breaking/test, review: pending at f85cc75bc, and needs: decision for one question the PR raises and I am not going to decide on your behalf.

The ask, in the PR's own words: "info_final/schema_final raising on re-decoration is my reading of the principle recorded on #514, not something you stated outright — say so and it goes back to a warning."

Two independent halves are in this diff and only the second is in question:

1. Refusing to subclass a finalised Info/Schema — this is the design: #514 recorded two rulings it never implemented — the Base/public collapse and the enforcing @final #778 ask itself, and it stays regardless.
2. Raising instead of warning when info_final/schema_final is applied twice — inferred, not ruled. Its collateral is real and worth knowing before you answer: tests/utilities/test_stacklevel.py used that warning as its probe and now uses a SeekWarning, and InfoWarning ends up raised nowhere in the library.

One line either way is enough — "keep it raising" or "back to a warning". If the latter, the change is local: restore the warn and put test_stacklevel.py's probe back.

Cross-review dispatched on a different model from the author; CI is still running (5 of 29 legs in at the time of writing). I will report both verdicts together and will not call this ready on CI alone.

[JarryShaw]

Raising instead of warning when info_final/schema_final is applied twice

If it's something like,

@info_final
@info_final
class FinaledInfo(Info): ...

then it should only warns. But if it's something like

@info_final
class A(Info): ...

@info_final
class B(A): ...

then B should raise.

[JarryShaw]

Ruling recorded, in your words:

If it's something like, @info_final @info_final class FinaledInfo(Info): ... then it should only warns. But if it's something like @info_final class A(Info): ... / @info_final class B(A): ... then B should raise.

Measured against this PR's head f85cc75bc, and your second case already behaves the way you want — it just does not come from the decorator. Both families:

CASE 1  @info_final @info_final class FinaledInfo(Info)
    RAISED InfoError: FinaledInfo: info class has already been finalised; ...   <- you want WARN
CASE 2  @info_final class B(A), A final
    RAISED InfoError: B2: cannot subclass A2, which is final                    <- you want RAISE
CASE 2b class D(C), A final, NO decorator
    RAISED InfoError: D2: cannot subclass C2, which is final
CASE 1  @schema_final x2       -> RAISED SchemaError: ... already been finalised
CASE 2  @schema_final class B(A) -> RAISED SchemaError: SB: cannot subclass SA, which is final

Case 2 raises from __init_subclass__, at class B(A) creation, before @info_final is ever applied — which is why 2b raises identically with no decorator at all. That is the #778 guard itself and it stays exactly as built.

So your ruling reduces to one change: re-decoration of the same class goes back to a warning. Two things fall out of it for free — InfoWarning becomes live again instead of being raised nowhere, and tests/utilities/test_stacklevel.py gets its original warning probe back, undoing the SeekWarning substitution that was this half's only collateral.

Removing needs: decision. Routing the change to the worker that wrote it; the cross-review currently running was dispatched against f85cc75bc, so I will say plainly which of its findings the new head changes rather than carrying its verdict across a force-push.

[JarryShaw]

Revised to d471e863a for your ruling. Your two cases, measured on the new head:

CASE 1 Info   @info_final x2                    WARN  ['InfoWarning']    -> F(1).to_dict() = {'x': 1}
CASE 1 Schema @schema_final x2                   WARN  ['SchemaWarning']  -> FS().to_dict() = {}
CASE 2 Info   @info_final class B(A), A final    RAISE InfoError: B: cannot subclass A, which is final
CASE 2b Info  bare subclass, no decorator        RAISE InfoError: D: cannot subclass C, which is final
CASE 2 Schema @schema_final class B(A)           RAISE SchemaError: SB: cannot subclass SA, which is final

The warned class still constructs correctly — that matters, because handing back a class without its generated __init__ would be a silent failure, not a warning.

The PR got smaller, not larger. Three test files are now byte-identical to main (verified with git diff --quiet origin/main): test_stacklevel.py, test_infoclass.py, test_schema_unit.py. The SeekWarning substitution is gone and InfoWarning is live again at infoclass.py:92. Still one commit; 6 files in the diff against f85cc75bc, and the __init_subclass__ guards do not appear in it at all.

One thing the downgrade genuinely makes worse, which the worker found rather than me. With a raise, keying the re-decoration check on getattr would have thrown on a legitimate call — loud. With a warning it instead warns and hands back a class with no generated __init__, whose construction then falls through to Info.__update__ and takes a mapping instead of the declared fields. So OwnDictRuleTests now asserts no warning and Child(1, 2).to_dict() == {'x': 1, 'y': 2} — the constructor working, not just the marker landing. That is now the most load-bearing test in the file.

Against-main count corrected in the body: 7 of 13, not 9. The two re-decoration tests now pass on main too, since main already warns there — they are preservation guards now. Also removed the "your call" paragraph you have answered, and the stale SeekWarning sentence.

Unit tier unchanged at 1806 passed / 31 skipped / 0 failed; coverage 100%/99%/100%; mypy 3 pre-existing; pylint only the 2 pre-existing R0801.

The earlier GOOD TO GO was at f85cc75bc and does not carry. Its ground on the __init_subclass__ guard, the compat.py 3.11 boundary, the EnumSchema reordering and the descendant counts is untouched by this diff — I re-confirmed the guards behave identically above. Dispatching a delta review on the changed part before I set a verdict.

[JarryShaw]

Delta cross-review at d471e863a (sonnet, a different model from the author): GOOD TO GO.

The mutation test I asked for is the part that matters, and it fired. It changed info_final to read getattr(cls, '__final__', False) instead of the own __dict__, and test_a_descendant_that_predates_the_marker_may_still_be_finalised failed immediately:

AssertionError: ['Child: info class has been finalised; now skipping'] != []

So OwnDictRuleTests genuinely catches the silent-half-built-class regression that the downgrade from raise to warning made possible, rather than merely asserting the marker lands. Reverted and re-confirmed clean.

Re-derived independently, both published numbers:

$ git diff --quiet origin/main <branch> -- pcapkit/utilities/warnings.py  ->  IDENTICAL
$ pytest tests/test_final_enforcement.py (on clean main)  ->  7 failed, 6 passed
$ python -m unittest tests.test_final_enforcement          ->  Ran 13, FAILED (failures=7)

Both runners agree at 7, so the corrected count in the body is right and InfoWarning/SchemaWarning are pre-existing rather than added here.

It also confirmed live, not by reading: triple decoration with several real fields still warns and stays usable (F3(1,2,3).to_dict() == {'x':1,'y':2,'z':3}); the subclass guard raises through MRO for a grandchild declared before its parent was finalised, and for a diamond; stacklevel attributes to the caller's line, not infoclass.py's; -W error on both warnings does not break import pcapkit.all; and the merge-base staleness is conflict-free — this branch touches 4 files, origin/main has moved 7, zero overlap.

Two honest caveats it raised rather than smoothing over. Its R0801 count was 7 against the claimed 2, which it attributes to scan scope — duplicate-code detection is corpus-size dependent, and the #783 review hit the same nondeterminism. And a tree-proof trap worth recording: a bare python script.py from inside the worktree with PYTHONSAFEPATH=1 still imported pcapkit from the main checkout, because suppressing the cwd insert leaves the venv's editable install as the only provider. It added a conftest.py asserting pcapkit.__file__ on collection and re-ran everything under it.

CI is fully green at 27✅ / 3⏭ / 0❌ on this head. Setting review: good-to-go — unpublished and unmerged, yours to take.

[JarryShaw]

but what if the class if @final'ed but not @info_final'ed. we still need @info_final to finalize and generate the functions in the class. and my thoughts were @info_final implies @final but @final must not be used without @info_final.

e.g.,

@info_final
class A:...   # all good, finalized and non-inheritable

@info_final
@final
class B:... # all good, finalized and non-inheritable

@final
@info_final
class C: ...  # all good, finalized and non-inheritable

@info_final
@info_final
class D: ...  # good but warn of finalized class

@final
class E: ...  # raise, this is not finalized and not acceptable

same ruling applies to Schema's @schema_final.

[JarryShaw]

You are right, and case B is broken today — not merely unguarded. Measured on d471e863a, all five of your cases, with the generated __init__ exercised rather than just the marker checked:

A  @info_final                warns=[]                constructs -> {'x': 1}
B  @info_final @final         warns=['InfoWarning']   CONSTRUCT FAILS: TypeError: 'int' object is not iterable
C  @final @info_final         warns=[]                constructs -> {'x': 1}
D  @info_final @info_final    warns=['InfoWarning']   constructs -> {'x': 1}
E  @final only                warns=[]                CONSTRUCT FAILS: TypeError: 'int' object is not iterable

B fails because decorators apply bottom-up. final(B) runs first and puts __final__ in B's own __dict__; info_final then sees it, warns "B: info class has been finalised; now skipping", and skips the generation. So B comes back marked final and with no generated __init__, and construction falls through to Info.__update__, which wants a mapping. C works only because the order puts info_final first.

E is worse — it is silent. No warning at all, and the class is equally unusable.

The root cause is exactly what you put your finger on: the re-decoration check keys on __final__, which conflates two different facts — "already finalised by info_final" and "merely marked by typing.final". They need distinguishing, and the marker for the first already exists in the tree (FinalisedState / __finalised__), which is what the check used before it was moved onto __final__.

So the ruling I will implement, in your terms:

	behaviour
@info_final	finalise, silent
@info_final @final / @final @info_final	finalise, silent — order must not matter
@info_final twice	finalise once, warn
@final alone on an Info subclass	raise — marked final but never finalised

That last one needs a hook that fires without info_final being called, so it lands in __init_subclass__/the metaclass rather than in the decorator. Same for Schema/schema_final.

Flipping #788 to review: needs-changes — I had set good-to-go on it, and this case was missed by me and by the cross-review alike: both of us exercised @info_final twice and bare subclassing, neither tried @info_final stacked on a bare @final. Routing it now.

[JarryShaw]

Revised to 0d197c8a8. The worker corrected my framing, against itself, and it is right. Verified on both trees:

main 0419c1c97   B Info @info_final @final  -> {'x': 1}     works
main 0419c1c97   E Info @final alone        -> {'x': 1}     works, silently unguarded
NEW  0d197c8a8   B Info @info_final @final  -> {'x': 1}     works
NEW  0d197c8a8   E Info @final alone        -> InfoError: E: marked final but never finalised

So case B was not a pre-existing defect — it was a regression this PR introduced at d471e863a, when the re-entry check moved onto __final__. My earlier reply measured it at that head and said so, but called it "broken today" without distinguishing whose breakage. It was the PR's. Case E is the genuinely new one: main silently accepted a bare @final as an unfinalised BASE class, and now it raises, naming the class and the fix.

One correction to my own measurement: my Schema probe constructed with cls(1), which is not a valid Schema construction regardless of finalisation, so the TypeError it showed on both trees was my bad call, not a finding. The worker's own Schema measurement is the one to trust — and it reports something worse than I had: on d471e863a, Schema B and E did not raise at all but returned a silently corrupt object, to_dict() giving {'__map__': {}, '__map_reverse__': {}, '__buffer__': {}, '__updated__': True}, because an unfinalised schema has an empty __excluded__. Both now raise SchemaError.

Where the case-E guard went, and the cost. Info.__new__/Schema.__new__, nested inside the existing one-shot if cls.__finalised__ == FinalisedState.NONE branch — not __init_subclass__, which cannot see a decorator applied after class creation, and not a metaclass __call__, because __new__ already owns the finalisation state machine. Measured:

nested inside the branch : 1369 -> 1387 ns  (+18 ns, +1.3%, inside a ±3-4% run-to-run spread)
hoisted above the branch : 1411 -> 1696 ns  (+285 ns, +20%)

It raises before the auto-finalisation so __finalised__ stays NONE and the refusal repeats rather than firing once — with a subtest per attempt. All four in-tree __new__ overrides call super().__new__ first, so none bypasses it.

Three limits it named in the ruling as stated, which are yours to weigh rather than mine:

1. A bare @final is only detectable if the final that was applied actually recorded __final__. On 3.10 typing.final records nothing, so from typing import final gets no guard there while typing_extensions.final does. Inherent to a marker-based rule.
2. Detection is deferred to first construction, so a mismarked class never instantiated is never reported — a runtime net, not a static one. Moot in-tree (zero classes carry __final__ without FinalisedState.FINAL).
3. A subclass of a BASE class is still not re-finalised, so @final class E2(SomeBase) escapes by inheriting BASE rather than NONE. Catching it means changing auto-finalisation for all 488 descendants — deliberately out of scope, and test_an_unfinalised_descendant_is_not_mistaken_for_a_mismarked_class documents the boundary without pinning the wart.

Tests 13 → 21, with B and C asserted to agree with each other rather than separately, since order-independence is the property that broke. Against unmodified main: 10 of 21 fail (was 7 of 13). Unit tier 1814 passed / 31 skipped / 0 failed; coverage 100/99/100; mypy and pylint unchanged.

Cross-review dispatched on a different model at this head. Label stays review: needs-changes until it lands.

[JarryShaw]

Revised to 94416199e. Both blocking findings verified closed by me, behaviourally rather than by grep, against a git archive export of the new head:

PROVENANCE: /tmp/v788n/pcapkit/__init__.py
-- 1: bare Info()/Schema() must not disable the guard --
  Info() -> {} | Info.__finalised__ now 1          <- stays NONE, was promoted to 2 (BASE)
  PlainBase(1) -> {'x': 1}                          <- was TypeError: 'int' object is not iterable
  Schema() -> {} | Schema.__finalised__ now 1
  Mis() RAISED SchemaError: Mis: marked final but never finalised...   <- was silently {}
-- 2: a refused Option subclass must leave __enum__ alone --
  BEFORE: CommentOption
  DECL RAISED SchemaError: Hijack: cannot subclass CommentOption, which is final
  AFTER : CommentOption | POLLUTED: False           <- was Hijack

The chosen fix for (1) is if cls is not Info: around the __finalised__ promotion — so Info/Schema no longer promote their own state, the guard's enclosing NONE branch stays reachable for every later-declared subclass, and the hot constructor path keeps its zero added bytecode. That is the cheap option rather than the +167 ns hoist, and it is the right one.

For (2), super().__init_subclass__() is now the first statement of Option.__init_subclass__ (pcapkit/protocols/schema/misc/pcapng.py:739), carrying a NOTE that states the invariant and why calling it last broke it.

Flipping review: needs-changes → review: pending: that verdict was pinned to the dead 0d197c8a8, and a verdict on a superseded head is worse than none. A delta re-check is running on the same reviewer, which still holds its own measurements — the remaining items are the three contradicted PR-body numbers, the field-less Sealed(Schema) rows, the two .rst automethod additions, the cls.__dict__-vs-getattr NOTE that named an unreachable case, the 3.10 typing.final caveat, and the BASE escape.

[JarryShaw]

Correcting the cross-review, not the PR: its coverage compat 100% finding is REFUTED, and the author was right to push back rather than "fix" a correct number.

The review argued 100% is structurally impossible on one interpreter, since compat.py has 10 top-level if sys.version_info gates and the line this PR adds (from typing_extensions import final) can never execute on 3.11+. The premise about the gates is true; the conclusion does not follow, because tests/utilities/test_compat.py fakes sys.version_info to (3, 5) and re-executes the module, so both sides of the gates run under instrumentation. Measured by me at 94416199e:

$ coverage run --branch --source=pcapkit.utilities.compat -m pytest tests/utilities/test_compat.py
5 passed
pcapkit/utilities/compat.py     101      0     42      0   100%

So "100%, unchanged from main" in the PR body is accurate and reproducible. The author recorded the disagreement plainly instead of deferring to the review — which is the right behaviour, and it is why this PR still says the true thing.

Two of my own measurement errors while settling it, worth recording since the pattern keeps recurring: --source=pcapkit/utilities/compat.py (a file path where coverage wants a module) collected nothing and warned Module ... was never imported; then --include='*utilities/compat.py' reported No data to report against a data file that plainly held data. Neither was evidence of anything. That is the sixth and seventh instance in this session of an empty result produced by the probe rather than by the code.

The other two numbers were handled correctly. The +285 ns / +20% claim is gone and no delta replaces it: the author could not reproduce a stable sign or magnitude — one ordering gave +2 ns, the reverse gave +174 ns — so the comment now rests on dis.dis(Info.__new__) showing a FINAL class jumps clean past the added bytecode, which is the argument that does not depend on a timer. And the unit tier is now a third figure — 1819 passed, 15 skipped, 0 failed, 7260 subtests, 28m07s — against the body's original 1814/31 and the review's 1759/69. Skip counts are gate-dependent (HAS_PYPCAP, HAS_VENDOR_DEPS, …) so the three legitimately differ; 0 failed is the part that held in all three environments and is the part that matters.

One thing the author could not verify and flagged rather than glossed: sphinx-build would not run, because this harness blocks any command containing the path component source — so the two new .. automethod:: __init_subclass__ directives are verified by inspection only (the identical directive already works for EnumSchema in the same file, and both methods exist). Worth a real docs build before merge.

[JarryShaw]

mergeStateStatus is BEHIND — 8 commits — and pcapkit/protocols/schema/misc/pcapng.py is touched by both sides, so I tested the actual merge rather than trusting "clean and green".

merge-base 0419c1c97 ; main has 8 commits #788 lacks (#784 #786 #783 #790 #793 #789 #795 #797)
main changed 21 files, #788 changed 7, OVERLAP: pcapkit/protocols/schema/misc/pcapng.py

Merged 94416199e into 477ed00c4 in a scratch worktree — real merge commit b972df37d, two parents — and both sides survived textually and behaviourally:

PROVENANCE: /tmp/m788/pcapkit/__init__.py
pcapng.py:739   super().__init_subclass__()            <- #788's hoist, still first
pcapng.py:1908  if not raw_line.endswith(b'\n') and raw_line.endswith(b'\x00')   <- #795's padding gate, intact

pytest tests/test_final_enforcement.py tests/protocols/schema/ tests/corekit/test_infoclass.py
68 passed, 15 subtests passed in 44.46s

So the overlap is benign: #795 edits the journal-export reader near line 1900 and #788 edits Option.__init_subclass__ near line 739, in the same file but not the same region, and the hoist's invariant still holds after the merge. No rebase is needed for correctness — BEHIND here is about the base pointer, not a real conflict. Rebasing is still yours to decide at merge time.

Worth noting one thing the merge surfaced that is not a defect: tests/protocols/schema/test_schema_metaclass_abc_cache_unit.py:345 emits a RegistryWarning about opt_comment already being registered, because it deliberately re-registers the code on a _RenamedOption. That warning is pre-existing and expected — but it is now firing in a file whose registry write order #788 changed, so it is worth knowing it was already there rather than introduced.

My own error while establishing this, recorded because it nearly produced a false all-clear: the first merge attempt printed fatal: Not possible to fast-forward, aborting (this repo sets merge.ff=only), leaving HEAD on plain main — and my greps then read main's unhoisted super().__init_subclass__() at :743 while appearing to describe a merged tree. Only re-reading git rev-parse HEAD caught it. Re-run with --no-ff gave the two-parent commit above.

[JarryShaw]

Delta re-check verdict: NEEDS CHANGES — one narrow item, and it is a new defect this revision introduced rather than a leftover. Everything else from round 1 is closed. I reproduced the blocking finding myself on git archive exports of both heads:

                                    0d197c8a8 (old)      94416199e (new)
Info.__excluded__ after 1000 bare Info()        56             56,999  (unique 57)
Schema.__excluded__ after 1000 bare Schema()    68             68,999  (unique 69)
bare Info()                                 1.2 us              49.1 us   (41x)
bare Schema()                                2.2 us              73.9 us   (34x)
declaring an Info subclass                  0.07 ms             19.77 ms  (282x)
Info(a=1).to_dict()                          {'a': 1}            {'a': 1}   <- correctness intact

Mechanism: because Info/Schema now never reach FinalisedState.BASE, every bare Info()/Schema() re-enters the NONE branch and re-runs info_final/schema_final in full — including cls.__excluded__.extend(cls.__builtin__), which appends in place to a class-level list. unique stays at 57/69 while the length grows without bound, so it is pure duplicate accumulation. The knock-on is worse than the construction cost, because InfoMeta.__new__ dedups a new subclass's __excluded__ against its bases' — hence 282× on a class declaration, i.e. at import, scaling linearly with how many bare constructions preceded it.

Required: dedup the extend, or short-circuit re-entry for the two base classes specifically. One line either way, and please add a test — nothing currently pins it, and grep confirms the library itself never bare-constructs (only the new NOTE comments match), so this would stay invisible.

Why it is blocking despite a narrow blast radius: it is unbounded, it is on documented public API (Info is "Turn dictionaries into object like instances"), and nothing tests it. Correctness is genuinely unaffected — subclasses still get a deduped list (Late.__excluded__ len 57, unique 57) and to_dict() is unchanged.

Everything else closed. Both round-1 blockers re-derived independently at the new head, not taken on trust. The attack battery is identical to round 1 — no new bypass, no new false positive (PlainI(1) → {'x': 1} after a bare Info()). Audit numbers unchanged: Info 487/453/0 mismarked, Schema 446/408/0, package-wide 865 own __final__, 2 not FINAL (NoValueType, _AbsentType, both outside the hierarchies). Ordering independence confirmed and pinned by two new tests written the right way round — each asserts __dict__.get('__finalised__') == NONE after the bare construction, which is exactly the assertion that dies if cls is not Info is removed, and which makes them robust under -n auto --dist load. 21 → 27 tests, all passing.

Mutation D is properly closed — it survived round 1 and now fails (test_a_subclass_that_predates_an_unfinalised_ancestors_bare_final_is_not_blamed_for_it), with the rewritten NOTE naming the real case. A–C all still caught, B more strongly than before.

One secondary item, your call. The reviewer disputes the replacement text in Info.__new__'s NOTE. It now says hoisting "gave inconsistent deltas run to run — the noise on this host is larger than the effect being measured", but the reviewer's round-1 measurement was back-to-back in one tree, min-of-9: nested 1474.5 ns vs hoisted 1641.4 ns, +167 ns / +11.3% against a nested spread of 1.4–3.2% — effect larger than noise, not smaller. The dis argument the NOTE now leads with is correct and is the right reason to trust the design; consider just dropping the noise sentence rather than asserting either number.

Flipping to review: needs-changes and routing the __excluded__ fix.

[JarryShaw]

Supplement: the cls is not Info / cls is not Schema guards are load-bearing and now defended. The reviewer's own mutation-E run was unre-derivable — its cleanup rm -rf fired mid-run and deleted the tree — so I derived it from scratch on a fresh git archive 94416199e export.

Mutation: replace if cls is not Info: and if cls is not Schema: with if True:, making the FinalisedState.BASE promotion unconditional again. (Note there is a third cls is not Info at infoclass.py:126, inside the '__init__' not in cls.__dict__ condition — deliberately not mutated.)

baseline, unmutated:  27 passed, 10 subtests passed in 0.09s
MUTATION E:            8 failed, 25 passed,  4 subtests passed in 0.18s
  FAILED  test_a_prior_bare_info_construction_does_not_defeat_the_guard
  FAILED  test_a_prior_bare_schema_construction_does_not_defeat_the_guard
  SUBFAILED(attempt=0,1,2)  test_the_refusal_repeats_rather_than_firing_once
  SUBFAILED(attempt=0,1,2)  test_schema_refusal_repeats_rather_than_firing_once

Four distinct methods catch it, across both families. The three-attempt failures on the refusal_repeats tests are exactly the round-1 flake signature — an earlier test bare-constructs Info(), Info gets promoted to BASE, the mismarked class inherits it, and the guard never fires. So removing the fix re-opens the original bypass and the suite catches it in the same breath. That is the falsification the re-check wanted and could not finish in budget.

My counts differ from the reviewer's (8 failed / 25 passed under pytest vs its 4 failures + 3 errors over 27 under unittest) — that is the known pytest-subtests counting divergence, not a disagreement about behaviour; the failing method names match exactly.

Verdict unchanged: review: needs-changes, solely on the unbounded __excluded__ growth, which is already routed. This supplement only removes a caveat from the record.

One methodological note the reviewer surfaced and I am recording because it is the inverse of the trap we keep hitting: its pgrep -f "n_probe1|n_mutate|…" reported RUNNING because the pattern matched its own shell wrapper — a false positive from a self-matching probe, where every other instance today has been a false zero. Both are the same error: a probe that has not been shown to distinguish the thing it is looking for from itself.

[JarryShaw]

Revised to 3edd196d5. The unbounded growth is genuinely fixed — but the per-call cost got worse, not better, and a 66× regression on public API remains. Measured by me across all three heads on git archive exports:

                              0d197c8a8      94416199e       3edd196d5 (new)
Info.__excluded__                    56    78,400 (u 56)              56   <- fixed
Schema.__excluded__                  68    95,200 (u 68)              68   <- fixed
declaring an Info subclass      0.059 ms       21.586 ms        0.091 ms   <- fixed
bare Info()                       1.0 us         48.3 us         66.0 us   <- WORSE
bare Schema()                     1.8 us         73.9 us         99.7 us   <- WORSE
Info(a=1).to_dict()             {'a': 1}        {'a': 1}        {'a': 1}

Why it got worse: extend(name for name in cls.__builtin__ if name not in cls.__excluded__) is O(n²) — 56 names each scanned against a 56-element list, on every bare construction, because the whole of info_final still re-runs. The dedup makes the function idempotent, which fixes the growth and the 282× declaration cost, but it does nothing about the re-run itself and adds work to it.

So the blocking item is closed and a different one is now visible. Info is documented as "Turn dictionaries into object like instances", so direct Info(**d) is a documented primary use, and 66 µs to wrap a dict — against 1.0 µs before this PR — is a real cost on that path. It is bounded and constant rather than unbounded, which is a genuine improvement, but it is 66× and it is on the API the class exists for.

The fix that addresses the cause rather than the symptom is the short-circuit that was considered and rejected. The stated reason for rejecting it was that it "needs no new marker" and that the cls is not Info guard's purpose must stay — but those do not conflict. The guard's purpose is keeping Info.__dict__['__finalised__'] at NONE so the ordering tests hold; a separate attribute recording "this base class has already been finalised" is invisible to those tests and lets the re-entry return immediately. That restores ~1.0 µs, keeps the growth fixed, keeps the declaration cost fixed, and keeps the guard intact.

Please do that instead, and keep the two new bound tests — they are good and they will still pass, since a short-circuit bounds the length just as effectively. Add one asserting the per-call cost does not regress, or at minimum that info_final's body is not re-entered on a repeat bare construction.

Everything else in this revision is right. The two new tests are well built — prime once, snapshot, loop 50, assert length equals its own unique count — and were shown failing on the pre-fix code with AssertionError: 3468 != 68, which is exactly 68 × (1 + 50). The noise sentence is gone from the NOTE, the PR body and the commit message, leaving the dis argument to stand alone with no number asserted. 29 passed, 10 subtests.

Staying at review: needs-changes — third round on this PR, but the remaining item is mechanical and the measurement above says exactly what to do.

One thing the author flagged honestly and I am carrying forward: CI's green run was against 94416199e, not this sha. Worth confirming before merge even though the change is behaviourally inert.

[JarryShaw]

Revised to 84faf77bf. All four properties restored together, verified by me across every head (min-of-5 over 2000 constructions, on git archive exports):

                         0d197c8a8      94416199e     3edd196d5      84faf77bf
                          (pre-fix)    (growth bug)     (dedup)    (short-circuit)
Info.__excluded__               56          56,000            56            56
bare Info()                  0.95 us       47.27 us      66.50 us       1.40 us
bare Schema()                1.73 us       72.77 us     100.34 us       2.24 us
declaring an Info subclass  0.075 ms      173.76 ms      0.091 ms      0.089 ms
Info.__dict__['__finalised__']   2 (BASE)    1 (NONE)     1 (NONE)      1 (NONE)
__base_ready__ own-dict        None          None          None          True
Info(a=1).to_dict()         {'a': 1}      {'a': 1}      {'a': 1}      {'a': 1}

The __base_ready__ marker is the right shape. It lives only in Info's/Schema's own __dict__, is never inherited, and never touches __finalised__ — so Info.__dict__['__finalised__'] stays NONE and the two ordering tests plus mutation E still hold, while the re-entry returns before any of the dir()-over-MRO / set(temp) / extend work. That is why bare construction is back at baseline rather than merely bounded, and it let the O(n²) dedup be reverted since the extend now runs exactly once.

The author caught its own probe artefact, and the mechanism is worth recording. Its first non-re-entry test snapshotted id(cls.__builtin__). Verified against the previous round's dedup-only code, the Info side correctly failed but the Schema side spuriously passed — the first set was garbage-collected and CPython immediately reused its address for the replacement, so two genuinely different objects compared equal by id(). Fixed by holding a live reference and asserting assertIs, then re-verified failing on the dedup-only code and passing on the short-circuit. That is the seventh probe artefact this session and the first of its kind: an identity check defeated by address reuse rather than a pattern that could not match.

31 passed, 10 subtests. isort clean on all three touched files.

Flipping review: needs-changes → review: pending — the previous verdict was pinned to the dead 3edd196d5. Delta re-check dispatched.

Carried forward honestly by the author and worth your eye at merge time: mypy and pylint were not re-run against this sha (verified on 3edd196d5 and 94416199e, same 2 pre-existing errors both times), the 28-minute unit tier was not re-run, and the PR body's 1819 passed / 15 skipped line is from an earlier sha — CI has not seen 84faf77bf yet. The body should say so rather than imply the number describes this head.

[JarryShaw]

Cross-review verdict at 84faf77bf: GOOD TO GO. Third round, delivered inside budget. Two informational findings, neither blocking, and I settled the one item the review left UNVERIFIED.

__base_ready__ opens no hole — re-derived by me. The marker does inherit through getattr, but cls is Info and cls.__dict__.get('__base_ready__') is doubly guarded, so planting it on a subclass neither skips that subclass's setup nor dodges the guard:

PROVENANCE: /tmp/n788d/pcapkit/__init__.py
Sneak.__base_ready__ = True ; Sneak(1) -> {'x': 1}   own __finalised__=2  own __init__=True   <- setup still ran
SneakF (@final + own __base_ready__) -> InfoError: marked final but never finalised...        <- guard holds
Sneak inherits getattr __base_ready__: True | own: True

Mutations A–F all caught, including the new F (remove the short-circuit → test_repeated_bare_info_construction_does_not_grow_excluded_without_bound and …_does_not_re_enter_the_base_setup both fail). So the short-circuit is defended, not decorative. The reviewer states a real scope limit honestly: its harness edits only infoclass.py, so F removed the Info short-circuit alone — the Schema side is covered instead by item 4's independent experiment, which shows both …_does_not_re_enter_the_base_setup tests failing against round 2's dedup-only code. Two experiments, both sides.

Audits unchanged — Info 487/453/0 mismarked, Schema 446/408/0, and a new column: zero subclasses anywhere in either hierarchy carry __base_ready__. Self-tested both ways (bad=1 ['KnownBad'], base_ready_subclasses=1 ['KnownBR'] after injection; KnownGood in bad? False).

The id() artefact is genuinely fixed. Against round 2's code both halves now fail — including the Schema one that previously passed by address reuse — and the failure message is its own argument for assertIs: "two sets that print identically and compare equal, yet are different objects". The tests also discriminate correctly: the two …_without_bound tests pass against round 2, because round 2 did fix the growth. Two defects, two independent assertions, neither firing on the other's fix.

Settling the UNVERIFIED item — the reviewer's derivation was right, and I measured it:

                              0d197c8a8 (pre-delta)    84faf77bf
__excluded__ before -> after        56 -> 112          56 -> 113
__builtin__                     56, marker absent   57, marker PRESENT
DRIFT in __builtin__                  none          {'__base_ready__'}

So the __excluded__ asymmetry predates this delta — it is 56 → 112 on the old head — and only the +1 is new. The __builtin__ drift is real and new, and its sole observable consequence is name-mangling of a field literally called __base_ready__ on a subclass declared after a bare Info(). Cosmetic, as rated.

Worth noting how long it took me to measure that correctly: my first two probes declared non-finalised subclasses, so __builtin__ was merely inherited from Info and the drift was invisible; the second died with AttributeError: type object 'Before' has no attribute '__builtin__' under a suppressed stderr. __builtin__ only exists once Info itself has been set up, and only a @info_final subclass walks dir() over the MRO. Eighth probe artefact of the session.

One informational item worth a one-line docstring note rather than a change: hand-setting Info.__base_ready__ = True before the first bare construction skips the one-time setup, giving AttributeError: 'Info' object has no attribute '__builtin__' for Info, and the round-1 silent-corruption shape for Schema. It does not defeat the guard or affect subclasses, and it is the same foot-gun that hand-setting __finalised__ = BASE already was — a new instance, not a new kind. __base_ready__ just looks more innocuous than a FinalisedState enum, so "internal, do not set" on the attribute would close it.

UNVERIFIED and accepted: mypy, pylint, the unit tier, the .rst directives under Sphinx, and the ~10-minute whole-package audit (865/2, both outside the hierarchies and untouched by this delta). CI is green on all 27 checks at this sha, which retires the author's earlier caveat — though the body's 1819 passed / 15 skipped still predates it and should say so.

Flipping to review: good-to-go.