Skip to content

fix(utils): walk MRO in get_class_field_annotations#302

Merged
allmonday merged 1 commit into
masterfrom
fix/get_class_field_annotations-mro
Jun 30, 2026
Merged

fix(utils): walk MRO in get_class_field_annotations#302
allmonday merged 1 commit into
masterfrom
fix/get_class_field_annotations-mro

Conversation

@allmonday

Copy link
Copy Markdown
Collaborator

Summary

  • get_class_field_annotations previously read only cls.__dict__['__annotations__'], missing fields inherited from parents.
  • This broke copy_dataloader_kls: the generated class NewLoader(Parent): pass has empty __annotations__ in its own __dict__, so _create_loader_instance could not see or inject the parent's loader params (user_id, permission, etc.), raising AttributeError at runtime.
  • Now walks cls.__mro__ and merges annotations from every class in the chain. The same fix also corrects the _context detection in analysis.py:385, so copies of context-aware loaders are now correctly recognized.

Why explicit MRO walk (not cls.__annotations__.keys())

Python's attribute lookup returns the first __annotations__ found in the MRO. So:

class Parent: user_id: int
class Mixin(Parent): extra: str
Mixin.__annotations__  # {'extra': str} — loses user_id

The explicit reversed-MRO loop merges across all levels, which is what loader param injection needs.

Side-effect check: DataLoader's own annotated fields

Walking the full MRO also surfaces aiodataloader.DataLoader's own annotations (batch, cache, max_batch_size). These all have class-level defaults, so _create_loader_instance skips them via its existing has_default and field not in param_config branch — no behavior change.

Test plan

  • Updated test_get_class_field_annotations_with_inheritance — derived class now includes base field
  • Updated test_get_class_field_annotations (test_utils.py) — D(C) and E(C) now reflect inherited hello
  • Added test_get_class_field_annotations_subclass_with_no_own_fields — covers class Child(Parent): pass
  • Added test_copy_dataloader_kls_inherits_parent_annotations — regression for the reported bug
  • Full suite: 707 passed, 1 skipped

🤖 Generated with Claude Code

get_class_field_annotations previously read only cls.__dict__['__annotations__'],
which misses inherited fields. This broke copy_dataloader_kls: the generated
`class NewLoader(Parent): pass` has no __annotations__ in its own __dict__, so
_create_loader_instance could not see (or inject) the parent's loader params,
raising AttributeError at runtime.

Walk cls.__mro__ and merge annotations from every class in the chain. Also
fixes the requires_context check in analysis.py, which now correctly detects
_context on loader subclasses (e.g. copies of a context-aware loader).

Updated two existing tests that encoded the old behavior, and added a
regression test for copy_dataloader_kls inheriting parent annotations.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@allmonday allmonday merged commit e103842 into master Jun 30, 2026
1 check passed
@allmonday allmonday mentioned this pull request Jun 30, 2026
2 tasks
nshiva-prasad pushed a commit to nshiva-prasad/pydantic-resolve that referenced this pull request Jun 30, 2026
Changelog entry includes the full history of the get_class_field_annotations
fix (KLR-Pattern#302): the 3-year-old latent bug from e637541 (2023-04-06), the
2026-05-14 copy_dataloader_kls refactor (ba8762e) that activated it, why
cls.__annotations__.keys() is insufficient, and the side effects on
_context detection and DataLoader's own annotated fields.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant