[claude-tmgbedu]

Fixes F6, F7 and F8 from task #3547. Design: docs/adr/039-default-relationship-keys-and-db-scaffolding.md.

F6: default relationship keys

Root cause: keys were only inferred in set_keys, which ran on instance access. with_, where_has and attach use class-level access, so they saw None keys (attribute name must be string, no such column posts.None). Several of the existing defaults were also wrong.

Fix: a lazy RelationshipKey data descriptor fills unset keys from Laravel-style default_keys() on first read. Explicit keys always win.

The defaults follow Laravel and come from class names, never from __table__ or singularization:

• A model's foreign key is snake(ClassName)_{pk}, so Media gives media_id and Label with __table__="blog_labels" gives label_id. A custom __primary_key__ is respected.
• HasOne/HasMany: (owner pk, owner FK).
• BelongsTo: ({attribute}_{related_pk}, related pk).
• BelongsToMany: the pivot is the two snake class names sorted and joined (essay_label, label_media). The keys are each side's FK and pk. The alphabetical key swap and the table-based get_pivot_table_name fallback are removed.
• HasOneThrough: (owner pk, owner FK on the intermediate, intermediate pk, intermediate FK on the distant model), using the select alias m_through_key.

Inheritance: reading a relationship through a subclass returns a copy bound to that subclass, which infers its keys from the subclass (KeyInvoice(KeyBaseItem) gives key_invoice_id). The copy is installed on the subclass with setattr, with no side cache, so a dropped subclass can still be garbage collected. A test covers this.

Upgrade notes / Breaking changes

Some default names change for configurations that worked on main. In each case, pass the old name explicitly to keep it.

Case	Before	After	Fix
(a) BelongsToMany without table= between models with a custom __table__	cp_item_cp_shop	cp_item_model_cp_shop_model	table="cp_item_cp_shop"
(b) A subclass sharing its parent's table that inherits a BelongsToMany	cp_person_cp_role	cp_admin_cp_role	table="cp_person_cp_role" on the parent
(c) Irregular plurals	medium_tag	media_tag	table="medium_tag"
(d) HasOne/HasMany default FK	{attribute}_id (posts_id)	{owner}_{pk} (user_id)	foreign_key="posts_id"
(e) Relative sqlite path (DB_DATABASE / DB_URL)	relative to the current working directory	relative to Application.base_path	use an absolute path

The pivot FKs in (a)–(c) follow the same class-name rule; pass local_key=/foreign_key= to keep the old ones.

Not affected:

• relationships with an explicit table=, local_key= or foreign_key=;
• multi-word classes on their default table (BlogPost/blog_posts gives blog_post_id both before and after);
• BelongsTo, HasOneThrough and implicit BelongsToMany reached through the class. These were broken on main.

F7: published database config

Root cause: default="pgsql", but only sqlite was defined.
Fix: default = env("DB_CONNECTION", "sqlite"), with sqlite, mysql and postgres connections.

F8: scaffolding and a fresh-app migrate

• db:make:model now defaults to app/models. All three generators (model, migration, seed) create their target directories.
• A relative sqlite database path resolves against the application base_path, not the working directory. This goes through a new Connection.resolve_url hook. The connection creates the parent directory, skipping :memory: and file: URIs.
• db:migrate treats a missing migrations directory as empty.
• Running provider:publish --provider=database then db:migrate on a fresh app works. So does db:make:model Post -m followed by db:migrate.

Tests

• test_default_relationship_keys.py covers: class-name keys, the pivot, a custom table, irregular plurals, a custom PK, HasOneThrough, inheritance and subclass GC, plus eager loads and where_has against sqlite.
• Database config tests cover: the default connection, each DB_CONNECTION value, a missing directory, and resolving a relative path against base_path.
• There is also a migrator test with a missing migrations directory, and the make-command tests.
• uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3314 passed, 10 skipped, coverage 91.71%.
• ruff and basedpyright are clean on the changed files.

Follow-ups (not in scope)

• HasManyThrough default keys, and the m_through_key alias for it.
• BelongsToMany where_has with a callback (pre-existing).
• has/with_count on the async builder.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[claude-tmgbedu]

Review + QA — head ee12ed2

Verdict: Changes requested (one small fix). Everything else is non-blocking.

Blocking

1. HasOne ignores a custom owner primary key. relationships/HasOne.py:11 still has self.local_key = local_key or "id", so default_keys()["local_key"] never applies. HasMany on the same owner gets this right.
   Repro: class Account(Model): __primary_key__ = "uuid"; setting = HasOne["Setting"](); invoices = HasMany["Invoice"]() gives Account.setting → ("id", "account_uuid") but Account.invoices → ("uuid", "account_uuid"). Eager loading then does relation._get_value("id") / owner.__attributes__["id"] on a model that has no id.
   Fix: self.local_key = local_key. A test with a custom PK would catch it, since every current test uses id.

Non-blocking / for discussion

2. Foreign keys and pivot names come from the table name, not the class name (BaseRelationship.py:99-105). Laravel uses snake(class) + "_" + pk. Two cases differ:
   • Irregular plurals: Media → tableize = media → singularize = medium, so Media.tags gives pivot medium_tag and key medium_id. Laravel gives media_tag / media_id.
   • Custom __table__: with Tag.__table__ = "blog_tags", Tag.posts uses FK blog_tag_id (Laravel: tag_id).
     The ADR states this choice, and old BelongsToMany did the same, so this is not a regression. It should still be a deliberate decision, because changing it after release would break apps.
3. Inherited relationships bind to the base class. owner_class is set in __set_name__, so SpecialItem(BaseItem) inheriting children = HasMany[...]() infers base_item_id. This is an edge case. Document it or leave it as is.
4. Fresh app: provider:publish --provider=database then db:migrate (before any make) still fails: first sqlite3.OperationalError: unable to open database file (no databases/), then FileNotFoundError .../databases/migrations. The ValueError: Connection 'pgsql' is fixed. publish → db:make:model Post -m → db:migrate works with no directories created beforehand. Could be a follow-up: have migrate create the directory or treat it as "nothing to migrate".
5. HasManyThrough still has the old set_keys/"id" defaults, and nothing calls its set_keys. It is out of scope, but it has the same class of bug as F6.
6. m_through_key appears in the distant model's attributes ({'id': 1, 'post_id': 1, 'path': 'a.png', 'm_through_key': 1}). Laravel's laravel_through_key does the same, so this is just FYI.

Verified OK

• Explicit keys still win: HasMany["Post"](foreign_key="author_id") and HasOneThrough(..., local_owner_key="author_id") both work. Keys are only filled when unset.
• F6 in a scratch sqlite app with default tables (users/profiles/posts/media/tags/post_tag), using deprecated string and generic forms:
  • Post.with_("author","tags","media") works.
  • Media.with_("post") works.
  • User.with_("profile","posts","latest_media") works.
  • Lazy await post.load("author","tags","media") and user.load("profile","latest_media") work.
  • Post.where_has("tags") works.
• Inferred keys: Post.author → author_id/id; Post.media → id/post_id; Post.tags → post_tag, post_id, tag_id; HasOneThrough → id, author_id, id, post_id.
• F7: published config/database.py defaults to sqlite and defines sqlite/mysql/postgres.
• F8: db:make:model Post -m writes app/models/Post.py and databases/migrations/..._create_posts_table.py without pre-existing directories. db:migrate succeeds, and from app.models.Post import Post imports (table posts).
• ADR 039 is present and indexed in docs/adr/AGENTS.md, and does not collide with 035–038. The ADR matches the diff against current main. The diff adds no explanatory comments or docstrings.
• Suite: uv run pytest --ignore=tests/masoniteorm/postgres --cov → 3304 passed, 10 skipped, coverage 91.69%.

[claude-tmgbedu]

Re-review + QA — head d4e544d

Verdict: Approve once the docs are fixed. The code fixes all three earlier findings. The class-name defaults are the intended Laravel-parity change, but they rename pivots that work on main today, and neither the ADR nor the PR body warns about this. That missing upgrade note is the only blocker.

Earlier findings: all fixed

1. HasOne custom PK. HasOne.py:11 now reads self.local_key = local_key. Account(__primary_key__="uuid").setting gives ("uuid", "key_account_uuid"). In a scratch DB, with_("setting"), await acc.load("setting"), where_has("setting") and the inverse BelongsTo all return the row.
2. Class-name keys.
   • FK is underscore(ClassName)_pk and the pivot is the sorted snake class names (BaseRelationship.py:101-105, BelongsToMany.py:41-50).
   • The table-based fallback and its 6 call sites are gone.
   • An inherited relationship rebinds to the subclass: KeyInvoice.notes gives key_invoice_id.
3. Fresh app. In a bare temp app with DatabaseProvider:
   • provider:publish --provider=database then db:migrate prints "Nothing To Migrate!" and creates databases/sqlite.db.
   • db:make:model Post -m then db:migrate migrates posts, and from app.models.Post import Post imports.
   • With no databases/migrations directory, db:migrate:status, rollback, refresh and fresh all exit cleanly.

Blocking: breaking default changes need a migration note

I ran the same DB-backed repros against main (via PYTHONPATH) and against this PR. Configs that work on main and fail on this PR:

Case	main pivot	PR pivot	Result on PR
BelongsToMany with explicit keys, no table=, custom __table__ (class ShopModel: __table__="cp_shops" → ItemModel/cp_items)	cp_item_cp_shop	cp_item_model_cp_shop_model	no such table
A subclass that shares its parent's table and inherits a BelongsToMany with no table= (class Admin(Person): __table__="cp_people")	cp_person_cp_role	cp_admin_cp_role	no such table
Irregular plural with a default table (Media)	medium_tag	media_tag	renamed
HasOne/HasMany default FK	{attribute}_id (lazy access only)	{owner}_id	renamed, low impact

The PR's own fixtures hit the first case. tests/masoniteorm/fixtures/typed/owners.py:24 and test_inferred_relationship_model.py:43 had to add table="product_store", because TypedStore/TypedProduct would otherwise resolve to typed_product_typed_store.

Unaffected:

• Relationships that already pass table= and keys.
• Multi-word classes whose table is the default (ProductStore → product_store, same as before).
• Every model in example/, which all use explicit table=.
• BelongsTo, HasOneThrough, and BelongsToMany with all keys implicit. These were broken on main anyway (id/id, None columns, swapped pivot keys), so nothing regresses.

Ask:

• Add a "Breaking changes / upgrade" section to ADR 039 and the PR body. It should list the cases above and give the fix: pass table= (and keys) explicitly, or rename the pivot.
• The PR body is also stale. It still says 3304 tests, and it describes neither the class-name or subclass rules nor the sqlite/migrator changes.

Non-blocking

• BaseRelationship.py:108-118, _bindings cache:
  • Async: safe, because there is no await between the check and the set.
  • Threads: a race can build two equivalent copies and keep the last one. That is benign.
  • Memory: the cache holds a strong reference to every subclass that accesses the relationship. Growth is bounded by the number of classes, but dynamically created model classes (tests, factories) are never collected. Consider weakref.WeakKeyDictionary.
• sqlite_connection.py:18-20:
  • A relative path resolves against the CWD, the same as sqlite itself, not against the app base path. Running artisan from another directory creates databases/ there.
  • When the parent directory is not writable, you get a clear [Errno 13] Permission denied: 'ro/sub' on first DB use. The connection is lazy, so db:make:model still works.
  • :memory: works.
• Pre-existing, unchanged: where_has("books", callback) on BelongsToMany still raises 'QueryBuilder' object is not iterable. HasManyThrough defaults and m_through_key leaking into attributes are unchanged. All of these are already listed as follow-ups.

Verified OK

• Explicit keys and tables always win:
  • BelongsToMany(table="authorships", local_foreign_key="author_ref", other_foreign_key="book_ref")
  • BelongsToMany(table="rv_writer_books"), which infers rv_writer_id/rv_book_id
  • HasOneThrough with all four keys explicit
  • HasMany(foreign_key="owner_id") on a subclass
• Default and custom pivots in the scratch DB: with_(), lazy load() and where_has() pass for BelongsToMany (default and custom pivot), HasOne, HasMany, BelongsTo and HasOneThrough (default and explicit keys).
• AGENTS.md:
  • No comments or docstrings were added. The added Python lines contain no # or """.
  • ADR 039 is indexed.
  • No number collision: open PRs use 035–038 and 040.
• Suite: uv run pytest --ignore=tests/masoniteorm/postgres --cov gives 3312 passed, 10 skipped, coverage 91.70%.

[claude-tmgbedu]

Round-3 review (d4e544df..f5cf9172): APPROVE, one non-blocking doc nit

Tests

• cd fastapi_startkit && uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3314 passed, 10 skipped, coverage 91.71% (needs 80%).
• example/database-app: uv sync fails on this machine because pydantic-core won't build (a local environment problem, not this PR). Instead I ran its artisan db:migrate with the framework venv, DB_CONNECTION=sqlite DB_DATABASE=databases/smoke.sqlite, from a different working directory. All migrations ran, and the file was created at example/database-app/databases/smoke.sqlite (the app's base_path), not under the working directory.

(3) Connection.resolve_url / sqlite base_path

Scratch repro: base_path=proj/, working directory=cwd/.

config	main (e6dee88b)	PR
database="db/a.sqlite"	relative to the working directory (fails with no dir: main doesn't mkdir)	proj/db/a.sqlite ✅
DB_URL=sqlite+aiosqlite:///db/b.sqlite	relative to the working directory	proj/db/b.sqlite (explicit URLs are rewritten too)
:memory:	:memory:	:memory: ✅
absolute path	same	unchanged, parent dir created ✅

• No new crash paths. On main, create_engine already called app().is_testing(), so standalone ORM use without an Application already raised Container not initialized. The PR reads app().base_path in the same place. Application.base_path defaults to os.getcwd(), so apps that don't pass base_path behave exactly as before.
• Postgres and MySQL inherit the identity resolve_url, so they're unaffected. url.set(database=...) keeps query params.
• Published config stub databases/sqlite.db, example/* (base_path=Path(__file__).parent.parent) and mcp-authentication/.env.testing (databases/testing.db): when these run from the project root, which is the usual case, the result is the same as before. When run from anywhere else, they now land in the right place.
• test_default_connection_is_defined was moved from chdir to patching base_path, and the new test checks that nothing is written under the working directory. Good.

Nit (non-blocking): The Upgrade notes / Breaking changes table in ADR 039 and the PR body has no row for this change. It only appears in the design section and in F8. It does change behaviour for any app whose working directory differs from base_path and uses a relative sqlite path, including a relative path inside DB_URL. Suggested row:
(e) relative sqlite path (DB_DATABASE / DB_URL) | relative to the working directory | relative to Application.base_path | use an absolute path.
Optionally, also note that relative file: URIs stay relative to the working directory.

(2) Subclass binding via setattr

Scratch repro (Base(notes, tags) → Mid → Leaf, Multi(Base, Other), Shadow(Base): notes = "user value"):

• Leaf accessed before Mid gives leaf_id, then Mid gives mid_id, and Base stays base_id. Base.__dict__["notes"].owner_class is still Base.
• Leaf.notes is Leaf.notes is True, and class-level access returns the HasMany descriptor.
• No clobbering. A subclass that defines its own notes shadows the parent in the MRO, so the parent's __get__ never runs (Shadow.notes == "user value").
• Multiple inheritance follows the MRO: Multi.notes binds Base's descriptor (multi_id), and Multi.tags._table == "multi_tag". This is correct Python semantics.
• Race: 50 threads read R.notes concurrently and all got the same keys. The losing setattr writes an equivalent copy, so this is harmless.
• __init_subclass__ computes __fillable__ before any binding, and caster only collects FieldDescriptor/ModelField. So the injected attribute doesn't leak into fillable or casts.
• The GC test (weakref + gc.collect) is sound, and the WeakKeyDictionary reasoning in the ADR is correct.

Upgrade table spot-check (main vs PR)

• (c) Media(__table__="media") ↔ Tag: main get_pivot_table_name gives medium_tag, PR gives media_tag ✅
• (d) User.posts = HasMany("Post"): main {attribute}_id (posts_id, from HasMany.py:24), PR user_id ✅. On main, class-level access returned (None, None), which matches the "Not affected" note.

AGENTS.md

No new comments or docstrings. ADR and index updated ✅