Repository navigation
feat(model)!: one declaration surface per fact, declared by accessor - #140
Merged
Merged
Conversation
A model used to declare the same fact twice: once as a class-body dunder and once as the classmethod that read it. Some consumers obeyed an overridden method, the rest read the dunder past it, and nothing said which was authoritative -- so an override could work in one code path and be silently ignored in another. This makes the accessor the declaration. table_name(), schema_name(), primary_key(), pk_auto_generated(), the four locators (deleted_at_field / created_at_field / updated_at_field / version_field), version_increment_by(), read_only(), logger_name(), no_track_fields() and uuid_version() are now declared by overriding them, and their defaults read no dunder at all. table_name() has no default: a model that does not declare one fails with NotImplementedError at the first read instead of carrying a None table name into a statement. The field-level surface gains composite declarations: UseIndex takes an authoritative columns list (the annotated field must be its leftmost column), UseConstraint gains the multi-column form for branches with a table-level counterpart, and index names are uniqueness-checked across both declaration surfaces at class creation. UsePartitionKey adds the partition key as an annotation, extracted in field order, with partition_key_columns() mirroring primary_key_columns(); the direct __partition_key__ tuple remains the entry point for introspected models and declaring both is a TypeError. Three additions round out the declaration story: uuid_version() selects time-ordered uuid7 keys (probed, not imported, since the stdlib gained it in 3.14), natural_key() lets find_one_or_create take its key from defaults, and reload() is the other half of the stale-record recipe -- re-read into the same instance and re-point the optimistic lock so a retry is a retry rather than a second conflict. The soft-delete and partial-unique-index recipe is documented and pinned by a round trip. Breaking, and deliberately so: there is no compatibility layer. Every model in this repository, the testsuite it drives, and nine backend repositories were migrated by an AST-located sweep (kept under scripts/) in the same change, so the family is consistent at this revision. The rules page .claude/rules/declaration-surfaces.md states the policy the code now implements.
|
This was referenced Nov 12, 2026
feat!: declare models by overriding accessors, not by dunder
rhosocial/python-activerecord-oracle#19
Merged
Merged
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.
Description
A model used to declare the same fact twice: once as a class-body dunder and once as the classmethod that read it.
__table_name__held the name,table_name()returned it;__primary_key__held the key,primary_key()returned it. Some consumers obeyed an overridden method, the rest read the dunder past it, so an override could work in one code path and be silently ignored in another — and nothing in the codebase said which of the two was authoritative.This PR makes the accessor the declaration and removes the second source:
table_name(),schema_name(),primary_key(),pk_auto_generated(),deleted_at_field(),created_at_field(),updated_at_field(),version_field(),version_increment_by(),read_only(),logger_name(),no_track_fields()anduuid_version()are now declared by overriding them, and their defaults read no dunder at all.table_name()has no default: a model that does not declare one fails withNotImplementedErrorat the first read, instead of carrying aNonetable name into a statement.__column_types_cache__— assigned but never read — is removed.__pk_auto_generated__or the connection trio directly;pk_auto_generated(),backend_class(),connection_config()andlogging_config()are the single read point.The declaration surface itself is audited and documented in the same change: a new rules page (
.claude/rules/declaration-surfaces.md) states the three surfaces a model may declare on, the accessor defaults, R1–R5, and the migration state of the legacy direct-write faces.Three additions round out the surface:
UseIndex.columns— composite indexes on the field-level annotation surface, anchored on the field's own column (must becolumns[0], the leftmost prefix), validated at class creation.UseConstraintmulti-column form — for branches that have a table-level counterpart (composite UNIQUE / FOREIGN KEY), surfaced throughtable_constraints().UsePartitionKey— partition key as field annotations (extracted in field order) or a direct__partition_key__tuple for introspected models; declaring both is aTypeError; a partition key the primary key does not contain is announced with aUserWarning; the query layer warns when a lookup constrains no partition column and the CLI says the same at filter time.And two capabilities that the accessor surface made possible:
uuid_version()—"uuid4"by default,"uuid7"for time-ordered keys. The v7 generator is probed withgetattr(uuid, "uuid7", None)rather than imported, because the stdlib gained it in 3.14 while this package supports 3.8 onward: selecting it on an older interpreter fails with a message naming the declaration and the requirement.natural_key()— a model declares the fields that identify a row, andfind_one_or_createacceptsdefaultsalone, taking the declared key's values from them. An explicit keyword lookup still wins; a model that declares nothing behaves exactly as before.reload()(sync and async) — the other half of the stale-record recipe: re-read the row into the same instance, reset change tracking, and re-point the optimistic lock at the committed version.Adds # (no issue filed; the work is specified by
.claude/plan/2026-11-11/declaration-surface-governance.md)Type of change
feat: A new featuredocs: Documentation only changes (if related to the new feature)test: Adding missing tests or correcting existing tests (for the new feature)perf: A code change that improves performance (if part of the feature)chore: Changes to the build process or auxiliary tools and libraries suchs as documentation generation (if related to the new feature)ci: Changes to our CI configuration files and scripts (if related to the new feature)build: Changes that affect the build system or external dependencies (if related to the new feature)revert: Reverts a previous commitBreaking Change
Impact on End-Users:
The ten declaration dunders no longer declare anything. A model must override the matching accessor instead:
__table_name__ = "users"@classmethod def table_name(cls): return "users"__schema_name__ = "crm"schema_name()__primary_key__ = ("a", "b")primary_key()returning the tuple (Nonefor a keyless model)__pk_auto_generated__ = Falsepk_auto_generated()returningFalse__deleted_at_field__/__created_at_field__/__updated_at_field__deleted_at_field()/created_at_field()/updated_at_field()__version_field__/__version_increment_by__version_field()/version_increment_by()__read_only__ = Trueread_only()(inheritingReadOnlyMixinalready answersTrue)A model with no
table_name()override now fails loudly withNotImplementedErrorinstead of running with aNonetable. There is no compatibility layer: this is a pre-1.0 breaking migration, made once and completely.__partition_key__remains a valid declaration (the entry point for introspected models);UsePartitionKey()is the other face, and declaring both is aTypeError.Impact on Backend Developers (Custom Implementations):
table_indexes()/table_constraints()now run the same unhandled-annotation gate every field-level accessor runs.DDLSourcegainspartition_key_columns(); the locator accessors are declared by the field mixins rather than byDDLSourceMixin.DDLAnnotationHandler.annotation_typesrather than replace it:UsePartitionKeyis now part of the core tuple.Related Repositories/Packages
Coordinated migration, all on
feature/declaration-surface, all verified against this core branch by their own CI:python-activerecord-testsuitepython-activerecord-mysqlpython-activerecord-postgrespython-activerecord-clickhousepython-activerecord-oraclepython-activerecord-sqlserverpython-activerecord-mariadbpython-activerecord-firebirdpython-activerecord-snowflakepython-activerecord-bigqueryEvery workflow on these branches installs core, testsuite and the backend under test from the matching
feature/declaration-surfaceref, so a run exercises one revision across the family.Testing
Test Plan:
test_ddl_composite_annotations.py(21 cases),test_ddl_partition_key.py(21),test_key_generation_and_natural_key.py(15),test_reload_stale_recipe.py(10, sync+async twins),test_soft_delete_partial_unique.py(5, round trip).UsePartitionKey;table_indexes()/table_constraints()carry the unhandled-annotation gate;pk_auto_generated()/logger_name()/no_track_fields()are pinned against their defaults.feature/domain 12103 passed, 156 skipped; the only failures (4) are Windows temp-file-lock issues that fail identically on the base commit.Checklist
README.md, etc.).docs/and.claude/rewritten to the method form; user-visible errors now name the accessor.changelog.d/<issue_number>.<type>.md).added+removed; the declaration removal is the breaking half).pk_auto_generated(),column_name()), and no new per-row work was added.Additional Notes
Evaluation evidence (
.claude/skills/dev-merge-evaluation): purpose achieved (plan P0–P4 all ✅); topologymain ⊆ release ⊆ featureholds; branch protection is a real ruleset with PR + status checks + linear history + signatures (note: anorg adminbypass actor exists, so history should not be read as "everything was reviewed"); backend adaptation matrix complete (0 stale symbol references in all nine backends, testsuite and devtools); expression lineage gate clean (iron rule 0,placement 0,integrity 0; the twodanglingTYPE_CHECKING imports are byte-identical to the release baseline).Migration tooling is kept in
scripts/(d5_migrate_declarations.py,d5_migrate_docs.py,d5_migrate_embedded.py,p3_fix_reads_and_writes.py): the sweep is AST-located rather than regex-matched, preserves line endings and spacing, and is the reference for any repository still to migrate.