refactor(engine): split asset routes and analytics middleware out of main#115
Merged
Conversation
Move favicon, /pygments.css, and static file routes into routers/assets.py, and the page-view tracking middleware, bot UA detection, and track_page_view into services/analytics_middleware.py. main.py is now a pure app factory and wiring module. No behavior, route, header, or middleware-ordering changes. Update moved-symbol imports in tests. Refs #111 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Refactors the FastAPI app wiring by moving asset-related routes and analytics page-view middleware out of main.py into dedicated modules, aiming to keep main.py focused on app construction and router/middleware registration.
Changes:
- Moved favicon,
/pygments.css, user static, and theme static routes intosrc/squishmark/routers/assets.pyand registered the router before the pages catch-all. - Extracted bot UA detection + page-view tracking middleware into
src/squishmark/services/analytics_middleware.pyand registered it frommain.py. - Updated tests to patch/match the new symbol locations.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_pygments_css.py | Updates patch target to the moved assets router module. |
| tests/test_analytics_filtering.py | Updates imports/patch targets for moved analytics middleware symbols. |
| src/squishmark/services/analytics_middleware.py | New module for bot UA detection + page-view tracking middleware registration. |
| src/squishmark/routers/assets.py | New router module containing the moved asset/static routes. |
| src/squishmark/main.py | Removes inlined assets routes + analytics middleware and wires the new router/middleware modules. |
Comments suppressed due to low confidence (1)
src/squishmark/main.py:141
- The linked issue’s acceptance criteria says “No route logic in
main.pybeyond registration”, butmain.pystill defines route handlers (/health,/redirect, and debug LiveReload endpoints). If the acceptance text is meant to be strict, these should move into routers (or the acceptance criteria should be clarified/updated).
— GitHub Copilot
# Middleware to track page views (non-blocking)
register_analytics_middleware(app)
# Health check endpoint
@app.get("/health")
async def health_check() -> dict[str, str]:
This was referenced Jul 3, 2026
…mplete Breaking out of the get_db_session loop closed the generator at the yield point, so the post-yield commit never ran and every flushed page view was rolled back. Remove the break (the generator yields exactly once) and add a regression test that persists a view and reads it back from a fresh session. Also log tracking failures with a traceback and annotate the middleware signature. Fixes #118 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The raw path went straight into get_binary_file, so in local content mode a request like /static/user/..%2f..%2fx.css escaped the content directory. Reject paths containing '..' or a backslash, or starting with '/', with a 404 (matching the extension check), mirroring the theme-static guard. Add tests for traversal rejection and normal nested serving. Fixes #119 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Closes #111
Pure move refactor, no behavior change:
routers/assets.py: favicon, /pygments.css, /static/user, and theme static routes moved verbatim, registered before the pages catch-allservices/analytics_middleware.py: bot UA detection,track_page_view, and the page-view middleware; registered viaBaseHTTPMiddlewarewithdispatch, which is exactly what@app.middleware("http")expands to, so middleware ordering is unchangedmain.pyis now pure app factory and wiring (lifespan, exception handlers, session middleware, /health, / redirect, livereload)Checks: format, lint, pytest (328), pyright all green.
🤖 Generated with Claude Code