fix: diagnostic false positives + Inertia 307 strip (#70) - #88
Merged
Conversation
- SM003 now resolves PAGE_X string constants imported from sibling files (e.g. feature_flags PAGE_BROWSE in constants.py used in views.py), eliminating the false orphan-page warning that fired every doctor run. - SM019 no longer warns when a module registers permissions but no menu items. Modules whose views are sub-pages of another module (e.g. Permissions' RoleEdit/UserEdit reached from Users admin) stay discoverable via the role editor without needing a sidebar entry. - wire_module_routes auto-mounts a bare-prefix alias for view routes at '/'. FastAPI's redirect_slashes 307 caused httpx to strip X-Inertia on follow, breaking Inertia navigation; the alias serves the same handler directly. Closes #70.
CI's 300-line cap was tripped by the diagnostic + app-builder changes in the previous commit. Split by responsibility, no behaviour change: - diagnostics/_pages.py — orphan/phantom-page detection, render-call resolution, and the cross-file PAGE constant collector. Test now imports the module-level helpers directly instead of poking ModuleDiagnostics' private methods. - _phase_helpers.py — gains wire_module_routes, including the bare-prefix view-route alias logic. app_builder.py re-imports it.
antosubash
marked this pull request as ready for review
April 30, 2026 16:36
…hecks Apply review findings from /simplify: - _pages.py parses each .py file once. Cross-file consts and render-call scanning share the parsed trees instead of rglob+ast.parse running twice. - _module_level_str_consts replaces the duplicated isinstance ladder that in-file and cross-file collectors both ran. - check_orphan_pages and check_phantom_renders merge into check_pages — one collect_tsx_pages call covers both directions of the diff. - SM019 if-chain flattens to a single boolean. - Trim narration in tests; the docstring on wire_module_routes already carries the X-Inertia rationale.
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
2 participants
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
Three concrete framework fixes surfaced by
make doctorand the open issue tracker. Many of the issues filed in #65–#86 were already addressed in #87 (merged); this PR cleans up the remaining real problems I could verify in this repo.SM003 false positive — cross-file PAGE constants
make doctorfalsely flaggedfeature_flags/pages/Browse.tsxas orphan because the AST scanner only resolvedinertia.render(NAME)constants defined in the same.pyfile.feature_flagsdefinesPAGE_BROWSEinconstants.pyand uses it inendpoints/views.py, so the const lookup missed._iter_render_componentsnow accepts anextra_constsregistry, and_find_render_callsbuilds one across every.pyfile in the module's source tree before walking renders. Truly orphan pages still fire SM003.SM019 — register_permissions is sufficient
The Permissions module surfaces its only views (RoleEdit, UserEdit) as deep-link edit pages reached from buttons inside the Users admin page. It registers permissions but no menu items. SM019 was firing on every boot.
Per the original issue (#85): a module is "silently invisible" only when admins can reach it through neither the sidebar nor the role editor. Loosened the check so registering either menu items or permissions silences the warning. Updated the diagnostic message and CLAUDE.md accordingly.
#70 — bare-prefix view alias
view_prefix="/foo"+@router.get("/")mounts at/foo/. A request to/foo(no slash) hits FastAPI'sredirect_slashes=True307, and httpx stripsX-Inertiaon follow — every Inertia link to a bare-prefix landing page broke.wire_module_routesnow auto-clones any view route atprefix + "/"toprefix(include_in_schema=False). Same handler, no redirect, X-Inertia survives. Confirmed viaTestRouteRegistration::test_expected_routes_registeredthat/dashboardand/dashboard/are both registered.Issues not addressed here
drop_constraintfor cross-module FKs) — needs a real Postgres DB to verify the include_object filter behavior; couldn't be tested safely in this session.main. Owner can close them as appropriate.laco_wiki_python) running this framework as a wheel-installed dependency. Reproducing them needssm newscaffolds and downstream module setups outside this repo.Test plan
make doctor— clean, 0 errors / 0 warnings (was 2 warnings: SM003 FeatureFlags/Browse, SM019 Permissions)uv run pytest --ignore=tests/e2e— 1044 passeduv run ruff check framework/ modules/— cleanuv run ruff format --check framework/ modules/— cleanTestSm003PageRenderResolution(2),test_silent_when_permissions_registered,/dashboardbare-prefix assertion intest_expected_routes_registeredGenerated by Claude Code