All gates re-verified green. Report: - **Verified (all 4 tasks already complete; this was the final pass):** `effective_settings` resolves the 3 brand strings panel-row → tab-row → env (`applied_override_keys`, presence-not-value); `PUT /api/ui-settings` ignores string body fields; Theme-tab form is colors-only (17 pickers, no `#theme-app-name`/`#theme-placeholder`/`#theme-footer`); the ONE shared `applyBrandFromConfig` re-applies title/`.brand-text`/footer/placeholder after a panel save-reset and a tab save-reset; boot path byte-identical (single `/api/config`). - **Completion criteria:** - strings order panel→tab→env, both branches — ✅ `tests/unit/test_theming.py::test_effective_app_name_prefers_settings_panel_row_over_ui_settings_row`, `..._fall_back_to_ui_settings_row_without_panel_row`, `..._per_field_panel_then_tab_then_env`, `..._ignore_panel_rows_when_the_overlay_is_off` - PUT ignores string fields + no 3 inputs in the tab — ✅ `tests/integration/test_ui_settings_api.py::test_put_ignores_string_fields_in_body` (+2) and the E2E `test_theme_tab_dom_is_colors_only` - live apply without reload + incident regression — ✅ `tests/e2e/test_app_name_settings_fix.py` tests 1–4 - byte-identical boot, no double-apply — ✅ `tests/unit/test_brand_live.py` markers + E2E "exactly one /api/config" - suite green, coverage >90%, ruff+pyright clean — ✅ - no behavior change in prior phases — ✅ (palette, phase-168 panel, env fallback suites all green) - atomic commit — **left to the harness** (pipeline rule overrides per-phase committing; no git write run) - **Gates:** `uv run pytest --cov=app --cov-report=term-missing` → **4630 passed, TOTAL 6941 16 99%**, exit 0 · `uv run pytest tests/e2e/test_app_name_settings_fix.py -v --no-cov` → **6 passed** in isolation · regression E2E (isolated, one process each, all exit 0): `test_smoke` `test_configurable_brand` `test_admin_theme_tab` `test_theme_save_and_coverage` `test_theme_semantic_completion` `test_themed_favicon` `test_dark_tech_theme` `test_admin_settings_panel` `test_admin_settings_layout` `test_settings_mobile_value_width` · `uv run ruff check .` → All checks passed · `uv run pyright` → 0 errors. - **Documentation:** README — fixed "saving one repaints *every* open page" (only the open document repaints; other pages on next load) and a broken paragraph wrap in *Customizing the look*; AGENTS.md rule 16 — `test_chunks_hnsw_recall.py` now described accurately (index paths forced with `enable_seqscan`/`indexonlyscan`/`bitmapscan` off, unforced plan asserted only on a pristine relation); `tests/unit/test_theme_frontend.py` — two stale section headers ("20 form fields"/"20 inputs" → 17 color). No AGENTS.md settings-overlay/resolution-order clause exists, so per the phase none was added. Full env-var cross-check (`BOR_*` in `app/config.py` vs README) → 0 gaps; 0 missing file references. - **Notable:** the working tree also carries this phase's flake hardening of the phase-188 `tests/integration/test_chunks_hnsw_recall.py` (verified legit and green). Live infra untouched; shared DB left clean (0 `app_settings`, 0 `ui_settings`). **Flagged, not changed:** `settings_overlay.applied_override_keys` lets a `ProgrammingError` (missing `app_settings`) propagate from public `GET /api/config`, where `load_overrides` deliberately fails soft — only reachable on an un-migrated (0027) DB, outside this phase's pinned A1 semantics, so left for the owner to rule on rather than silently deviating. - **Next pending phase:** `191_source_name_labels_tags`.
321 lines
13 KiB
Python
321 lines
13 KiB
Python
"""The DB-over-env settings overlay (phase 168, task 02).
|
|
|
|
``TODO.md`` L3: "…save it to the database. The database value should
|
|
override the env var."
|
|
|
|
:func:`resolve_settings` layers the ``app_settings`` rows (phase 168
|
|
task 01: bare ``Settings`` field name → env-native string) over the
|
|
environment and hands every existing consumer one valid ``Settings`` —
|
|
through :func:`app.config.get_settings` (which delegates here, lazily,
|
|
under its unchanged ``lru_cache``). No consumer changes, no restart
|
|
(D2). The kill switch is env-only: ``BOR_SETTINGS_DB=0`` (read
|
|
straight from ``os.environ`` per call — it is not a ``Settings`` field,
|
|
so the escape hatch cannot disable itself).
|
|
|
|
Fail loud on write, fail soft on read (LOCKED A5, D5): a write
|
|
validates the FULL candidate set (task 04); 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.
|
|
|
|
The overlay's engine is a lazily-created ``NullPool`` engine built from
|
|
``Settings().database_url`` (never the app's real pool): importing
|
|
``app.db`` from here would be a cycle (``app.db`` → ``app.config`` at
|
|
import time), and the pool whose size this table can change must not
|
|
be the one that reads the table. ``NullPool`` = one connection per
|
|
resolution, none held.
|
|
|
|
A missing ``app_settings`` table (fresh deployment, migrations not
|
|
applied) means "no overrides" + one DEBUG line per process (K3 — a
|
|
pre-migration deployment is a normal state); any other DB failure is
|
|
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 :func:`reset_state_for_tests`.
|
|
|
|
Phase 190 (task 02) adds the one question another resolver asks this
|
|
table: :func:`applied_override_keys` — "which keys does the Settings
|
|
panel own right now?" (the brand-string ownership order in
|
|
:func:`app.core.theming.effective_settings`). It reads on the CALLER's
|
|
session, so answering it costs no second engine and never a stale view.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import logging
|
|
import os
|
|
from collections.abc import Callable, Mapping
|
|
from typing import Any, get_origin
|
|
|
|
from pydantic import ValidationError
|
|
from sqlalchemy import create_engine, text
|
|
from sqlalchemy.engine import Engine
|
|
from sqlalchemy.exc import ProgrammingError, SQLAlchemyError
|
|
from sqlalchemy.orm import Session
|
|
from sqlalchemy.pool import NullPool
|
|
|
|
from app.config import Settings
|
|
from app.core.settings_catalog import CATALOG
|
|
|
|
logger = logging.getLogger("app.settings")
|
|
|
|
#: ``BOR_SETTINGS_DB`` — env-only, read per call from ``os.environ`` so
|
|
#: tests can flip it; unset/``1`` = on, the house bool off-spellings
|
|
#: are off. It is NOT a ``Settings`` field, so the escape hatch cannot
|
|
#: disable itself from the database (D6).
|
|
_DISABLED_SPELLOS = frozenset({"0", "false", "no", "off"})
|
|
|
|
_engine: Engine | None = None
|
|
_warned_db_failure = False
|
|
_debugged_missing_table = False
|
|
_invalidators: list[Callable[[], None]] = []
|
|
|
|
#: The one ``list[str]`` field (derived — a new list field would join
|
|
#: automatically); every other field is a scalar or CSV string (K2).
|
|
LIST_FIELDS: frozenset[str] = frozenset(
|
|
name
|
|
for name, field in Settings.model_fields.items()
|
|
if get_origin(field.annotation) in (list, tuple)
|
|
)
|
|
|
|
|
|
def enabled() -> bool:
|
|
"""The ``BOR_SETTINGS_DB`` kill switch — read per call (D6)."""
|
|
value = os.environ.get("BOR_SETTINGS_DB", "1")
|
|
return value.strip().lower() not in _DISABLED_SPELLOS
|
|
|
|
|
|
def _get_engine() -> Engine:
|
|
"""The overlay's lazily-created ``NullPool`` engine (see module
|
|
docstring — never the app's real pool: import cycle + the pool
|
|
whose size this table can change must not read the table)."""
|
|
global _engine
|
|
if _engine is None:
|
|
_engine = create_engine(
|
|
Settings().database_url, poolclass=NullPool, pool_pre_ping=True
|
|
)
|
|
return _engine
|
|
|
|
|
|
def load_overrides() -> dict[str, str]:
|
|
"""``SELECT key, value FROM app_settings ORDER BY key`` — fail-soft.
|
|
|
|
A missing table (pre-migration deployment) is one DEBUG line per
|
|
process (K3); any other DB failure is one WARNING per process
|
|
naming the exception class. Both return ``{}`` — the env values
|
|
stay in effect, and the NEXT resolution retries (a boot-time miss
|
|
is never a permanently cached env-only view — the ``create_app``
|
|
prime, task 02 step 4).
|
|
"""
|
|
global _warned_db_failure, _debugged_missing_table
|
|
try:
|
|
with _get_engine().connect() as conn:
|
|
rows = conn.execute(text("SELECT key, value FROM app_settings ORDER BY key"))
|
|
return {key: value for key, value in rows}
|
|
except ProgrammingError:
|
|
# Undefined table (psycopg ``UndefinedTable`` → SQLAlchemy
|
|
# ProgrammingError): the pre-migration deployment — DEBUG (K3).
|
|
if not _debugged_missing_table:
|
|
_debugged_missing_table = True
|
|
logger.debug(
|
|
"app_settings table missing — no DB overrides in effect "
|
|
"(pre-migration deployment, phase 168 K3)"
|
|
)
|
|
return {}
|
|
except (SQLAlchemyError, OSError) as exc:
|
|
if not _warned_db_failure:
|
|
_warned_db_failure = True
|
|
logger.warning(
|
|
"settings overrides unavailable — env values in effect (%s)",
|
|
type(exc).__name__,
|
|
)
|
|
return {}
|
|
|
|
|
|
def applied_override_keys(session: Session) -> frozenset[str]:
|
|
"""The ``app_settings`` keys IN EFFECT right now — phase 190 task 02.
|
|
|
|
:func:`app.core.theming.effective_settings` needs exactly this to
|
|
answer "does the admin's Settings panel own this brand string?" (A1:
|
|
the panel row beats the Theme-tab row, and only a REAL row counts —
|
|
comparing values would mis-read a panel row that happens to equal
|
|
the env value, the exact shadow the phase-190 incident is).
|
|
|
|
Read on the CALLER's session — one ``SELECT`` on a connection the
|
|
caller already holds, transactionally consistent with the
|
|
``ui_settings`` read beside it (a cross-request memo of the key set
|
|
would go stale the moment a row is written outside the panel and
|
|
could re-shadow a just-saved value). The resolver calls this ONCE
|
|
per resolution and tests the three string fields for membership, so
|
|
a resolution costs no second query per field.
|
|
|
|
The kill switch (``BOR_SETTINGS_DB``, D6) short-circuits before any
|
|
read — with the overlay off no row is in effect, and the Theme-tab
|
|
row keeps its phase-91 fallback. A non-overridable key (LOCKED A3)
|
|
and an orphan row (a field a later release removed) are never
|
|
"in effect": :func:`merge` drops them, so they must not win here
|
|
either.
|
|
"""
|
|
if not enabled():
|
|
return frozenset()
|
|
keys = session.execute(text("SELECT key FROM app_settings")).scalars().all()
|
|
return frozenset(
|
|
key for key in keys if key in Settings.model_fields and CATALOG[key].overridable
|
|
)
|
|
|
|
|
|
def _coerce(field_name: str, raw: str) -> Any:
|
|
"""Coerce one env-native string row value for the candidate set.
|
|
|
|
The one ``list[str]`` field (``suggestions``) is ``json.loads``-ed
|
|
— it recovers exactly what the env spelling (a JSON array, K2)
|
|
wrote. Every other field is the raw string: pydantic-settings
|
|
lax-coerces ``str`` → ``int`` / ``float`` / ``bool``, and init
|
|
kwargs rank ABOVE env in pydantic-settings — precisely "the
|
|
database value overrides the env var" (D2). A malformed
|
|
``suggestions`` JSON raises :class:`ValueError`, which
|
|
:func:`merge` turns into a per-key skip (LOCKED A5: one bad row
|
|
costs one setting, never the whole table). The return type is
|
|
``Any`` on purpose — the value is whatever pydantic will
|
|
lax-coerce, decided in exactly one place (the ``Settings``
|
|
validators).
|
|
"""
|
|
if field_name in LIST_FIELDS:
|
|
try:
|
|
return json.loads(raw)
|
|
except json.JSONDecodeError as exc:
|
|
raise ValueError(f"not a JSON array ({exc.msg})") from exc
|
|
return raw
|
|
|
|
|
|
def merge(
|
|
rows: Mapping[str, str],
|
|
) -> tuple[Settings, list[tuple[str, str]]]:
|
|
"""Layer ``rows`` over the env base (``Settings()``) and validate.
|
|
|
|
Returns ``(settings, skipped)`` — ``skipped`` is the
|
|
``(key, reason)`` pairs the read-side fail-soft (LOCKED A5)
|
|
dropped so one unusable row costs one setting, not the whole
|
|
table. Dropped keys: an orphan row (not a ``Settings`` field —
|
|
the setting was removed in a later release), a LOCKED A3 key
|
|
(a hand-inserted ``database_url`` / ``embedding_dim`` row must
|
|
never hijack the engine / the fixed 768-dim table), a malformed
|
|
``suggestions`` JSON, and any value the real ``Settings``
|
|
validators reject (retried per field — ``error["loc"][0]``). Each
|
|
dropped key is logged once at ERROR
|
|
(``settings override ignored key=… reason=…``).
|
|
"""
|
|
kwargs: dict[str, Any] = {}
|
|
skipped: list[tuple[str, str]] = []
|
|
for key, raw in rows.items():
|
|
if key not in Settings.model_fields:
|
|
skipped.append((key, "unknown setting (orphan row from a removed field)"))
|
|
continue
|
|
if not CATALOG[key].overridable:
|
|
skipped.append((key, "not DB-overridable (LOCKED A3 — read-only)"))
|
|
continue
|
|
try:
|
|
kwargs[key] = _coerce(key, raw)
|
|
except ValueError as exc:
|
|
skipped.append((key, str(exc)))
|
|
while True:
|
|
try:
|
|
settings = Settings(**kwargs)
|
|
break
|
|
except ValidationError as exc:
|
|
err = exc.errors()[0]
|
|
bad = str(err["loc"][0])
|
|
if bad in kwargs:
|
|
# The offending row is an override — drop it and retry
|
|
# (LOCKED A5: one bad row costs one setting).
|
|
kwargs.pop(bad)
|
|
skipped.append((bad, err["msg"]))
|
|
continue
|
|
# The validator blames a field with NO override — its value
|
|
# comes from env/.env and collides with a cross-field
|
|
# override (e.g. the env lexical_support_floor above a
|
|
# DB-overridden relevance_threshold). The env base alone is
|
|
# always valid (the app booted with it pre-phase), so try
|
|
# dropping the override(s) until the candidate set is valid
|
|
# again — one unresolvable collision costs the colliding
|
|
# row, the rest of the table stands.
|
|
resolved = False
|
|
for candidate in list(kwargs):
|
|
trial = dict(kwargs)
|
|
trial.pop(candidate)
|
|
try:
|
|
settings = Settings(**trial)
|
|
except ValidationError:
|
|
continue
|
|
skipped.append(
|
|
(
|
|
candidate,
|
|
f"cross-field collision with env/base {bad}: {err['msg']} "
|
|
"— override skipped, env value for the other field stands",
|
|
)
|
|
)
|
|
kwargs = trial
|
|
resolved = True
|
|
break
|
|
if not resolved:
|
|
# Nothing to drop (no overrides at all) or no single drop
|
|
# heals the set: the env base alone would boot (pre-phase
|
|
# proof), so return it — overrides skipped, boot intact.
|
|
skipped.append(
|
|
(
|
|
bad,
|
|
f"unresolvable cross-field collision: {err['msg']} — "
|
|
"all overrides skipped, env values in effect",
|
|
)
|
|
)
|
|
settings = Settings()
|
|
break
|
|
for key, reason in skipped:
|
|
logger.error("settings override ignored key=%s reason=%s", key, reason)
|
|
return settings, skipped
|
|
|
|
|
|
def resolve_settings() -> Settings:
|
|
"""The whole overlay: env base + kill switch + DB rows.
|
|
|
|
Never calls :func:`app.config.get_settings` (recursion guard — that
|
|
resolver delegates HERE): ``Settings()`` is the env-only entry
|
|
point, and the kill switch short-circuits before any DB access.
|
|
"""
|
|
if not enabled():
|
|
return Settings()
|
|
settings, _skipped = merge(load_overrides())
|
|
return settings
|
|
|
|
|
|
def register_invalidator(fn: Callable[[], None]) -> None:
|
|
"""Register a runtime reset hook (task 03: the chat LLM client +
|
|
semaphore). Hooks run on every :func:`app.config.invalidate_settings`
|
|
— after the cache clear — so the next resolution rebuilds the
|
|
singletons from the fresh settings."""
|
|
_invalidators.append(fn)
|
|
|
|
|
|
def run_invalidators() -> None:
|
|
"""Run the registered reset hooks, in order. A hook that raises is
|
|
logged, never propagated (a failed reset must not undo a
|
|
successful save — LOCKED A5's "the save stands")."""
|
|
for fn in list(_invalidators):
|
|
try:
|
|
fn()
|
|
except Exception:
|
|
logger.exception(
|
|
"settings invalidator failed — the save stands, this "
|
|
"runtime reset did not run"
|
|
)
|
|
|
|
|
|
def reset_state_for_tests() -> None:
|
|
"""Clear the module engine + the once-per-process flags + the hook
|
|
list (the unit suites flip the kill switch and stub the engine)."""
|
|
global _engine, _warned_db_failure, _debugged_missing_table
|
|
_engine = None
|
|
_warned_db_failure = False
|
|
_debugged_missing_table = False
|
|
_invalidators.clear()
|