From bd3307a5d29931ace67d236ecb1a5a81c78e2af6 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 5 Nov 2026 15:24:40 +0100 Subject: [PATCH 1/2] perf(inertia): page audit skill, dev Server-Timing, fast-pages guide; background-tasks pages stop waiting on the broker - Development responses carry Server-Timing (app time, db time, SQL statement count); simple_module_db.query_stats exposes the count to tests. - skills/simple-module-page-perf: SKILL.md + scripts/page_audit, which audits every page (server, browser, phone width, UX checks) against budgets. - docs/frontend/performance.md: building pages that are fast and feel fast. - Background tasks: worker presence and the Workers snapshot are deferred props with same-size placeholders (server ~1,050 ms -> ~15-20 ms); links to the executions list use the canonical trailing-slash URL. - Users admin list: test pins a flat SQL count as rows grow. Claude-Session: https://claude.ai/code/session_01FXHA3ZJ5EgoJN4zXTAhBR2 --- CHANGELOG.md | 27 ++ CLAUDE.md | 5 +- docs/.vitepress/config.ts | 1 + docs/frontend/performance.md | 371 ++++++++++++++++++ framework/cli/tests/test_skill_page_audit.py | 159 ++++++++ framework/db/simple_module_db/query_stats.py | 82 ++++ framework/db/tests/test_db_query_stats.py | 66 ++++ .../simple_module_hosting/_app_boot.py | 4 + .../_middleware_pipeline.py | 5 + .../simple_module_hosting/_server_timing.py | 64 +++ .../tests/test_hosting_server_timing.py | 70 ++++ .../background_tasks/endpoints/views.py | 39 +- .../background_tasks/locales/de.json | 8 +- .../background_tasks/locales/en.json | 8 +- .../background_tasks/pages/Detail.tsx | 6 +- .../background_tasks/pages/Index.tsx | 13 +- .../background_tasks/pages/Logs.tsx | 6 +- .../background_tasks/pages/Outbox.tsx | 6 +- .../background_tasks/pages/Workers.tsx | 36 +- .../pages/components/TasksEmpty.tsx | 73 ++-- .../pages/components/WorkerCard.tsx | 28 ++ .../background_tasks/pages/constants.ts | 2 + .../background_tasks_IndexPage.test.tsx | 19 + .../background_tasks_WorkersPage.test.tsx | 55 +++ .../tests/test_bg_index_worker_presence.py | 57 ++- .../tests/test_workers_endpoints.py | 8 +- .../tests/test_workers_uptime.py | 8 +- .../test_users_admin_list_query_count.py | 67 ++++ packages/i18n/src/generated-resources.ts | 4 + packages/i18n/src/keys.generated.ts | 4 + pyproject.toml | 2 + skills/README.md | 1 + skills/simple-module-inertia-pages/SKILL.md | 1 + skills/simple-module-page-perf/SKILL.md | 157 ++++++++ .../scripts/page_audit/__main__.py | 242 ++++++++++++ .../scripts/page_audit/audit_browser.py | 189 +++++++++ .../scripts/page_audit/audit_crawl.py | 79 ++++ .../scripts/page_audit/audit_report.py | 172 ++++++++ .../scripts/page_audit/audit_server.py | 169 ++++++++ 39 files changed, 2221 insertions(+), 92 deletions(-) create mode 100644 docs/frontend/performance.md create mode 100644 framework/cli/tests/test_skill_page_audit.py create mode 100644 framework/db/simple_module_db/query_stats.py create mode 100644 framework/db/tests/test_db_query_stats.py create mode 100644 framework/hosting/simple_module_hosting/_server_timing.py create mode 100644 framework/hosting/tests/test_hosting_server_timing.py create mode 100644 modules/background_tasks/tests-js/background_tasks_WorkersPage.test.tsx create mode 100644 modules/users/tests/test_users_admin_list_query_count.py create mode 100644 skills/simple-module-page-perf/SKILL.md create mode 100644 skills/simple-module-page-perf/scripts/page_audit/__main__.py create mode 100644 skills/simple-module-page-perf/scripts/page_audit/audit_browser.py create mode 100644 skills/simple-module-page-perf/scripts/page_audit/audit_crawl.py create mode 100644 skills/simple-module-page-perf/scripts/page_audit/audit_report.py create mode 100644 skills/simple-module-page-perf/scripts/page_audit/audit_server.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f6c16b03..2b74dc6cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,33 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] +### Added +- Development responses carry a `Server-Timing` header with the request's app + time, database time and SQL statement count (`app;dur=…, db;dur=…;desc="N + queries"`), so an N+1 shows in the browser's network panel. Never sent + outside development. `simple_module_db.query_stats` (`install_query_stats`, + `track_queries`) exposes the same count to tests. +- `simple-module-page-perf` skill: `scripts/page_audit` signs in, finds every + page from the menus and two levels of links, and reports server time, SQL + statements, payload and prop sizes, deferred props, client-side navigation + time, LCP, CLS, JS bytes, DOM size, console errors, phone-width overflow and + basic UX checks against budgets. Install with + `smpy skills add simple-module-page-perf`. +- `docs/frontend/performance.md`: how to build pages that are fast and feel + fast (server, client, loading/empty/error states, layout shift, polling, + prefetch) and how to pin a page's query count in a test. + +### Fixed +- Background tasks: the executions list (when empty) and the Workers page no + longer wait out the broker's ~1 s inspect timeout before rendering. The + worker poll is now a deferred prop: the list shows *Checking for workers…* + in the empty state and Workers shows skeleton cards until the broker + answers (server time about 1,050 ms → 15–20 ms). **Behaviour change:** + `worker_presence` and `snapshot` are no longer in the first Inertia + response; they arrive in the deferred `workers` group. +- Background tasks pages linked to `/admin/background-tasks` without its + trailing slash, so every "back to executions" click cost a redirect. + ## [0.0.43] — 2026-11-04 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index a53f8d73e..1be36206c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -264,7 +264,10 @@ by `InertiaLayoutDataMiddleware`; `auth.user` is null unless `app.state.principal_serializer` is set, and `i18n.messages` is null on every `X-Inertia` request when the tab reports a matching locale and audience via `X-SM-I18n`; clients without that header use the legacy session markers. -Layout props are resolved lazily by page and error rendering; permissions stay eager. Inertia +Layout props are resolved lazily by page and error rendering; permissions stay eager. +Development responses carry `Server-Timing` (app time, SQL statement count); +`docs/frontend/performance.md` covers fast pages and the +`simple-module-page-perf` skill audits them. Inertia `router.post/patch/put/delete` must target view routes, never `/api/*` (`SM018`). `docs/frontend/`. diff --git a/docs/.vitepress/config.ts b/docs/.vitepress/config.ts index 7086fd7af..ee0385ee5 100644 --- a/docs/.vitepress/config.ts +++ b/docs/.vitepress/config.ts @@ -165,6 +165,7 @@ export default defineConfig({ { text: "Inertia basics", link: "/frontend/inertia" }, { text: "Pages & discovery", link: "/frontend/pages" }, { text: "Shared props & layout", link: "/frontend/shared-props" }, + { text: "Fast pages", link: "/frontend/performance" }, ], }, ], diff --git a/docs/frontend/performance.md b/docs/frontend/performance.md new file mode 100644 index 000000000..e51ebe910 --- /dev/null +++ b/docs/frontend/performance.md @@ -0,0 +1,371 @@ +# Fast pages + +How to build an Inertia page that is fast, and that *feels* fast, in a +simple_module app. It covers the server (what a visit costs), the client +(bundle and render), the UX that decides how waiting feels, and how to measure +all three with the `simple-module-page-perf` skill. + +## What "fast" means here + +People notice three thresholds: about **100 ms** feels instant, about **1 s** +keeps their train of thought, and past **10 s** they leave. Google's Core Web +Vitals add two numbers for the first load: the largest element should paint +within **2.5 s** (LCP) and the page should not jump around after it does +(CLS ≤ **0.1**). + +The page audit turns these into budgets. A value over the first number is a +warning; over the second, a failure: + +| Budget | Warn | Fail | Measured as | +| --- | --- | --- | --- | +| Server time | 100 ms | 300 ms | `app` in the `Server-Timing` header, warm median | +| SQL statements | 12 | 25 | statements per visit, cheapest warm round | +| Page JSON | 100 KB | 300 KB | the Inertia response, without the i18n catalog | +| One prop | 50 KB | 150 KB | the largest page prop | +| Navigation | 300 ms | 1 s | click on a link → new page painted | +| Deferred props | 1 s | 2.5 s | click → every deferred prop painted | +| LCP (first load) | 2.5 s | 4 s | production build only | +| JS transferred | 350 KB | 600 KB | first load, production build only | +| CLS | 0.1 | 0.25 | first load and navigation | +| DOM nodes | 1,500 | 3,000 | after everything settled | +| Table rows | 100 | 250 | the longest `` | + +For scale, the framework's own pages on a production build (SQLite, one +worker, 2,000 users, 20,000 audit entries): server time 2–25 ms, 0–6 SQL +statements, client-side navigation 27–125 ms, LCP 144–328 ms, CLS 0. + +## Where the time goes + +A **first load** fetches the HTML document (with the page object inside it), +the shared bundle, the page's own chunk and its CSS, then renders. A +**client-side navigation** (any `` or `router.visit`) skips all of that: +one JSON request, the page chunk if it isn't cached yet, a React render. + +``` +click ─┬─ request ── server: auth, handler, queries, props, encode ─┐ + │ │ + └──────────────── transfer (gzip) ── JSON parse ── chunk ── render ── paint + │ + deferred props: one more request per group +``` + +So most of the work for a fast page is on the server, and most of the work for +a page that *feels* fast is deciding what can arrive later. + +## Measure first + +### `Server-Timing` in development + +A development host adds a `Server-Timing` header to every response: + +``` +Server-Timing: app;dur=12.4, db;dur=3.1;desc="5 queries" +``` + +`app` is the time the request spent in the app (every middleware, the +handler, the commit). `db` is the time spent in the database driver, and +`desc` counts the SQL statements. Your browser's network panel shows both on +the request's *Timing* tab. The header is never sent outside development: +per-request timings help an attacker time lookups. + +### The page audit + +The `simple-module-page-perf` skill (`smpy skills add simple-module-page-perf`) +ships a script that signs in, finds every page from the menus, follows links +two levels deep (one sample per URL pattern, so a 50-row table adds one page), +and measures each page from the server, through a real browser, and at phone +width: + +```bash +uv run --with playwright python .claude/skills/simple-module-page-perf/scripts/page_audit \ + --base-url http://localhost:8000 --email admin@example.com --password '…' \ + --json perf.json +``` + +Run it twice. Against a **development** server you get server time and SQL +counts, but dev-server bundle numbers are meaningless. Against a **production +build** (`make build`, then a non-development `SM_ENVIRONMENT`) you get LCP and +JS bytes, but no `Server-Timing`. The skill's `SKILL.md` has both recipes and +explains each finding. + +### Pin it in a test + +Once a page is fast, keep it that way. `track_queries()` counts the statements +a block runs on an engine instrumented with `install_query_stats()`. The test +that catches an N+1 compares a small table with a bigger one: + +```python +from simple_module_db.query_stats import install_query_stats, track_queries + + +async def _statements(app, client) -> int: + install_query_stats(app.state.sm.db.engine) + counts = [] + # Auth adds statements on its own schedule (loading the principal, then a + # session-version check), so the cheapest of three requests is the page's. + for _ in range(3): + with track_queries() as stats: + resp = await client.get("/admin/orders/", headers={"X-Inertia": "true"}) + assert resp.status_code == 200 + counts.append(stats.count) + return min(counts) + + +async def test_list_query_count_does_not_grow_with_rows(app, admin_client, db): + await add_orders(db, 2) + few = await _statements(app, admin_client) + await add_orders(db, 20) + assert await _statements(app, admin_client) == few +``` + +`modules/users/tests/test_users_admin_list_query_count.py` is a working copy. + +## The server + +### Send only what the page shows + +Every byte of every prop is serialized, gzipped, sent, parsed and held in +memory by the client. Build a DTO with exactly the fields the page renders +rather than dumping a model, and keep large text (bodies, JSON blobs, audit +diffs) for the detail page. + +### Paginate every list + +A list that grows with the data is paginated on the server, 20–50 rows a +page. `paginate()` (`simple_module_db.pagination`) runs the count and the +page, and clamps a page past the end; `lenient_page_params()` +(`simple_module_hosting.pagination`) turns a hand-edited `?page=` into a +default instead of a 422; `PaginationBar` renders the control. For a feed the +user scrolls, use `scroll()` with Inertia's infinite scroll instead of pages. + +### No query per row + +The usual N+1: a list of N rows, then one more query per row for its roles, +owner or counts. Load relations in one query (`selectinload()` for +collections, `joinedload()` for a single parent), resolve users in one call +(`get_user_directory(app).get_many(ids)`), and compute per-row numbers with a +single `GROUP BY` instead of a `count()` per row. The `Server-Timing` count, or +the test above, tells you whether it worked. + +### Pick the right kind of prop + +`InertiaResponse` props can be plain values, callables, or wrapped with the +factories from `simple_module_inertia`: + +| Prop | Sent on the first visit | On a partial reload | Use it for | +| --- | --- | --- | --- | +| `"rows": rows` | yes | only if named | cheap data, already computed | +| `"rows": load_rows` (a callable) | yes, evaluated | only if named, else **not evaluated** | anything the page reloads around | +| `optional(load)` | no | only if named | data a later interaction asks for | +| `always(value)` | yes | always, even when not named | things every reload must refresh | +| `defer(load, group=...)` | no: loaded right after the page renders | when named | slow parts of an otherwise fast page | +| `once(value)` | yes, then kept by the client | skipped while the client has it | large, rarely changing data (option lists) | +| `merge()` / `scroll()` | yes | appended to what the client has | infinite lists, "load more" | + +Two rules follow: + +- **Pass expensive props as callables.** A plain value is computed on every + request, including a partial reload that leaves it out. A callable (sync or + async, `lambda: service.stats()` is enough) is only evaluated when the + response includes it. +- **Defer what is slow, not what is important.** The page should paint its + structure and its main content at once; a deferred prop is for the part that + depends on something slow (an external API, a broker, a big aggregate). + Props in different `group=`s load in parallel; props in one group share a + request. + +The Background tasks *Workers* page is the worked example. Asking Celery which +workers are alive waits out a broker timeout, about one second. Before, the +whole page waited for it (server time about 1,050 ms). Now the snapshot is +deferred and everything else paints at once (about 20 ms): + +```python +return await inertia.render( + "BackgroundTasks/Workers", + { + # The routes and the start command render at once; the fleet fills + # in behind a skeleton when the broker answers. + "snapshot": defer(_snapshot_json(request), group="workers"), + "queues": queues, + "routes": routes, + }, +) +``` + +### Shared props are on every response + +Everything a provider registered with `register_inertia_shared_provider()` +returns goes into every Inertia response (`docs/frontend/shared-props.md`). +Keep providers cheap, and return callables for anything that costs a query, +so a partial reload that does not name them skips them. The framework's own (`auth`, `menus`, +`i18n`) come to about 3 KB, and the translation catalog is sent once per tab: +the client echoes the catalog it holds in `X-SM-I18n`, so later visits come +back without it. + +### Link to the canonical URL + +A link to `/admin/orders` when the route is `/admin/orders/` costs a redirect: +one more round trip before anything paints. Menu URLs are checked for this +(`test_menu_urls_are_canonical.py`); links inside pages are not, so keep one +constant for the index URL with its trailing slash. + +### Caching + +Inertia responses are forced to `private, no-store`, so don't try to cache them +in the browser or a CDN. Cache *inside* the app instead: per-process caches +keyed on what they depend on, with a TTL, and `register_invalidations` to drop +them when the data changes (`docs/framework/invalidation.md`). + +## The client + +### Keep each page's JavaScript small + +Each page is its own chunk (the `import.meta.glob` in the client app), so a +heavy library imported by one page only costs that page, *unless* it is +imported from a shared component. Load a heavy widget (a chart, an editor, a +map) where it is used: + +```tsx +const Chart = lazy(() => import('./components/Chart')); + +}> + + +``` + +`make build` prints chunk sizes; the audit reports JS transferred per first +load. + +### Render less + +- **Keep the DOM small.** Past about 1,500 nodes, every render and every style + recalculation gets slower. Paginate, or virtualize a list that must show + hundreds of rows. +- **Derive once.** Filtering or grouping a large prop on every render belongs + in `useMemo`, keyed on the prop. +- **Stable keys.** Key list items by id, never by index, or React rebuilds + rows it could have kept. + +### Filters and search + +A filter change is a visit to the same page with new query parameters. Reload +only what changes, keep the scroll position and the input's state, and don't +push a history entry per keystroke: + +```tsx +router.get(INDEX_URL, params, { + only: ['rows', 'pagination'], + preserveState: true, + preserveScroll: true, + replace: true, +}); +``` + +Debounce text inputs (about 300 ms) so typing a word costs one request, not +one per letter. + +## UX: how waiting feels + +Speed is measured in milliseconds, but judged by what the user sees while +waiting. These patterns matter as much as the numbers above. + +### Show progress only when there is a wait + +The client app shows Inertia's progress bar after 150 ms, so fast visits show +nothing and slow ones show that something is happening. Don't add a spinner of +your own for a whole-page visit. + +### Placeholders keep the page still + +A deferred prop or a lazy component needs a placeholder, and the placeholder +must take up the **same space** as what replaces it. Otherwise the content +below jumps when the data lands: that jump is layout shift, the CLS budget +above, and it makes people click the wrong thing. + +- Use the `Skeleton` component (`@simple-module-py/ui/components/ui/skeleton`) + in the shape of the real content: same grid, same card padding, same + row heights. +- Or render the real component with "checking…" copy, the way the Background + tasks list shows *Checking for workers…* in the same empty-state block that + will hold the answer. +- Mark the placeholder `aria-busy="true"` (and `role="status"` with a label + when it stands alone), so screen readers wait for the real content. +- Inertia's `` does the bookkeeping for + you. To handle it yourself, a deferred prop is `undefined` until it loads, + which is different from `null` (loaded, nothing there). Type it as optional, + `stats?: Stats`, and branch on `=== undefined`. +- A component that copies a deferred prop into state (`useState(initial)`) + must adopt it when it arrives: + `useEffect(() => { if (initial) setValue(initial); }, [initial]);`. + +### Below the fold can wait + +Content the user has to scroll to reach can load when it scrolls into view: +pass it as `optional()` and wrap it in +``. The first response gets smaller +and nothing is fetched for users who never scroll. + +### Distinct loading, empty and error states + +"Loading", "there is nothing here", "your filter hides everything" and +"something is broken" need different messages, or a slow load reads as an +empty list and an outage reads as "no data". Give each its own copy, and give +the empty and error states a next step (clear the filter, start a worker). + +### Buttons answer immediately + +A submit button disables itself and shows progress while the request is in +flight (`form.processing` from `useForm`), so a slow save is never clicked +twice. On a validation error the server redirects back with the errors +(`docs/frontend/inertia.md`), and a `useForm` form still holds what the user +typed, so they fix one field instead of retyping the form. + +### Polling and freshness + +A page showing live state either polls or says how old it is. + +- To poll, use `usePoll(interval, { only: [...] })` with a modest interval + (5 s or more) and only the props that change. Inertia slows polling down + while the tab is in the background. +- For anything that costs the server real work, prefer a manual *Refresh* + button and a visible "last updated" time that turns amber when it goes + stale. That is what the Workers page does. + +### Prefetching: only where data can be a little old + +`` fetches a page when the pointer rests on the link, so the +click feels instant. The response is cached briefly (`cacheFor`), so the click +can show data that is a few seconds old. That suits documentation and +reference pages. It does not suit admin lists right after an edit, or anything +whose GET has a cost. Every hover is a request. + +### Phones + +Check every page at phone width: nothing should scroll sideways (wrap wide +tables in `overflow-x-auto`), and touch targets should be at least 44 px tall +(`max-lg:min-h-11` on buttons). The audit loads every page at 390 px and +reports horizontal overflow. + +### Small things that read as quality + +One `

` per page, an accessible name on every icon-only button, a document +title per page (``). The audit checks all three. They cost +nothing and make the page faster to *understand*, which is what the user +perceives. + +## Checklist + +Before calling a page done: + +- [ ] Server time under 100 ms in development; anything slow is `defer()`ed + with a same-size placeholder. +- [ ] The SQL count doesn't grow with the rows; a test pins it. +- [ ] Lists are paginated; no prop over 50 KB. +- [ ] Expensive props are callables, so partial reloads skip them. +- [ ] Filters reload with `only`, `preserveState`, `preserveScroll` and `replace`. +- [ ] No layout shift when deferred content lands (CLS ≤ 0.1). +- [ ] Loading, empty, filtered-empty and error states each have their own copy. +- [ ] Buttons disable while their request is in flight. +- [ ] Nothing scrolls sideways at 390 px. +- [ ] Links point at canonical URLs (no redirects). +- [ ] The page audit is clean on a development server and on a production build. diff --git a/framework/cli/tests/test_skill_page_audit.py b/framework/cli/tests/test_skill_page_audit.py new file mode 100644 index 000000000..3a8893cd4 --- /dev/null +++ b/framework/cli/tests/test_skill_page_audit.py @@ -0,0 +1,159 @@ +"""The pure parts of the page-perf skill's audit script. + +The script ships inside ``skills/simple-module-page-perf`` (bundled with the +CLI), so it is tested here: URL patterns and crawl sampling, Server-Timing +parsing, page description and the budget verdicts. The browser half needs a +running app and is exercised by running the audit itself. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +import pytest + +pytest.importorskip("playwright") + +_SCRIPTS = Path(__file__).resolve().parents[3] / "skills/simple-module-page-perf/scripts/page_audit" +sys.path.insert(0, str(_SCRIPTS)) + +import audit_crawl # noqa: E402 +import audit_report # noqa: E402 +import audit_server # noqa: E402 +from audit_browser import BrowserResult # noqa: E402 + +_UUID = "91a1b900-5c0c-45c8-9e0c-894d78f7c425" + + +class TestCrawl: + def test_ids_collapse_to_one_pattern(self) -> None: + assert audit_crawl.pattern(f"/admin/users/{_UUID}/edit?tab=2") == "/admin/users/:id/edit" + assert audit_crawl.pattern("/orders/42") == "/orders/:id" + assert audit_crawl.pattern("/admin/settings/store") == "/admin/settings/store" + + def test_one_sample_per_pattern(self) -> None: + links = {"/admin/users/": [f"/admin/users/{_UUID}", "/admin/users/7", "/admin/users/add"]} + found = audit_crawl.extra_pages(["/admin/users/"], links, [], 10) + assert found == { + f"/admin/users/{_UUID}": "/admin/users/", + "/admin/users/add": "/admin/users/", + } + + def test_a_covered_section_is_sampled_from_its_own_pages(self) -> None: + """An audit row linking into settings is a worse sample than settings' own list.""" + links = { + "/admin/audit-log/": [f"/admin/settings/{_UUID}/edit"], + "/admin/settings/store": ["/admin/settings/1/edit"], + } + known = ["/admin/audit-log/", "/admin/settings/store"] + found = audit_crawl.extra_pages(known, links, [], 10) + assert found == {"/admin/settings/1/edit": "/admin/settings/store"} + + def test_excluded_links_and_the_limit(self) -> None: + links = {"/a/": ["/a/logout", "/a/1", "/a/x/y", "/a/z"]} + excluded = [re.compile("logout")] + assert list(audit_crawl.extra_pages(["/a/"], links, excluded, 2)) == ["/a/1", "/a/x/y"] + + def test_hubs_prefer_parent_then_same_area_then_home(self) -> None: + known = ["/dashboard/", "/admin/sharing/", "/admin/users/"] + assert audit_crawl.hubs_for("/admin/users/", None, known) == ["/admin/sharing/", "/"] + assert audit_crawl.hubs_for("/admin/users/7", "/admin/users/", known) == [ + "/admin/users/", + "/admin/sharing/", + "/", + ] + + +class TestServer: + def test_server_timing_is_parsed(self) -> None: + header = 'app;dur=12.4, db;dur=3.1;desc="5 queries"' + assert audit_server._timing(header) == (12.4, 3.1, 5) + assert audit_server._timing(None) == (None, None, None) + + def test_menu_urls_are_discovered(self) -> None: + props = { + "menus": { + "sidebar": [{"url": "/orders/", "children": [{"url": "/orders/new"}]}], + "admin": [{"href": "/admin/x/"}, {"url": "https://elsewhere.test/"}], + } + } + assert audit_server.discover_paths(props) == ["/orders/", "/orders/new", "/admin/x/"] + + def test_page_description_separates_shared_and_deferred_props(self) -> None: + result = audit_server.ServerResult(path="/orders/") + page = { + "component": "Orders/Index", + "props": { + "auth": {"user": None}, + "i18n": {"locale": "en", "messages": {"a": "b"}}, + "rows": [{"id": i} for i in range(50)], + "filters": {}, + }, + "deferredProps": {"stats": ["totals", "chart"]}, + "mergeProps": ["rows"], + } + audit_server._describe_page(result, page) + assert result.component == "Orders/Index" + assert result.top_props[0][0] == "rows" + assert "auth" not in dict(result.top_props) + assert result.i18n_bytes > 0 + assert result.deferred == ["chart", "totals"] + assert result.merge == ["rows"] + + +def _server(**over) -> audit_server.ServerResult: + base = {"path": "/p/", "app_ms": 10.0, "queries": 3, "json_bytes": 2048} + return audit_server.ServerResult(**{**base, **over}) + + +def _checks(findings) -> dict[str, str]: + return {f.check: f.level for f in findings} + + +class TestBudgets: + def test_a_fast_small_page_is_clean(self) -> None: + assert audit_report.evaluate(_server(), None) == [] + + def test_warn_and_fail_thresholds(self) -> None: + checks = _checks(audit_report.evaluate(_server(app_ms=150.0, queries=40), None)) + assert checks == {"app_ms": "warn", "queries": "fail"} + + def test_the_i18n_catalog_does_not_count_against_the_page(self) -> None: + big = _server(json_bytes=400 * 1024, i18n_bytes=390 * 1024) + assert audit_report.evaluate(big, None) == [] + + def test_redirects_and_server_errors(self) -> None: + redirected = _server(redirected=True, final_url="http://t/p/") + assert _checks(audit_report.evaluate(redirected, None)) == {"redirect": "warn"} + broken = _server(error="HTTP 500 (not an Inertia page response)") + assert _checks(audit_report.evaluate(broken, None)) == {"server": "fail"} + + def test_bundle_budgets_only_apply_to_a_production_build(self) -> None: + load = {"lcp_ms": 5000, "js_bytes": 900 * 1024, "cls": 0} + dev = BrowserResult(path="/p/", load={**load, "dev_build": True}) + prod = BrowserResult(path="/p/", load={**load, "dev_build": False}) + assert _checks(audit_report.evaluate(_server(), dev)) == {} + assert _checks(audit_report.evaluate(_server(), prod)) == { + "lcp_ms": "fail", + "js_kb": "fail", + } + + def test_ux_checks(self) -> None: + browser = BrowserResult( + path="/p/", + load={"cls": 0.3, "dev_build": True}, + ux={"dom_nodes": 2000, "stuck_loaders": 1, "unnamed_controls": 2, "h1_count": 0}, + mobile_overflow_x=40, + console_errors=["TypeError: x is undefined"], + ) + assert _checks(audit_report.evaluate(_server(), browser)) == { + "cls": "fail", + "dom_nodes": "warn", + "console": "fail", + "overflow": "warn", + "stuck_loaders": "warn", + "unnamed_controls": "warn", + "h1": "warn", + } diff --git a/framework/db/simple_module_db/query_stats.py b/framework/db/simple_module_db/query_stats.py new file mode 100644 index 000000000..b10e9fa10 --- /dev/null +++ b/framework/db/simple_module_db/query_stats.py @@ -0,0 +1,82 @@ +"""Per-request SQL statement counting. + +Development hosts install this on their engine and expose the totals in a +``Server-Timing`` header (see ``simple_module_hosting._server_timing``), so a +page that issues one query per row shows up in the browser's network panel and +in the page-perf audit instead of only under load. + +Counting is opt-in twice over: :func:`install_query_stats` adds the engine +listeners, and only code inside :func:`track_queries` is counted. Statements +run outside a tracked block (startup, background work) cost one ContextVar +read each. +""" + +from __future__ import annotations + +import time +from collections.abc import Generator +from contextlib import contextmanager +from contextvars import ContextVar +from dataclasses import dataclass + +from sqlalchemy import event +from sqlalchemy.ext.asyncio import AsyncEngine + +__all__ = ["QueryStats", "install_query_stats", "track_queries"] + +_START_KEY = "_sm_query_started" +_INSTALLED_FLAG = "_sm_query_stats_installed" + + +@dataclass(slots=True) +class QueryStats: + """Statements executed inside one :func:`track_queries` block.""" + + count: int = 0 + seconds: float = 0.0 + + +_current: ContextVar[QueryStats | None] = ContextVar("sm_query_stats", default=None) + + +def _before(conn, cursor, statement, parameters, context, executemany) -> None: + if _current.get() is not None and context is not None: + setattr(context, _START_KEY, time.perf_counter()) + + +def _after(conn, cursor, statement, parameters, context, executemany) -> None: + stats = _current.get() + if stats is None: + return + stats.count += 1 + started = getattr(context, _START_KEY, None) if context is not None else None + if started is not None: + stats.seconds += time.perf_counter() - started + + +def install_query_stats(engine: AsyncEngine) -> None: + """Count statements on *engine* for any :func:`track_queries` block. + + Idempotent per engine. + """ + sync_engine = engine.sync_engine + if getattr(sync_engine, _INSTALLED_FLAG, False): + return + event.listen(sync_engine, "before_cursor_execute", _before) + event.listen(sync_engine, "after_cursor_execute", _after) + setattr(sync_engine, _INSTALLED_FLAG, True) + + +@contextmanager +def track_queries() -> Generator[QueryStats]: + """Count the statements run by this task (and tasks it spawns) until exit. + + The stats object is shared by reference, so work SQLAlchemy runs in a + greenlet or a child task still lands in the same totals. + """ + stats = QueryStats() + token = _current.set(stats) + try: + yield stats + finally: + _current.reset(token) diff --git a/framework/db/tests/test_db_query_stats.py b/framework/db/tests/test_db_query_stats.py new file mode 100644 index 000000000..42783ca1d --- /dev/null +++ b/framework/db/tests/test_db_query_stats.py @@ -0,0 +1,66 @@ +"""Per-request SQL statement counting (feeds the dev Server-Timing header).""" + +from __future__ import annotations + +import asyncio + +from simple_module_db.query_stats import install_query_stats, track_queries +from sqlalchemy import text +from sqlalchemy.ext.asyncio import create_async_engine + + +async def test_counts_only_statements_inside_the_tracked_block() -> None: + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + install_query_stats(engine) + try: + async with engine.connect() as conn: + await conn.execute(text("select 1")) # untracked + with track_queries() as stats: + await conn.execute(text("select 1")) + await conn.execute(text("select 2")) + await conn.execute(text("select 3")) # after the block + assert stats.count == 2 + assert stats.seconds > 0 + finally: + await engine.dispose() + + +async def test_install_is_idempotent() -> None: + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + install_query_stats(engine) + install_query_stats(engine) + try: + with track_queries() as stats: + async with engine.connect() as conn: + await conn.execute(text("select 1")) + # A second install must not register the listeners twice. + assert stats.count == 1 + finally: + await engine.dispose() + + +async def test_child_tasks_share_the_request_totals() -> None: + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + install_query_stats(engine) + + async def one_query() -> None: + async with engine.connect() as conn: + await conn.execute(text("select 1")) + + try: + with track_queries() as stats: + await asyncio.gather(one_query(), one_query()) + assert stats.count == 2 + finally: + await engine.dispose() + + +async def test_engine_without_install_counts_nothing() -> None: + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + try: + with track_queries() as stats: + async with engine.connect() as conn: + await conn.execute(text("select 1")) + assert stats.count == 0 + finally: + await engine.dispose() diff --git a/framework/hosting/simple_module_hosting/_app_boot.py b/framework/hosting/simple_module_hosting/_app_boot.py index 21a168f94..d5ed107f5 100644 --- a/framework/hosting/simple_module_hosting/_app_boot.py +++ b/framework/hosting/simple_module_hosting/_app_boot.py @@ -13,6 +13,7 @@ from simple_module_core.permissions import PermissionRegistry from simple_module_core.public_routes import PublicRouteRegistry from simple_module_db.listeners import register_listeners +from simple_module_db.query_stats import install_query_stats from simple_module_db.session import init_db from simple_module_hosting._db_health import register_database_check @@ -93,6 +94,9 @@ def init_database(settings: Settings, health_registry: HealthRegistry): if not settings.multi_tenant and settings.default_tenant: db_state.default_tenant_id = settings.default_tenant register_listeners(db_state) + if settings.is_development: + # Feeds the development Server-Timing header (query count per request). + install_query_stats(db_state.engine) configure_process_services(settings.redis_url, engine=db_state.engine) # The host's own readiness signal, and the only probe-safe check in a # default install — module checks reach third parties and are on-demand. diff --git a/framework/hosting/simple_module_hosting/_middleware_pipeline.py b/framework/hosting/simple_module_hosting/_middleware_pipeline.py index f2ae2f7d2..2b780e8f0 100644 --- a/framework/hosting/simple_module_hosting/_middleware_pipeline.py +++ b/framework/hosting/simple_module_hosting/_middleware_pipeline.py @@ -18,6 +18,7 @@ from simple_module_hosting._forwarded_host import ForwardedHostMiddleware from simple_module_hosting._rate_limit import RateLimitMiddleware from simple_module_hosting._route_access import API_PREFIX +from simple_module_hosting._server_timing import ServerTimingMiddleware from simple_module_hosting.i18n_middleware import LocaleMiddleware from simple_module_hosting.maintenance import MaintenanceMiddleware from simple_module_hosting.middleware import ( @@ -174,6 +175,10 @@ def install_middleware( if api_policy is not None: cors_registry.add_prefix(API_PREFIX, api_policy) app.add_middleware(CorsMiddleware, registry=cors_registry) + if settings.is_development: + # Inside RequestLogging, outside everything that does work: its ``app`` + # time covers the whole pipeline and every query the request ran. + app.add_middleware(ServerTimingMiddleware) app.add_middleware(RequestLoggingMiddleware) app.add_middleware(CorrelationIdMiddleware) if sentry.is_enabled(): diff --git a/framework/hosting/simple_module_hosting/_server_timing.py b/framework/hosting/simple_module_hosting/_server_timing.py new file mode 100644 index 000000000..bd3425325 --- /dev/null +++ b/framework/hosting/simple_module_hosting/_server_timing.py @@ -0,0 +1,64 @@ +"""Development-only ``Server-Timing`` header: request time and SQL statements. + +Every response in a development host carries:: + + Server-Timing: app;dur=12.4, db;dur=3.1;desc="5 queries" + +``app`` is the time from this middleware until the response starts (the +handler, every inner middleware and the commit); ``db`` is the time spent in +the database driver, and ``desc`` the number of statements. Browsers show both +in the network panel's Timing tab, and the page-perf audit +(``skills/simple-module-page-perf``) reads them to flag N+1 pages. + +Never installed outside development: per-request timings help an attacker +time lookups (user enumeration, cache probing). +""" + +from __future__ import annotations + +import time + +from simple_module_db.query_stats import track_queries +from starlette.datastructures import MutableHeaders +from starlette.types import ASGIApp, Message, Receive, Scope, Send + +__all__ = ["SERVER_TIMING_HEADER", "ServerTimingMiddleware", "format_server_timing"] + +SERVER_TIMING_HEADER = "Server-Timing" +_SCOPE_HTTP = "http" +_MSG_RESPONSE_START = "http.response.start" + + +def format_server_timing(app_seconds: float, db_seconds: float, queries: int) -> str: + """Render the header value (durations in milliseconds, one decimal).""" + noun = "query" if queries == 1 else "queries" + return ( + f'app;dur={app_seconds * 1000:.1f}, db;dur={db_seconds * 1000:.1f};desc="{queries} {noun}"' + ) + + +class ServerTimingMiddleware: + """Add a ``Server-Timing`` header with app time and SQL statement totals.""" + + def __init__(self, app: ASGIApp) -> None: + self.app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != _SCOPE_HTTP: + await self.app(scope, receive, send) + return + + start = time.perf_counter() + with track_queries() as stats: + + async def send_with_timing(message: Message) -> None: + if message["type"] == _MSG_RESPONSE_START: + MutableHeaders(scope=message).append( + SERVER_TIMING_HEADER, + format_server_timing( + time.perf_counter() - start, stats.seconds, stats.count + ), + ) + await send(message) + + await self.app(scope, receive, send_with_timing) diff --git a/framework/hosting/tests/test_hosting_server_timing.py b/framework/hosting/tests/test_hosting_server_timing.py new file mode 100644 index 000000000..da4d9389b --- /dev/null +++ b/framework/hosting/tests/test_hosting_server_timing.py @@ -0,0 +1,70 @@ +"""Development-only Server-Timing header: app time plus SQL statement count.""" + +from __future__ import annotations + +import re + +import httpx +import pytest +from simple_module_db.query_stats import install_query_stats +from simple_module_hosting._server_timing import ServerTimingMiddleware, format_server_timing +from simple_module_hosting.app_builder import create_app +from simple_module_hosting.settings import Settings +from sqlalchemy import text +from sqlalchemy.ext.asyncio import create_async_engine +from starlette.applications import Starlette +from starlette.responses import PlainTextResponse +from starlette.routing import Route + +_HEADER = re.compile(r'^app;dur=\d+\.\d, db;dur=\d+\.\d;desc="(\d+) quer(?:y|ies)"$') + + +def test_format_server_timing() -> None: + assert format_server_timing(0.0124, 0.0031, 5) == 'app;dur=12.4, db;dur=3.1;desc="5 queries"' + assert format_server_timing(0.001, 0.0, 1).endswith('desc="1 query"') + + +async def test_header_counts_the_requests_queries() -> None: + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + install_query_stats(engine) + + async def three_queries(_request): + async with engine.connect() as conn: + for _ in range(3): + await conn.execute(text("select 1")) + return PlainTextResponse("ok") + + app = ServerTimingMiddleware(Starlette(routes=[Route("/", three_queries)])) + try: + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://t") as client: + first = await client.get("/") + second = await client.get("/") + finally: + await engine.dispose() + + for response in (first, second): + match = _HEADER.match(response.headers["server-timing"]) + assert match, response.headers["server-timing"] + # Per request, not cumulative across requests. + assert match.group(1) == "3" + + +def _names(settings: Settings) -> tuple[str, ...]: + return tuple(m.cls.__name__ for m in create_app(settings).user_middleware) + + +@pytest.mark.parametrize(("environment", "installed"), [("development", True), ("testing", False)]) +def test_installed_only_in_development(environment: str, installed: bool) -> None: + names = _names( + Settings( + database_url="sqlite+aiosqlite:///:memory:", + environment=environment, + secret_key="x" * 32, + ) + ) + assert ("ServerTimingMiddleware" in names) is installed + if installed: + # Right inside request logging, so it times the whole pipeline. + logging_at = names.index("RequestLoggingMiddleware") + assert names[logging_at + 1] == "ServerTimingMiddleware" diff --git a/modules/background_tasks/background_tasks/endpoints/views.py b/modules/background_tasks/background_tasks/endpoints/views.py index 909ae0ed3..716988261 100644 --- a/modules/background_tasks/background_tasks/endpoints/views.py +++ b/modules/background_tasks/background_tasks/endpoints/views.py @@ -2,7 +2,6 @@ from __future__ import annotations -import asyncio import uuid from fastapi import APIRouter, Depends, HTTPException, Query, Request @@ -10,7 +9,7 @@ from simple_module_hosting.inertia_deps import InertiaDep from simple_module_hosting.pagination import lenient_page_params from simple_module_hosting.permissions import RequiresPermission -from simple_module_inertia import InertiaResponse +from simple_module_inertia import InertiaResponse, defer from background_tasks.celery_app import describe_routes from background_tasks.constants import ( @@ -30,6 +29,9 @@ # Window the "Succeeded 24h" tile counts over. SUCCESS_WINDOW_HOURS = 24 +# Deferred group for props that wait on the broker. +_WORKERS_GROUP = "workers" + async def _worker_presence(request: Request) -> dict[str, object]: """Whether anything is actually able to run a task right now. @@ -41,7 +43,8 @@ async def _worker_presence(request: Request) -> dict[str, object]: Polling costs one inspect timeout (~1s), so this is called only when the unfiltered list is genuinely empty — the one moment the answer changes what - the screen should say. The normal path never pays for it. + the screen should say — and as a deferred prop: the page paints at once + and the empty state says it is checking until the answer lands. """ snapshot = await poll_workers(request) return { @@ -50,6 +53,15 @@ async def _worker_presence(request: Request) -> dict[str, object]: } +def _snapshot_json(request: Request): + """A deferred loader for the Workers page's fleet snapshot.""" + + async def load() -> dict[str, object]: + return (await poll_workers(request)).model_dump(mode="json") + + return load + + @router.get("/", response_model=None) async def index( inertia: InertiaDep, @@ -98,13 +110,7 @@ async def strip_data() -> tuple[dict[str, int], list[str]]: # queue they are in, so it keeps offering the ones they are not in. return counts, await service.queues() - # `_worker_presence` polls the broker over a thread, not the DB session the - # counts use — the two don't share state, so run them concurrently instead - # of paying the ~1s inspect timeout serially on top of the queries. - if unfiltered_and_empty: - (counts, queues), presence = await asyncio.gather(strip_data(), _worker_presence(request)) - else: - (counts, queues), presence = await strip_data(), None + counts, queues = await strip_data() return await inertia.render( "BackgroundTasks/Index", { @@ -116,7 +122,13 @@ async def strip_data() -> tuple[dict[str, int], list[str]]: "queues": queues, "tenant_ids": await service.tenant_ids(), "platform_tenant_value": PLATFORM_TENANT_FILTER, - "worker_presence": presence, + # Deferred: the broker poll waits out a ~1s inspect timeout, which + # would otherwise hold back the whole page on a fresh install. + "worker_presence": ( + defer(lambda: _worker_presence(request), group=_WORKERS_GROUP) + if unfiltered_and_empty + else None + ), "has_executions": has_executions, "pagination": { "page": response.page, @@ -140,7 +152,6 @@ async def workers( service: BackgroundTaskService = Depends(get_background_task_service), ) -> InertiaResponse: settings = request.app.state.background_tasks.settings - snapshot = await poll_workers(request) # The start command the empty state offers has to name the queues this # install actually uses, or it starts a worker that consumes nothing. The # default queue leads because it is the one that always exists, even before @@ -154,7 +165,9 @@ async def workers( return await inertia.render( "BackgroundTasks/Workers", { - "snapshot": snapshot.model_dump(mode="json"), + # Deferred for the same ~1s inspect timeout: the routes and the + # start command render at once, the fleet fills in behind a skeleton. + "snapshot": defer(_snapshot_json(request), group=_WORKERS_GROUP), "broker_url_redacted": redact_broker_url(settings.broker_url), "queues": queues, "routes": routes, diff --git a/modules/background_tasks/background_tasks/locales/de.json b/modules/background_tasks/background_tasks/locales/de.json index 067a21f9d..e102c409e 100644 --- a/modules/background_tasks/background_tasks/locales/de.json +++ b/modules/background_tasks/background_tasks/locales/de.json @@ -125,7 +125,9 @@ "no_workers_title": "Kein Worker läuft", "no_workers_description": "Der Broker ist verfügbar, aber kein Worker verarbeitet die Warteschlange. Jetzt eingereihte Aufgaben warten auf ihre Ausführung.", "healthy_title": "Noch keine Aufgabe ausgeführt", - "healthy_description": "Registrierte Aufgaben erscheinen hier bei ihrer ersten Ausführung." + "healthy_description": "Registrierte Aufgaben erscheinen hier bei ihrer ersten Ausführung.", + "checking_title": "Suche nach Workern…", + "checking_description": "Der Broker wird gefragt, ob etwas läuft, das Aufgaben ausführt." }, "workers": { "title": "Worker", @@ -155,7 +157,9 @@ "routes_module": "Festgelegt durch", "routes_override": "Betreiberüberschreibung", "routes_default_queue": "Standardwarteschlange", - "routes_empty": "Keine Aufgabenrouten konfiguriert. Jede Aufgabe wird an die Standardwarteschlange gesendet. Fügen Sie Routen in task_config.py (TASK_ROUTES) eines Moduls hinzu." + "routes_empty": "Keine Aufgabenrouten konfiguriert. Jede Aufgabe wird an die Standardwarteschlange gesendet. Fügen Sie Routen in task_config.py (TASK_ROUTES) eines Moduls hinzu.", + "checking": "Broker wird abgefragt…", + "checking_label": "Worker werden geladen" }, "nav": { "background_tasks": "Hintergrundaufgaben", diff --git a/modules/background_tasks/background_tasks/locales/en.json b/modules/background_tasks/background_tasks/locales/en.json index ec802ce0e..7354dbfc0 100644 --- a/modules/background_tasks/background_tasks/locales/en.json +++ b/modules/background_tasks/background_tasks/locales/en.json @@ -125,7 +125,9 @@ "no_workers_title": "No worker is running", "no_workers_description": "The broker is up, but nothing is consuming the queue. Tasks enqueued now will wait rather than run.", "healthy_title": "No task has run yet", - "healthy_description": "Registered tasks appear here the first time they execute." + "healthy_description": "Registered tasks appear here the first time they execute.", + "checking_title": "Checking for workers…", + "checking_description": "Asking the broker whether anything is running to execute tasks." }, "workers": { "title": "Workers", @@ -155,7 +157,9 @@ "routes_module": "Set by", "routes_override": "Operator override", "routes_default_queue": "Default queue", - "routes_empty": "No task routes are configured, so every task goes to the default queue. Add routes in a module's task_config.py (TASK_ROUTES)." + "routes_empty": "No task routes are configured, so every task goes to the default queue. Add routes in a module's task_config.py (TASK_ROUTES).", + "checking": "Polling the broker…", + "checking_label": "Loading workers" }, "nav": { "background_tasks": "Background tasks", diff --git a/modules/background_tasks/background_tasks/pages/Detail.tsx b/modules/background_tasks/background_tasks/pages/Detail.tsx index 4e91c4bf1..3cb009d4a 100644 --- a/modules/background_tasks/background_tasks/pages/Detail.tsx +++ b/modules/background_tasks/background_tasks/pages/Detail.tsx @@ -14,7 +14,7 @@ import { ProgressCard } from './components/ProgressCard'; import { RetryConfirmDialog } from './components/RetryConfirmDialog'; import { StatusPill } from './components/StatusPill'; import { TracebackCard } from './components/TracebackCard'; -import { RETRYABLE_STATUSES, type TaskDetail, VIEW_BASE } from './constants'; +import { RETRYABLE_STATUSES, type TaskDetail, VIEW_BASE, VIEW_INDEX } from './constants'; import { retryExecution } from './retry'; interface Props { @@ -50,7 +50,7 @@ function Detail({ execution, max_retries: maxRetries }: Props) { title={execution.task_name} titleClassName="font-mono text-[22px]" mono - back={VIEW_BASE} + back={VIEW_INDEX} // Both are hidden on phones: the strip below restates them at a size // the 390px frame can read, and the shell's header showed the same // pill and the same attempt count immediately above it. @@ -70,7 +70,7 @@ function Detail({ execution, max_retries: maxRetries }: Props) { chevron, and the retry lives at the foot of the page where a thumb is. */} - } - /> - ) : ( - - - - {t(keys.background_tasks.worker_health.view_workers)} - - - } - /> - )} + {/* aria-busy: assistive tech waits for the poll's answer instead of reading the placeholder. */} +
+ {filtered ? ( + + {t(keys.background_tasks.tasks_empty.clear_filters)} + + } + /> + ) : ( + + + + {t(keys.background_tasks.worker_health.view_workers)} + + + } + /> + )} +
); } diff --git a/modules/background_tasks/background_tasks/pages/components/WorkerCard.tsx b/modules/background_tasks/background_tasks/pages/components/WorkerCard.tsx index 6e2b8ae77..d650f6834 100644 --- a/modules/background_tasks/background_tasks/pages/components/WorkerCard.tsx +++ b/modules/background_tasks/background_tasks/pages/components/WorkerCard.tsx @@ -1,5 +1,6 @@ import { keys, useT } from '@simple-module-py/i18n'; import { Card } from '@simple-module-py/ui/components/ui/card'; +import { Skeleton } from '@simple-module-py/ui/components/ui/skeleton'; import { cn } from '@simple-module-py/ui/lib/utils'; import type { ReactNode } from 'react'; import { EM_DASH, formatSoftware, formatUptime, type WorkerInfo } from '../constants'; @@ -100,3 +101,30 @@ export function WorkerCard({ worker }: { worker: WorkerInfo }) { ); } + +/** + * Stand-in for the fleet while the deferred broker poll is in flight. + * + * Two cards' worth of space in the same grid, so the routes table below does + * not jump when the answer lands — whether that is cards, or an empty state. + */ +export function WorkersSkeleton({ label }: { label: string }) { + return ( +
+ {[0, 1].map((i) => ( + +
+ + + +
+
+ {[0, 1, 2, 3].map((j) => ( + + ))} +
+ + ))} +
+ ); +} diff --git a/modules/background_tasks/background_tasks/pages/constants.ts b/modules/background_tasks/background_tasks/pages/constants.ts index 15db2d753..4864d1548 100644 --- a/modules/background_tasks/background_tasks/pages/constants.ts +++ b/modules/background_tasks/background_tasks/pages/constants.ts @@ -6,6 +6,8 @@ import { formatDayMonthTime } from '@simple-module-py/ui/lib/date-format'; export const API_BASE = '/api/background_tasks/admin'; export const VIEW_BASE = '/admin/background-tasks'; +/** The list page. Its canonical url has the slash: without it, every link costs a redirect. */ +export const VIEW_INDEX = `${VIEW_BASE}/`; /** Rendered wherever a value is genuinely absent, so the columns still line up. */ export const EM_DASH = '—'; diff --git a/modules/background_tasks/tests-js/background_tasks_IndexPage.test.tsx b/modules/background_tasks/tests-js/background_tasks_IndexPage.test.tsx index da4125265..fe3e97fce 100644 --- a/modules/background_tasks/tests-js/background_tasks_IndexPage.test.tsx +++ b/modules/background_tasks/tests-js/background_tasks_IndexPage.test.tsx @@ -72,6 +72,25 @@ describe('background_tasks Index page', () => { vi.clearAllMocks(); }); + it('says it is checking for workers while the deferred poll runs, then answers', () => { + const empty = { + executions: [], + pagination: { page: 1, per_page: 20, total: 0 }, + has_executions: false, + }; + const props = () => state.props as unknown as ComponentProps; + setProps({ ...empty, worker_presence: undefined }); + const { rerender } = render(); + expect(screen.getByText(keys.background_tasks.tasks_empty.checking_title)).toBeInTheDocument(); + + setProps({ ...empty, worker_presence: { broker_reachable: true, worker_count: 0 } }); + rerender(); + expect( + screen.getByText(keys.background_tasks.tasks_empty.no_workers_title), + ).toBeInTheDocument(); + expect(screen.queryByText(keys.background_tasks.tasks_empty.checking_title)).toBeNull(); + }); + it('lists executions and offers retry controls to managers only', () => { render()} />); expect(screen.getByText('demo.task')).toBeInTheDocument(); diff --git a/modules/background_tasks/tests-js/background_tasks_WorkersPage.test.tsx b/modules/background_tasks/tests-js/background_tasks_WorkersPage.test.tsx new file mode 100644 index 000000000..e0c562062 --- /dev/null +++ b/modules/background_tasks/tests-js/background_tasks_WorkersPage.test.tsx @@ -0,0 +1,55 @@ +import '@testing-library/jest-dom/vitest'; +import { configureI18n, keys } from '@simple-module-py/i18n'; +import { render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +configureI18n({ locale: 'en', messages: {} }); + +vi.mock('@inertiajs/react', () => ({ + Head: () => null, + usePage: () => ({ props: { auth: { permissions: [] } } }), + Link: ({ children, href }: { children: React.ReactNode; href: string }) => ( + {children} + ), +})); +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +import type { WorkerSnapshot } from '../background_tasks/pages/constants'; +import Workers from '../background_tasks/pages/Workers'; + +const w = keys.background_tasks.workers; +const base = { broker_url_redacted: 'redis://localhost:6379/0', queues: ['default'], routes: [] }; +const snapshot: WorkerSnapshot = { + broker_reachable: true, + polled_at: '2026-01-01T12:00:00Z', + error: null, + workers: [ + { + hostname: 'celery@host-a', + online: true, + queues: ['default'], + active_task_count: 0, + pool_size: 4, + total_processed: 10, + software: null, + uptime_seconds: 60, + }, + ], +}; + +describe('background_tasks Workers page', () => { + it('holds the fleet space with a busy skeleton until the deferred snapshot lands', () => { + const { rerender } = render(); + expect(screen.getByRole('status', { name: w.checking_label })).toHaveAttribute( + 'aria-busy', + 'true', + ); + expect(screen.getByText(w.checking)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: w.refresh })).toBeDisabled(); + + rerender(); + expect(screen.queryByRole('status', { name: w.checking_label })).toBeNull(); + expect(screen.getByText('celery@host-a')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: w.refresh })).toBeEnabled(); + }); +}); diff --git a/modules/background_tasks/tests/test_bg_index_worker_presence.py b/modules/background_tasks/tests/test_bg_index_worker_presence.py index c40f5f43c..800327b6e 100644 --- a/modules/background_tasks/tests/test_bg_index_worker_presence.py +++ b/modules/background_tasks/tests/test_bg_index_worker_presence.py @@ -4,7 +4,8 @@ enqueued, or no worker is running and the queue is unattended — and the page cannot tell them apart without asking the broker. Polling costs an inspect timeout, so the view only pays it when the answer would change what the screen -says: an unfiltered list that came back empty. +says: an unfiltered list that came back empty. Even then it is a deferred prop, +so the page paints first and the poll's ~1s lands in a follow-up request. """ from __future__ import annotations @@ -15,6 +16,7 @@ import pytest from background_tasks import worker_inspector as wi from background_tasks.contracts.schemas import WorkerInfo, WorkerSnapshot +from simple_module_test import InertiaPage # Trailing slash: the index is registered at "/" under the prefix, and the # bare path 307s to it. @@ -52,6 +54,13 @@ async def _index(client: httpx.AsyncClient, **params: str) -> dict: return body["props"] +async def _presence(client: httpx.AsyncClient) -> dict: + """The deferred ``worker_presence`` as the client loads it after paint.""" + page = InertiaPage.from_response(await client.get(VIEW_BASE, headers=INERTIA_HEADERS)) + page.deferred("workers", "worker_presence").missing("worker_presence") + return (await page.load_deferred(client, "workers")).props["worker_presence"] + + class TestWorkerPresenceIsPolled: async def test_empty_unfiltered_list_reports_a_live_fleet( self, @@ -62,10 +71,12 @@ async def test_empty_unfiltered_list_reports_a_live_fleet( wi.WorkerInspector, "snapshot", lambda self: _snapshot(reachable=True, online_workers=2) ) - props = await _index(authenticated_client) - - assert props["pagination"]["total"] == 0 - assert props["worker_presence"] == {"broker_reachable": True, "worker_count": 2} + assert (await _index(authenticated_client))["pagination"]["total"] == 0 + presence = await _presence(authenticated_client) + assert presence == { + "broker_reachable": True, + "worker_count": 2, + } async def test_distinguishes_no_worker_from_nothing_enqueued( self, @@ -77,9 +88,11 @@ async def test_distinguishes_no_worker_from_nothing_enqueued( wi.WorkerInspector, "snapshot", lambda self: _snapshot(reachable=True, online_workers=0) ) - props = await _index(authenticated_client) - - assert props["worker_presence"] == {"broker_reachable": True, "worker_count": 0} + presence = await _presence(authenticated_client) + assert presence == { + "broker_reachable": True, + "worker_count": 0, + } async def test_unreachable_broker_is_reported_rather_than_raising( self, @@ -92,9 +105,11 @@ async def test_unreachable_broker_is_reported_rather_than_raising( lambda self: _snapshot(reachable=False, online_workers=0), ) - props = await _index(authenticated_client) - - assert props["worker_presence"] == {"broker_reachable": False, "worker_count": 0} + presence = await _presence(authenticated_client) + assert presence == { + "broker_reachable": False, + "worker_count": 0, + } async def test_offline_workers_are_not_counted( self, @@ -106,9 +121,27 @@ async def test_offline_workers_are_not_counted( snapshot.workers[0].online = False monkeypatch.setattr(wi.WorkerInspector, "snapshot", lambda self: snapshot) + presence = await _presence(authenticated_client) + assert presence == { + "broker_reachable": True, + "worker_count": 0, + } + + +class TestFirstPaintNeverWaitsForTheBroker: + async def test_initial_response_does_not_poll( + self, + monkeypatch, + authenticated_client: httpx.AsyncClient, + ): + def _fail(self): # pragma: no cover - asserts it is never reached + raise AssertionError("the first response must not wait on inspect()") + + monkeypatch.setattr(wi.WorkerInspector, "snapshot", _fail) + props = await _index(authenticated_client) - assert props["worker_presence"] == {"broker_reachable": True, "worker_count": 0} + assert "worker_presence" not in props class TestWorkerPresenceIsSkipped: diff --git a/modules/background_tasks/tests/test_workers_endpoints.py b/modules/background_tasks/tests/test_workers_endpoints.py index 7485108c7..4bc177461 100644 --- a/modules/background_tasks/tests/test_workers_endpoints.py +++ b/modules/background_tasks/tests/test_workers_endpoints.py @@ -8,6 +8,7 @@ import pytest from background_tasks import worker_inspector as wi from background_tasks.contracts.schemas import WorkerInfo, WorkerSnapshot +from simple_module_test import InertiaPage JSON_BASE = "/api/background_tasks/admin" VIEW_BASE = "/admin/background-tasks" @@ -92,9 +93,10 @@ async def test_renders_page_with_snapshot_prop( headers={"X-Inertia": "true", "Accept": "application/json"}, ) assert resp.status_code == 200 - body = resp.json() - assert body["component"] == "BackgroundTasks/Workers" - snapshot = body["props"]["snapshot"] + page = InertiaPage.from_response(resp).assert_component("BackgroundTasks/Workers") + # Deferred: the broker poll can take ~1s, so the page paints without it. + page.deferred("workers", "snapshot").missing("snapshot") + snapshot = (await page.load_deferred(authenticated_client, "workers")).props["snapshot"] assert snapshot["broker_reachable"] is True assert snapshot["workers"][0]["hostname"] == "celery@host-a" diff --git a/modules/background_tasks/tests/test_workers_uptime.py b/modules/background_tasks/tests/test_workers_uptime.py index e4d9f171a..01ba57dc2 100644 --- a/modules/background_tasks/tests/test_workers_uptime.py +++ b/modules/background_tasks/tests/test_workers_uptime.py @@ -18,6 +18,7 @@ from background_tasks import worker_inspector as wi from background_tasks.contracts.schemas import WorkerInfo, WorkerSnapshot from background_tasks.worker_inspector import redact_broker_url +from simple_module_test import InertiaPage VIEW_WORKERS = "/admin/background-tasks/workers" JSON_BASE = "/api/background_tasks/admin" @@ -39,11 +40,12 @@ def _stats(**overrides) -> dict: async def _workers_props(client: httpx.AsyncClient) -> dict: + """The page's props once the deferred fleet snapshot has loaded too.""" resp = await client.get(VIEW_WORKERS, headers=INERTIA_HEADERS) assert resp.status_code == 200 - body = resp.json() - assert body["component"] == "BackgroundTasks/Workers" - return body["props"] + page = InertiaPage.from_response(resp).assert_component("BackgroundTasks/Workers") + deferred = await page.load_deferred(client, "workers") + return {**page.props, **deferred.props} class TestUptimeSeconds: diff --git a/modules/users/tests/test_users_admin_list_query_count.py b/modules/users/tests/test_users_admin_list_query_count.py new file mode 100644 index 000000000..0dada3956 --- /dev/null +++ b/modules/users/tests/test_users_admin_list_query_count.py @@ -0,0 +1,67 @@ +"""The users admin list costs the same number of SQL statements for 2 rows or 22. + +A per-row lookup (roles, sessions, tenants) makes the count grow with the +page: the N+1 that the development ``Server-Timing`` header shows in the +browser and that ``docs/frontend/performance.md`` tells authors to pin with a +test like this one. +""" + +from __future__ import annotations + +import uuid + +import httpx +from fastapi import FastAPI +from fastapi_users.password import PasswordHelper +from simple_module_db.query_stats import install_query_stats, track_queries +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession +from users.models import Role, User, UserRole + +_pw = PasswordHelper() + + +async def _add_users(session: AsyncSession, prefix: str, count: int) -> None: + role = (await session.execute(select(Role).where(Role.name == "admin"))).scalar_one() + for i in range(count): + user = User( + id=uuid.uuid4(), + email=f"{prefix}{i}@example.com", + hashed_password=_pw.hash("SecurePass1!"), + is_active=True, + is_verified=True, + ) + session.add(user) + await session.flush() + session.add(UserRole(user_id=user.id, role_id=role.id)) + await session.commit() + + +async def _statements(app: FastAPI, client: httpx.AsyncClient) -> int: + """The page's own statement count. + + Auth adds statements on its own schedule (loading the principal, the + session-version check that follows it), so take the cheapest of three requests. + """ + install_query_stats(app.state.sm.db.engine) + counts = [] + for _ in range(3): + with track_queries() as stats: + resp = await client.get("/admin/users/", headers={"X-Inertia": "true"}) + assert resp.status_code == 200 + assert resp.json()["component"] == "Users/Users/Index" + counts.append(stats.count) + return min(counts) + + +async def test_list_query_count_does_not_grow_with_rows( + users_app: FastAPI, admin_client: httpx.AsyncClient, users_db: AsyncSession +) -> None: + await _add_users(users_db, "few", 2) + few = await _statements(users_app, admin_client) + + await _add_users(users_db, "many", 20) + many = await _statements(users_app, admin_client) + + assert few > 0, "statements were not counted; is the engine instrumented?" + assert many == few diff --git a/packages/i18n/src/generated-resources.ts b/packages/i18n/src/generated-resources.ts index acb7c807f..bc2a6ef22 100644 --- a/packages/i18n/src/generated-resources.ts +++ b/packages/i18n/src/generated-resources.ts @@ -275,6 +275,8 @@ export default { 'background_tasks.table.tenant': '', 'background_tasks.tasks_empty.broker_unreachable_description': '', 'background_tasks.tasks_empty.broker_unreachable_title': '', + 'background_tasks.tasks_empty.checking_description': '', + 'background_tasks.tasks_empty.checking_title': '', 'background_tasks.tasks_empty.clear_filters': '', 'background_tasks.tasks_empty.filtered_description': '', 'background_tasks.tasks_empty.filtered_title': '', @@ -299,6 +301,8 @@ export default { 'background_tasks.workers.back_button': '', 'background_tasks.workers.broker_unreachable_description': '', 'background_tasks.workers.broker_unreachable_title': '', + 'background_tasks.workers.checking': '', + 'background_tasks.workers.checking_label': '', 'background_tasks.workers.description': '', 'background_tasks.workers.last_updated': '', 'background_tasks.workers.no_error_reported': '', diff --git a/packages/i18n/src/keys.generated.ts b/packages/i18n/src/keys.generated.ts index 1d4543c38..e2713d1ab 100644 --- a/packages/i18n/src/keys.generated.ts +++ b/packages/i18n/src/keys.generated.ts @@ -354,6 +354,8 @@ export const keys = { tasks_empty: { broker_unreachable_description: 'background_tasks.tasks_empty.broker_unreachable_description', broker_unreachable_title: 'background_tasks.tasks_empty.broker_unreachable_title', + checking_description: 'background_tasks.tasks_empty.checking_description', + checking_title: 'background_tasks.tasks_empty.checking_title', clear_filters: 'background_tasks.tasks_empty.clear_filters', filtered_description: 'background_tasks.tasks_empty.filtered_description', filtered_title: 'background_tasks.tasks_empty.filtered_title', @@ -387,6 +389,8 @@ export const keys = { back_button: 'background_tasks.workers.back_button', broker_unreachable_description: 'background_tasks.workers.broker_unreachable_description', broker_unreachable_title: 'background_tasks.workers.broker_unreachable_title', + checking: 'background_tasks.workers.checking', + checking_label: 'background_tasks.workers.checking_label', description: 'background_tasks.workers.description', last_updated: 'background_tasks.workers.last_updated', no_error_reported: 'background_tasks.workers.no_error_reported', diff --git a/pyproject.toml b/pyproject.toml index 712f04288..3b59eb161 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,6 +71,8 @@ ignore = [ [tool.ty.environment] extra-paths = [ "framework/cli", + # The page-perf skill's audit script: its modules import each other flat. + "skills/simple-module-page-perf/scripts/page_audit", "framework/core", "framework/db", "framework/hosting", diff --git a/skills/README.md b/skills/README.md index 8c5789881..f0a137ac9 100644 --- a/skills/README.md +++ b/skills/README.md @@ -44,6 +44,7 @@ The CLI is [vercel-labs/skills](https://github.com/vercel-labs/skills); see its | [simple-module-database](./simple-module-database/SKILL.md) | Adding SQLModel tables, picking a mixin, or debugging session/transaction behavior | | [simple-module-migrations](./simple-module-migrations/SKILL.md) | Generating, applying, or reviewing Alembic migrations after installing or changing a module | | [simple-module-inertia-pages](./simple-module-inertia-pages/SKILL.md) | Adding or debugging an Inertia page in a module — render keys, shared props, common pitfalls | +| [simple-module-page-perf](./simple-module-page-perf/SKILL.md) | Auditing pages for speed and perceived speed — server time and SQL counts (`Server-Timing`), payloads, deferred props, navigation timing, layout shift, UX checks; ships `scripts/page_audit` | | [simple-module-locales](./simple-module-locales/SKILL.md) | Adding or debugging i18n in a module — `locale_dirs()`, namespaces, CLDR plurals, the Zod-in-hook rule | | [simple-module-registries](./simple-module-registries/SKILL.md) | Contributing menu items, permissions, feature flags, or events from a module | | [simple-module-testing](./simple-module-testing/SKILL.md) | Writing pytest tests — picking the right fixture (`db_session` / `app` / `authenticated_client`), single-test runs, e2e | diff --git a/skills/simple-module-inertia-pages/SKILL.md b/skills/simple-module-inertia-pages/SKILL.md index 6f079c340..e55508e4b 100644 --- a/skills/simple-module-inertia-pages/SKILL.md +++ b/skills/simple-module-inertia-pages/SKILL.md @@ -108,3 +108,4 @@ The typed client (#504): - **simple-module-creating** — where the PascalCase rule originates (matching `ModuleMeta.name`) - **simple-module-doctor** — full reference for `SM003` / `SM004` / `SM018` / `SM019` +- **simple-module-page-perf** — auditing a page's speed, N+1s, deferred props and loading UX diff --git a/skills/simple-module-page-perf/SKILL.md b/skills/simple-module-page-perf/SKILL.md new file mode 100644 index 000000000..78a74ec56 --- /dev/null +++ b/skills/simple-module-page-perf/SKILL.md @@ -0,0 +1,157 @@ +--- +name: simple-module-page-perf +description: Use when checking or improving the speed and perceived speed of Inertia pages in a simple_module_python app — a page or navigation feels slow, a list may have an N+1, a payload looks large, content jumps while loading, or before shipping a new page. Triggers on "page is slow", "audit page performance", "Server-Timing", "N+1", "defer this prop", "layout shift", "skeleton", "LCP", or "perf check the pages". +--- + +# simple_module_python: page performance audit + +`scripts/page_audit` (next to this file) signs in to a running app, finds every +page, and measures each one three ways: the Inertia JSON a navigation fetches, +a real browser load and click-through, and a phone-width load. It prints a +Markdown table and a findings list, and exits 1 if any budget fails. + +## 1. Run it against a development server (server-side numbers) + +A development host sends `Server-Timing: app;dur=…, db;dur=…;desc="N queries"` +on every response. The audit reads it to report server time and SQL +statements per page. + +```bash +uv run --with playwright playwright install chromium # once +uv run --with playwright python .claude/skills/simple-module-page-perf/scripts/page_audit \ + --base-url http://localhost:8000 \ + --email admin@example.com --password '' \ + --json perf-dev.json +``` + +- Pages come from the `menus` shared prop, then two levels of links found in + each page's `
`, one sample per URL pattern (`/orders/` is audited + once). Add pages with `--path /x/` (repeatable), audit only those with + `--no-discover --crawl-depth 0`. +- Logout, delete, export, download, `/api/` and `/setup` links are never + followed. Add your own with `--exclude `. +- Public pages: run without `--email` and pass them with `--path` + (`--path / --path /users/login`). +- No `Server-Timing` (older framework, or a non-development server): the + Server ms and SQL columns show `—`; everything else still works. +- Seed realistic volumes first. Ten rows hide the N+1 and the oversized prop + that ten thousand expose. + +## 2. Run it against a production build (what users download) + +Dev-server bundles are unminified and served module by module, so LCP and JS +bytes only mean something on a build. The audit only applies those budgets +when the page is not served by Vite. + +```bash +make build # the client bundle +SM_ENVIRONMENT=staging SM_SECRET_KEY=<32+ chars> \ + uv run uvicorn main:app --port 8001 # any non-development environment serves the build +uv run --with playwright python .claude/skills/simple-module-page-perf/scripts/page_audit \ + --base-url http://localhost:8001 --email … --password … --json perf-prod.json +``` + +Outside development the session cookie is `Secure`. Browsers accept it on +`http://localhost`; over plain HTTP on any other host, set +`SM_SESSION_COOKIE_SECURE=false` for the audit run. + +## 3. Read the report + +| Column / check | Budget (warn / fail) | What it means | +| --- | --- | --- | +| Server ms | 100 / 300 | warm median of `app` in Server-Timing | +| SQL | 12 / 25 | statements, cheapest warm round (auth adds a few now and then) | +| JSON KB | 100 / 300 | the Inertia response without the i18n catalog | +| `prop_kb` | 50 / 150 | the largest single page prop | +| Deferred | — | props the page loads after it paints | +| Nav ms | 300 / 1000 | click on a visible link → new page painted (`—`: no visible link to it) | +| `deferred_ms` | 1000 / 2500 | click → every deferred prop painted | +| LCP ms | 2500 / 4000 | production build only | +| `js_kb` | 350 / 600 | JS transferred on first load, production build only | +| CLS | 0.1 / 0.25 | layout shift on first load or navigation | +| DOM / `table_rows` | 1500 / 3000, 100 / 250 | rendered size after everything settled | +| `redirect` | warn | the GET redirected: link to the canonical URL | +| `console`, `failed_requests` | fail | runtime errors, 4xx/5xx requests while on the page | +| `overflow` | warn | page scrolls sideways at 390 px | +| `stuck_loaders` | warn | skeleton/spinner/`aria-busy` still visible after everything settled | +| `unnamed_controls`, `h1` | warn | icon buttons without a name; not exactly one `

` | + +Override a budget with `--budget queries=20,40`. Use `--no-fail` to report +without failing. + +## 4. Fix what it finds + +| Finding | Usual cause | Fix | +| --- | --- | --- | +| High SQL count | a query per row (N+1) | `selectinload()`/`joinedload()`, `get_user_directory(app).get_many(ids)`, one `GROUP BY` for per-row counts | +| High server ms, few queries | a slow call: broker, HTTP API, big aggregate | `defer(load, group="…")` with a same-size placeholder, or cache with a TTL plus invalidation | +| Large JSON / prop | whole models, unpaginated lists | a DTO with only the rendered fields; `paginate()` 20–50 rows; detail data on the detail page | +| Partial reloads still slow | props passed as values, so computed every time | pass expensive props as callables (`"stats": lambda: svc.stats()`): only evaluated when included | +| `deferred_ms` warning | the deferred work itself is slow | fine if the placeholder says what is loading; split independent slow props into separate groups (parallel requests) | +| CLS | content appears without reserved space | `Skeleton` in the real layout's shape; same component with "checking…" copy; `aria-busy="true"` | +| `redirect` | a link without the trailing slash, or to an old URL | one constant for the index URL, with its slash | +| `js_kb` | heavy library in a shared component | `lazy(() => import('./Heavy'))` + `}>` where it is used | +| DOM / rows | long unpaginated list | paginate, or virtualize past a few hundred rows | +| `overflow` | wide table or fixed widths | `overflow-x-auto` around tables; no fixed pixel widths | + +How deferred props behave on the client: + +- A deferred prop is `undefined` until it loads, which is different from `null` + (loaded, nothing there). Type it optional (`stats?: Stats`) and branch on + `=== undefined`, or wrap the part in Inertia's + ``. +- A component that copies the prop into state must adopt it when it arrives: + `useEffect(() => { if (initial) setValue(initial); }, [initial]);`. +- Test the server side with `InertiaPage` from `simple_module_test`: + `page.deferred("group", "prop").missing("prop")`, then + `await page.load_deferred(client, "group")`. + +## 5. Pin it + +Keep a fixed page fixed with a statement-count test. The cheapest of three +requests excludes the auth layer's own periodic queries: + +```python +from simple_module_db.query_stats import install_query_stats, track_queries + + +async def _statements(app, client, url) -> int: + install_query_stats(app.state.sm.db.engine) + counts = [] + for _ in range(3): + with track_queries() as stats: + assert (await client.get(url, headers={"X-Inertia": "true"})).status_code == 200 + counts.append(stats.count) + return min(counts) + + +# Seed 2 rows, measure; seed 20 more, measure again; assert the counts are equal. +``` + +## UX checklist the numbers don't cover + +Walk through these by hand on each page you touched: + +- A visit under about 150 ms shows nothing; Inertia's progress bar covers longer + ones. Don't add page-level spinners. +- Loading, empty, filtered-empty and error states each say something different + and offer a next step. +- Submit buttons disable while `form.processing` is true; validation errors keep + what the user typed. +- Filters reload with `only`, `preserveState`, `preserveScroll` and + `replace: true`; text inputs are debounced (about 300 ms). +- Live data either polls modestly (`usePoll(5000+, { only })`) or shows + "last updated" next to a Refresh button. +- `` only where slightly stale data is fine; every hover is a request. +- Touch targets are at least 44 px tall on phones (`max-lg:min-h-11`). + +## Gotchas + +- Run the audit against a server that nothing else is changing: a Vite HMR + reload in the middle of a run shows up as layout shift and console errors. +- `Nav ms` needs a visible link to the page from its parent, a sibling page in + the same area, or home. Pages only reachable from a dropdown show `—`. +- A `fail` on a crawled detail page with HTTP 404 or 422 is usually seed data + linking to rows that don't exist. Check the link before chasing it. +- The browser pass drives one tab; don't run two audits against one server + at the same time, or the timings measure each other. diff --git a/skills/simple-module-page-perf/scripts/page_audit/__main__.py b/skills/simple-module-page-perf/scripts/page_audit/__main__.py new file mode 100644 index 000000000..94c9f820d --- /dev/null +++ b/skills/simple-module-page-perf/scripts/page_audit/__main__.py @@ -0,0 +1,242 @@ +"""Audit every Inertia page of a running simple_module app for speed and UX. + + uv run --with playwright python /scripts/page_audit \\ + --base-url http://localhost:8000 --email admin@example.com --password '...' + +Signs in, discovers pages from the ``menus`` shared prop (plus any ``--path``), +then for each page measures the Inertia JSON response (server time and SQL +statements from the development ``Server-Timing`` header, payload size, the +largest props, deferred props), a full document load (FCP, LCP, CLS, bytes), a +client-side navigation from a sidebar link (click -> painted -> deferred +props painted) and UX checks (DOM size, table rows, stuck loaders, unnamed +icon buttons, headings, horizontal overflow on a phone). Prints a Markdown +report, optionally writes JSON, and exits 1 when any check fails. +""" + +from __future__ import annotations + +import argparse +import os +import re +import sys +from pathlib import Path +from typing import Any + +from audit_browser import ( + INIT_SCRIPT, + BrowserResult, + client_navigation, + first_load, + mobile_overflow, + settle, +) +from audit_crawl import extra_pages, hubs_for +from audit_report import BUDGETS, Finding, evaluate, markdown, write_json +from audit_server import discover_paths, inertia_headers, probe +from playwright.sync_api import BrowserContext, Page, ViewportSize, sync_playwright + +DESKTOP: ViewportSize = {"width": 1440, "height": 900} +PHONE: ViewportSize = {"width": 390, "height": 844} +DEFAULT_EXCLUDE = ( + r"logout|log-out|sign-?out|switch|impersonat|delete|remove|revoke|" + r"/api/|/static/|/setup|download|export|\.(csv|json|pdf|zip)$" +) +_PAGE_OBJECT = """ +() => { + const el = document.querySelector('script[data-page]'); + if (el) return JSON.parse(el.textContent); + const app = document.getElementById('app'); + return app && app.dataset.page ? JSON.parse(app.dataset.page) : null; +} +""" + + +def _args(argv: list[str]) -> argparse.Namespace: + ap = argparse.ArgumentParser(prog="page_audit", description=__doc__.split("\n\n")[0]) + ap.add_argument( + "--base-url", default=os.environ.get("PAGE_AUDIT_BASE_URL", "http://localhost:8000") + ) + ap.add_argument("--email", default=os.environ.get("PAGE_AUDIT_EMAIL")) + ap.add_argument("--password", default=os.environ.get("PAGE_AUDIT_PASSWORD")) + ap.add_argument("--login-path", default="/users/login") + ap.add_argument("--storage-state", help="Playwright storage state of a signed-in session") + ap.add_argument("--path", action="append", default=[], help="page to audit (repeatable)") + ap.add_argument("--no-discover", action="store_true", help="only audit --path pages") + ap.add_argument("--crawl-depth", type=int, default=2, help="link levels to follow (0: none)") + ap.add_argument("--max-pages", type=int, default=80, help="cap including crawled pages") + ap.add_argument("--exclude", action="append", default=[DEFAULT_EXCLUDE], help="regex to skip") + ap.add_argument("--rounds", type=int, default=5, help="warm server samples per page") + ap.add_argument("--server-only", action="store_true", help="skip the browser checks") + ap.add_argument("--no-mobile", action="store_true", help="skip the phone-width pass") + ap.add_argument("--budget", action="append", default=[], help="override: queries=20,40") + ap.add_argument("--json", type=Path, help="also write the full results here") + ap.add_argument("--headed", action="store_true") + ap.add_argument("--no-fail", action="store_true", help="exit 0 even when checks fail") + return ap.parse_args(argv) + + +def _apply_budgets(overrides: list[str]) -> None: + for item in overrides: + key, _, values = item.partition("=") + warn, _, fail = values.partition(",") + if key not in BUDGETS or not fail: + raise SystemExit(f"bad --budget {item!r}; keys: {', '.join(BUDGETS)}") + BUDGETS[key] = (float(warn), float(fail)) + + +def _sign_in(page: Page, args: argparse.Namespace) -> None: + if not (args.email and args.password): + return + page.goto(args.login_path) + page.locator('input[type="email"], input[name="email"], #email').first.fill(args.email) + page.locator('input[type="password"]').first.fill(args.password) + page.locator('button[type="submit"]').first.click() + page.wait_for_url(lambda url: args.login_path not in url, timeout=20_000) + + +def _paths(found: list[str], args: argparse.Namespace) -> list[str]: + excluded = [re.compile(p) for p in args.exclude] + candidates = [*args.path, *([] if args.no_discover else found)] + seen: list[str] = [] + for path in candidates: + if path not in seen and not any(rx.search(path) for rx in excluded): + seen.append(path) + return seen + + +def _watch(page: Page, holder: dict[str, BrowserResult]) -> None: + def on_console(msg: Any) -> None: + if msg.type == "error" and "current" in holder: + holder["current"].console_errors.append(msg.text) + + def on_error(exc: Any) -> None: + if "current" in holder: + holder["current"].console_errors.append(str(exc)) + + def on_response(resp: Any) -> None: + if resp.status >= 400 and "current" in holder: + holder["current"].failed_requests.append(f"{resp.status} {resp.url}") + + page.on("console", on_console) + page.on("pageerror", on_error) + page.on("response", on_response) + + +def _load_pass( + page: Page, paths: list[str], holder: dict[str, BrowserResult] +) -> dict[str, BrowserResult]: + """Full document load of every path (paint timings, bytes, UX checks).""" + results = {p: BrowserResult(path=p) for p in paths} + for path in paths: + holder["current"] = results[path] + try: + first_load(page, path, results[path]) + except Exception as exc: + results[path].error = f"first load: {exc}"[:300] + holder.pop("current", None) + return results + + +def _nav_pass( + page: Page, + results: dict[str, BrowserResult], + parents: dict[str, str], + holder: dict[str, BrowserResult], +) -> None: + """Client-side navigation to every path from a page that links to it.""" + known = list(results) + for path, result in results.items(): + holder.pop("current", None) + + def attribute(result: BrowserResult = result) -> None: + holder["current"] = result + + try: + hubs = hubs_for(path, parents.get(path), known) + result.nav = client_navigation(page, path, hubs, attribute) + except Exception as exc: + result.error = result.error or f"navigation: {exc}"[:300] + holder.pop("current", None) + + +def _mobile_pass( + context: BrowserContext, base: str, paths: list[str], results: dict[str, BrowserResult] +) -> None: + browser = context.browser + if browser is None: + return + phone = browser.new_context( + base_url=base, viewport=PHONE, storage_state=context.storage_state() + ) + phone.add_init_script(INIT_SCRIPT) + page = phone.new_page() + for path in paths: + try: + results[path].mobile_overflow_x = mobile_overflow(page, path) + except Exception as exc: + results[path].error = results[path].error or f"phone load: {exc}"[:300] + phone.close() + + +def main(argv: list[str]) -> int: + args = _args(argv) + _apply_budgets(args.budget) + base = args.base_url.rstrip("/") + with sync_playwright() as pw: + browser = pw.chromium.launch(headless=not args.headed) + context = browser.new_context( + base_url=base, viewport=DESKTOP, storage_state=args.storage_state + ) + context.add_init_script(INIT_SCRIPT) + page = context.new_page() + _sign_in(page, args) + page.goto("/") + settle(page) + page_object = page.evaluate(_PAGE_OBJECT) or {} + props = page_object.get("props") or {} + i18n = props.get("i18n") or {} + i18n_key = f"{i18n['locale']}:{i18n['audience']}" if i18n.get("audience") else None + headers = inertia_headers(page_object.get("version"), i18n_key) + paths = _paths(discover_paths(props), args) + if not paths: + print("No pages found: pass --path, or check the login (--email/--password).") + return 2 + servers = [probe(context.request, base, p, headers, args.rounds) for p in paths] + browsers: dict[str, BrowserResult] = {} + parents: dict[str, str] = {} + if not args.server_only: + holder: dict[str, BrowserResult] = {} + _watch(page, holder) + browsers = _load_pass(page, [s.path for s in servers if not s.error], holder) + excluded = [re.compile(x) for x in args.exclude] + frontier = list(browsers) + for _ in range(args.crawl_depth): + links = {p: browsers[p].ux.get("links", []) for p in frontier} + room = max(0, args.max_pages - len(servers)) + found = extra_pages([s.path for s in servers], links, excluded, room) + if not found: + break + parents |= found + extra = [probe(context.request, base, p, headers, args.rounds) for p in found] + servers += extra + loaded = _load_pass(page, [s.path for s in extra if not s.error], holder) + browsers |= loaded + frontier = list(loaded) + _nav_pass(page, browsers, parents, holder) + if not args.no_mobile: + _mobile_pass(context, base, list(browsers), browsers) + browser.close() + + rows = [(s, browsers.get(s.path)) for s in servers] + findings: list[Finding] = [f for s, b in rows for f in evaluate(s, b)] + print(markdown(rows, findings)) + if args.json: + dev = any(b.load.get("dev_build") for b in browsers.values()) + write_json(args.json, rows, findings, {"base_url": base, "dev_build": dev}) + print(f"\nJSON written to {args.json}") + failed = any(f.level == "fail" for f in findings) + return 0 if args.no_fail or not failed else 1 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/skills/simple-module-page-perf/scripts/page_audit/audit_browser.py b/skills/simple-module-page-perf/scripts/page_audit/audit_browser.py new file mode 100644 index 000000000..d996711db --- /dev/null +++ b/skills/simple-module-page-perf/scripts/page_audit/audit_browser.py @@ -0,0 +1,189 @@ +"""Browser side of the audit: what the user sees and feels. + +An init script installed on every document records paint timings (FCP, LCP), +layout shifts (CLS, ignoring shifts the user caused) and every Inertia visit +through the public ``inertia:start`` / ``inertia:finish`` DOM events, so a +deferred-prop reload is told apart from the navigation that triggered it. +""" + +from __future__ import annotations + +import contextlib +from collections.abc import Callable +from dataclasses import dataclass, field +from typing import Any + +from playwright.sync_api import Error as PlaywrightError +from playwright.sync_api import Page + +SETTLE_TIMEOUT_MS = 15_000 +_QUIET_MS = 300 + +INIT_SCRIPT = """ +(() => { + const s = (window.__pa = { + cls: 0, shifts: 0, lcp: 0, fcp: 0, visits: [], pending: 0, last: 0, + }); + const watch = (type, fn) => { + try { + new PerformanceObserver((l) => l.getEntries().forEach(fn)).observe({ type, buffered: true }); + } catch (e) { /* unsupported entry type */ } + }; + watch('layout-shift', (e) => { if (!e.hadRecentInput) { s.cls += e.value; s.shifts += 1; } }); + watch('largest-contentful-paint', (e) => { s.lcp = e.startTime; }); + watch('paint', (e) => { if (e.name === 'first-contentful-paint') s.fcp = e.startTime; }); + document.addEventListener('inertia:start', (ev) => { + const visit = (ev.detail && ev.detail.visit) || {}; + s.pending += 1; + s.visits.push({ start: performance.now(), partial: (visit.only || []).length > 0 }); + }); + document.addEventListener('inertia:finish', () => { + s.pending = Math.max(0, s.pending - 1); + s.last = performance.now(); + const v = s.visits.find((x) => x.finish === undefined); + if (v) { + v.finish = s.last; + requestAnimationFrame(() => { v.painted = performance.now(); }); + } + }); +})(); +""" + +_SETTLED = f""" +() => window.__pa && window.__pa.pending === 0 + && window.__pa.visits.every((v) => v.painted !== undefined) + && performance.now() - window.__pa.last > {_QUIET_MS} +""" + +_RESET = "() => { const s = window.__pa; s.cls = 0; s.shifts = 0; s.visits = []; }" + +_LOAD_METRICS = """ +() => { + const s = window.__pa; + const nav = performance.getEntriesByType('navigation')[0] || {}; + const res = performance.getEntriesByType('resource'); + const size = (r) => r.transferSize || r.encodedBodySize || 0; + const bytes = (pred) => res.filter(pred).reduce((t, r) => t + size(r), 0); + const deferred = s.visits.filter((v) => v.partial && v.painted !== undefined); + return { + ttfb_ms: nav.responseStart || 0, + dcl_ms: nav.domContentLoadedEventEnd || 0, + fcp_ms: s.fcp, lcp_ms: s.lcp, cls: s.cls, shifts: s.shifts, + deferred_done_ms: deferred.length ? Math.max(...deferred.map((v) => v.painted)) : null, + html_bytes: nav.transferSize || nav.encodedBodySize || 0, + js_bytes: bytes((r) => r.initiatorType === 'script' || /\\.(m?js|tsx?)(\\?|$)/.test(r.name)), + css_bytes: bytes((r) => r.initiatorType === 'link' || /\\.css(\\?|$)/.test(r.name)), + requests: res.length, + dev_build: !!document.querySelector('script[src*="@vite/client"]'), + }; +} +""" + +_NAV_METRICS = """ +() => { + const s = window.__pa; + const main = s.visits.find((v) => !v.partial); + if (!main) return null; + const partial = s.visits.filter((v) => v.partial && v.painted !== undefined); + return { + nav_ms: main.painted - main.start, + server_wait_ms: main.finish - main.start, + deferred_done_ms: partial.length + ? Math.max(...partial.map((v) => v.painted)) - main.start + : null, + cls: s.cls, + }; +} +""" + +UX_CHECKS = """ +() => { + const visible = (el) => { + const r = el.getBoundingClientRect(); + return r.width > 0 && r.height > 0; + }; + const named = (el) => (el.innerText || '').trim() || el.getAttribute('aria-label') + || el.getAttribute('aria-labelledby') || el.getAttribute('title') + || (el.labels && el.labels.length > 0); + const controls = [...document.querySelectorAll('button, a[href], [role="button"]')] + .filter(visible); + const loaders = [...document.querySelectorAll( + '[aria-busy="true"], .animate-pulse, .animate-spin, [data-slot="skeleton"]')].filter(visible); + const tbodies = [...document.querySelectorAll('tbody')]; + const root = document.documentElement; + return { + dom_nodes: document.getElementsByTagName('*').length, + max_table_rows: tbodies.length ? Math.max(...tbodies.map((t) => t.rows.length)) : 0, + unnamed_controls: controls.filter((el) => !named(el)).length, + stuck_loaders: loaders.length, + h1_count: document.querySelectorAll('h1').length, + title: document.title, + links: [...new Set([...document.querySelectorAll('main a[href^="/"]')] + .filter(visible).map((a) => a.getAttribute('href')))], + overflow_x: Math.max(0, root.scrollWidth - root.clientWidth), + }; +} +""" + + +@dataclass(slots=True) +class BrowserResult: + path: str + load: dict[str, Any] = field(default_factory=dict) + nav: dict[str, Any] | None = None + ux: dict[str, Any] = field(default_factory=dict) + mobile_overflow_x: int | None = None + console_errors: list[str] = field(default_factory=list) + failed_requests: list[str] = field(default_factory=list) + error: str = "" + + +def settle(page: Page) -> None: + """Wait until no Inertia visit (including deferred reloads) is in flight.""" + with contextlib.suppress(PlaywrightError): + page.wait_for_load_state("networkidle", timeout=SETTLE_TIMEOUT_MS) + page.wait_for_function(_SETTLED, timeout=SETTLE_TIMEOUT_MS) + + +def first_load(page: Page, url: str, result: BrowserResult) -> None: + """A full document load of *url*: paint timings, bytes, shifts, UX checks.""" + page.goto(url, wait_until="load") + settle(page) + result.load = page.evaluate(_LOAD_METRICS) + result.ux = page.evaluate(UX_CHECKS) + + +def client_navigation( + page: Page, path: str, hubs: list[str], before_click: Callable[[], None] +) -> dict[str, Any] | None: + """Click a visible link to *path* and time click -> painted -> deferred done. + + Tries the current page first, then loads each of *hubs* (a parent list + page, the section index, the home page) until one links to *path*. + Returns ``None`` when none does. + """ + link = page.locator(f'a[href="{path}"]:visible').first + for hub in hubs: + if link.count(): + break + page.goto(hub) + settle(page) + link = page.locator(f'a[href="{path}"]:visible').first + if link.count() == 0: + return None + page.evaluate(_RESET) + before_click() # errors from here on belong to this page, not to the hubs + link.click() + page.wait_for_function( + "() => window.__pa.visits.some((v) => !v.partial && v.painted !== undefined)", + timeout=SETTLE_TIMEOUT_MS, + ) + settle(page) + return page.evaluate(_NAV_METRICS) + + +def mobile_overflow(page: Page, url: str) -> int: + """Horizontal overflow in CSS pixels at a phone-sized viewport (0 is good).""" + page.goto(url, wait_until="load") + settle(page) + return int(page.evaluate(UX_CHECKS)["overflow_x"]) diff --git a/skills/simple-module-page-perf/scripts/page_audit/audit_crawl.py b/skills/simple-module-page-perf/scripts/page_audit/audit_crawl.py new file mode 100644 index 000000000..fcd9c91fe --- /dev/null +++ b/skills/simple-module-page-perf/scripts/page_audit/audit_crawl.py @@ -0,0 +1,79 @@ +"""One-level crawl: pages linked from the content of the menu pages. + +Menus only reach list pages. Detail and edit pages (``/admin/users/``) +are reached from links in those lists, so after the first pass every link in +``
`` is collected and one sample per URL *pattern* is added: ids, +numbers and long tokens collapse to ``:id``, so a 50-row table adds one page, +not 50. +""" + +from __future__ import annotations + +import re +from collections.abc import Iterable +from urllib.parse import urlsplit + +_ID_SEGMENT = re.compile( + r"^(\d+|[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}|[0-9a-f]{16,}|[\w-]{24,})$", + re.IGNORECASE, +) + + +def pattern(path: str) -> str: + """``/admin/users/3f2a…/edit?tab=1`` -> ``/admin/users/:id/edit``.""" + parts = urlsplit(path).path.split("/") + return "/".join(":id" if _ID_SEGMENT.match(p) else p for p in parts) + + +def extra_pages( + known: Iterable[str], + links_by_page: dict[str, list[str]], + excluded: list[re.Pattern[str]], + limit: int, +) -> dict[str, str]: + """Map each new sample path to the page that links to it (its parent).""" + known = list(known) + seen = {pattern(p) for p in known} + covered = {section(p) for p in known} + found: dict[str, str] = {} + # A list page's own rows first (``/admin/users/`` -> ``/admin/users/``): + # a link into another section (an audit entry pointing at a setting) is a + # worse sample of that section's pages than the section's own list. + ordered = sorted( + ((parent, href) for parent, links in links_by_page.items() for href in links), + key=lambda item: section(item[0]) != section(item[1]), + ) + for parent, href in ordered: + if len(found) >= limit: + return found + path = href.split("#", 1)[0] + key = pattern(path) + if not path or key in seen or any(rx.search(path) for rx in excluded): + continue + # A section the audit already reaches gets its samples from its own + # pages (next level), not from rows elsewhere that point into it. + if section(path) != section(parent) and section(path) in covered: + continue + seen.add(key) + found[path] = parent + return found + + +def section(path: str, depth: int = 2) -> str: + """The first *depth* path segments: ``/admin/users/`` -> ``admin/users``.""" + return "/".join(urlsplit(path).path.strip("/").split("/")[:depth]) + + +def hubs_for(path: str, parent: str | None, known: list[str]) -> list[str]: + """Known pages likely to link to *path*. + + Its parent, then a page of the same area (``/admin/...`` pages share the + admin sidebar, which the home page does not render), then home. + """ + area = [p for p in known if p != path and section(p, 1) == section(path, 1)] + candidates = [parent, *area[:1], "/"] + out: list[str] = [] + for hub in candidates: + if hub and hub != path and hub not in out: + out.append(hub) + return out diff --git a/skills/simple-module-page-perf/scripts/page_audit/audit_report.py b/skills/simple-module-page-perf/scripts/page_audit/audit_report.py new file mode 100644 index 000000000..5ee97089b --- /dev/null +++ b/skills/simple-module-page-perf/scripts/page_audit/audit_report.py @@ -0,0 +1,172 @@ +"""Budgets, findings and the Markdown/JSON report. + +Every budget is a ``(warn, fail)`` pair. The defaults follow the thresholds +users notice: ~100 ms feels instant, ~1 s keeps the flow of thought, and +Google's Core Web Vitals for LCP (2.5 s / 4 s) and CLS (0.1 / 0.25). +""" + +from __future__ import annotations + +import json +from dataclasses import asdict, dataclass +from pathlib import Path +from typing import Any + +from audit_browser import BrowserResult +from audit_server import ServerResult + +KB = 1024 + +BUDGETS: dict[str, tuple[float, float]] = { + "app_ms": (100, 300), # server time for an Inertia visit (warm) + "queries": (12, 25), # SQL statements per visit + "page_kb": (100, 300), # JSON without the i18n catalog + "prop_kb": (50, 150), # one prop + "nav_ms": (300, 1000), # click -> new page painted + "deferred_ms": (1000, 2500), # click -> every deferred prop painted + "lcp_ms": (2500, 4000), # first load + "js_kb": (350, 600), # JS transferred by a first load (production build) + "cls": (0.1, 0.25), + "dom_nodes": (1500, 3000), + "table_rows": (100, 250), +} + +HINTS = { + "app_ms": "profile the handler; move slow work behind defer()/optional() or a cache", + "queries": "likely N+1: eager-load (selectinload/joinedload) or batch the lookups", + "page_kb": "paginate, send only the columns the page shows, defer() the heavy part", + "prop_kb": "trim the DTO, paginate, or load it with defer()/WhenVisible", + "nav_ms": "check server time first, then the page chunk size and render cost", + "deferred_ms": "deferred groups load in parallel: split slow props into their own group", + "lcp_ms": "largest element paints late: smaller first payload, defer below-the-fold data", + "js_kb": "a heavy dependency on this page: lazy-load it (import()) where it is used", + "cls": "reserve space for deferred/async content (fixed-height skeletons)", + "dom_nodes": "paginate or virtualize long lists", + "table_rows": "paginate (<=50 rows) or virtualize; use scroll() for infinite lists", + "redirect": "link to the canonical URL; each redirect is an extra round trip", + "console": "fix runtime errors/warnings; they often mean wasted renders", + "failed_requests": "a request on this page failed", + "overflow": "content wider than a phone screen: wrap tables in overflow-x-auto", + "stuck_loaders": "a skeleton/spinner is still visible after every request settled", + "unnamed_controls": "icon-only buttons/links need aria-label (screen readers, tooltips)", + "h1": "give the page exactly one

so its purpose is clear at a glance", +} + + +@dataclass(slots=True) +class Finding: + path: str + level: str # "fail" | "warn" + check: str + detail: str + + @property + def hint(self) -> str: + return HINTS.get(self.check, "") + + +def _budget(path: str, check: str, value: float | None, label: str) -> list[Finding]: + if value is None: + return [] + warn, fail = BUDGETS[check] + if value > fail: + return [Finding(path, "fail", check, f"{label} {value:g} > {fail:g}")] + if value > warn: + return [Finding(path, "warn", check, f"{label} {value:g} > {warn:g}")] + return [] + + +def evaluate(server: ServerResult, browser: BrowserResult | None) -> list[Finding]: + p = server.path + if server.error: + return [Finding(p, "fail", "server", server.error)] + out: list[Finding] = [] + out += _budget(p, "app_ms", server.app_ms and round(server.app_ms), "server ms") + out += _budget(p, "queries", server.queries, "SQL statements") + page_kb = round((server.json_bytes - server.i18n_bytes) / KB, 1) + out += _budget(p, "page_kb", page_kb, "page JSON KB") + for name, size in server.top_props[:1]: + out += _budget(p, "prop_kb", round(size / KB, 1), f"prop '{name}' KB") + if server.redirected: + out.append(Finding(p, "warn", "redirect", f"GET redirects to {server.final_url}")) + if browser is None: + return out + if browser.error: + return [*out, Finding(p, "fail", "browser", browser.error)] + load, ux, nav = browser.load, browser.ux, browser.nav or {} + if not load.get("dev_build"): # dev-server timings and bytes are not what ships + out += _budget(p, "lcp_ms", round(load.get("lcp_ms") or 0), "LCP ms") + out += _budget(p, "js_kb", round((load.get("js_bytes") or 0) / KB), "JS KB") + out += _budget(p, "cls", round(max(load.get("cls", 0), nav.get("cls", 0)), 3), "CLS") + out += _budget(p, "nav_ms", nav.get("nav_ms") and round(nav["nav_ms"]), "navigation ms") + out += _budget( + p, + "deferred_ms", + nav.get("deferred_done_ms") and round(nav["deferred_done_ms"]), + "deferred ms", + ) + out += _budget(p, "dom_nodes", ux.get("dom_nodes"), "DOM nodes") + out += _budget(p, "table_rows", ux.get("max_table_rows"), "table rows") + if browser.console_errors: + out.append(Finding(p, "fail", "console", "; ".join(browser.console_errors[:3])[:300])) + if browser.failed_requests: + out.append(Finding(p, "fail", "failed_requests", ", ".join(browser.failed_requests[:3]))) + if browser.mobile_overflow_x: + out.append(Finding(p, "warn", "overflow", f"{browser.mobile_overflow_x}px at 390px wide")) + for check, key in ( + ("stuck_loaders", "stuck_loaders"), + ("unnamed_controls", "unnamed_controls"), + ): + if ux.get(key): + out.append(Finding(p, "warn", check, f"{ux[key]} element(s)")) + if ux.get("h1_count", 1) != 1: + out.append(Finding(p, "warn", "h1", f"{ux.get('h1_count')}

elements")) + return out + + +def _fmt(value: float | None, digits: int = 0) -> str: + if value is None: + return "—" + return f"{value:.{digits}f}" + + +def markdown(rows: list[tuple[ServerResult, BrowserResult | None]], findings: list[Finding]) -> str: + levels = {f.path: f.level for f in findings if f.level == "warn"} + levels.update({f.path: f.level for f in findings if f.level == "fail"}) + lines = [ + "| Page | Component | Server ms | SQL | JSON KB | Deferred | Nav ms | LCP ms | CLS | DOM " + "| Status |", + "|---|---|---|---|---|---|---|---|---|---|---|", + ] + for server, browser in rows: + nav = (browser.nav if browser else None) or {} + load = browser.load if browser else {} + ux = browser.ux if browser else {} + kb = (server.json_bytes - server.i18n_bytes) / KB + lines.append( + f"| `{server.path}` | {server.component or '—'} | {_fmt(server.app_ms)} " + f"| {_fmt(server.queries)} | {kb:.1f} | {', '.join(server.deferred) or '—'} " + f"| {_fmt(nav.get('nav_ms'))} | {_fmt(load.get('lcp_ms'))} " + f"| {_fmt(max(load.get('cls', 0), nav.get('cls', 0)), 3)} | {ux.get('dom_nodes', '—')} " + f"| {levels.get(server.path, 'ok')} |" + ) + if findings: + lines += ["", "## Findings", ""] + for f in sorted(findings, key=lambda f: (f.level != "fail", f.path)): + lines.append(f"- **{f.level}** `{f.path}` {f.check}: {f.detail} — {f.hint}") + return "\n".join(lines) + + +def write_json( + target: Path, + rows: list[tuple[ServerResult, BrowserResult | None]], + findings: list[Finding], + meta: dict[str, Any], +) -> None: + payload = { + "meta": meta, + "budgets": BUDGETS, + "pages": [{"server": asdict(s), "browser": asdict(b) if b else None} for s, b in rows], + "findings": [{**asdict(f), "hint": f.hint} for f in findings], + } + target.write_text(json.dumps(payload, indent=2, default=str)) diff --git a/skills/simple-module-page-perf/scripts/page_audit/audit_server.py b/skills/simple-module-page-perf/scripts/page_audit/audit_server.py new file mode 100644 index 000000000..d2ff4cd29 --- /dev/null +++ b/skills/simple-module-page-perf/scripts/page_audit/audit_server.py @@ -0,0 +1,169 @@ +"""Server side of the audit: Inertia JSON responses fetched like the client does. + +Each page is requested with the same headers ``router.visit`` sends, so the +response is exactly what a client-side navigation costs: the JSON body, the +``Server-Timing`` header a development host adds (app time, SQL statement +count), any redirect, and the deferred/merge/once props the page declares. +""" + +from __future__ import annotations + +import gzip +import json +import re +import statistics +import time +from dataclasses import dataclass, field +from typing import Any +from urllib.parse import urlsplit + +from playwright.sync_api import APIRequestContext + +_APP_DUR = re.compile(r"\bapp;dur=([\d.]+)") +_DB = re.compile(r'\bdb;dur=([\d.]+);desc="(\d+) quer') +_GZIP_LEVEL = 5 # what the framework's GZipMiddleware uses +_TOP_PROPS = 5 +# Shared props every page carries; reported apart from the page's own props. +SHARED_PROPS = ("auth", "menus", "i18n", "errors", "flash", "csrf_token", "branding") + + +@dataclass(slots=True) +class ServerResult: + path: str + status: int = 0 + component: str = "" + final_url: str = "" + redirected: bool = False + wall_ms: float = 0.0 + app_ms: float | None = None + cold_app_ms: float | None = None + db_ms: float | None = None + queries: int | None = None + json_bytes: int = 0 + gzip_bytes: int = 0 + i18n_bytes: int = 0 + shared_bytes: int = 0 + top_props: list[tuple[str, int]] = field(default_factory=list) + deferred: list[str] = field(default_factory=list) + merge: list[str] = field(default_factory=list) + once: list[str] = field(default_factory=list) + error: str = "" + + +def _size(value: Any) -> int: + return len(json.dumps(value, separators=(",", ":"), default=str).encode()) + + +def _timing(header: str | None) -> tuple[float | None, float | None, int | None]: + if not header: + return None, None, None + app = _APP_DUR.search(header) + db = _DB.search(header) + return ( + float(app.group(1)) if app else None, + float(db.group(1)) if db else None, + int(db.group(2)) if db else None, + ) + + +def inertia_headers(version: str | None, i18n_key: str | None) -> dict[str, str]: + """The headers ``@inertiajs/react`` sends on a client-side visit.""" + headers = { + "X-Inertia": "true", + "X-Requested-With": "XMLHttpRequest", + "Accept": "text/html, application/xhtml+xml", + } + if version: + headers["X-Inertia-Version"] = version + if i18n_key: + # The tab already holds this catalog, so the server can leave it out. + headers["X-SM-I18n"] = i18n_key + return headers + + +def probe( + request: APIRequestContext, + base_url: str, + path: str, + headers: dict[str, str], + rounds: int, +) -> ServerResult: + """Fetch *path* ``rounds + 1`` times (the first is the cold sample).""" + result = ServerResult(path=path) + walls: list[float] = [] + apps: list[float] = [] + dbs: list[float] = [] + queries: list[int] = [] + page: dict[str, Any] | None = None + for attempt in range(rounds + 1): + started = time.perf_counter() + response = request.get(base_url + path, headers=headers) + elapsed = (time.perf_counter() - started) * 1000 + result.status = response.status + result.final_url = response.url + if response.status != 200 or "x-inertia" not in response.headers: + result.error = f"HTTP {response.status} (not an Inertia page response)" + return result + app_ms, db_ms, count = _timing(response.headers.get("server-timing")) + if attempt == 0: + result.cold_app_ms = app_ms + body = response.body() + page = json.loads(body) + result.json_bytes = len(body) + result.gzip_bytes = len(gzip.compress(body, _GZIP_LEVEL)) + continue + walls.append(elapsed) + if app_ms is not None: + apps.append(app_ms) + if db_ms is not None and count is not None: + dbs.append(db_ms) + queries.append(count) + # Every redirect is one more round trip before anything paints. + result.redirected = urlsplit(result.final_url).path != urlsplit(path).path + result.wall_ms = statistics.median(walls) if walls else 0.0 + result.app_ms = statistics.median(apps) if apps else None + result.db_ms = statistics.median(dbs) if dbs else None + # The cheapest round is the page's own cost: auth adds statements on its + # own schedule (loading the principal, the session-version re-check). + result.queries = min(queries) if queries else None + if page is not None: + _describe_page(result, page) + return result + + +def _describe_page(result: ServerResult, page: dict[str, Any]) -> None: + props: dict[str, Any] = page.get("props") or {} + result.component = str(page.get("component", "")) + i18n = props.get("i18n") or {} + result.i18n_bytes = _size(i18n.get("messages")) if i18n.get("messages") else 0 + result.shared_bytes = sum(_size(props[k]) for k in SHARED_PROPS if k in props) + own = {k: v for k, v in props.items() if k not in SHARED_PROPS} + sizes = sorted(((k, _size(v)) for k, v in own.items()), key=lambda kv: -kv[1]) + result.top_props = sizes[:_TOP_PROPS] + deferred = page.get("deferredProps") or {} + result.deferred = sorted({p for group in deferred.values() for p in group}) + result.merge = sorted( + set(page.get("mergeProps") or []) + | set(page.get("prependProps") or []) + | set(page.get("deepMergeProps") or []) + ) + result.once = sorted((page.get("onceProps") or {}).keys()) + + +def discover_paths(page_props: dict[str, Any]) -> list[str]: + """Every root-relative ``url``/``href`` found in the shared ``menus`` prop.""" + found: list[str] = [] + + def walk(node: Any) -> None: + if isinstance(node, dict): + for key, value in node.items(): + if key in ("url", "href") and isinstance(value, str) and value.startswith("/"): + found.append(value) + else: + walk(value) + elif isinstance(node, list): + for item in node: + walk(item) + + walk(page_props.get("menus")) + return found From d9abd88f47001829774cda501a54fe1569461c4d Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 5 Nov 2026 15:31:29 +0100 Subject: [PATCH 2/2] fix(page-audit): hold pages without a visible link to the deferred-props budget (first-load timing) Claude-Session: https://claude.ai/code/session_01FXHA3ZJ5EgoJN4zXTAhBR2 --- framework/cli/tests/test_skill_page_audit.py | 11 +++++++++++ .../scripts/page_audit/audit_report.py | 14 ++++++++------ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/framework/cli/tests/test_skill_page_audit.py b/framework/cli/tests/test_skill_page_audit.py index 3a8893cd4..4d26a46f1 100644 --- a/framework/cli/tests/test_skill_page_audit.py +++ b/framework/cli/tests/test_skill_page_audit.py @@ -157,3 +157,14 @@ def test_ux_checks(self) -> None: "unnamed_controls": "warn", "h1": "warn", } + + def test_deferred_budget_falls_back_to_the_first_load(self) -> None: + """A page with no visible link to click is still held to the budget.""" + slow = BrowserResult(path="/p/", load={"deferred_done_ms": 6000, "dev_build": True}) + assert _checks(audit_report.evaluate(_server(), slow)) == {"deferred_ms": "fail"} + clicked = BrowserResult( + path="/p/", + load={"deferred_done_ms": 6000, "dev_build": True}, + nav={"nav_ms": 100, "deferred_done_ms": 400, "cls": 0}, + ) + assert _checks(audit_report.evaluate(_server(), clicked)) == {} diff --git a/skills/simple-module-page-perf/scripts/page_audit/audit_report.py b/skills/simple-module-page-perf/scripts/page_audit/audit_report.py index 5ee97089b..ea265bbb5 100644 --- a/skills/simple-module-page-perf/scripts/page_audit/audit_report.py +++ b/skills/simple-module-page-perf/scripts/page_audit/audit_report.py @@ -99,12 +99,14 @@ def evaluate(server: ServerResult, browser: BrowserResult | None) -> list[Findin out += _budget(p, "js_kb", round((load.get("js_bytes") or 0) / KB), "JS KB") out += _budget(p, "cls", round(max(load.get("cls", 0), nav.get("cls", 0)), 3), "CLS") out += _budget(p, "nav_ms", nav.get("nav_ms") and round(nav["nav_ms"]), "navigation ms") - out += _budget( - p, - "deferred_ms", - nav.get("deferred_done_ms") and round(nav["deferred_done_ms"]), - "deferred ms", - ) + # Click -> deferred painted when the page was reached by a link; otherwise + # document start -> deferred painted from the full load, so a page with no + # visible link is still held to the budget. + if nav.get("deferred_done_ms") is not None: + deferred, label = nav["deferred_done_ms"], "deferred ms" + else: + deferred, label = load.get("deferred_done_ms"), "deferred ms (first load)" + out += _budget(p, "deferred_ms", deferred and round(deferred), label) out += _budget(p, "dom_nodes", ux.get("dom_nodes"), "DOM nodes") out += _budget(p, "table_rows", ux.get("max_table_rows"), "table rows") if browser.console_errors: