phase: 93_theme_semantic_completion
All verification is complete. Final report: **Phase 93 — Theme semantic completion: FINAL VERIFICATION PASS — ALL GREEN** - Verified full implementation in tree: migration `0016` (8 nullable semantic columns, applied at head), 17-var `BUILTIN_COLORS`/`COLOR_FIELDS`/`effective_settings`, API validation, `#view-theme` State-colors fieldset (17 pickers), `theme.js` FIELDS/PAIRS (5→8), `.page-head` surface panel (6 shell views + doc-edit + shared.html; login card / document sticky header audited as already-surfaced), mock_llm `content: None` fix - Fixed 2 pre-existing defects (both fail identically on baseline `d4f38ad`, proven via worktree A/B): `test_nav_rename_sources` — expected nav tail missing the phase-91 "Theme" link; `test_stale_ui_copy` — now truncates `saved_chats` before/after (house `test_suggestion_chips` pattern) so the seed-chip contract is deterministic on the shared dev DB (owner's 22 saved chats triggered phase-80 last-3-questions) - Tests: `uv run pytest --cov=app --cov-report=term-missing` → **1868 passed, app/ 99%** (>90% ✓); `uv run ruff check .` → clean; `uv run pyright` → **0 errors** - E2E: dedicated `uv run pytest tests/e2e/test_theme_semantic_completion.py -v --no-cov` → **8/8 in isolation** (all-gray 17-color theme: zero residual color on saved-result/Stale/Revoked/Local/tool-call elements, text labels intact, gray heads non-transparent, pre-paint tag, Reset → byte-identical no-tag); 15 theme/header/nav/responsive suites green in isolation; full 85-file combined run: only the 2 fixed pre-existing failures + 1 combined-run artifact (`test_sync_upload_progress`, green in isolation) - Completion criteria: (1) monochrome E2E ✓ (2) default byte-identical, no `#bor-theme` tag ✓ (3) all page heads on solid surface ✓ (4) suite/coverage/lint/E2E green ✓ (5) phases 01–92 no behavior change ✓ (6) commit left to harness per protocol - Notable: cleaned stray uvicorn leftovers from prior implementation pass (owner's `--reload` dev server untouched); no deviations from the phase design - Next pending phase: `94_ls_tree_drilldown`
This commit is contained in:
+87
-25
@@ -7,12 +7,12 @@ authoring guide before task 03 deleted it) and the DB-over-env /
|
||||
DB-over-built-in resolver shared by ``/api/ui-settings`` and
|
||||
``/api/config``:
|
||||
|
||||
* ``BUILTIN_COLORS`` — the DRIFT GUARD: the 9 built-ins must equal the
|
||||
values parsed straight out of ``frontend/assets/styles.css``'s
|
||||
``:root`` block, so the Python palette and the stylesheet can never
|
||||
silently diverge;
|
||||
* ``BUILTIN_COLORS`` — the DRIFT GUARD: the 17 built-ins (9 identity +
|
||||
8 semantic state, phase 93) must equal the values parsed straight
|
||||
out of ``frontend/assets/styles.css``'s ``:root`` block, so the
|
||||
Python palette and the stylesheet can never silently diverge;
|
||||
* ``theme_style_tag`` — the byte-identical contract (all built-in →
|
||||
``""``) and the exact tag shape (all 9 variables, ``COLOR_FIELDS``
|
||||
``""``) and the exact tag shape (all 17 variables, ``COLOR_FIELDS``
|
||||
order, lowercased hex);
|
||||
* ``effective_settings`` — missing row → env strings + built-ins; a DB
|
||||
row's set columns win; an empty-string DB string falls back to env
|
||||
@@ -67,13 +67,16 @@ def _root_declarations() -> dict[str, str]:
|
||||
def test_builtin_colors_match_styles_css_root() -> None:
|
||||
"""The drift guard: every built-in equals the stylesheet's ``:root``
|
||||
value for the same variable (and ``BUILTIN_COLORS`` names exactly
|
||||
the 9 identity variables — no more, no fewer)."""
|
||||
the 17 palette variables — the 9 identity + the 8 semantic state,
|
||||
no more, no fewer; phase 93, task 01)."""
|
||||
decls = _root_declarations()
|
||||
builtin_names = set(theming.BUILTIN_COLORS)
|
||||
assert builtin_names == {
|
||||
"bg", "surface", "ink", "ink_soft", "line", "grid_line",
|
||||
"brand", "brand_soft", "brand_ink",
|
||||
}, f"BUILTIN_COLORS must name exactly the 9 identity variables, got {sorted(builtin_names)}"
|
||||
"ok_bg", "ok_ink", "err_bg", "err_ink", "err_line",
|
||||
"accent_bg", "accent_ink", "accent_line",
|
||||
}, f"BUILTIN_COLORS must name exactly the 17 palette variables, got {sorted(builtin_names)}"
|
||||
for name, value in theming.BUILTIN_COLORS.items():
|
||||
css_name = f"--{name.replace('_', '-')}"
|
||||
assert css_name in decls, f"styles.css :root is missing {css_name}"
|
||||
@@ -83,16 +86,19 @@ def test_builtin_colors_match_styles_css_root() -> None:
|
||||
)
|
||||
|
||||
|
||||
def test_color_fields_are_the_nine_keys_in_readme_order() -> None:
|
||||
"""``COLOR_FIELDS`` is the 9 keys in the themes-README order — the
|
||||
order the resolver, the API, and the tag renderer all rely on.
|
||||
(Phase 92, task 01: ``grid_line`` is the 9th identity variable,
|
||||
slotting in between ``line`` and ``brand`` — structural colors
|
||||
first, brand last.)"""
|
||||
def test_color_fields_are_the_17_keys_in_order() -> None:
|
||||
"""``COLOR_FIELDS`` is the 17 keys — the 9 identity in the
|
||||
themes-README order (phase 92: ``grid_line`` between ``line`` and
|
||||
``brand``), then the 8 semantic state variables (phase 93: ok,
|
||||
err, accent — identity, brand, then state) — the order the
|
||||
resolver, the API, and the tag renderer all rely on."""
|
||||
assert theming.COLOR_FIELDS == (
|
||||
"bg", "surface", "ink", "ink_soft",
|
||||
"line", "grid_line", "brand", "brand_soft", "brand_ink",
|
||||
"ok_bg", "ok_ink", "err_bg", "err_ink", "err_line",
|
||||
"accent_bg", "accent_ink", "accent_line",
|
||||
)
|
||||
assert len(theming.COLOR_FIELDS) == 17
|
||||
assert theming.STRING_FIELDS == ("app_name", "input_placeholder", "footer_text")
|
||||
|
||||
|
||||
@@ -113,12 +119,14 @@ def _env_settings() -> Settings:
|
||||
|
||||
def test_effective_missing_row_is_env_strings_plus_builtins(db: Session) -> None:
|
||||
"""A missing row (GET creates nothing) means "defaults": the env
|
||||
strings + the built-in palette, all 12 keys."""
|
||||
strings + the built-in palette, all 20 values (3 strings + 17
|
||||
colors — phase 93, task 01)."""
|
||||
_start_row_missing(db)
|
||||
row = db.execute(select(UiSettings).where(UiSettings.id == 1)).scalars().first()
|
||||
assert row is None, "the test starts from a row-missing state"
|
||||
effective = theming.effective_settings(db, _env_settings())
|
||||
assert set(effective) == set(theming.STRING_FIELDS) | set(theming.COLOR_FIELDS)
|
||||
assert len(effective) == 20 # 3 strings + 17 colors (9 identity + 8 semantic)
|
||||
assert effective["app_name"] == "Env Name"
|
||||
assert effective["input_placeholder"] == "Env placeholder…"
|
||||
assert effective["footer_text"] == "Env footer"
|
||||
@@ -169,7 +177,7 @@ def test_effective_empty_string_db_string_falls_back_to_env(db: Session) -> None
|
||||
def test_effective_without_explicit_settings_uses_get_settings(db: Session) -> None:
|
||||
"""``settings=None`` (the design's call shape) resolves the env
|
||||
fallback from the cached :func:`app.config.get_settings` — the
|
||||
values it reports must be real ``str``s for all 12 keys."""
|
||||
values it reports must be real ``str``s for all 20 values."""
|
||||
from app.config import get_settings
|
||||
|
||||
_start_row_missing(db)
|
||||
@@ -196,27 +204,40 @@ def test_theme_style_tag_all_builtins_is_empty_string() -> None:
|
||||
assert theming.theme_style_tag(colors) != ""
|
||||
|
||||
|
||||
def test_theme_style_tag_one_changed_carries_all_nine_in_order() -> None:
|
||||
"""A single non-built-in color still emits ALL 9 variables, in
|
||||
``COLOR_FIELDS`` order, with the exact tag shape (no whitespace).
|
||||
Phase 92 (task 01): the tag carries ``--grid-line:#4a2626;`` between
|
||||
``--line`` and ``--brand`` (the 9th identity variable — the
|
||||
background grid texture)."""
|
||||
def test_theme_style_tag_one_changed_semantic_carries_all_17_in_order() -> None:
|
||||
"""A single NON-BUILT-IN SEMANTIC variable (phase 93, task 01) still
|
||||
emits ALL 17 declarations, in ``COLOR_FIELDS`` order, with the exact
|
||||
tag shape (no whitespace): the 9 identity variables keep their
|
||||
built-ins, the 8 semantic variables carry ``--ok-ink:#444444;`` (the
|
||||
change) plus the 7 other semantic built-ins — and the CSP hash
|
||||
matches the tag's content (the runtime exemption contract)."""
|
||||
colors = dict(theming.BUILTIN_COLORS)
|
||||
colors["brand"] = "#818cf8"
|
||||
colors["ok_ink"] = "#444444" # one non-default SEMANTIC var
|
||||
tag = theming.theme_style_tag(colors)
|
||||
assert tag == (
|
||||
'<style id="bor-theme">:root{'
|
||||
"--bg:#0f0a0a;--surface:#1a0f0f;--ink:#f0e6e6;--ink-soft:#b8a8a8;"
|
||||
"--line:#2d1a1a;--grid-line:#4a2626;--brand:#818cf8;"
|
||||
"--line:#2d1a1a;--grid-line:#4a2626;--brand:#f43f5e;"
|
||||
"--brand-soft:#2d0a0a;--brand-ink:#fca5a5;"
|
||||
"--ok-bg:#10241b;--ok-ink:#444444;--err-bg:#2d0a0a;--err-ink:#fca5a5;"
|
||||
"--err-line:#ef4444;--accent-bg:#2b2110;--accent-ink:#fbbf24;"
|
||||
"--accent-line:#f59e0b;"
|
||||
"}</style>"
|
||||
)
|
||||
# The changed value lands under the dashed CSS name…
|
||||
assert "--brand:#818cf8;" in tag
|
||||
assert "--ok-ink:#444444;" in tag
|
||||
# …and the underscored field (ink_soft) renders as --ink-soft.
|
||||
assert "--ink-soft:#b8a8a8;" in tag
|
||||
assert "--ink_soft" not in tag
|
||||
# Exactly 17 declarations, COLOR_FIELDS order.
|
||||
names = re.findall(r"--([a-z-]+):", tag)
|
||||
assert names == [k.replace("_", "-") for k in theming.COLOR_FIELDS]
|
||||
# The hash is computed from the tag's EXACT content (CSP3 §13.4).
|
||||
content = tag.split(">", 1)[1].rsplit("</style>", 1)[0]
|
||||
expected = "sha256-" + base64.b64encode(
|
||||
hashlib.sha256(content.encode("utf-8")).digest()
|
||||
).decode("ascii")
|
||||
assert theming.theme_csp_hash(tag) == expected
|
||||
|
||||
|
||||
def test_theme_style_tag_multiple_changed() -> None:
|
||||
@@ -226,11 +247,13 @@ def test_theme_style_tag_multiple_changed() -> None:
|
||||
colors = dict(theming.BUILTIN_COLORS)
|
||||
colors["bg"] = "#0a0e1a"
|
||||
colors["brand_ink"] = "#c7d2fe"
|
||||
colors["accent_ink"] = "#cccccc" # a semantic var joins the mix too
|
||||
tag = theming.theme_style_tag(colors)
|
||||
assert tag.startswith('<style id="bor-theme">:root{--bg:#0a0e1a;')
|
||||
assert "--brand-ink:#c7d2fe;" in tag
|
||||
assert "--accent-ink:#cccccc;" in tag
|
||||
assert tag.endswith("}</style>")
|
||||
# The order of the 9 dashed names is the COLOR_FIELDS order.
|
||||
# The order of the 17 dashed names is the COLOR_FIELDS order.
|
||||
names = re.findall(r"--([a-z-]+):", tag)
|
||||
assert names == [k.replace("_", "-") for k in theming.COLOR_FIELDS]
|
||||
|
||||
@@ -327,3 +350,42 @@ def test_theme_csp_hash_changes_with_the_palette() -> None:
|
||||
assert first != second
|
||||
assert first.startswith("sha256-")
|
||||
assert second.startswith("sha256-")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 93 (task 02): the docstring ↔ theme.js PAIRS mirror — the
|
||||
# authoritative eight-pair table and the client-side warning list must
|
||||
# NEVER diverge (the docstring names the mirror; this test pins it).
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _theme_js_pairs() -> list[tuple[str, str]]:
|
||||
"""The (foreground, background) entries of ``theme.js``'s ``PAIRS``
|
||||
array, in order (frontend source read as text — the house
|
||||
pattern)."""
|
||||
js = (REPO_ROOT / "frontend" / "assets" / "theme.js").read_text(encoding="utf-8")
|
||||
start = js.index("const PAIRS = [")
|
||||
end = js.index("];", start)
|
||||
return re.findall(r'\[\s*"([a-z_]+)"\s*,\s*"([a-z_]+)"\s*\]', js[start:end])
|
||||
|
||||
|
||||
def test_docstring_pair_table_matches_theme_js_pairs() -> None:
|
||||
"""The mirror contract: ``theme.js``'s ``PAIRS`` is exactly the
|
||||
EIGHT pairs the module docstring's authoritative table names —
|
||||
every PAIRS entry appears in the docstring as ``fg`` on ``bg``
|
||||
(and the list has exactly eight entries, so a pair silently added
|
||||
to ONE side fails)."""
|
||||
# Line-wrap-tolerant: the docstring table wraps at 79 columns
|
||||
# (``ink_soft``\non ``surface``), so newlines become spaces.
|
||||
doc = (theming.__doc__ or "").replace("\n", " ")
|
||||
pairs = _theme_js_pairs()
|
||||
assert len(pairs) == 8, f"PAIRS must hold exactly 8 pairs, got {pairs}"
|
||||
for fg, bg in pairs:
|
||||
assert f"``{fg}`` on ``{bg}``" in doc, (
|
||||
f"the docstring's authoritative pair table must name "
|
||||
f"``{fg}`` on ``{bg}`` (the theme.js mirror)"
|
||||
)
|
||||
# The two *_line state variables stay EXCLUDED from the warning
|
||||
# surface in BOTH places (decorative borders — no contrast duty).
|
||||
assert "err_line" not in [f for f, _ in pairs]
|
||||
assert "accent_line" not in [f for f, _ in pairs]
|
||||
|
||||
Reference in New Issue
Block a user