[anth-volk]

Fixes #4204

Summary

#4191 added the v2-layers quality guard and rule api.v2.files-assigned-to-layers: every file under policyengine_api/services/v2 and policyengine_api/fastapi_routes/v2 must have a name that gives it a layer, or a [[rule.exception]] for that rule in rules/registry.toml. #4191 merged at e19f0d5, before the fixes from its last self-review round were pushed. This PR brings those fixes onto the latest master.

What master accepts that it shouldn't

The guard on master applies every files-rule exception the registry loader accepts, as long as it names one existing v2 file whose name gives no layer. The loader requires an expires date only when an exception sets neither category nor removal, and the guard reads no other field. So master:

1. accepts exceptions with no expiry date. An exception that sets category = "forever" or removal = "never" and no expires loads, passes the registry checks and is applied. The file then keeps a name with no layer permanently.
2. ignores match. An exception with match = "session.get" reads as if it covered one call, but the guard applies it to the whole file.
3. accepts exceptions for files in a connector package. It applies an exception for a misnamed module in a database_connectors package, such as policies/database_connectors/helpers.py. Rule api.v2.crud-checks-cover-every-resource (pytest) still fails a module directly in services/v2/<resource>/database_connectors whose name doesn't start with an operation, so for such a module the exception can't work.
4. gives an incomplete fix message. For a services file with no layer, the message lists services.py, validators.py, transformations.py, types.py, process_state.py, database_session.py and database_connectors/<operation>[_name].py, but not where each must sit. database_session.py is only accepted directly in a resource package, so following the message can fail again.
5. accepts an exception path it never applies (found in this PR's self-review). Another spelling of a real file passes the existence check: a ., .. or empty segment (household_calculations/./client.py), a path through a symlinked directory, or different letter case on macOS. The guard compares exceptions with the paths git ls-files lists, so the exception covers nothing, and the file still fails with a fix that asks for the exception it already has.
6. has a proof gap. The registered proof fixture for api.v2.files-assigned-to-layers has no unassigned file under fastapi_routes/v2, so the proof doesn't show the check working there. Other unit tests do cover it.

Changes

• scripts/rules/v2_layers.py reports, and doesn't apply, a files-rule exception that:
  • has no expires date (or one the loader couldn't read);
  • sets category, removal or match;
  • names a module in a database_connectors package, at any depth;
  • names a path the guard doesn't scan. The path must be one git ls-files lists; any other spelling of a real file, or a file git doesn't track, is reported, and the fix says to check the spelling and to git add an untracked file. run_check lists the files once and uses the list for both the exception check and the scan.
• The services fix message says where each name must sit: the five module names in a resource package under services/v2, database_session.py directly in one, and connector modules directly in their database_connectors package.
• An unassigned module in a database_connectors package gets its own message: if it reads or writes rows, name it after its operation and put it directly in services/v2/<resource>/database_connectors; otherwise move it out of the package and name it after its layer. An exception can't stand in. This covers a file that already has an operation's name but sits one level too deep, such as database_connectors/legacy/creates.py. The message names that location, rather than any database_connectors package, because it is the only one the CRUD checks find.
• The refusal of a connector-package exception gives its reason as "a database_connectors package holds only connector modules", and adds that api.v2.crud-checks-cover-every-resource fails a module directly in services/v2/<resource>/database_connectors whose name doesn't start with an operation. That is what the rule's test checks: it finds only that location, and its patterns (creates*.py and so on) pass a name such as createsfoo.py, which this guard gives no layer.
• The files rule's bad proof fixture gains fastapi_routes/v2/reports/helpers.py, which has no layer.
• The guard is split in two to stay under the 800-line file limit:
  • scripts/rules/v2_layers.py (463 lines) keeps the layer assignment, exceptions, scanning, baselines and the names other modules import;
  • the new scripts/rules/v2_layer_checks.py (438 lines) holds the per-file import and call checks.
  • On master, v2_layers.py is 786 lines; this change would have taken it past 800.
• docs/engineering/skills/v2-code-organization.md says an exception's path is written as git ls-files lists it, for a tracked file, that an exception covers the whole file and sets no match, category or removal, and that it can't name a connector-package module, which moves out of the package if it isn't a connector.
• The statement of api.v2.files-assigned-to-layers in rules/registry.toml says the exception is for a file outside a database_connectors package, and docs/generated/rules.md is regenerated.

No exception exists for this rule today, so nothing in the repository is newly reported.

CI and environment variables

• No workflow changes. The guard still runs in the "Run quality guards" step of the quality-guards job, and its tests in the "Test the rules registry tool" step.
• The guard reads no environment variable.

Known limits, not changed here

• Far-future dates. An expires date far ahead, such as 9999-12-30, is accepted. No rule in the registry sets a maximum. 9999-12-31 is reported as "no expires date", because the loader uses that date for a missing or unreadable one.
• Deeper connector packages. The guard gives an operation-named module a connector layer in any database_connectors package, including services/v2/database_connectors/ and services/v2/<group>/<resource>/database_connectors/. The CRUD checks find only services/v2/<resource>/database_connectors/. This was already so on master; the fix messages now point to the checked location.
• Directory names with spaces. The files rule's fix message is chosen from the path read back out of the finding's text, so a directory name containing a space gets the services message instead of the connector one. Such a directory can't be a Python package.
• Exceptions for the other v2-layers rules. The guard reads exceptions only for api.v2.files-assigned-to-layers. A [[rule.exception]] for one of its other five rules is neither applied nor reported, so the finding it targets still fails. This was already so on master.
• Loader problems. run_check returns the files rule's exception problems but not the registry loader's own; the rules-registry guard reports those. This was already so on master.
• Datetime dates (outside this guard). The registry loader accepts a TOML datetime such as expires = 2099-01-01T00:00:00, because a datetime is a date. scripts/rules/validate.py then raises TypeError comparing it with today's date. This affects every rule's exceptions, not only this one.

Testing

Test first:

• cb01ffa adds the tests, with "(failing)" in its message. Against master's guard, 4 of the 5 touched tests fail:

  • test_an_unassigned_file_fails_and_names_the_fix (fix message placement);
  • test_an_unassigned_connector_file_must_be_renamed;
  • test_an_exception_ends_on_its_expires_date_and_covers_the_whole_file, with one subtest each for category, removal and match;
  • test_a_connector_module_cannot_be_excepted.

  test_rule_proof_files_assigned_to_layers passes on master too: its new route-side file closes a coverage gap rather than showing a behaviour change.

• f651b4a makes the change, and the 5 tests pass.

These two commits were first made on #4191's branch after it merged (0dc5686 and 12889c2) and were cherry-picked here without conflicts. cbe0c87 then merges master with #4183 (the suppressions guard), also without conflicts.

Three self-review rounds on this PR found more. Round 1:

• c991814 adds or changes the tests, with "(failing)" in its message. Three fail before the fix:

  • test_an_unassigned_connector_file_is_told_its_name_and_place (renamed from test_an_unassigned_connector_file_must_be_renamed; adds the legacy/creates.py case);
  • test_a_connector_module_cannot_be_excepted (new wording, and the nested case);
  • test_an_exception_path_must_be_written_as_git_lists_it.

  test_an_exception_with_no_readable_expires_date_is_reported passes before and after. It covers the guard's check for an exception with no expires, category or removal, or an unreadable date, which no test reached: with that check disabled, every other exception test still passed.

• 14b1d9c makes the change, and the tests pass.

Round 2:

• c915861 changes the tests, with "(failing)" in its message. Three fail before the fix: the two connector tests (the message names services/v2/<resource>/database_connectors), and test_an_exception_path_must_be_written_as_git_lists_it (one message for every unscanned spelling). The .. subtest now creates its middle directory, so the path reaches the real file as the bug needs. The connector-exception test removes each subtest's file and checks that the refused module is still reported as having no layer. test_an_exception_ends_on_its_expires_date_and_covers_the_whole_file is renamed test_an_exception_needs_an_expires_date_and_covers_the_whole_file, because the guard checks that a date is set, and the registry's expiry check ends the exception.
• bcc4756 makes the change, and the tests pass.

Round 3:

• 9f973eb adds or changes the tests, with "(failing)" in its message. All four fail before the fix: the two connector tests (the move-out remedy and the CRUD rule's wording), test_an_exception_path_must_be_written_as_git_lists_it (the git add fix), and the new test_an_exception_for_a_file_git_does_not_track_is_reported. The new test runs the guard in a git init directory, so the exception is checked against git ls-files rather than a directory listing; the other exception tests use plain directories.
• ed5fcca makes the change, and the tests pass.

4c64934 merges master with #4188 (the asserting-tests guard), without conflicts; docs/generated/rules.md and AGENTS.md regenerate unchanged.

After these changes:

• the 8 tests added or changed in scripts/rules/tests/test_v2_layers.py, run individually by node ID, pass;
• python scripts/run_quality_guards.py (including the new suppressions and asserting-tests guards), python -m scripts.rules guard --base-ref origin/master, and ruff check and ruff format --check on scripts/rules pass.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.63%. Comparing base (7835f6a) to head (4c64934).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #4205       +/-   ##
===========================================
+ Coverage   53.43%   88.63%   +35.20%     
===========================================
  Files         290      304       +14     
  Lines       16967    18442     +1475     
  Branches     2784     3084      +300     
===========================================
+ Hits         9066    16346     +7280     
+ Misses       7132     1357     -5775     
+ Partials      769      739       -30     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.