Skip to content

fix: resolve relationship refs to explicit override DTOs#176

Open
adiberk wants to merge 5 commits into
gazorby:mainfrom
levco:lev/relationship-dto-resolution-pr
Open

fix: resolve relationship refs to explicit override DTOs#176
adiberk wants to merge 5 commits into
gazorby:mainfrom
levco:lev/relationship-dto-resolution-pr

Conversation

@adiberk

@adiberk adiberk commented Jun 3, 2026

Copy link
Copy Markdown

Summary

Fixes relationship field references that stay pointed at an earlier generated/default DTO when a user-defined override=True DTO for the related model is registered later.

This can happen when one model type is decorated before the explicit related model type has been imported. The relationship field records a reference to the generated DTO, and the later explicit override previously did not satisfy that pending reference.

Change

  • Add RegistryTypeInfo.resolves_scoped_references so a user-defined override=True DTO with the model default name can update pending references for the generated/default DTO of the same model.
  • Preserve existing scoped behavior, including scope="schema" and internal global handling.
  • Add a regression test where FruitNode.color resolves to the later explicit ColorNode instead of a generated ColorType.

Validation

  • uv run pytest tests/unit -q --snapshot-warn-unused -> 205 passed, 2 skipped
  • uv run ruff check src/strawchemy/utils/registry.py tests/unit/mapping/test_schemas.py
  • uv run ruff format --check src/strawchemy/utils/registry.py tests/unit/mapping/test_schemas.py

Related to #175.

Summary by CodeRabbit

  • Enhancements
    • Improved handling of scoped references for generated DTOs, ensuring correct resolution across scopes.
    • More robust updating of forward type references during registration to avoid stale links.
    • Relationship type selection now reliably respects explicitly registered related types, regardless of decoration/import order.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8f695d22-a4b4-42eb-9805-466e070f1f66

📥 Commits

Reviewing files that changed from the base of the PR and between cbc3665 and 6ff2820.

📒 Files selected for processing (2)
  • src/strawchemy/utils/registry.py
  • tests/unit/mapping/test_schemas.py

📝 Walkthrough

Walkthrough

Adds RegistryTypeInfo.resolves_scoped_references and a helper to detect existing forward-ref targets, then updates StrawberryRegistry._register to conditionally register namespace default_name and selectively refresh forward and scoped type references. Adds a unit test asserting explicit related-type registration wins over auto-generated defaults.

Changes

Scoped Reference Resolution

Layer / File(s) Summary
RegistryTypeInfo.resolves_scoped_references property
src/strawchemy/utils/registry.py
New properties compute whether a registration resolves scoped references from model, exclude_from_scope, scope, and a scope is None default-name override flag.
Registry registration logic and selective forward-ref updates
src/strawchemy/utils/registry.py
StrawberryRegistry._register now uses type_info.resolves_scoped_references to decide when to register default_name and to selectively update _forward_type_refs and _type_refs (refresh-all for scope == "global", otherwise update only refs still pointing at the prior default).
Forward-ref helper
src/strawchemy/utils/registry.py
Adds _TypeReference.contains_type(...) to test whether a tracked forward reference currently targets a specific Strawberry type, used to avoid overwriting unrelated tracked refs.
Test: explicit related-type registration
tests/unit/mapping/test_schemas.py
New test test_relationship_uses_late_explicit_related_type_registration registers multiple related DTOs and asserts the schema uses the explicitly registered related type and excludes incorrect/alternate generated types.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

Poem

🐇 I hop through registry rows,
late types find their place,
scoped paths now gently know
which DTO wears the grace.
No more wrong defaults—hooray!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main fix: resolving relationship references to explicitly registered override DTOs, directly matching the core change in the registry logic and test additions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@adiberk

adiberk commented Jun 3, 2026

Copy link
Copy Markdown
Author

Follow-up pushed for the review comment and diff scope.

  • Restored the unrelated unused snapshot deletion, so the PR no longer deletes the large scope_schema_in_the_middle snapshot file.
  • Added a guard so default-name reference rewrites only update references still pointing at the previous generated/default DTO. References that were already resolved to an explicit override are left alone.
  • Added a regression assertion: after FruitNode.color resolves to ColorNode, registering a later alternate Color DTO does not retarget FruitNode.color.

Validation rerun:

  • uv run pytest tests/unit -q --snapshot-warn-unused -> 205 passed, 2 skipped
  • uv run ruff check src/strawchemy/utils/registry.py tests/unit/mapping/test_schemas.py
  • uv run ruff format --check src/strawchemy/utils/registry.py tests/unit/mapping/test_schemas.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/strawchemy/utils/registry.py`:
- Around line 125-135: The boolean in resolves_scoped_references is doing two
distinct checks (explicit global scope vs. implicit override/default-name
takeover); extract the second branch into a clearly named helper property (e.g.,
_is_override_with_default_name) or at minimum add an inline comment explaining
the two branches, then have resolves_scoped_references return the combined
result using that helper and keep the original semantics (referencing
self.model, self.exclude_from_scope, self.scope, self.override,
self.user_defined, and self.default_name to preserve behavior).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 04392e2c-1aa6-474d-b26d-333d9d44a830

📥 Commits

Reviewing files that changed from the base of the PR and between 654b352 and cbc3665.

⛔ Files ignored due to path filters (2)
  • tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[override_with_custom_name].gql is excluded by !**/__snapshots__/**
  • tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[scope_schema_in_the_middle].gql is excluded by !**/__snapshots__/**
📒 Files selected for processing (2)
  • src/strawchemy/utils/registry.py
  • tests/unit/mapping/test_schemas.py

Comment thread src/strawchemy/utils/registry.py
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