Repository navigation
Assign every v2 route and service file to a layer and enforce the v2 layer rules - #4191
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4191 +/- ##
===========================================
+ Coverage 53.84% 88.44% +34.59%
===========================================
Files 283 297 +14
Lines 16502 17960 +1458
Branches 2681 2975 +294
===========================================
+ Hits 8886 15885 +6999
+ Misses 6876 1347 -5529
+ Partials 740 728 -12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
anth-volk
force-pushed
the
v2-architecture-layer-check
branch
from
November 4, 2026 16:57
80e0f9d to
d18175c
Compare
scripts/rules/tests/test_v2_layers.py specifies a quality guard over every .py file under services/v2 and fastapi_routes/v2: - each file is assigned a layer by its name or by rules/v2-layer-manifest.toml, and an unassigned file fails; - route modules import no SQLAlchemy, SQLModel, connector, session or v2 database module and call no SQL builder or session method; - services build no SQL, call no data-access session method and import no FastAPI or Starlette; - connectors don't open, commit, roll back or close a session. Calls count only on a receiver named like a session (session.add, not set.add), and SQL builders only when imported from SQLAlchemy or SQLModel. Imports under TYPE_CHECKING count. Proof fixture trees for the four rules include a route that imports a connector and a file with no layer. These fail until scripts/rules/v2_layers.py exists. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add the v2-layers quality guard (scripts/rules/v2_layers.py, standard library only), run by scripts/run_quality_guards.py in the quality-guards job. - It discovers every .py file under services/v2 and fastapi_routes/v2 and takes its layer from its file name, or from rules/v2-layer-manifest.toml for the 9 files with other names. A file with neither fails: new rule api.v2.files-assigned-to-layers. - api.v2.routes-no-sessions-or-sql is ratcheted. Its baseline lists the 4 route modules that import sqlalchemy.exc to catch SQLAlchemyError. - api.v2.services-no-sql and api.v2.connectors-never-commit are enforced, with no findings today. - Session calls count only on a receiver named like a session, and SQL builders and session factories only when imported from SQLAlchemy, SQLModel or the v2 database module. Imports under TYPE_CHECKING count. Each rule has proof fixture trees, and v2-code-organization.md cites each rule id. The test fix resolves "from .. import database_session" to the resource's own session module, as Python does. Fixes #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Validators, transformations, types, process-state modules and package __init__ files touch no session: new rule api.v2.pure-modules-no-session-calls, with proof fixtures. A database_connectors/__init__.py counts as a connector. - The repository's own session factories count: the v1 ones in policyengine_api.data.orm, build_v2_session_factory, the engines, and a database_session container's transaction() in a route. - Services import nothing from SQLAlchemy or SQLModel but the Session type and sqlalchemy.exc, so and_, or_, func and col count, as does a select() on a model's __table__. - A name that only contains "session" (sessions_router, session_ids, request.session) is not a session. - Each occurrence of a finding needs its own baseline entry, and a duplicated entry is stale. - The manifest gives only the layers of the file's directory, never "package", and every entry needs a reason. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- New rule api.v2.pure-modules-no-session-calls: validators, transformations, types, process-state modules and package __init__ files call no session method, SQL builder or session factory. A database_connectors/__init__.py is checked as a connector. - Session factories are any call into policyengine_api.data.orm or policyengine_api.data.v2.database, or SQLAlchemy's Session, sessionmaker and create_engine. Routes and connectors may not import the v1 ORM module, and a route may not call a database_session container's read() or transaction(). - Services import nothing from SQLAlchemy or SQLModel but the Session type and sqlalchemy.exc, so and_, or_, func and col are caught, and a builder called on a model's __table__ counts. - A session receiver is named session or <name>_session, and request.session and http_session are not sessions, so sessions_router and session_ids are not flagged. - Each occurrence of a finding needs its own baseline line, and a duplicated line is reported as no longer applying. - The manifest may give only the layers of the file's directory, and every entry needs a reason. - v2-code-organization.md has a "File layers" section, which the files rule's rationale now links to. The real code still has exactly the 4 tolerated sqlalchemy.exc imports. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Routes and the pure layers may call no session method at all, so get_one, bulk_save_objects, connection and expunge count; services' and connectors' lists gain get_one, the bulk methods, reset and invalidate. - A manifest file in a pure or HTTP layer gets that layer's import and call checks, including session factories. - A database_session.py below a resource package has no layer. - Every call into SQLAlchemy or SQLModel but its exception classes counts, so "import sqlalchemy.exc" doesn't open sqlalchemy.and_. - request.session on any request object, and client or requests sessions, are not database sessions. - Only a builder called on <Model>.__table__ counts, once. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Routes and the pure layers may call no method on a session, so a method missing from a list can't slip through. Services' and connectors' lists gain get_one, the bulk methods, connection, expunge, reset and invalidate. - The pure layers also get import checks (nothing from SQLAlchemy or SQLModel but the Session type; no connector, session, v1 ORM or v2 database module), so a file the manifest assigns to one is checked as fully as a named one. - A database_session.py below a resource package has no layer, so it can't escape the session-module rule. - Every call into SQLAlchemy or SQLModel counts in routes, services and the pure layers, except to its exception classes, so "import sqlalchemy.exc" no longer opens sqlalchemy.and_. - A request's .session and client or requests sessions are not database sessions; only <Model>.__table__.<builder>() counts as a table builder. - The pure rule's scope leaves out the connector package __init__ files, which are checked as connectors. The docs list what the guard recognises and what it doesn't. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- An injected session factory opens a session: session_factory.begin() and self._session_factory() count in connectors and routes, and so does a database_session container's transaction() in a connector. - New rule api.v2.session-modules-no-data-access: a database_session module opens, commits, rolls back and closes sessions, but adds, gets, merges or executes nothing and builds no SQL. Proof fixtures included. - A request's .session is recognised by its owner (req, websocket, *_request), so request_context.session and request_session are database sessions. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Injected session factories open sessions: session_factory(), self._session_factory() and session_factory.begin() count in routes, connectors and the pure layers, and a connector may not call a database_session container's read() or transaction(). - New rule api.v2.session-modules-no-data-access: database_session modules call no data-access session method or SQL builder and import no connector. Today's modules only open, commit, roll back and close. - A request's .session is recognised by its owner (req, request, websocket, ws, *_request), so request_context.session and request_session are database sessions, and requests_session is not. - The docs say services may open sessions, which their rule doesn't forbid. The real code still has exactly the 4 tolerated sqlalchemy.exc imports. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
force-pushed
the
v2-architecture-layer-check
branch
from
November 5, 2026 12:08
5af955d to
efb919d
Compare
…ling) - New name rules: process_state.py is process-state, and under fastapi_routes/v2 any <name>_models.py is http-model and any <name>_support.py is http-support. - A file whose name gives no layer can only be allowed by a [[rule.exception]] for api.v2.files-assigned-to-layers. The exception chooses no layer: the file gets its directory's strictest checks, the route checks under fastapi_routes/v2 and the pure-module checks under services/v2. An exception for a missing file, a file whose name already gives a layer, a glob, or a path outside the two directories fails. - The files rule's proof shows both an unassigned name and two excepted files getting the strictest checks. - The tests import the eight renamed modules by their new names: process_state, errors_support, user_auth_support, user_resources_support, route_support, query_support, response_cache_support and search_file_support. These fail until the guard reads exceptions instead of the manifest and the files are renamed. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The v2-layers guard no longer reads rules/v2-layer-manifest.toml, which
is deleted. New name rules: process_state.py under services/v2, and
<name>_models.py and <name>_support.py under fastapi_routes/v2.
- A file that can't be renamed needs a [[rule.exception]] for
api.v2.files-assigned-to-layers in rules/registry.toml. It gets no
layer, but its directory's strictest checks; an exception for a missing
file, a file whose name gives a layer, a glob or a path outside the two
directories fails. None is needed today.
- Renamed, with every import updated:
- services/v2/metadata/selected_catalogs.py -> process_state.py
- fastapi_routes/v2/errors.py -> errors_support.py
- fastapi_routes/v2/user_auth.py -> user_auth_support.py
- fastapi_routes/v2/user_resources.py -> user_resources_support.py
- fastapi_routes/v2/metadata/common.py -> route_support.py
- fastapi_routes/v2/metadata/query_dependencies.py -> query_support.py
- fastapi_routes/v2/metadata/response_cache.py ->
response_cache_support.py
- fastapi_routes/v2/metadata/search_file_response.py ->
search_file_support.py
- v2-code-organization.md, the registry, the routes baseline and the
rendered rules.md and AGENTS.md follow.
Refs #4190
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… (failing) - The proofs compare "<rule id>: <path> <detail>", so a finding reported under the wrong rule fails the rule's own proof. - The exception tests require their registry to load without problems; an exception needs an expires date or a removal condition as well as a path and a reason, which the fix message must say. - An unassigned file is told only the names that give a layer in its directory. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Each bad proof tree lists its findings as "<rule id>: <path> <detail>", so a finding reported under the wrong rule fails the proof. The files rule's tree shows its excepted files' findings under the route and pure-module rules. - The files rule's fix message names only the layers of the file's directory, and says an exception needs an expires date or removal condition as well as a path and a reason; the test helper's exceptions now carry one and load cleanly. - The registry statements say which checks an excepted file gets, and the pure-module rule's covers excepted files under services/v2. - The Import Linter routes fixture uses route_support.py instead of the old common.py name. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rules-registry.md reserves an exception's removal condition for rules whose guard reads it, which the v2-layers guard doesn't, so a files-rule exception should end on an expires date. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The files rule's fix message and v2-code-organization.md ask for an exception's path, reason and expires date; rules-registry.md reserves removal conditions for rules whose guard reads them. - The guard, its fixtures and the docs say an excepted file gets the pure-module checks under services/v2 and the route checks under fastapi_routes/v2, instead of its directory's "strictest" checks, which overstated the services case. - Two fixture comments no longer mention the deleted manifest. Refs #4190 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er-check # Conflicts: # docs/generated/rules.md # scripts/run_quality_guards.py
anth-volk
marked this pull request as ready for review
November 5, 2026 14:31
anth-volk
added a commit
that referenced
this pull request
Nov 5, 2026
Brings in #4191's v2 layer check. scripts/run_quality_guards.py keeps both the suppressions and v2-layers guards; docs/generated/rules.md and AGENTS.md were rendered again from the merged registry. #4191 moved the unexplained `from fastapi import Request # noqa: TC002` from fastapi_routes/v2/user_resources.py to user_resources_support.py, so its baseline entry now names the new file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Nov 5, 2026
Brings in master (#4191's v2 layer check and #4020) through #4183's branch. scripts/run_quality_guards.py keeps both the asserting-tests and v2-layers guards; docs/generated/rules.md and AGENTS.md were rendered again. #4020 removed tests/unit/v2/test_catalog_extraction_failures.py::test_national_region_without_a_dataset_path_uses_the_release_default, so its baseline entry no longer applies: it is removed and the rule's baseline count goes from 22 to 21, with the testing skill and the changelog fragment updated to match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 5, 2026
This branch was previously deployed
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.
Fixes #4190
Summary
Adds the
v2-layersquality guard. It gives every API v2 route and service file a layer from its file name alone, and checks each layer's imports and calls. It turns threedoc-onlyv2 rules into checked ones and adds three rules. This is step 2.6 of the agent-proof code-quality plan (its AST part), and builds on #4184 (Import Linter, merged).Eight files whose names gave no layer are renamed to fit. Open pull requests that import them need updating; see below.
What the guard checks
scripts/rules/v2_layers.py(786 lines) assigns the layers, reads exceptions, scans the files, runs the per-file import and call checks and compares findings with the baselines. #4205 moves the per-file checks to a newscripts/rules/v2_layer_checks.py, so that both stay under the 800-line limit. The guard uses the standard library only, like the rest of the rules tool, andscripts/run_quality_guards.pyruns the guard in thequality-guardsjob ofpr.yml.api.v2.files-assigned-to-layers(new).pyfile underpolicyengine_api/services/v2andpolicyengine_api/fastapi_routes/v2gets a layer from its file name. A file whose name gives none fails.api.v2.routes-no-sessions-or-sql(wasdoc-only)fastapi_routes/v2.database_sessionmodule, the v1 ORM modulepolicyengine_api.data.ormorpolicyengine_api.data.v2.database.database_sessioncontainer'sread()ortransaction().*session_factory.api.v2.services-no-sql(wasdoc-only)services.pymodules.- From SQLAlchemy and SQLModel, they import only the
- They call no SQLAlchemy or SQLModel function except its exception classes and session factories. This catches
- No data-access session methods:
- No FastAPI or Starlette imports.
Services may open sessions, which the rule's statement doesn't forbid.Sessiontype andsqlalchemy.exc.and_,or_,funcandcol, even throughimport sqlalchemy.exc.exec,execute,add,get,get_one, the bulk methods,connection,deleteand so on.api.v2.connectors-never-commit(wasdoc-only)database_connectors/__init__.py.commit,rollback,close,begin,begin_nested,resetorinvalidateon a session.session_factory, no containerread()ortransaction().database_sessionmodule.api.v2.session-modules-no-data-access(new)database_session.pymodules sit directly in their resource package.- No data-access session methods (
- No SQL builders.
- No connector imports.
They open, commit, roll back and close sessions.add,get,merge,exec,executeand so on).api.v2.pure-modules-no-session-calls(new)__init__files, and files underservices/v2that a[[rule.exception]]names.Sessiontype.The fourth
doc-onlyrule,api.v2.connectors-no-cross-calls, became ratcheted in #4184 through Import Linter's independence contract.Layers by file name (no manifest):
services/v2/<resource>/:services.py,validators.py,transformations.pyandtypes.py;process_state.py, for data a service keeps for the process;database_session.py, which must sit directly in its resource package, whereapi.v2.session-modules-no-queries(pytest) also checks it;database_connectors/<creates|reads|updates|deletes>[_name].py.fastapi_routes/v2/:routes.pyand<name>_routes.py;<name>_models.py, for request, response and document schemas;<name>_support.py, for HTTP code the route modules share.__init__.pyanywhere.The fix message for an unassigned file lists only the names that give a layer in its directory. It doesn't say where each must sit (for example,
database_session.pyonly directly in a resource package); #4205 adds that.A file that can't be renamed needs a
[[rule.exception]]forapi.v2.files-assigned-to-layersinrules/registry.toml, with a path, a reason and anexpiresdate. The fix message andv2-code-organization.mdask for these. None is needed today.expiresonly when an exception sets neithercategorynorremoval, and the guard doesn't check for it. So an exception that sets either and noexpiresis applied and never ends.match, so an exception withmatchstill covers the whole file.database_connectorspackage, thoughapi.v2.crud-checks-cover-every-resourcestill fails a connector module that isn't named after its operation.expiresdate fails the rules guard once that date passes, through the registry's existing expiry check.fastapi_routes/v2, or the validator, transformation and types checks underservices/v2. The latter don't forbid FastAPI imports, unlike the service checks.Renamed (8 files), with every import updated:
services/v2/metadata/selected_catalogs.pyprocess_state.pyfastapi_routes/v2/errors.pyerrors_support.pyfastapi_routes/v2/user_auth.pyuser_auth_support.pyfastapi_routes/v2/user_resources.pyuser_resources_support.pyfastapi_routes/v2/metadata/common.pyroute_support.pyfastapi_routes/v2/metadata/query_dependencies.pyquery_support.pyfastapi_routes/v2/metadata/response_cache.pyresponse_cache_support.pyfastapi_routes/v2/metadata/search_file_response.pysearch_file_support.pymetadata/common.pybecameroute_support.pyrather thancommon_support.py: it runs the metadata reads for the route modules, maps their failures to HTTP, and sets the caching headers.Effects of the renames:
/v2/openapi.jsonis unchanged.user_resources_support.pyis named after its module, so its name changes frompolicyengine_api.fastapi_routes.v2.user_resourcesto...user_resources_support. No tracked log filter uses the old name.households/document_models.pyalready matched*_models.py.How calls are recognised.
sessionor<name>_session..sessionof a request or websocket (Starlette's session data, for receivers namedrequest,req,websocket,wsor*_request), and HTTP or client sessions such ashttp_session,requests_sessionand_client_session.request_sessionandrequest_context.sessiondo count.set.add,cache.get,sessions_router.getandsession_ids.adddon't count.<Model>.__table__.<builder>().policyengine_api.data.ormorpolicyengine_api.data.v2.databasecounts, as do SQLAlchemy'sSession,sessionmakerandcreate_engine, and an injected*session_factory(session_factory(),self._session_factory(),session_factory.begin()).from package import submoduleare resolved. Imports made only underTYPE_CHECKINGcount (decided 2026-11-04).Tolerated (baseline
rules/baselines/v2-layers-routes.txt, 4 entries, one per occurrence).sqlalchemy.excto catchSQLAlchemyError:countries/routes.pyhouseholds/routes.pypolicies/routes.pyuser_resources_support.pyLimits.
sessionin it isn't recognised.TABLE = Policy.__table__) isn't recognised.Import Linter (#4184) covers the import side transitively.
Open pull requests this affects
I scanned each open PR's head. Once this merges, each needs these changes. The registry baselines can't grow, so each PR must clear its own new findings.
fastapi_routes.v2.errors(incountries/andreports/routes.pyandresponse_models.py) and offastapi_routes.v2.user_resources(inreports/routes.py). They raiseModuleNotFoundErroruntil they use the new names."policyengine_api/fastapi_routes/v2/user_resources.py"in its newtests/unit/v2/test_simulation_entrypoint_contract.py. That test reads the file, so it would fail withFileNotFoundError.errors.pyanduser_resources.py. A simulated merge carries both edits onto the new names without conflict.masterincountries/routes.pyandcountries/response_models.py, which this PR doesn't cause.fastapi_routes/v2/reports/routes.pyimportssqlalchemy.exc, which failsapi.v2.routes-no-sessions-or-sql. The service should raise typed failures for the route to map.fastapi_routes/v2/reports/household_response_models.pynow gets a layer from the*_models.pyrule and needs nothing.from policyengine_api.fastapi_routes.v2.metadata import common, which raiseImportErrorafter the rename.metadata/common.py, which this PR renames toroute_support.py; a merge reports a modify/delete conflict there.query_dependencies.py,response_cache.pyandselected_catalogs.pyfrom its old merge base, with no edits relative tomaster. A merge silently keeps those copies alongside the renamed files, and the files rule then flags them. Delete them rather than moving anything.services/v2/household_calculations/simulation_client.pymatches no layer name. Either move it out ofservices/v2(for example topolicyengine_api/libs/), or give it a[[rule.exception]]with anexpiresdate; it then gets the validator, transformation and types checks, which it passes.fastapi_routes/v2/household_calculations/routes.pyimportssqlalchemy.exc, the same finding as Serve economy reports at /v2/reports and save v2 report and simulation IDs #4108's.masterafter Check declared dependencies with deptry, with today's findings as a baseline #4178 merged) and Check that every test function asserts something #4188 (stacked on Check that every noqa and type: ignore comment names a code and gives a reason #4183):rules/baselines/suppressions.txtlistspolicyengine_api/fastapi_routes/v2/user_resources.py :: from fastapi import Request # noqa: TC002.user_resources_support.pyas new, so the entry needs the new path.# noqa: TC002inuser_resources_support.pya reason and delete the entry.scripts/run_quality_guards.pyand changedocs/generated/rules.md, which are text conflicts.No other open PR adds files under these directories or refers to the renamed modules.
Docs and registry
v2-code-organization.mdhas a "File layers" section on the naming rules, exceptions and how calls are recognised. Its HTTP adapter layout uses the new names, and it cites each rule id where it states the rule.docs/generated/rules.mdandAGENTS.mdare regenerated.rules/fixtures/<rule id>/. They include a route that imports a connector, a services file with no layer, and excepted files that get the route or pure-module checks. Make v2 file-layer exceptions expire and cover whole files #4205 adds a route-side file with no layer.CI and environment variables
pythonand no project dependencies. Its proofs run in the existing "Test the rules registry tool" step.git ls-files, through the file-length guard's helper, to list tracked files.Testing
Each change came test first, in a commit that failed before the change that follows it:
0dc5686 and 12889c2 (exceptions expire and cover whole files; connector names; fix-message placement; splitting the guard) were pushed after the merge and are in #4205.
Expectations changed in implementing commits, each because of an intended change:
from .. import database_session(not...) names the resource's session module.imports sqlalchemyforimport sqlalchemy as sa.sqlmodel.selectimport.test_metadata_route_caching.pywas reordered, formatting only.expected_findingsgained their rule ids, the form 1780783's proofs compare. The test helper's generated exceptions gained theexpiresdate that 1780783's clean-load check requires. Those were test-side failures. The guard change in this commit fixed the two fix-message tests.e19f0d5 merges
master(with #4178'sdeclared-dependenciesguard) into this branch. Onlyscripts/run_quality_guards.py(both guards kept) and the generateddocs/generated/rules.mdconflicted.scripts/rules/tests/test_v2_layers.pyhas 37 tests:<rule id>: <path> <detail>findings;process_state.py,*_models.pyand*_support.py;TYPE_CHECKING, relative and package imports;__table__builders;After the changes:
V2_MIGRATION_DATABASE_URLisn't set; CI's cross-database job runs them;python scripts/run_quality_guards.py,python -m scripts.rules guard --base-ref origin/master,lint-imports --config pyproject.toml,ruff format --check .,ruff check .andmypypass.🤖 Generated with Claude Code