docs(phases): convert TODO.md L3 into phase 168 (admin settings panel)
One owner item, one phase: an admin-only Settings view that writes any BOR_* setting to the database, resolves DB-over-env inside get_settings(), and applies on Save without a restart. Six tasks: the app_settings table + reversible migration 0028, the overlay resolver and field catalog, the live-apply reset of the two in-process singletons (the shared LLMClient and the chat semaphore), the admin-only /api/admin/settings surface, the shell's eighth view, and the dedicated Playwright suite. Owner-locked during authoring: DB-only persistence (.env is never written), the overlay lives inside get_settings() behind the BOR_SETTINGS_DB kill switch, database_url and embedding_dim are not DB-overridable, boot-wired settings stay editable with a "restart required" badge, secrets are masked and write-only, and every write is validated by the real Settings validators while an unusable stored row is skipped at read so boot can never brick. Protocol B append: PLAN.md and AGENTS.md are untouched, so PLAN's roadmap table does not list phase 168 and its "get_settings() is lru_cache'd" note in 13 is extended (cached env base + DB overlay + invalidation). TODO.md's L3 item is now cleared - it lives in this phase directory. LLM-Generated: true
This commit is contained in:
@@ -0,0 +1,81 @@
|
||||
# Phase 168 — Admin Settings panel: edit any `BOR_*` setting in the browser, DB over env, live on Save
|
||||
|
||||
**Source:** `TODO.md` L3 — "I want an admin panel that can edit anything in .env and save it to the database. The database value should override the env var. The change should take effect on clicking 'save' on that page."
|
||||
**Story:** n/a (owner-directed feature)
|
||||
**Context:** `app/config.py` (75 `Settings` fields, `env_prefix="BOR_"`, `env_file=".env"`, `extra="ignore"`, `get_settings()` `@lru_cache`d, ~40 call sites across `app/`), `app/db.py` (module-level `settings = get_settings()` + `create_engine`), `app/main.py` (`settings = get_settings()` at import, middleware wired at boot, `_shell_routes`), `app/core/theming.py::effective_settings` (the DB-over-env resolver precedent, B1), `app/models.py::UiSettings` (the single-row admin-persistence precedent), `app/api/ui_settings.py` + `app/api/tokens.py` (router-wide `require_admin`), `app/core/auth.py::require_admin`, `frontend/index.html` (`#view-*` shell views; `#view-tokens` L1077, `#view-theme` L1269), `frontend/assets/router.js` (`VIEW` / `VIEW_PATH` / `VIEW_MODULES`), `frontend/assets/header.js` (~L197-230, the ship-hidden / reveal-for-admin nav contract), `frontend/assets/styles.css` (`.table-wrap` L2825 + `.table-wrap table thead th` sticky L2841 — the phase-156 bounded-scroll precedent), `app/core/caching.py::HTML_PAGES` (L169), `alembic/versions/` (next free: **0028**), `tests/e2e/conftest.py` (the `os.environ["BOR_PDF_PICTORIAL"] = "0"` global-pin precedent, ~L52).
|
||||
|
||||
## Objective
|
||||
|
||||
An admin opens **Settings** in the navbar, edits any setting that today lives only in `.env` (75 fields, grouped, type-correct inputs), clicks **Save**, and the new value is (a) stored in Postgres, (b) preferred over the env var from that instant on with **no restart**, and (c) visible in the running UI. Per-row **Reset** deletes the DB row so the env value wins again. The `.env` file is never written by the app.
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
|
||||
**D1 — Storage: one `app_settings` table (task 01).** `key` = the **bare `Settings` field name** (`relevance_threshold`), `value` = the **env-native string form** (`"0.55"`, `"true"`, `"md,txt"`, a JSON array for the one `list[str]` field), `updated_at`. Storing bare strings (not typed columns, not a wide row) is what makes "anything in .env" work for all 75 fields with zero per-field DDL, and keeps the coercion in exactly one place (the pydantic validators). The `BOR_` prefix stays a *presentation* concern (`env_var = "BOR_" + key.upper()`), never stored — a stored prefix is a double-prefix typo waiting to happen.
|
||||
|
||||
**D2 — The overlay lives INSIDE `get_settings()` (task 02).** The ~40 existing call sites (`app/rag/retriever.py`, `app/api/chat.py`, middlewares, `scripts/`) must not change — that is the only way "anything" becomes live rather than "the three fields someone remembered to thread through". `get_settings()` keeps its `@lru_cache` and its zero-arg signature (`tests/{integration,e2e}/*.py` call `get_settings.cache_clear()` in 6 files — that surface must survive), and resolves: `Settings()` (env/`.env`, exactly today) → overlay the DB rows → validate → return. `invalidate_settings()` clears the cache and runs the reset hooks (D3). Hot-path cost after the first call: one `lru_cache` dict hit.
|
||||
|
||||
**D3 — "Takes effect on Save" = cache + the two in-process singletons (task 03).** Everything except two objects is read per use, so clearing the cache is enough for ~70 fields. The exceptions are `app/api/chat.py::_llm` (an `LLMClient` built once from `get_settings()` — holds `llm_base_url` / `effective_api_key` / `llm_chat_model` / `llm_embed_model` / timeouts / retries, L318-326) and `_chat_semaphore` (built once from `chat_max_concurrent`, L302-315). The overlay exposes a hook registry; `app/main.py::create_app()` registers the chat reset (`_llm = None`, `_chat_semaphore = None`) so `app/core/` never imports `app/api/` (dependency direction, PLAN §3). Settings wired at boot cannot be applied in-process (engine pools, `SessionMiddleware`, the static mount, the gzip middleware, logging) — they stay **editable** but carry a `restart_required` badge (LOCKED A4).
|
||||
|
||||
**D4 — The catalog is code, derived-checked, and complete (task 02).** `app/core/settings_catalog.py` declares, per field: `group` (mirrors `app/config.py`'s own section comments: App & branding, HTTP & logging, Database, LLM, Chat history, RAG & retrieval, Agent tools, Import & sources, Images, PDF, Docs push, Admin & security), `label`, `secret`, `restart_required`, `overridable`. A unit test asserts the catalog covers **exactly** `Settings.model_fields` (a new setting added without catalog metadata fails the test → it can never silently disappear from the panel).
|
||||
|
||||
**D5 — Fail loud on write, fail soft on read (tasks 02/04, LOCKED A5).** A write validates the FULL candidate set — every stored override plus the new one, layered over the env base — through the real `Settings` validators, so a cross-field typo (`lexical_support_floor` above a DB-overridden `relevance_threshold`) is a 422 that names the field and writes nothing. A read that finds an unusable row (hand-edited, or a validator changed in a later release) skips THAT key, logs `settings override ignored key=… reason=…` and applies the rest: a DB value must never brick boot, because a bricked app cannot be un-bricked from the panel.
|
||||
|
||||
**D6 — The kill switch, and the E2E poison guard (task 02).** `BOR_SETTINGS_DB=0` (env-only, never itself DB-overridable, read straight from `os.environ`) disables the overlay entirely — the pre-phase behavior, the house `agent_max_rounds`/`compress_min_bytes` convention. `tests/e2e/conftest.py` pins it to `0` for **every** E2E app the same way it already pins `BOR_PDF_PICTORIAL`: a row left behind by one suite would otherwise silently retune every later suite's app subprocess. The phase-168 suite opts back in on its own app fixture (the `test_pictorial_pdf.py` pattern) and `TRUNCATE app_settings` before AND after (the `test_admin_theme_tab.py:286` precedent).
|
||||
|
||||
**D7 — The API (task 04).** `app/api/admin_settings.py`, prefix `/admin/settings`, router-wide `dependencies=[Depends(require_admin)]` (the `ui_settings`/`tokens` pattern: anonymous AND token users 403): `GET` the catalog + effective values with a `source` badge (`db` | `env` | `default`), `PUT /{key}` set one value, `DELETE /{key}` reset to env, `DELETE` reset all (the panel's "Clear all overrides"). Secrets are **write-only** (LOCKED A4): `GET` reports `is_set`, never the plaintext, and the write log line carries the key and the action, never the value.
|
||||
|
||||
**D8 — The view (task 05).** The shell's eighth view: `#view-settings` in `frontend/index.html` + `frontend/assets/settings.js`, `/settings.html` added to `main.py::_shell_routes` and `caching.HTML_PAGES`, `VIEW`/`VIEW_PATH`/`VIEW_MODULES` in `router.js`, and `#nav-settings` (ship-hidden) in `index.html` / `document.html` / `login.html` / `shared.html` revealed by `header.js` — the exact phase-91 Theme-view wiring. Layout per PLAN §7 + AGENTS rule 5: the 72rem `.container`, one `.table-wrap` card (bounded `max-height: min(70vh, 64rem)`, sticky `thead`) with a `<tbody>` per group, type-correct controls (`<input type=number>` for int/float, a `true/false` `<select>` for bool, `<textarea>` for the CSV/JSON and long strings, a password input for secrets), a text source badge (B5: never color alone), a client-side `type=search` filter (the phase-101 tokens pattern), a page-level **Save settings** (dirty rows only), per-row **Reset**, and a `role="status"` / `role="alert"` live region (§7.4).
|
||||
|
||||
## Dependencies
|
||||
- `91_admin_theme_tab` + `92_theme_save_and_coverage` + `93_theme_semantic_completion` (complete) — the DB-over-default resolver (B1), the admin-only shell view + gate pattern this view copies, and the nav reveal contract.
|
||||
- `79_api_tokens` (complete) — router-wide `require_admin` + the bounded, searchable admin catalogue table with per-row actions.
|
||||
- `156_table_ux_polish` (complete) — `.table-wrap` bounded scroll + sticky header, the `cell-clip` ellipsis contract for long action labels.
|
||||
- `167_api_payload_diet` (complete) — the response byte budget: the new `GET /api/admin/settings` (~75 rows ≈ 20 KB) must stay under `BOR_LOG_LARGE_RESPONSE_BYTES` and the audit script's 256 KB bar; never a 413.
|
||||
- `82_security_headers` + `81_login_rate_limit` (complete) — the CSP/`default-src 'self'` posture the new JS must live inside (no CDN, rule 6).
|
||||
|
||||
## Tasks
|
||||
1. `01_app_settings_table.md` — `AppSetting` model + reversible migration `0028_app_settings.py` + its migration test.
|
||||
2. `02_settings_overlay_resolver.md` — `app/core/settings_catalog.py` + `app/core/settings_overlay.py`, `get_settings()` becomes the cached env+DB resolver, `invalidate_settings()`, the `BOR_SETTINGS_DB` kill switch + the E2E conftest pin, the catalog-completeness unit pins.
|
||||
3. `03_live_apply_on_save.md` — the reset-hook registry, the chat `_llm` / `_chat_semaphore` reset registered from `create_app()`, and the integration proof that a saved change bites **without a restart**.
|
||||
4. `04_admin_settings_api.md` — `/api/admin/settings` GET / PUT `/{key}` / DELETE `/{key}` / DELETE all, the schemas, the full-candidate validation, the masked secrets, the value-free log line, the README API note.
|
||||
5. `05_settings_view_ui.md` — the Settings shell view (`#view-settings`, `settings.js`, router + shell route + `HTML_PAGES` + nav reveal, `.table-wrap` catalogue, Save/Reset + live region).
|
||||
6. `06_e2e_settings_panel.md` — `tests/e2e/test_admin_settings_panel.py`, the regression set, the full gates, the atomic commit.
|
||||
|
||||
## Testing & Quality
|
||||
- **Unit:** `tests/unit/test_settings_catalog.py` (catalog ⇄ `Settings.model_fields` equality; the masked set is exactly `llm_api_key` + `admin_password` + `session_secret` + `database_url` (the last one is read-only but its URL carries credentials); the non-overridable set is exactly `database_url` + `embedding_dim`), `tests/unit/test_settings_overlay.py` (merge/coercion/validation-skip/`BOR_SETTINGS_DB=0`), `tests/unit/test_settings_frontend_view.py` (static source pins in the house style: the view section exists, `router.js` maps `/settings.html`, no CDN, `settings.js` builds DOM with `createElement`/`textContent`).
|
||||
- **Integration:** `tests/integration/test_migration_0028.py` (up/down, A13), `tests/integration/test_admin_settings_api.py` (auth matrix anonymous/token-user/admin, 404 unknown key, 422 validator failures naming the field, masked secrets in the response body, the DB-over-env flip visible through `GET /api/config`, reset-to-env, `get_settings()` reflecting a write with no restart), plus the existing `get_settings.cache_clear()` suites staying green untouched.
|
||||
- **Coverage:** **>90%** on `app/` — `uv run pytest --cov=app --cov-report=term-missing`.
|
||||
- **E2E:** `uv run pytest tests/e2e/test_admin_settings_panel.py -v --no-cov` green **in isolation**; regression set green in isolation: `test_admin_theme_tab.py`, `test_api_tokens.py`, `test_configurable_brand.py`, `test_global_tuning.py`, `test_admin_auth.py`, `test_cache_busting.py`.
|
||||
- **Lint/types:** `uv run ruff check . && uv run pyright`.
|
||||
- **Log contract (AGENTS rule 10):** every settings write logs one line (`settings action=set|clear|clear_all key=… source=db|env`) with no value; `GET /api/admin/settings` appears in the `grep ' :: api '` byte audit and under the 256 KB bar (`uv run python -m scripts.api_payload_audit`).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `/settings.html` is an admin-only shell view: anonymous sees the sign-in gate, a token user never sees the nav link, and all four API routes 403 for both.
|
||||
- [ ] Editing a value + **Save settings** stores it in `app_settings`, flips the row's badge from `env`/`default` to `DB`, and the effect is visible in the running app with **no restart** (pinned E2E: `BOR_INPUT_PLACEHOLDER` shows up in the composer; `BOR_RELEVANCE_THRESHOLD` / `BOR_LLM_CHAT_MODEL` are read from the overlay by a live request).
|
||||
- [ ] **Reset** deletes the row and the env value wins again; **Clear all overrides** empties the table; `.env` is never opened for writing by any code path (unit-pinned: no `open(` on the env file in `app/`).
|
||||
- [ ] Secrets never leave the server (`GET` body has no plaintext, `page.content()` has no secret), no write log line contains a value, and a bogus value is a 422 that names the field and stores nothing.
|
||||
- [ ] A stored-but-invalid row (inserted directly, bypassing the API) is ignored at read with a logged reason — the app still boots and `/api/health` is 200.
|
||||
- [ ] `uv run pytest` green, `app/` coverage >90%, ruff + pyright clean, the phase E2E + the regression set green in isolation.
|
||||
- [ ] One atomic `--no-gpg-sign` Conventional Commit; the phase dir moves to `.agents/phases/complete/`.
|
||||
|
||||
## Locked decisions (owner-confirmed 2026-10-09 during phase authoring)
|
||||
- **A1 — DB-only persistence:** the panel writes `app_settings` and **never** touches `.env`; the env var stays the base and the DB row wins.
|
||||
- **A2 — the overlay sits inside `get_settings()`** (cached, `cache_clear()`-compatible) so all ~40 consumers go DB-over-env at once, behind the `BOR_SETTINGS_DB=0` kill switch.
|
||||
- **A3 — `database_url` and `embedding_dim` are NOT DB-overridable** (read-only rows, reason shown: the engine is built from `database_url`, so an override is a chicken-and-egg brick; A6 fixes `chunks.embedding` at 768, so a dim override would corrupt retrieval).
|
||||
- **A4 — boot-wired settings stay editable with a "restart required" badge** (pools, session middleware, static mount, gzip, log level); **secrets are masked and write-only** and never appear in a response body or a log line.
|
||||
- **A5 — validate the full candidate set on write (422, nothing stored), fail soft on read** (skip the bad key, log it, boot anyway).
|
||||
- **A6 — one story, one phase, one Playwright file** (AGENTS rule 4) — the backend tasks are covered by unit + integration, the live-effect contract by `tests/e2e/test_admin_settings_panel.py`.
|
||||
|
||||
## Assumptions (the TODO left these open; the executor does not guess)
|
||||
- **K1:** the key is the bare field name; `BOR_UPPER` is derived for display/API presentation only.
|
||||
- **K2:** the one `list[str]` field (`suggestions`) is edited as a JSON array in a `<textarea>` and stored in that same JSON form (pydantic parses it exactly like the env var); every other field is a scalar or CSV string.
|
||||
- **K3:** a missing `app_settings` table (fresh deployment, migrations not applied) means "no overrides" and logs at DEBUG once — only an unusable *value* logs at ERROR.
|
||||
- **K4:** "Save settings" sends only the dirty rows; the first failing row aborts the batch, shows its 422 detail, and keeps the other rows' edits in the form for correction.
|
||||
- **K5:** history/audit beyond `updated_at` (who changed what, when, old value) is out of scope — single-admin homelab (PLAN §1 "out of scope").
|
||||
- **K6:** the overlay is single-process (PLAN A12: in-process state, restart-clearing accepted, no Valkey); with more than one uvicorn worker a save is visible in the worker that served it and in every worker after its next resolution — the deployment runs one app process, and the panel notes nothing about it.
|
||||
- **K7:** `.env` keys that are not `Settings` fields (`extra="ignore"`, e.g. `AIPI_KEY`, `DEBUGPY*`) are not editable — they have no `app/` consumer to retune. `BOR_LLM_API_KEY` remains the way to set the key (it wins over `$AIPI_KEY` via `effective_api_key`).
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add app/ alembic/ frontend/ tests/ .env.example README.md .agents/phases/ && git commit --no-gpg-sign -m "feat(settings): admin settings panel with DB-over-env overrides applied live on save"
|
||||
```
|
||||
@@ -0,0 +1,60 @@
|
||||
# Task 01 — `app_settings`: the table that holds DB-side setting overrides
|
||||
|
||||
**Phase:** `168_admin_settings_panel` · **Source:** `TODO.md:3` — "I want an admin panel that can edit anything in .env and save it to the database."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
One narrow key/value table stores the admin's overrides, keyed by the bare `Settings` field name, with the value kept in its **env-native string form** — the storage half of phase-168 D1, with zero per-field DDL for the 75 settings.
|
||||
|
||||
## Work
|
||||
1. `app/models.py` — append the `AppSetting` class (after `FolderSummary`, the file's tail) in the house docstring style, naming the phase and the TODO line:
|
||||
```python
|
||||
class AppSetting(Base):
|
||||
"""One admin setting override (phase 168, task 01).
|
||||
|
||||
``TODO.md`` L3: the admin Settings panel persists what used to be
|
||||
``.env``-only knobs here, and the row WINS over the environment
|
||||
(resolved by ``app.core.settings_overlay``, consumed by
|
||||
``app.config.get_settings``). …
|
||||
"""
|
||||
|
||||
__tablename__ = "app_settings"
|
||||
|
||||
#: The BARE ``Settings`` field name (``relevance_threshold``) — never
|
||||
#: the ``BOR_…`` form: the prefix is presentation
|
||||
#: (``"BOR_" + key.upper()``), so a stored prefix could double up.
|
||||
key: Mapped[str] = mapped_column(String(100), primary_key=True)
|
||||
#: The value in the SAME textual form the env var would carry
|
||||
#: (``"0.55"``, ``"true"``, ``"md,txt"``, a JSON array for the one
|
||||
#: ``list[str]`` field) — coercion and validation belong to
|
||||
#: pydantic, in exactly one place (phase 168 D1/A5).
|
||||
value: Mapped[str] = mapped_column(Text, nullable=False)
|
||||
#: When the admin last saved it (the ``folder_summaries`` /
|
||||
#: ``kb_overview.updated_at`` house style: server-side now()).
|
||||
updated_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True), server_default=func.now(), nullable=False
|
||||
)
|
||||
```
|
||||
Import `func` from `sqlalchemy` if the module does not already (it imports `DateTime`, `String`, `Text`, `Integer`, `select`-adjacent names — check the header first, do not duplicate an import).
|
||||
No index: the whole table is read by primary key order in one `SELECT` per settings resolution (the `ui_settings` "display fact, never a query surface" note, 0027).
|
||||
2. `alembic/versions/0028_app_settings.py` — `revision = "0028"`, `down_revision = "0027"`, docstring in the 0027 style (what the table is for, why the value is TEXT, why no index, why the downgrade is a clean drop). `upgrade()` = `op.create_table("app_settings", sa.Column("key", sa.String(length=100), primary_key=True), sa.Column("value", sa.Text(), nullable=False), sa.Column("updated_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False))`; `downgrade()` = `op.drop_table("app_settings")` with the A13 comment (dropping it leaves the 0027 schema byte-identical; the values are re-enterable from the panel).
|
||||
3. `tests/integration/test_migration_0028.py` (new) — copy the `test_migration_0017.py` harness verbatim (the `alembic` fixture with the `db_available()` skip + the teardown `upgrade(cfg, "head")`, `_version`, the `information_schema` assertion helpers) and pin:
|
||||
- 0027 → 0028 creates `app_settings` with `key` VARCHAR(100) NOT NULL and PK, `value` TEXT NOT NULL, `updated_at` TIMESTAMPTZ NOT NULL with the `now()` server default, and `alembic_version = '0028'`;
|
||||
- the 0027 schema survives (`documents.page_strip_mode` still present);
|
||||
- rows round-trip (insert `relevance_threshold`/`"0.55"` and `import_extensions`/`"md,txt"`, read them back);
|
||||
- a duplicate `key` raises `IntegrityError` (the PK is the uniqueness guard — two rows for one setting would make "which one wins" undefined);
|
||||
- `NULL` value is rejected (NOT NULL — "clear the override" is a DELETE, never an empty value, so the resolver never has to distinguish them);
|
||||
- downgrade to 0027 → the table is GONE, the rest survives; re-upgrade → the table is back (A13 round-trip).
|
||||
4. `.env.example` — add a short comment block at the top (under the header, before the first `BOR_` line) stating the precedence rule and the kill switch: the app reads `BOR_*` from the environment, an admin override saved in the database wins per key, the panel never writes this file, and `BOR_SETTINGS_DB=0` disables DB overrides (set it in the container env, not here — a `0` committed here is a silent "my change did nothing"). Task 02 implements what this comment describes; write it so it stays true.
|
||||
ASSUMPTION (K3): `.env.example` is documentation only — nothing parses it at runtime except pydantic's `env_file=".env"`, so a comment cannot change behavior.
|
||||
|
||||
## Testing & Quality
|
||||
- Integration: `uv run pytest tests/integration/test_migration_0028.py -v` (needs `podman compose up -d db`).
|
||||
- Unit: no new module yet; `uv run pytest tests/unit -q` stays green (adding a model cannot change behavior — if something moved, it is a bug in this task).
|
||||
- Coverage: **>90%** on `app/` — the added model is exercised by the migration test's insert/read plus task 02's resolver tests.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run alembic upgrade head` applies 0028 and `uv run alembic downgrade 0027` removes `app_settings` cleanly.
|
||||
- [ ] `tests/integration/test_migration_0028.py` green (including the duplicate-key and NOT-NULL pins).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] No behavior change anywhere: `get_settings()` still reads env only (that is task 02), and no route exists yet.
|
||||
@@ -0,0 +1,76 @@
|
||||
# Task 02 — The overlay: `get_settings()` resolves DB-over-env, behind a kill switch
|
||||
|
||||
**Phase:** `168_admin_settings_panel` · **Source:** `TODO.md:3` — "…save it to the database. The database value should override the env var."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
A cached resolver that layers the `app_settings` rows over the environment and hands every existing consumer a valid `Settings` — no consumer changes, no restart, and a `BOR_SETTINGS_DB=0` switch that puts the pre-phase behavior back exactly.
|
||||
|
||||
## Work
|
||||
1. `app/core/settings_catalog.py` (new) — the presentation + capability metadata, code-owned and complete:
|
||||
- `@dataclass(frozen=True, slots=True) class SettingMeta`: `key: str`, `group: str`, `label: str`, `secret: bool = False`, `restart_required: bool = False`, `overridable: bool = True`.
|
||||
- `CATALOG: dict[str, SettingMeta]` covering **every one** of `Settings.model_fields` (75 today). Groups, in this display order, with these members (they mirror `app/config.py`'s own section comments — if the executor disagrees with one placement, keep the count and note it in the commit body):
|
||||
| Group | Fields |
|
||||
|---|---|
|
||||
| App & branding | `app_name` `app_version` `environment` `input_placeholder` `footer_text` `suggestions` |
|
||||
| HTTP & logging | `log_level`↻ `log_large_response_bytes` `compress_min_bytes`↻ `static_dir`↻ |
|
||||
| Database | `database_url`⛔ `db_pool_size`↻ `db_pool_max_overflow`↻ `db_pool_recycle`↻ `chat_max_concurrent` |
|
||||
| LLM (aipi) | `llm_base_url` `llm_api_key`🔒 `llm_chat_model` `llm_embed_model` `llm_summary_model` `stream_thinking` `llm_retries` `llm_retry_delay` `llm_timeout` |
|
||||
| Chat history | `history_max_turns` `history_max_chars` |
|
||||
| Retrieval & honesty | `embedding_dim`⛔ `top_n_docs` `relevance_threshold` `lexical_support_floor` `source_usefulness_floor` `related_max_docs` `suggested_docs` `suggestion_preview_chars` `hybrid_vector_candidates` `hybrid_lexical_candidates` `rrf_k` `recency_boost` `recency_half_life_days` `name_hit_bonus` |
|
||||
| Chunking & embedding | `chunk_target_chars` `chunk_overlap_chars` `embed_batch_size` `embed_question_max_chars` |
|
||||
| Prompt budgets | `max_output_tokens` `steering_max_chars` `summary_max_chars` `kb_overview_max_chars` `overview_input_max_chars` `folder_summary_input_max_chars` |
|
||||
| Agent tools | `agent_max_rounds` `read_max_chars` |
|
||||
| Import & sources | `import_extensions` `git_sources` `sources_dir` `upload_dir` `upload_max_mb` |
|
||||
| Images | `images` `image_extensions` `image_dir` `chat_image_dir` `chat_image_max_mb` `answer_images` |
|
||||
| PDF | `pdf_pictorial` `pdf_pictorial_min_chars` `pdf_image_skip_blank` `pdf_page_dir` |
|
||||
| Docs push | `docs_repo` `docs_branch` `docs_base_branch` `docs_work_dir` |
|
||||
| Admin & security | `admin_password`🔒 `session_secret`🔒↻ `session_max_age`↻ `session_cookie`↻ |
|
||||
`↻` = `restart_required=True` (wired once at boot: logging, the gzip middleware in `app/main.py`, the static mount, `create_engine` in `app/db.py`, `SessionMiddleware`); `⛔` = `overridable=False` (LOCKED A3 — `database_url` is what builds the engine that would read the override, `embedding_dim` is fixed in the `chunks` table by A6); `🔒` = `secret=True`, plus `database_url` as a fourth masked field (its URL carries credentials) even though it is read-only.
|
||||
- `ENV_PREFIX = "BOR_"`, `def env_var(key: str) -> str` → `"BOR_" + key.upper()`; `GROUPS: tuple[str, ...]` (the table order above); `SECRETS`, `RESTART_REQUIRED`, `NON_OVERRIDABLE` derived from `CATALOG` (never re-listed by hand).
|
||||
- Module docstring: the phase, the TODO line, why the catalog exists (the panel is generated, not hand-written) and the completeness pin.
|
||||
2. `app/core/settings_overlay.py` (new) — the mechanism:
|
||||
- `resolve_settings() -> Settings` — the whole overlay: `base = Settings()`; `if not enabled(): return base`; `rows = load_overrides()`; `return merge(rows)[0]`. **Never call `app.config.get_settings()` here** — `Settings()` is the env-only entry point (recursion guard, pinned by test).
|
||||
- `enabled() -> bool` — `os.environ.get("BOR_SETTINGS_DB", "1")` is not one of `0/false/no/off` (the house bool parsing), read **per call** from `os.environ` so tests can flip it; the flag itself is not a `Settings` field, so it can never be turned off from the database (the escape hatch cannot disable itself).
|
||||
- `load_overrides() -> dict[str, str]` — `SELECT key, value FROM app_settings ORDER BY key` through a module-level lazily-created engine: `create_engine(Settings().database_url, poolclass=NullPool, pool_pre_ping=True)` (NullPool: one connection per resolution, none held — the app's real pool belongs to `app/db.py` and must not be used here: importing `app.db` from the overlay would be a cycle, `app/db.py` imports `app.config`, and the pool whose size this table can change must not be the one that reads the table). Fail-soft: `ProgrammingError`/undefined-table → `{}` + one DEBUG line per process (K3 — a pre-migration deployment); any other `SQLAlchemyError`/`OSError` → `{}` + one WARNING per process naming the exception class ("settings overrides unavailable — env values in effect"). Both "once per process" guards are module-level flags, reset by `reset_state_for_tests()`.
|
||||
- `merge(rows: Mapping[str, str]) -> tuple[Settings, list[tuple[str, str]]]` — returns `(settings, skipped)` where `skipped` is `(key, reason)` pairs. Build the kwargs from the rows: drop keys not in `Settings.model_fields` (an orphan row from a removed setting) and keys with `overridable=False` (a hand-inserted `database_url` row must never hijack the engine — LOCKED A3), coerce each value (`_coerce(field, raw)`: `json.loads` for the one `list[str]` field `suggestions`, the raw string for everything else — pydantic-settings lax-coerces `str`→`int`/`float`/`bool`, and init kwargs rank ABOVE env in pydantic-settings, which is precisely "the database value overrides the env var"). Then `Settings(**kwargs)`; on `ValidationError` drop the named field (`error["loc"][0]`), append it to `skipped` with the validator's message, and retry — so one bad row costs one setting, not the whole table (LOCKED A5). Log each skipped key once at ERROR: `settings override ignored key=… reason=…`.
|
||||
- `register_invalidator(fn: Callable[[], None]) -> None` + `run_invalidators()` — the hook list task 03 uses; a hook that raises is logged, never propagated (a failed reset must not undo a successful save).
|
||||
- `reset_state_for_tests() -> None` — clears the module flags/engine for the unit suites.
|
||||
3. `app/config.py` — keep the `Settings` class 100% untouched and swap the resolver tail:
|
||||
```python
|
||||
@lru_cache
|
||||
def get_settings() -> Settings:
|
||||
"""Effective settings: env/.env with the admin's DB overrides layered
|
||||
on top (phase 168, TODO L3). … cached exactly as before —
|
||||
``get_settings.cache_clear()`` still works — and
|
||||
``invalidate_settings()`` is the save-side wrapper that also runs the
|
||||
runtime reset hooks (task 03)."""
|
||||
from app.core.settings_overlay import resolve_settings # lazy: app.core imports app.config
|
||||
|
||||
return resolve_settings()
|
||||
|
||||
|
||||
def invalidate_settings() -> None:
|
||||
get_settings.cache_clear()
|
||||
from app.core.settings_overlay import run_invalidators
|
||||
|
||||
run_invalidators()
|
||||
```
|
||||
The lazy imports are load-bearing (documented in the comment): `app.core.*` imports `app.config` at module scope, and `app/db.py` → `app.config` runs at import time; a module-scope `app.config` → `app.core` import would deadlock the import graph. The 6 existing `get_settings.cache_clear()` test call sites keep working unchanged (they only retune env, so skipping the hooks there is correct).
|
||||
4. `app/main.py` — inside `create_app()`, immediately after `ensure_admin_configured(settings)`: `invalidate_settings()` then `effective = get_settings()` and ONE boot log line, `settings overrides loaded count=N` (or `source=env-only disabled` when the kill switch is off) — the prime that turns a boot-time DB miss into a retry rather than a permanently cached env-only view. Keep the module-level `settings = get_settings()` as is (it is the pre-boot value the middleware wiring uses today).
|
||||
5. `tests/conftest.py` — next to the existing `os.environ.setdefault("BOR_ADMIN_PASSWORD", …)` block, add `os.environ.setdefault("BOR_SETTINGS_DB", "0")` with a comment: the shared test DB must never retune the in-process suites (the same poison-guard reasoning as the phase-140 `BOR_PDF_PICTORIAL` pin in `tests/e2e/conftest.py`), and tests that WANT the overlay set the var + `get_settings.cache_clear()` themselves.
|
||||
6. `tests/e2e/conftest.py` — same pin beside the `os.environ["BOR_PDF_PICTORIAL"] = "0"` line (~L52): `os.environ["BOR_SETTINGS_DB"] = "0"`, comment it as the cross-suite poison guard (one suite's saved row would silently change every later suite's app subprocess). The phase-168 E2E suite overrides it to `"1"` in its own module app fixture (the `test_pictorial_pdf.py:339` `env["BOR_PDF_PICTORIAL"] = "1"` pattern) — task 06 does that.
|
||||
7. `tests/unit/test_settings_catalog.py` (new) — `set(CATALOG) == set(Settings.model_fields)` (both directions, the diff in the failure message — this is the pin that stops a new setting from being invisible in the panel); `every group in GROUPS exists in CATALOG and every CATALOG group is in GROUPS`; `SECRETS == {llm_api_key, admin_password, session_secret, database_url}`; `NON_OVERRIDABLE == {database_url, embedding_dim}`; `env_var("relevance_threshold") == "BOR_RELEVANCE_THRESHOLD"`; no label is empty and no label duplicates another.
|
||||
8. `tests/unit/test_settings_overlay.py` (new) — pure/unit level, no DB: `enabled()` matrix (`"0"/"false"/"no"/"off"` off; unset/`"1"/"true"` on); `_coerce` for int/float/bool/str and `suggestions` JSON (a malformed JSON value is skipped, not fatal); `merge` happy path over the real env (`{"relevance_threshold": "0.55"}` → float 0.55, everything else byte-equal to `Settings()`), `merge` skips an unknown key and a `NON_OVERRIDABLE` key with a reason, `merge` drops one invalid value and keeps the rest, a cross-field violation (`lexical_support_floor` above a DB-overridden `relevance_threshold`) lands in `skipped` instead of raising, bool spellings (`"false"`, `"0"`) behave like the env, `load_overrides()` returns `{}` + one WARNING when the engine raises, `register_invalidator`/`run_invalidators` runs hooks in order and survives a raising hook, and a pin that `app/core/settings_overlay.py` contains no `get_settings(` call (the recursion guard) and that no file this phase owns writes `.env` (LOCKED A1 — assert the absence of an `open(` / `.env` write in `app/core/settings_overlay.py`, `app/config.py` and — once it exists — `app/api/admin_settings.py`).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: the two new files; then `uv run pytest tests/unit -q` (the `get_settings` surface is unchanged for every existing pin) and `uv run pytest tests/integration/test_api.py -q` (it calls `get_settings.cache_clear()` — it must pass untouched).
|
||||
- Integration: `get_settings()` with `BOR_SETTINGS_DB=0` is byte-identical to `Settings()` — pin it in `tests/integration/test_admin_settings_api.py` (task 04) or here if the DB is up.
|
||||
- Coverage: **>90%** on the new modules — the fail-soft branches (undefined table, generic error, invalid value, non-overridable row) each need a test.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run python -c "from app.config import Settings, get_settings; print(get_settings().relevance_threshold)"` prints the env value, and `BOR_RELEVANCE_THRESHOLD=0.11 …` still prints 0.11 (the env path is untouched).
|
||||
- [ ] With a live DB and one hand-inserted `app_settings` row + `BOR_SETTINGS_DB=1` + `invalidate_settings()`, `get_settings()` reports the DB value; with `BOR_SETTINGS_DB=0` it reports the env value.
|
||||
- [ ] A hand-inserted row for `database_url` or `embedding_dim` changes nothing and logs the skip.
|
||||
- [ ] `tests/{conftest,e2e/conftest}.py` pin the switch off; the whole `uv run pytest` suite passes with a *populated* `app_settings` table present (the poison guard proven by running the suite twice: once with the table empty, once with a garbage row in it that must change nothing).
|
||||
- [ ] `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,68 @@
|
||||
# Task 03 — Live apply: Save re-points the two in-process singletons
|
||||
|
||||
**Phase:** `168_admin_settings_panel` · **Source:** `TODO.md:3` — "The change should take effect on clicking \"save\" on that page."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
After `invalidate_settings()`, the next request behaves under the new value with **no restart**: the ~70 per-use settings are covered by the cache clear alone, and the two objects that captured a `Settings` snapshot — the shared `LLMClient` and the chat concurrency semaphore — are rebuilt.
|
||||
|
||||
## Why only two objects (traced 2026-10-09, do not re-derive)
|
||||
Every other setting is read through `get_settings()` at the point of use (`app/rag/retriever.py:408,700,807,860`, `app/core/response_budget.py:172`, `app/api/sync.py:528`, the `LLMClient()` constructions in `app/api/sync.py:532`, `app/api/docs.py:480`, `app/api/git_sources.py:944`, the path fields resolved per operation), so clearing the cache is the whole job for them. `app/core/caching.py`'s `functools.cache` `asset_version()` is the deploy token (A19), not a setting. The genuine snapshots are:
|
||||
|
||||
- `app/api/chat.py:318-326` — `_llm: LLMClient | None`, built once by `get_llm()`; `LLMClient.__init__` (`app/rag/llm.py:246-254`) holds `self.settings` and bakes `llm_base_url`, `effective_api_key` and `llm_timeout` into the `AsyncOpenAI` client, while `llm_chat_model` / `llm_embed_model` / `llm_summary_model` are read off that captured snapshot on every call.
|
||||
- `app/api/chat.py:301-315` — `_chat_semaphore: asyncio.Semaphore | None`, built once from `chat_max_concurrent`.
|
||||
|
||||
Boot-wired values (`db_pool_*`, `session_*`, `static_dir`, `log_level`, `compress_min_bytes`, `database_url`) are LOCKED as `restart_required` / non-overridable — this task does not attempt them (A3/A4).
|
||||
|
||||
## Work
|
||||
1. `app/api/chat.py` — add, directly below `get_llm()` (and documented as the phase-168 counterpart of the lazy-init comments above it):
|
||||
```python
|
||||
def reset_runtime_caches() -> None:
|
||||
"""Drop the settings-derived singletons (phase 168, task 03).
|
||||
|
||||
Called by ``invalidate_settings()`` after the admin saves. The next
|
||||
turn rebuilds the client from the CURRENT ``get_settings()`` — an
|
||||
in-flight turn keeps the client it started with (its ``AsyncOpenAI``
|
||||
transport is mid-request; swapping it would kill the stream), which
|
||||
is the same accepted-semantics note as A12's in-process state.
|
||||
``_chat_active`` is deliberately untouched: it counts live turns, and
|
||||
the new ``chat_max_concurrent`` applies as permits are taken.
|
||||
"""
|
||||
global _llm, _chat_semaphore
|
||||
_llm = None
|
||||
_chat_semaphore = None
|
||||
```
|
||||
2. `app/main.py` — in `create_app()`, right after the task-02 prime line: register the hook and log it once.
|
||||
```python
|
||||
# Phase 168 (task 03): the settings panel's Save must retune the running
|
||||
# process, and app/core must never import app/api (PLAN §3's direction) —
|
||||
# so create_app(), the composition root, wires the reset hook.
|
||||
register_invalidator(reset_runtime_caches)
|
||||
```
|
||||
with `from app.api.chat import reset_runtime_caches` added to the existing `app.api.chat` import block and `from app.core.settings_overlay import register_invalidator` alongside the other `app.core` imports. Importing `app.api.chat` here is already done (the router import at the top of the file), so this adds no new import cost or cycle.
|
||||
3. `tests/unit/test_settings_live_apply.py` (new) — source/behaviour pins in the house style:
|
||||
- `reset_runtime_caches()` nulls both globals: set `app.api.chat._llm = object()` and `app.api.chat._chat_semaphore = object()`, call it, assert both are `None`; assert `_chat_active` is untouched by the call;
|
||||
- `get_llm()` rebuilds after a reset and returns the SAME instance on two calls with no reset (the singleton contract still holds — this pin stops a future "always construct" shortcut that would break connection pooling);
|
||||
- `_get_chat_semaphore()` rebuilds after a reset and reflects a changed `chat_max_concurrent` (drive it with `monkeypatch.setenv("BOR_CHAT_MAX_CONCURRENT", "3")` + `get_settings.cache_clear()`, assert `_chat_semaphore._value == 3`), then restore;
|
||||
- `app/main.py`'s source contains `register_invalidator(reset_runtime_caches)` inside `create_app` (the static pin that stops the wiring being "cleaned up" — without it a Save silently stops retuning the LLM client);
|
||||
- `invalidate_settings()` runs registered hooks and clears the cache (call it after registering a counter, assert the counter moved and that `get_settings()` afterwards re-resolves);
|
||||
- a hook that raises does not propagate out of `invalidate_settings()` (a failed reset must not turn a committed save into a 500).
|
||||
4. `tests/integration/test_admin_settings_live.py` (new; DB required — use the existing `db` fixture + `pytest.skip` idiom from `tests/integration/test_ui_settings_api.py`) — the end-to-end proof of "no restart", in-process:
|
||||
- start from an empty `app_settings` and `monkeypatch.setenv("BOR_SETTINGS_DB", "1")` + `invalidate_settings()`, teardown `DELETE FROM app_settings` + `invalidate_settings()` + `get_settings.cache_clear()` (never leave a row behind for the suites that run after);
|
||||
- `relevance_threshold`: read `get_settings().relevance_threshold` (the env/test value), `INSERT` the row `('relevance_threshold','0.5')` directly, `invalidate_settings()`, assert `get_settings().relevance_threshold == 0.5` and that a retrieval call that consults it (`app/rag/retriever.py`'s gate reads it at `:408`) now behaves under 0.5 — assert through the function the way `tests/integration/test_recency_boost.py` does, no chat turn needed;
|
||||
- `llm_chat_model`: with the row `('llm_chat_model','gpt-test')`, `invalidate_settings()`, then `app.api.chat.get_llm().settings.llm_chat_model == "gpt-test"` **and** the rebuilt client's `AsyncOpenAI` base_url/timeout come from the new snapshot (set `('llm_timeout','7')` in the same batch and assert `client._client.timeout.connect == 7.0`, the way `app/rag/llm.py` builds it) — this is the pin that a Save actually swaps the client;
|
||||
- `chat_max_concurrent`: same shape, assert the rebuilt semaphore's `_value` (via `_get_chat_semaphore()`) follows the saved number;
|
||||
- the kill-switch counterpart: with rows present but `BOR_SETTINGS_DB=0`, `invalidate_settings()` → the env values are back (the switch really is the whole feature's kill switch);
|
||||
- `import_extensions` (the `scripts/` consumer): after the save, `get_settings().import_extension_set` reflects the DB value — the importer and Sync read the same overlay (no separate code path).
|
||||
5. `.env.example` — extend task 01's comment block with the one sentence that matters operationally: values marked "restart required" in the panel (DB pool sizes, session cookie wiring, log level, compression, the static dir) apply on the next `podman compose restart app`, everything else applies immediately.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `uv run pytest tests/unit/test_settings_live_apply.py -v`, then `uv run pytest tests/unit -q`; the existing chat pins (`tests/unit/test_chat_*`, `tests/integration/test_chat_api.py`, `test_chat_concurrency.py`) pass untouched — the reset only runs when `invalidate_settings()` is called, which nothing in them calls.
|
||||
- Integration: `uv run pytest tests/integration/test_admin_settings_live.py -v` with the DB up; then `uv run pytest tests/integration -q` to prove the teardown left nothing behind.
|
||||
- Coverage: **>90%** on `app/api/chat.py`'s new function and `app/core/settings_overlay.py`'s hook paths.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Saving a value (row insert + `invalidate_settings()`) changes what the NEXT request does — pinned for a retrieval setting, the LLM client (model + timeout), and the chat concurrency cap, with no process restart.
|
||||
- [ ] An in-flight turn is never swapped mid-stream (the docstring's contract; the unit test proves `get_llm()` hands out the same object until a reset).
|
||||
- [ ] `register_invalidator(reset_runtime_caches)` is wired in `create_app()` and unit-pinned; `app/core/` imports no `app/api/` symbol (grep-clean).
|
||||
- [ ] `uv run pytest` green, coverage >90%, ruff + pyright clean.
|
||||
@@ -0,0 +1,51 @@
|
||||
# Task 04 — `/api/admin/settings`: the admin-only read/write surface
|
||||
|
||||
**Phase:** `168_admin_settings_panel` · **Source:** `TODO.md:3` — "…can edit anything in .env and save it to the database."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
One admin-only router that serves the whole catalogue (every setting, its effective value, and where that value came from) and writes single values back to `app_settings` — validated by the real `Settings` validators before anything is stored, masked where the value is a secret.
|
||||
|
||||
## Work
|
||||
1. `app/schemas.py` — append the four models (docstrings in the `UiSettingsIn`/`UiSettingsOut` style, naming the phase and the route):
|
||||
- `AdminSettingItem`: `key: str`, `env_var: str`, `group: str`, `label: str`, `type: Literal["int","float","bool","str","json"]`, `secret: bool`, `restart_required: bool`, `overridable: bool`, `source: Literal["db","env","default"]`, `value: str | None` (the effective value in env-native string form; **`None` when masked**), `masked: bool`, `has_value: bool` (for a masked field: is a non-empty value in effect at all), `has_override: bool` (a row exists), `db_value: str | None` (the stored string when it is a stored non-secret override, else `None`), `default: str` (the code default stringified — for a masked field `""`).
|
||||
- `AdminSettingsOut`: `items: list[AdminSettingItem]`, `totals: {"count": int, "overridden": int}`, `overlay_enabled: bool` (the `BOR_SETTINGS_DB` state — when `false` the panel shows a banner instead of pretending a save took effect).
|
||||
- `AdminSettingIn`: `value: str` — always a string, coercion belongs to pydantic (the same reason D1 stores strings).
|
||||
- `AdminSettingsCleared`: `cleared: int`.
|
||||
Stringify helpers live in the router (`_display(field, value)`), not in the schemas: `bool` → `"true"/"false"`, `list[str]` → `json.dumps(...)`, everything else `str(value)`.
|
||||
2. `app/api/admin_settings.py` (new) — `router = APIRouter(prefix="/admin/settings", tags=["admin-settings"], dependencies=[Depends(require_admin)])` (the `app/api/ui_settings.py` pattern: anonymous AND token users 403 on every route). Module docstring lists the routes and the three invariants (validate-before-store, masked secrets, never write `.env`).
|
||||
- `GET ""` → `AdminSettingsOut`: read the rows once (`SELECT key, value FROM app_settings`), resolve the effective `Settings` through `get_settings()`, then emit one `AdminSettingItem` per catalog entry **in `GROUPS` order, catalog order within the group** (a stable order the E2E pins against). `source`: `db` when an *allowed* row is in effect, `env` when the process env has the `BOR_*` var set (check `os.environ`), else `default`. A masked field emits `masked=True`, `value=None`, `db_value=None` and `has_value=bool(effective)`.
|
||||
- `PUT "/{key}"` (`AdminSettingIn` → `AdminSettingItem`, 200):
|
||||
- unknown key → 404 `f"Unknown setting {key}"`;
|
||||
- `overridable=False` → 422 `f"{env_var} is not settable from the database"` + the reason in the docstring (A3: the engine is built from `database_url`; `chunks.embedding` is fixed at `embedding_dim` by A6);
|
||||
- `secret=True` and the value is empty/whitespace → 422 `f"{env_var} must not be blank — use Reset to fall back to the environment"` (an empty API key or session secret is a self-lockout; `DELETE` is the explicit clear);
|
||||
- **candidate-set validation** (A5/D5): the stored rows for the allowed keys + this key/value, run through the same `app.core.settings_overlay.merge()` path — a `ValidationError` (bad shape OR a cross-field rule like `lexical_support_floor` above a DB-overridden `relevance_threshold`) is a 422 whose detail names the field, and **nothing is written**;
|
||||
- write: `INSERT … ON CONFLICT (key) DO UPDATE SET value = excluded.value, updated_at = now()` (`sqlalchemy.dialects.postgresql.insert`), commit, then `invalidate_settings()` (task 03 makes it live) and return the refreshed item.
|
||||
- `DELETE "/{key}"` → `AdminSettingItem`: delete the row (idempotent — deleting an absent key is a success, not a 404), `invalidate_settings()`, return the item as `source="env"|"default"`.
|
||||
- `DELETE ""` → `AdminSettingsCleared`: `DELETE FROM app_settings`, `invalidate_settings()`, `{"cleared": rowcount}` (the panel's "Clear all overrides").
|
||||
- **Log one line per write, never the value** (AGENTS rule 10, A4): `settings action=set key=… status=200`, `settings action=clear key=…`, `settings action=clear_all count=N` on the module logger `app.api.admin_settings`. GETs log nothing here — the phase-167 `ResponseBudgetMiddleware` already writes the `api method=… bytes=…` line, and the whole catalogue must stay under `BOR_LOG_LARGE_RESPONSE_BYTES`' INFO bar (75 items ≈ 20 KB; verified in task 06 with the audit script).
|
||||
- Never opens `.env`, never shells out, never `eval`s a value — the module contains no file writes (unit-pinned).
|
||||
3. `app/main.py` — `from app.api.admin_settings import router as admin_settings_router` in the router-import block and `app.include_router(admin_settings_router, prefix="/api")` right after the `ui_settings_router` line, with the one-line comment the neighbours carry ("admin-only (router-wide require_admin; a token user stays 403)").
|
||||
4. `tests/integration/test_admin_settings_api.py` (new; the `db`-fixture + skip idiom of `tests/integration/test_ui_settings_api.py`, admin client per that file's convention). Set `BOR_SETTINGS_DB=1` + `invalidate_settings()` in the fixture, and in teardown `DELETE FROM app_settings` + `invalidate_settings()` + `get_settings.cache_clear()` — **no row may survive this module** (the same discipline `test_admin_theme_tab.py:286` applies to `ui_settings`). Pin:
|
||||
- **auth matrix:** anonymous `403`, token user `403` (issue a token the way `test_tokens_api.py` does), admin `200` — on all four routes;
|
||||
- **shape:** `items` length == `len(Settings.model_fields)`, groups in `GROUPS` order, `totals.count`/`overridden` correct, `overlay_enabled` true; on an empty table a field whose `BOR_*` var the test env sets reports `source="env"`, an untouched one `source="default"`, and none reports `"db"`;
|
||||
- **write → live:** `PUT {"value": "0.5"}` for `relevance_threshold` → 200 with `source="db"`, `get_settings().relevance_threshold == 0.5`, `totals.overridden == 1`, and the row present with a non-null `updated_at`;
|
||||
- **visible-through-the-app proof:** `PUT input_placeholder` then `GET /api/config` (no auth) shows the DB value — the DB-over-env rule reaches the public config endpoint through the same overlay;
|
||||
- **errors:** unknown key → 404; `database_url` / `embedding_dim` → 422 naming the env var; `"abc"` for `relevance_threshold` → 422 naming `relevance_threshold` and the table stays empty; a cross-field pair that only breaks in combination (store `relevance_threshold=0.2`, then `lexical_support_floor=0.9`) → the second `PUT` 422s and the first row is unchanged;
|
||||
- **masking:** store `llm_api_key` directly, `GET` reports `masked=True`, `value is None`, `has_value=True`, and the plaintext appears NOWHERE in `response.text` (assert on the raw text, not the parsed model); blank `PUT` for it → 422; `DELETE` clears it;
|
||||
- **reset:** `DELETE /{key}` → the item is `source="env"` (with the env var set in the fixture) and `get_settings()` is back to the env value; `DELETE ""` with 3 rows → `{"cleared": 3}` and `overridden == 0`;
|
||||
- **kill switch:** with `BOR_SETTINGS_DB=0` (monkeypatch + `invalidate_settings()`) `overlay_enabled` is `false` while a `PUT` still stores the row and `get_settings()` still reports the env value — the switch disables the mechanism, not the panel;
|
||||
- **byte budget:** `len(response.content)` for the full `GET` is under 64 KB (the phase-167 INFO bar) — so the catalogue never floods the log.
|
||||
5. `README.md` — under **Configuration reference** (L1071): a short lead paragraph stating the precedence order (`app_settings` row → `BOR_*` environment → code default), that the panel never writes `.env`, that `BOR_SETTINGS_DB=0` disables DB overrides, and that the values marked "restart required" apply on the next `podman compose restart app`.
|
||||
|
||||
## Testing & Quality
|
||||
- Integration: `uv run pytest tests/integration/test_admin_settings_api.py -v` (DB up), then `uv run pytest tests/integration -q` twice with a `app_settings` row left in on purpose ONCE inside the run only — the module's teardown must clean it (proven by a second clean run).
|
||||
- Unit: the module's masking + `_display` helpers (bool/JSON/scalar stringification), the `.env`-never-written and no-shell static pins.
|
||||
- Coverage: **>90%** on `app/api/admin_settings.py` (every 404/422 branch + both `DELETE`s + the masked branch).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `GET /api/admin/settings` returns all 75 settings grouped, each with its effective value and a `db`/`env`/`default` badge; secrets report `masked`.
|
||||
- [ ] `PUT` stores, validates first (nothing written on failure, 422 names the field) and makes the value live through `get_settings()` in the same process.
|
||||
- [ ] `DELETE /{key}` restores the env value; `DELETE ""` clears everything; both log a value-free line (`grep " :: app.api.admin_settings " <logs>`).
|
||||
- [ ] Anonymous and token-user callers get 403 on every route; the byte audit keeps this endpoint under the 256 KB budget and, pinned, under 64 KB.
|
||||
- [ ] `uv run pytest` green, coverage >90%, ruff + pyright clean.
|
||||
@@ -0,0 +1,53 @@
|
||||
# Task 05 — The Settings view: the shell's eighth view, `#view-settings`
|
||||
|
||||
**Phase:** `168_admin_settings_panel` · **Source:** `TODO.md:3` — "I want an admin panel that can edit anything in .env … The change should take effect on clicking 'save' on that page."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
An admin opens **Settings**, sees every setting grouped with the value in effect and where it came from, edits a few, presses **Save settings**, and sees the badges flip to `DB` — while the anonymous visitor gets the sign-in gate and a token user never sees the nav link. Same wiring as the phase-91 Theme view, same table discipline as the phase-156 catalogue tables.
|
||||
|
||||
## Work
|
||||
1. `frontend/index.html`
|
||||
- nav: after `#nav-theme` (L62) add `<a href="/settings.html" class="nav-link" id="nav-settings" hidden>Settings</a>` (ship-hidden, revealed for admin only — the phase-34 one-bar contract; the same one line goes into `document.html` (~L74), `login.html` (~L67) and `shared.html` (~L68) after their Theme links).
|
||||
- view: after `#view-theme` add `<section class="view" id="view-settings" hidden inert aria-label="Settings" tabindex="-1">` following the `#view-tokens` skeleton exactly:
|
||||
- `<div class="container settings-shell">` (PLAN §7.1's 72rem centered column — no new width token),
|
||||
- `.page-head` with `<h1>Settings</h1>` and a `.page-sub` that states the rule in the owner's terms: values saved here are stored in the database and win over the server's environment variables; clearing one hands control back to the environment,
|
||||
- `<section class="sources-gate" id="settings-gate" aria-labelledby="settings-gate-title" hidden>` — the `#tokens-gate` shape, heading "Sign in to change settings", sub-copy naming what is admin-only, `<a class="sources-gate-link" href="/login.html?next=/settings.html">Sign in</a>`,
|
||||
- `<div id="settings-content" hidden>` holding, in order:
|
||||
- `<div class="kb-banner is-error" id="settings-disabled" role="alert" hidden>` — shown when `overlay_enabled` is `false` ("DB overrides are switched off on the server (`BOR_SETTINGS_DB=0`) — saving stores the value but it will not take effect."),
|
||||
- `<span class="settings-status" id="settings-status" role="status" aria-live="polite"></span>` (the ONE status region, the `#tokens-status` precedent) and `<p class="settings-error" id="settings-error" role="alert" hidden></p>` (the `#theme-error` precedent),
|
||||
- the search row: `<label for="settings-filter">Search settings</label>` + `<input id="settings-filter" type="search" autocomplete="off">` (client-side filter, no fetch — the phase-101 D4 contract),
|
||||
- the scroll card: `<div class="table-wrap"><table class="settings-table"><caption class="visually-hidden">Application settings — the value in effect and where it comes from</caption><thead><tr><th scope="col">Setting</th><th scope="col">Value</th><th scope="col">Source</th><th scope="col"><span class="visually-hidden">Actions</span></th></tr></thead></table></div>` — the `tbody` elements are created by `settings.js` (one per group, each opening with a `<tr class="settings-group-row"><th colspan="4" scope="colgroup">Group</th></tr>`), and the `.table-wrap` + sticky-`th` rules at `styles.css:2825/2841` are already the bounded-scroll + sticky-header contract (AGENTS rule 5) — `.visually-hidden` inside the header needs the `.table-wrap` `position: relative` that phase 86 added,
|
||||
- the action row: `<button type="button" class="btn primary" id="settings-save" disabled>Save settings</button>` + `<button type="button" class="btn ghost" id="settings-clear-all">Clear all overrides</button>` + `<span class="settings-dirty" id="settings-dirty" aria-hidden="true"></span>` (an "N unsaved changes" hint, `textContent` only).
|
||||
- No `<script>` tag for the new module: the router lazy-imports it (rule 6 — everything local).
|
||||
2. `frontend/assets/router.js` — three entries in the phase-91 style: `"/settings.html": "settings"` in `VIEW`, `settings: "/settings.html"` in `VIEW_PATH`, `settings: () => import("./settings.js")` in `VIEW_MODULES` (a STATIC specifier so the Containerfile's `esbuild --bundle` of `router.js` inlines it — `settings.js` needs no new esbuild line because no page references it directly; `tests/integration/test_containerfile_assets.py` proves it).
|
||||
3. `app/main.py` — add `"/settings.html"` to the `_shell_routes(...)` tuple (after `"/theme.html"`) with the usual comment.
|
||||
4. `app/core/caching.py` — add `"/settings.html"` to `HTML_PAGES` (L169 block) with the `# phase 168: the admin settings view (shell route)` comment, so it gets the `no-cache` + `?v=` contract (A19).
|
||||
5. `frontend/assets/header.js` — after the Theme reveal block (~L225) add the null-safe `#nav-settings` block, admin-only, with the comment naming the contract it copies (a token user never sees it).
|
||||
6. `frontend/assets/settings.js` (new) — `export async function mount(root)`, the `theme.js` shape (gate first via `fetchIsAdmin()`/`fetchWhoami()` from `header.js`; anonymous shows `#settings-gate` and fetches nothing; then unhide `#settings-content`, load, bind; listen for `bor:view-refresh` to re-load on a re-show):
|
||||
- `GET /api/admin/settings` → render the groups in the server's order (`createElement` + `textContent` only, never HTML strings — PLAN §7 frontend rule). One row per setting:
|
||||
- **Setting cell:** the `label` (`.cell-clip` ellipsis when it truncates, the phase-156 contract with the full text in the control's `aria-label`), the `BOR_*` name in a `<code class="settings-env">`, and — when `restart_required` — a `.settings-flag` with the literal text `restart required` (text, never color alone, B5) plus `title`/`aria-describedby` copy explaining when it applies;
|
||||
- **Value cell:** the control chosen by `type` — `int`/`float` → `<input type="number">` (`step="any"` for float), `bool` → a two-option `<select>` (`true`/`false`), `json` → `<textarea spellcheck="false">`, `str` → `<input type="text">` (a `<textarea rows="2">` for the CSV/path fields whose value exceeds 40 chars), `secret` → `<input type="password" autocomplete="new-password" placeholder="•••••• (stored)">` with `aria-label` naming the env var. Every control gets an `id` (`set-<key>`) and a `<label for>` (visually the Setting cell's label, the house pattern for table-hosted inputs). For a masked field an empty input means "leave unchanged" (never an accidental wipe);
|
||||
- **Source cell:** `<span class="settings-src is-db|is-env|is-default">DB</span>` / `env` / `default` — literal text;
|
||||
- **Actions cell:** a `Reset` button, `disabled` when `!has_override` (nothing to reset) or `!overridable` (with `title` naming the reason).
|
||||
- dirty tracking in module state (`Map<key, value>`): an `input`/`change` marks the row (`.is-dirty` + the `N unsaved changes` hint) and enables `#settings-save`; a row that returns to its served value un-marks.
|
||||
- `#settings-save` → for each dirty row in DOM order `PUT /api/admin/settings/<key>` with `{ value }`; **stop at the first failure** (K4), show its server detail in `#settings-error` (role=alert), keep the remaining edits in the form, re-`GET` so the badges show the truth, keep focus on the offending input. All good → one `#settings-status` line: `Saved 3 settings.` (then `role=status`, not alert) + a re-`GET` so every badge/value is the settled server state. While in flight the button is `disabled` with `aria-busy="true"` (§7.4: every control state defined; a failure path always re-enables).
|
||||
- `Reset` → `DELETE /api/admin/settings/<key>` → status line `BOR_X is back to the environment value.` + re-`GET`. `#settings-clear-all` → the house two-step inline confirm (the `history-confirm-*` classes, focus moves to the confirm button, Escape/`No` cancels) → `DELETE /api/admin/settings` → status line `Cleared N overrides.` + re-`GET`.
|
||||
- `#settings-filter` → case-insensitive substring over the label + `BOR_*` text cells, applied on `input` with no fetch, re-applied after every re-`GET` (the query survives re-shows, phase-77); a group with no visible rows hides its group row; zero matches with a non-empty query → one no-match row `No settings match "<query>"` (`textContent`).
|
||||
- A `restart_required` row that saves successfully adds "— applies on the next restart" to the status line (so the owner is never told "saved" and then sees nothing change).
|
||||
7. `frontend/assets/styles.css` — a `---------- Settings view (phase 168) ----------` block: `.settings-shell` (no new container), `.settings-table { width: 100%; }` with `.git-sources-table`-style column proportions (never a skinny list — PLAN §7.1), `.settings-group-row th` (a `--surface` band, `--ink` text ≥4.5:1), `.settings-env` (mono, `--ink-soft`), `.settings-src` + the three `is-*` variants (solid `--brand-soft` / `--surface` / `--line` backgrounds with the existing ink tokens — no new palette variable, the phase-156 B3 rule; the badge is TEXT, the color is decoration), `.settings-flag` (`--accent-bg`/`--accent-ink`, the state-family pair), `.settings-row.is-dirty` (a `--brand` left border on the row + the `N unsaved changes` hint, so "unsaved" survives grayscale), `.settings-status` (the `.tokens-status` mirror), `.settings-error` (the `.theme-error` mirror), `input`/`select`/`textarea` sizing with `:focus-visible` inherited from the global rule, and ≥44px touch targets for the buttons/inputs at `≤640px` (§7.1 mobile). Nothing animates, so `prefers-reduced-motion` needs no new rule.
|
||||
8. `tests/unit/test_settings_frontend_view.py` (new — the house static-source-assertion style, cf. `tests/unit/test_thinking_scroll.py` / `test_frontend_router.py`): `index.html` contains the `#view-settings` section, the `#settings-gate`, `#settings-content`, `#settings-status role="status"`, `#settings-error role="alert"`, `#settings-save`, `#settings-clear-all`, and `#nav-settings` with `hidden`; `router.js` maps `/settings.html` in all three tables; `main.py`'s shell tuple and `caching.HTML_PAGES` both list `/settings.html`; `header.js` reveals `#nav-settings` and keys it off `admin` (not `signedIn`); `settings.js` contains no `innerHTML =` with a template literal and no `http(s)://` reference (rule 6 + the no-HTML-strings rule); `styles.css` declares `.settings-table` and the three `.settings-src.is-*` variants and does NOT add a `:root` variable (B3); the four pages that carry the Theme link now carry the Settings link too.
|
||||
9. `README.md` — a **Settings (`/settings.html`)** subsection under "Pages at a glance" (after "Access tokens", L205): what it is, the precedence rule, "restart required", and that it is admin-only.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: the new file, plus `uv run pytest tests/unit/test_frontend_router.py tests/unit/test_caching.py -v` (both enumerate the views/pages and must be *extended*, not broken) and `uv run pytest tests/integration/test_containerfile_assets.py -v` (the new asset is reachable through the bundle).
|
||||
- Integration: `uv run pytest tests/integration/test_caching_revalidation.py -v` — `/settings.html` must serve `no-cache` with rewritten `?v=` asset refs.
|
||||
- The E2E for this view is task 06's file; here the view must be reachable by hand (`uv run uvicorn app.main:app --reload` → sign in → Settings).
|
||||
- Coverage: **>90%** on the `app/` diff of this task (the shell route + `HTML_PAGES`), untouched elsewhere.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `/settings.html` deep-links to the Settings view (the shell document, `router.js` picks the view), anonymous sees the gate and no data, a token user never sees the nav link.
|
||||
- [ ] The table lists every group with a bounded scroll region and a sticky header at 72rem and at 375px; long labels clip with the ellipsis and stay reachable through the control's `aria-label`.
|
||||
- [ ] Editing marks the row dirty, **Save settings** reports one status line and every badge/value re-reads from the server; a rejected value shows the server's detail in the `role="alert"` region and keeps the other edits.
|
||||
- [ ] Keyboard-only operation works end to end (filter → edit → Save → Reset → two-step Clear-all), `:focus-visible` visible on every control, and the page introduces no CDN reference and no HTML-string DOM building.
|
||||
- [ ] `uv run pytest` green, coverage >90%, ruff + pyright clean.
|
||||
@@ -0,0 +1,57 @@
|
||||
# Task 06 — `tests/e2e/test_admin_settings_panel.py`: the live-on-Save contract, proven in a browser
|
||||
|
||||
**Phase:** `168_admin_settings_panel` · **Source:** `TODO.md:3` — "…The change should take effect on clicking 'save' on that page."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The phase's dedicated Playwright suite (AGENTS rule 4/9): a real admin signs in, changes a setting through the panel, presses **Save**, and the running app behaves differently — no restart, no `.env` edit — plus the gate, the masking, the rejection path, and the reset. Then the full gates and the one atomic commit.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/test_admin_settings_panel.py` (new) — copy the module-scoped app fixture from `tests/e2e/test_admin_theme_tab.py:205-270` verbatim (mock LLM, `BOR_ENVIRONMENT=e2e`, `BOR_STATIC_DIR`, the E2E admin password/secret, the docs-repo/suggestions/app-name leak guards) and change exactly these lines:
|
||||
- `env["BOR_SETTINGS_DB"] = "1"` — the module opts back in over `tests/e2e/conftest.py`'s global `0` pin (task 02), the `test_pictorial_pdf.py:339` opt-in pattern;
|
||||
- `env["BOR_INPUT_PLACEHOLDER"] = "Env placeholder for panel E2E"` and `env["BOR_FOOTER_TEXT"] = "Env footer for panel E2E"` — a distinctive env base, so "the DB row now wins" is a visible DOM change and not a no-op against the default;
|
||||
- `env["BOR_RELEVANCE_THRESHOLD"] = "0.30"` (the E2E calibration, so a test that saves `0.31` proves the retrieval path read the new number, not a broken gate).
|
||||
Add the module docstring in the house style: what the suite proves, the DB-isolation rule, and the Source line (`TODO.md:3`).
|
||||
2. **DB isolation** (module autouse fixture, the `test_admin_theme_tab.py:286` `TRUNCATE ui_settings` idiom): `TRUNCATE app_settings` before AND after every test, plus `invalidate_settings()`-equivalent on the server side is unnecessary (the app runs in its own subprocess and re-reads on the save's own invalidation). Teardown must leave the shared dev E2E DB with an empty `app_settings` — a leftover row is the cross-suite poison this phase's D6 exists to prevent.
|
||||
3. Tests (names pinned; helpers for `login(page, app_url)` from `tests/e2e/auth_helpers.py` and the `httpx` + cookies pattern those suites already use):
|
||||
- `test_anonymous_sees_gate_and_no_data` — `/settings.html` serves the shell with `#view-settings` visible, `#settings-gate` visible, `#settings-content` hidden, `#nav-settings` hidden; `GET /api/admin/settings` unauthenticated is 403; no setting label or value appears in `page.content()`.
|
||||
- `test_token_user_never_sees_the_panel` — a token user (issue one as admin via the API the way `test_api_tokens.py` does, then drive the token gate with `login_with_token`) sees `#nav-settings` hidden and gets 403 from the API.
|
||||
- `test_admin_sees_groups_and_source_badges` — after `login()`, `#nav-settings` reveals, the view mounts, rows render, `table.settings-table` is inside `.table-wrap` (bounded scroll, sticky `thead` computed style), at least the `Retrieval & honesty` and `LLM (aipi)` group rows appear, `input_placeholder`'s row shows the served env value and an `env` badge, an untouched knob (e.g. `top_n_docs`) shows its code default with a `default` badge, `llm_api_key` renders a password input whose `value` is `""` and whose placeholder is the "stored" hint, and `database_url` / `embedding_dim` render read-only with no Reset and no input.
|
||||
- `test_save_takes_effect_without_restart` — **the TODO's own acceptance test.** Filter to "placeholder", set the value to `DB placeholder — panel saved this`, press **Save settings**, expect: the `#settings-status` line (`Saved 1 setting`), the badge flips to `DB`, then `goto("/")` (a fresh page load, chat view) and expect the composer's `placeholder` attribute to be `DB placeholder — panel saved this` — the same browser, the same server process, no `.env` touched. Assert `footer_text` is still the env value (only one row was saved) and that the DB value survives a reload of `/settings.html`.
|
||||
- `test_saved_value_reaches_a_real_turn` — set `BOR_RELEVANCE_THRESHOLD` from the panel to `0.31`, POST `/api/chat` (the `test_admin_theme_tab.py`/`test_honest_deflection.py` HTTP helper style) and assert the turn's honesty classification follows the new bar for a fixture question whose cosine sits between `0.30` and `0.31` (if the fixture corpus has no such question — verify with `uv run python -m scripts.eval_retrieval "<question>"` — pin the strongest available proxy: `query_log.top_score` is unchanged while the deflected/grounded decision flips when the saved bar moves above/below it, which proves the live read). Then **Reset** it and assert the pre-save classification returns.
|
||||
- `test_invalid_value_is_refused_and_nothing_changes` — set `relevance_threshold` to `abc`, Save → the `#settings-error` (`role="alert"`) region is visible and names `relevance_threshold`, the badge is still `env`, the row keeps its typed value (K4), `GET /api/admin/settings` still reports `source=env`, and the composer placeholder (a second dirty row that was NOT saved because the batch stopped) is unchanged server-side.
|
||||
- `test_cross_field_pair_is_refused` — save `relevance_threshold = 0.2`, then try `lexical_support_floor = 0.9` → the alert names the field and the stored pair never becomes effective (`GET /api/admin/settings` still shows the first saved value only).
|
||||
- `test_secret_never_reaches_the_browser` — set `llm_api_key` to a sentinel (`sk-e2e-secret-…`) via the API (not through the DOM), reload `/settings.html`, and assert the sentinel is absent from `page.content()` while the row's password input shows the "stored" placeholder and `has_override` renders a live Reset button; then set it from the panel's password input, Save, and assert the sentinel still never appears in the served HTML or the response body.
|
||||
- `test_reset_hands_control_back_to_the_env` — after a DB override on `footer_text`, press its Reset → status line, badge back to `env`, and the footer on `/` reads the env value again.
|
||||
- `test_clear_all_overrides` — save three overrides, `#settings-clear-all` → the two-step confirm (Escape cancels and the overrides survive), then confirm → `Cleared 3 overrides`, every badge back to `env`/`default`.
|
||||
- `test_disabled_switch_is_announced` (subprocess-level, its own module fixture or an `httpx`-only variant): a second app subprocess with `BOR_SETTINGS_DB=0` → the panel shows `#settings-disabled` (`role="alert"`) and a save still stores without changing the app. If a second app fixture is too heavy for the file, pin the `overlay_enabled=false` render through `page.route` interception of `GET /api/admin/settings` instead — state which was done in the commit body.
|
||||
- `test_view_layout_and_accessibility` — no horizontal page overflow at 375px and at 1440px (§7.1: the table is ≥80% of the container, not a skinny list), keyboard tab order reaches filter → first input → Save → Reset (a `page.keyboard` walk asserting focus lands on the real controls), the `.settings-flag` text `restart required` present on `db_pool_size`'s row, and zero external requests during the whole test (`page.on("request")` — rule 6).
|
||||
4. **Regression set, all in isolation** (`--no-cov`): `test_admin_theme_tab.py` (the Theme view's gate/save contract that this view copies), `test_api_tokens.py`, `test_configurable_brand.py` (`/api/config`'s strings — now DB-over-env-capable), `test_global_tuning.py`, `test_admin_auth.py`, `test_cache_busting.py`, `test_wide_desktop_column.py` (the 72rem container contract, new view included).
|
||||
5. **Full gates** (AGENTS rule 9, all green before the commit):
|
||||
```bash
|
||||
uv run pytest
|
||||
uv run pytest --cov=app --cov-report=term-missing # >90% on app/
|
||||
uv run pytest tests/e2e/test_admin_settings_panel.py -v --no-cov
|
||||
uv run ruff check . && uv run pyright
|
||||
```
|
||||
Plus the operational proof of AGENTS rule 10: with the app running, save a setting, then `grep ' :: app.api.admin_settings ' <app log>` shows the key-only line and `grep ' :: api ' <app log>` shows the `GET /api/admin/settings` byte line; `uv run python -m scripts.api_payload_audit` lists the endpoint under the 256 KB budget.
|
||||
6. **Docs & close-out:** confirm `README.md` carries both additions from tasks 04/05 (the Settings page section + the Configuration-reference precedence note); move the phase directory:
|
||||
then move the phase directory and commit (AGENTS rule 8, `--no-gpg-sign`):
|
||||
```bash
|
||||
git mv .agents/phases/todo/168_admin_settings_panel .agents/phases/complete/168_admin_settings_panel
|
||||
```
|
||||
```bash
|
||||
git add app/ alembic/ frontend/ tests/ .env.example README.md .agents/phases/ && git commit --no-gpg-sign -m "feat(settings): admin settings panel with DB-over-env overrides applied live on save"
|
||||
```
|
||||
|
||||
## Testing & Quality
|
||||
- The suite passes **in isolation** (`-v --no-cov`) and the app it drives writes no `.env` change (`git status --porcelain .env` empty after the run — `.env` is gitignored, so assert by `stat` mtime instead if the file is untracked).
|
||||
- Unit + integration already exist from tasks 02/03/04 — this file adds the browser contract only; if it needs a behavior the API lacks, fix the API in this task and say so in the commit body.
|
||||
- Coverage: **>90%** on `app/` after everything (`--cov` runs exclude E2E by the repo's config; the gate is the unit+integration number).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `test_save_takes_effect_without_restart` is green: a value saved through the panel shows up in the running app in the same browser session with no restart.
|
||||
- [ ] Gate, masking, invalid-value, cross-field, reset, clear-all and mobile/keyboard coverage all green in isolation.
|
||||
- [ ] `app_settings` is empty after the suite (`SELECT count(*) FROM app_settings` → 0) and the rest of the E2E suite still passes with that pin in place.
|
||||
- [ ] `uv run pytest` green, `app/` coverage >90%, ruff + pyright clean, the byte audit clean.
|
||||
- [ ] Phase directory moved to `.agents/phases/complete/168_admin_settings_panel/` and one atomic `--no-gpg-sign` Conventional Commit exists (verify with `git log -1 --stat`).
|
||||
Reference in New Issue
Block a user