docs: catch the reference docs up to v0.0.32 - #285
Open
antosubash wants to merge 3 commits into
Open
Conversation
Thirteen commits landed since the docs were last swept (admin section, Inertia cache guard, maintenance mode, the i18n gate, git module sources, the default Docker image). The PRs updated the pages they touched; this covers what they left behind. Broken links first. The admin move relocated every admin *view* URL and the reference docs largely still pointed at the old ones: /settings/modules, /users/admin, /audit_log/, /feature_flags/, /dashboard/doctor, /permissions/. Retargeted against each module's actual view_prefix/admin_view_prefix. The /api/* paths are a separate contract and did not move, so they are left alone. Three of those were wrong in a second way that a URL rewrite alone would have preserved: - /settings/modules/<package> names a route that does not exist at all — /admin/settings/ is one master/detail page, with no per-package path. - framework-conventions' MenuItem example used group_key "ui.nav_groups.administration"; the vocabulary is access|appearance| content|system, so the example shipped a key with no catalog entry. - fixtures.md posted form data to /users/admin/invite; the endpoint is /api/users/admin/invite and takes JSON. Then the features with no docs at all: - middleware.md listed 9 of 14 middlewares. Adds ProxyHeaders, GZip, InertiaCache, Maintenance and CommitBeforeResponse to both order blocks, with a section each, and states why the innermost three are ordered as they are — Maintenance inside InertiaCache so its short-circuited 503 does not ship a per-user payload past the cache guard, CommitBeforeResponse innermost so its send-wrapper sees the response first. - lifecycle.md was missing register_admin_routes and register_audit_links, and still showed register_event_handlers' one-arg signature. - i18n.md gains label_key/group_key, the cross-locale fallback, the ci-check-untranslated gate with its three exemption forms and its documented blind spot, and the installed-vs-active type generation split. - Maintenance mode had no operator documentation anywhere; deployment.md now covers what stays reachable, that it fails open, and that it is not a security boundary. - The CLI reference never listed smpy add / update / module verify / build. - pages.md gains AdminLayout, PageShell's section prop, and the topbar / breadcrumb / palette chrome. Two factual corrections beyond the missing pieces. env-vars.md claimed task_always_eager was "the one field still read from the environment" — #281 made broker_url and result_backend env-readable too, which is what lets a container boot before any DB row exists. And i18n.md documented SM_I18N_* as app configuration; those are read only by the standalone diagnostics runner, while the app takes its locale set from DB-backed HostSettings. Also adds SM_TRUSTED_PROXY, previously undocumented in the reference despite being required behind a TLS-terminating proxy. Verified: vitepress build clean (no dead links), check_readmes and check_metadata pass. Every prefix, hook, field and route name in the diff was read out of the source rather than inferred. Claude-Session: https://claude.ai/code/session_01JJtbN97VhtDr28Fuy5JKEF
Deploying simple-module-python with
|
| Latest commit: |
64e3d31
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2c1c5a25.simple-module-python.pages.dev |
| Branch Preview URL: | https://docs-update-latest.simple-module-python.pages.dev |
`ruff format --check` covers Python fenced blocks inside markdown, and the aligned inline comments in the new MenuItem and ModuleMeta examples used column padding where ruff wants exactly two spaces before `#`. Caught by `make lint`, which is what CI runs. Whitespace only — no wording or code changes. Claude-Session: https://claude.ai/code/session_01JJtbN97VhtDr28Fuy5JKEF
antosubash
marked this pull request as ready for review
August 26, 2026 15:58
|
You have reached your Codex usage limits for security reviews. Please try again later. |
A review flag on the "Tenant (only if SM_MULTI_TENANT=true)" line prompted
checking it. The line is right — but checking it surfaced that the
surrounding story was wrong in both directions, including a claim this
branch had just introduced.
`Settings` is `HostSettings` + `BootstrapSettings` and inherits the latter's
`env_prefix="SM_"`, so every `HostSettings` field resolves from `SM_*` at
boot. That instance lands on `app.state.sm.settings` and is what configures
middleware at construction. The DB-hydrated instance is a plain
`HostSettings` on `app.state.host.settings`, which declares no prefix.
Verified by construction, not by reading:
SM_I18N_DEFAULT_LOCALE=es -> Settings().i18n_default_locale == 'es'
SM_MULTI_TENANT=true -> Settings().multi_tenant is True
SM_MAINTENANCE_MODE=true -> Settings().maintenance_mode is True
HostSettings().maintenance_mode is False
Three corrections follow:
- i18n.md claimed `SM_I18N_*` were read only by the diagnostics runner and
that setting them "does not change what the running app serves". False,
and introduced by this branch. They are the app's actual source:
LocaleMiddleware, the i18n manifest, the shared-props builder and
i18n_deps all read them off `app.state.sm.settings`. Editing those rows in
the admin UI is what does nothing — the middleware captured its locale set
at boot, and a settings save swaps a different object.
- env-vars.md's "Host settings (DB-backed, not env)" said the same thing of
tenancy and i18n. Now a per-field table naming the env var and which
instance each field is read from, since the answer differs per field.
- The maintenance fields have no working env var, which is worth stating
because the code's shape suggests otherwise: `SM_MAINTENANCE_MODE=true`
sets the boot object, nothing reads maintenance from there, and the
hydrated `HostSettings` never sees the prefix. It looks plausible and
silently does nothing.
Also fixes middleware.md claiming `X-Tenant-ID` is the default tenant
header. `TenantMiddleware.__init__` takes `header: str | None = None` and
`tenant_header` defaults to `""`, so header lookup is off until a name is
set; `X-Tenant-ID` is a suggested constant, not a default.
Verified: make lint clean, 2152 passed / 2 skipped, 122 JS passed,
vitepress build clean (new cross-page anchor resolves).
Claude-Session: https://claude.ai/code/session_01JJtbN97VhtDr28Fuy5JKEF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thirteen commits landed since the docs were last swept (admin section, Inertia cache guard, maintenance mode, the i18n gate, git module sources, the default Docker image). Those PRs updated the pages they touched; this covers what they left behind.
Broken links
The admin move relocated every admin view URL and the reference docs largely still pointed at the old ones —
/settings/modules,/users/admin,/audit_log/,/feature_flags/,/dashboard/doctor,/permissions/. Retargeted against each module's actualview_prefix/admin_view_prefix. The/api/*paths are a separate contract and did not move, so they are untouched.Three were wrong in a second way a URL rewrite alone would have preserved:
/settings/modules/<package>names a route that does not exist —/admin/settings/is one master/detail page, no per-package pathframework-conventions.mdMenuItemexample usedui.nav_groups.administration; the vocabulary isaccess|appearance|content|system, so the example shipped a key with no catalog entrytesting/fixtures.md/users/admin/invite; the endpoint is/api/users/admin/inviteand takes JSONFeatures with no docs
middleware.mdlisted 9 of 14 middlewares. Adds ProxyHeaders, GZip, InertiaCache, Maintenance, CommitBeforeResponse to both order blocks with a section each, and states why the innermost three are ordered as they are.lifecycle.mdwas missingregister_admin_routesandregister_audit_links, and still showedregister_event_handlers' one-arg signature.i18n.mdgainslabel_key/group_key, the cross-locale fallback, theci-check-untranslatedgate with its three exemption forms and documented blind spot, and the installed-vs-active type generation split.deployment.mdnow covers what stays reachable, that it fails open, and that it is not a security boundary.smpy add/update/module verify/module build.pages.mdgainsAdminLayout,PageShell'ssectionprop, and the topbar / breadcrumb / palette chrome.Factual corrections
env-vars.mdclaimedtask_always_eagerwas "the one field still read from the environment". feat(docker): default app image, standalone bydocker run#281 madebroker_urlandresult_backendenv-readable too — that is what lets a container boot before any DB row exists.i18n.mddocumentedSM_I18N_*as app configuration. Those are read only by the standalone diagnostics runner (python -m simple_module_core); the app takes its locale set from DB-backedHostSettings. Setting them does not change what the app serves.SM_TRUSTED_PROXY, previously absent from the reference despite being required behind a TLS-terminating proxy (without it Inertia's pushState throws and login breaks).Verification
vitepress buildclean — no dead linkscheck_readmes.py,check_metadata.pypassAuditLink.url_templateneeds the literal{id}(a{entity_id}template raises at boot), and the registry method isregister(), notadd().Docs-only — no code changes.
https://claude.ai/code/session_01JJtbN97VhtDr28Fuy5JKEF
Ship pipeline
Ran the full review → verify pipeline on this branch.
Code review (high effort): 0 findings. Fanned out 6 verification agents to fact-check every concrete claim in the diff against source — admin route moves and redirect status codes, middleware install/execution order, lifecycle hook ordering and the
register_event_handlersback-compat dispatch,AuditLinkboot validation, i18n DB-backed settings and the untranslated-string gate,SM_TRUSTED_PROXYand the Celery env vars, and the frontend layout/chrome claims. ~35 claims, all CONFIRMED with file:line citations. Both new link anchors (#audience,#maintenance-mode) resolve.Local CI: green.
make lint(ruff format + ruff + ty + biome + per-workspace tsc + file-size + hardcoded-strings + untranslated-strings + metadata + readmes)uv run pytestnpm testvitepress buildOne real defect surfaced by the gate and fixed in
3c2754a:ruff format --checkalso covers Python fenced blocks inside markdown, and the aligned inline comments in the newMenuItem/ModuleMetaexamples used column padding where ruff wants two spaces. Whitespace only.Browser QA: deliberately skipped. This branch changes 17 markdown files and zero application code, so browser tests would exercise
main's behaviour and QA's auto-fix agents would edit unrelated app code onto a docs PR — no signal available about this diff.Follow-up: host settings, env vs DB (
64e3d31)A review flag on middleware.md's
Tenant (only if SM_MULTI_TENANT=true)line turned out to be a false positive — that line is correct — but checking it surfaced that the surrounding story was wrong in both directions, including a claim this branch had just introduced.SettingsisHostSettings+BootstrapSettingsand inherits the latter'senv_prefix="SM_", so everyHostSettingsfield resolves fromSM_*at boot. That instance lands onapp.state.sm.settingsand configures middleware at construction. The DB-hydrated instance is a plainHostSettingsonapp.state.host.settings, which declares no prefix. Verified by construction rather than by reading:SM_I18N_*were read only by the diagnostics runner, and that setting them "does not change what the running app serves". That was false and it was introduced by this branch. They are the app's actual source —LocaleMiddleware, the i18n manifest, the shared-props builder andi18n_depsall read them offapp.state.sm.settings. It is editing those rows in the admin UI that does nothing.SM_MAINTENANCE_MODE=truesets the boot object, nothing reads maintenance from there, and the hydratedHostSettingsnever sees the prefix. It looks plausible and silently does nothing.X-Tenant-IDis the default tenant header.TenantMiddleware.__init__takesheader: str | None = Noneandtenant_headerdefaults to"", so header lookup is off until a name is set.Gate re-run on the final commit:
make lintclean, 2152 passed / 2 skipped, 122 JS passed, vitepress build clean (the new cross-page anchor resolves).