fix(resolver): port v5.10.3 state isolation + collector + context fixes#88
Merged
Conversation
Three bug fixes from pydantic-resolve v5.10.3 that apply to nexusx:
#289-2 (concurrent resolve guard): Resolver holds per-call mutable state
on self (_node_collectors, _loader_cache, levels list). Two overlapping
resolve() calls on the same instance clobbered each other and surfaced
as a cryptic KeyError. Added _in_resolve flag + try/finally that raises
a clear RuntimeError instead.
#293 (Collector subclass preservation): _phase_b_prepare_collectors
hardcoded `Collector(alias=alias, flat=flat)` when instantiating
per-node collectors, silently downgrading any ICollector implementation
or Collector subclass with extra __init__ config (key_fn, n, dict-valued
aggregator) to the base Collector. Now deepcopies the prototype, and
the isinstance check widened from Collector to ICollector so direct
ICollector implementations are detected.
#291 (context input validation): Resolver(context={}) was silently
coerced to {} via `context or {}`, and non-dict values (Resolver(context=
[1,2,3])) slipped through to confuse _traverse. Now rejects empty dict
with ValueError and non-dict with TypeError at __init__.
Also evaluates the other two v5.10.3 fixes — #290 (Timer.timeset leak)
and #292 (inherited __pydantic_resolve_expose__ via getattr MRO) — and
documents why neither applies to nexusx (no Timer module; expose scan
reads model_fields.metadata, not class-level __xxx__ attrs).
Tests: +17 migrated from pydantic-resolve
- test_resolver_concurrency.py (4): concurrent reject, sequential reuse,
guard resets after exception, separate instances concurrent ok
- test_context.py::TestCollectorSubclass (5): MapCollector dedupe,
sibling branch isolation, sequential no-leak, TopNCollector preserves
n, SimpleSubCollector backward-compat baseline
- test_resolver_context_validation.py (8): None / non-empty dict allowed,
empty dict ValueError, 5 parametrized non-dict TypeError
Full suite: 1025 passed, 6 skipped (3.14-only).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three bug-fix ports from pydantic-resolve v5.10.3 that apply to nexusx's Resolver / Collector surface:
Resolverholds per-call mutable state onself(_node_collectors,_loader_cache, thelevelslist inside_traverse). Two overlappingresolve()calls on the same instance clobbered each other and surfaced as a crypticKeyError. Added a_in_resolveflag +try/finallythat raises a clearRuntimeErrorinstead._phase_b_prepare_collectorshardcodedCollector(alias=alias, flat=flat)when instantiating per-node collectors, silently downgrading anyICollectorimplementation orCollectorsubclass with extra__init__config (key_fn,n, dict-valued aggregator) to the baseCollector. Nowdeepcopys the prototype, and theisinstance(param.default, Collector)check widened toICollectorso direct implementations are detected.Resolver(context={})was silently coerced to{}viacontext or {}, and non-dict values slipped through to confuse_traverse. Now rejects empty dict withValueErrorand non-dict withTypeErrorat__init__.The other two v5.10.3 fixes were evaluated and intentionally skipped — see
Test plannotes.Test plan
tests/test_resolver_concurrency.py— 4 new tests for the concurrent-call guardtests/test_context.py::TestCollectorSubclass— 5 new tests migrated from pydantic-resolvetest_collector_subclass.pytests/test_resolver_context_validation.py— 8 new tests for context validation (5 parametrized)ruff checkcleanN/A fixes (documented for reviewers):
Profile.Timer.timesetleak) — nexusx has noTimer/ profile module; only useslogging.__pydantic_resolve_expose__viagetattrMRO false-positive) — nexusx'sscan_expose_fieldsreadsmodel_fields[field].metadatadirectly, notgetattr(kls, '__xxx__'), so the MRO false-positive doesn't exist.object_level_collect_alias_map_storeaccumulating across calls) — already handled:_node_collectors.clear()and_loader_cache.clear()run at the top of everyresolve().dict.get(k, expensive_default)evaluating default eagerly) — no such pattern in nexusx's resolver (audited every.get(call).🤖 Generated with Claude Code