refactor(models): split entities.py into one model module per domain - #1255
Conversation
|
Important Review skippedToo many files! This PR contains 186 files, which is 86 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (186)
You can disable this status message by setting the 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. Comment |
…ase.py `Base` lived in entities.py and `UtcDateTime` plus the SQLModel mixins lived in tenancy.py, so every model module imported one or both of those files for a type rather than a table. Splitting the tables by domain (#1192) needs a neutral home for them first, or each new domain module would import another domain to get a column type. Pure move: no table, column, constraint, or class name changes.
First domain out of entities.py (#1192). Importers name the domain module, and the package import list registers it so the metadata stays whole. Pure move: the table, its columns and constraints, and the class name are unchanged. The `workspace_id` comment is cut to what a reader needs, since its pointer to "the three tables below" no longer held.
Budgets, scoped ceilings, the reservation ledger, reset logs, workspace budget defaults, and MAX_COUNT_LIMIT leave entities.py for their domain module (#1192). Importers name the domain module, and the package import list registers it. Pure move: tables, columns, constraints, and class names are unchanged.
…guardrails.py The guardrails domain already had a model module for the request-body field, so its two tables join it rather than a second module (#1192). The package import list now registers it, since it declares tables. Pure move: tables, columns, constraints, and class names are unchanged. One comment that leaned on "its neighbors", which stayed behind, now states its own reason.
Batches belong to the inference domain (#1192). The package import list registers the new module and moves to one module per line, since it no longer fits on one. Pure move: the table, its columns, and the class name are unchanged.
…m.py Runtime settings belong to the platform domain (#1192). Importers name the domain module, and the package import list registers it. Pure move: the table, its columns, and the class name are unchanged.
The deployment price list, organization overrides, catalog snapshots and their history, and the unit and origin vocabularies leave entities.py for their domain module (#1192). Importers name the domain module, and the package import list registers it. Pure move: tables, columns, constraints, and class names are unchanged. One comment that pointed at the tables "above", which stayed behind, now states its own reason.
…models/providers.py Runtime provider instances and model aliases belong to the providers domain (#1192). They get a domain-named module beside provider_keys.py, which keeps its name until the providers domain is reshaped on its own. Importers name the domain module, and the package import list registers it. Pure move: tables, columns, constraints, and class names are unchanged. Docstrings that named these classes through entities now name their module, and one comment drops the word "above". Three of those docstrings belong to request schemas, so the OpenAPI spec and the dashboard client are regenerated with the new module names.
The routing domain already had a model module for the policy schema, so stored policies, routing memory, and router preferences join it rather than a second module (#1192). The package import list now registers it, since it declares tables, and its `__all__` names the three tables. Pure move: tables, columns, constraints, and class names are unchanged.
Search tool credentials, uploaded file metadata, and the workspace MCP server, code execution, and web search tables belong to the tools domain (#1192). They get a domain-named module beside mcp.py, which stays a request schema module. Importers name the domain module, and the package import list registers it. Pure move: tables, columns, constraints, and class names are unchanged. Docstrings that named these classes through entities now name their module, and two comments that leaned on tables elsewhere now state their own reason. One of those docstrings belongs to a request schema, so the OpenAPI spec and the dashboard client are regenerated with the new module name.
…ls/usage.py Usage rows and coding-agent telemetry belong to the usage domain (#1192). Importers name the domain module, and the package import list registers it. Pure move: tables, columns, constraints, and class names are unchanged.
…to models/tenancy.py Dashboard sessions and the setup guide's per-workspace state belong to the tenancy domain (#1192), so they join its model module as its two declarative tables. The module docstring now says why both styles sit in one module, and `__all__` names the two tables. Pure move: tables, columns, constraints, and class names are unchanged. One comment that pointed at a table "above", now in budgets, states its own reason.
…d drop entities.py The gateway's string-keyed `users` table is the last one in entities.py (#1192). It gets its own module rather than joining tenancy.py, because tenancy.py already declares the sign-in `User` and the two are different identities whose convergence is still undecided. With it gone entities.py is empty, so the module and its entry in the package import list go too. Pure move: the table, its columns, and the class name are unchanged.
With entities.py gone (#1192), the gateway and skill guidance, the architecture overview, and two model docstrings named a module that no longer exists. They now say where a table goes: the model module of its domain, registered in the package import list. The gateway guidance also names the two `User` classes, since the split puts them in different modules and the names alone do not say which identity is which.
4fe7cf6 to
31bbec6
Compare
tbille
left a comment
There was a problem hiding this comment.
Verified independently on the PR head: the full SQLAlchemy metadata (all 48 tables: columns, types, defaults, FKs + ondelete, constraints, indexes, mappers) is byte-identical to main; all 138 top-level definitions from the old modules exist with AST-identical code; every changed line outside models/ and docs is an import rewrite; the model-module dependency graph is acyclic (table modules depend only on base, money, secret_fields); lint, architecture check, and single-head check pass locally.
One leftover the sweep missed, in a file this PR does not touch so it cannot carry an inline comment: alembic/versions/a7d3f5b1c9e4_add_organization_model_pricing.py line 11 still points readers at gateway.models.entities.OrganizationModelPricing, which this PR deletes. Should read gateway.models.pricing.OrganizationModelPricing. Fine as a follow-up commit here or a separate one-line PR.
Reviewed by Claude Code (Fable 5), run by the reviewer.
Description
Every table the gateway stores lived in one 2,016-line models file, so nearly every feature that touched the database edited it: 52 commits from 6 authors in the last 90 days. This moves each table into the model module of the domain it belongs to (budgets, pricing, tools, and so on), and moves the shared base types into a module of their own.
Nothing changes for someone running Otari. The database schema, the API, and the dashboard behave exactly as before, and no migration is added. The only wire-visible difference is three schema descriptions in the OpenAPI spec that named the old module.
Open PRs that add or edit a table will conflict with this. The update is mechanical; see "Updating an open PR" below.
Updating an open PR
src/gateway/models/<domain>.py.Base,UtcDateTimeand the SQLModel mixins come fromgateway.models.base.src/gateway/models/__init__.py. Without it, Alembic does not see its tables.from gateway.models.entities import Xbecomesfrom gateway.models.<domain> import X. The billingUser(tableusers) is nowgateway.models.users.User. The dashboard sign-inUser(tableuser) stays ingateway.models.tenancy.hybridmode #1185 (the package import list).Downstream: otari-ai. The enterprise overlay imports the old module in its runtime and tests, and three of its migration drift guards read the old file. A matching change is prepared and has to land with the otari pin bump to the first release that contains this PR. Nothing breaks before that bump, because the pin does not move on its own.
Measures
entities.pyentities.py, 2,016 linestenancy.py, 1,588 linesThe issue's own measure (
entities.pyleaves the 90-day churn list) can only be read once 90 days have passed.How to test it locally
To confirm the schema did not move, migrate a PostgreSQL database to head and run
uv run alembic checkonmainand on this branch. Both report the same single item, a duplicate unique constraint onapi_keys.key_hashthat already exists onmain, and nothing else. That item is why the issue's "no drift" criterion is not literally met; fixing it needs a migration, which this PR must not add.What was checked:
make lint,make typecheck,make openapi-check,make postman-check, and a clean import of every model module and app entry point in a fresh interpreter.CREATE TABLEandCREATE INDEXstatements for all 48 tables on SQLite and PostgreSQL, plus every mapper, column attribute and relationship, matchmainbyte for byte.test_mcp_dependency_ceiling, fails locally because this machine's Python cannot build the throwaway virtualenv the test creates. It fails the same way onmain.PR Type
Relevant issues
Fixes #1192
Part of #1171
Checklist
tests/unit,tests/integration).make lint,make typecheck,make test).uv run python scripts/generate_openapi.py).No tests were added: behavior does not change, and the existing suites cover it. Test files changed only their imports. The API contract did not change either; the spec, Postman collection and dashboard client were regenerated because three schema descriptions named the old module.
AI Usage
AI Model/Tool used:
An AI coding agent, run by the author.
Any additional AI details you'd like to share:
The author made the layout decisions: one module per domain; a separate module for the billing
Userrather than renaming eitherUser; new domain modules beside the existing ones whose names do not match a domain (provider_keys,playground,mcp), which keep their names for now; deleting the old module rather than keeping a re-export shim; and a shared base module. The agent did the moves and import rewrites, wrote the schema comparison run at every commit, and drafted this description.NOTE:
When responding to reviewer questions, please respond yourself rather than copy/pasting reviewer comments into an AI and pasting back its answer. We want to discuss with you, not your AI :)