Repository navigation
refactor: route SM012 through print_diagnostics; depublicize migrations helper - #33
Merged
Merged
Conversation
…lper - `check_settings_registration` now returns `list[Diagnostic]` and the boot path routes it through `print_diagnostics`, so SM012 shows up with the same framing as every other diagnostic code instead of a stray `logger.warning`. - Rename `_migrations.py` to `migrations.py` — test conftests import `resolve_head_revision` across the package boundary, so the leading underscore was misleading. The remaining `_*.py` hosting files are genuinely intra-package and stay underscored. - CLAUDE.md: `app.state.<module>_settings` was stale (the 2026-04-17 app.state reorg moved to `app.state.<module_lower>`). Also list SM007 and SM012 in the diagnostic-code summary.
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.
Summary
check_settings_registrationpreviously built aDiagnosticobject and threw it away withlogger.warning("%s", diag). It now returnslist[Diagnostic]and the boot path inapp_builder.pyfeeds it throughprint_diagnostics, so the warning renders with the same framing as every other SM-code._migrations.py→migrations.py. Two conftests (conftest.py,modules/users/tests/conftest.py) already importresolve_head_revisionacross the package boundary, so the leading underscore was signalling something that wasn't true. The other_*.pyfiles insimple_module_hostinggenuinely are intra-package and stay underscored.app.state.<module>_settings. That hasn't been true since the 2026-04-17app.statereorganization (commit 04b133f) — current convention isapp.state.<module_lower>, which is also what SM012 checks for.SM007(emitted byModuleDiagnostics) andSM012(emitted here). Both added.Why
Came out of an architecture audit that started from a sprawl complaint ("6 registries, 16 diagnostic codes, 10 hooks"). Most of the claims reversed under scrutiny —
versioning.pyguards ABI compat at boot,health.pyfeeds/health/ready, the "unused" lifecycle hooks are wired extension points per recently-landed specs, and the 3-package split has a clean linear dep graph. The two genuine findings are in this PR.What's explicitly not in this PR and why:
versioning.py/health.py— both are load-bearing (boot gate +/health/readyrespectively), just zero module overrides today.template_dirs,static_mounts,register_event_handlers,register_exception_handlers,on_shutdown) — all wired end-to-end and backed by a landed design doc; removing them is a product decision, not cleanup._*.pyfiles — no real sprawl; the split is principled, and the remaining underscore files are only consumed withinsimple_module_hosting.Test plan
make test-py— 562 passed, 4 deselecteduv run ruff check framework/— cleanframework/hosting/tests/test_app.py,framework/core/tests/) — 152 passedregister_settingswithout touchingapp.state.<module>in dev mode; SM012 should now render with the diagnostic header instead of a bare warning log lineReviewer notes
print_diagnosticscalls at boot (phase 2, phase 4). SM012 requiresregister_settingsto have run first, so it can't fold into the phase-2 pass without reordering fail-fast semantics. Considered and skipped — cost is one extra header print when SM012 fires.if settings_diagnostics:guard inapp_builder.pyis intentional:print_diagnosticslogs "No issues found" on empty input, and we already emit that in phase 2.