generated from john/python-template
Completes the table consolidation deferred within Phase 5. Section 4 of the review lists table construction as a duplication pattern; three call sites had been left bypassing the canonical builder. - table/common.py: build_table gains a row_key parameter so callers with a non-"id" primary key can use it. - linked_people.py: replaces its hand-rolled ui.table with build_table (row_key="person_id", selection="multiple", rows_per_page=0, no search). - print_preview_page.py: _render_metadata_table and _render_job_table now share a local _render_print_table helper. Print tables deliberately do not use build_table - they must never paginate or render a search box, and they carry print-only styling. The helper docstring records that rationale. - tests/test_ui_boundaries.py: new AST guard asserting ui.table() is called from exactly two modules - components/table/common.py and pages/print_preview_page.py. Also closes the intermittent tests/ui/test_jobs_page.py failure observed once after Phase 5 as environmental. Unreproduced across ~54 sequential full-suite runs (including a 25-run soak) and 5 concurrent-process runs. No code change. Verification: ruff check src tests clean; 275 passed, 4 skipped. Co-authored-by: Copilot App <[email protected]>
116 lines
4.1 KiB
Python
116 lines
4.1 KiB
Python
"""Structural rules for the NiceGUI page layer.
|
|
|
|
`.github/instructions/ui.instructions.md:23,31` forbids pages from owning
|
|
persistence or query-building concerns, and forbids components from resolving
|
|
request or application state. HIGH-07 recorded three violations of those rules;
|
|
these tests keep them from coming back.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import ast
|
|
from pathlib import Path
|
|
|
|
UI_DIR = Path(__file__).resolve().parents[1] / "src" / "transcription" / "ui"
|
|
PAGES_DIR = UI_DIR / "pages"
|
|
COMPONENTS_DIR = UI_DIR / "components"
|
|
|
|
# Names a page must not pull in: they hand the page a session, a transaction, ORM
|
|
# loader introspection, or process-global configuration.
|
|
FORBIDDEN_PAGE_IMPORTS = frozenset(
|
|
{
|
|
"session_scope",
|
|
"transaction_scope",
|
|
"get_session_factory",
|
|
"resolve_session_factory",
|
|
"get_settings",
|
|
"get_engine",
|
|
"upgrade_schema",
|
|
"create_all",
|
|
}
|
|
)
|
|
|
|
FORBIDDEN_PAGE_MODULES = frozenset({"sqlalchemy", "sqlmodel"})
|
|
|
|
|
|
def _page_paths() -> list[Path]:
|
|
return sorted(path for path in PAGES_DIR.glob("*.py") if path.stem != "__init__")
|
|
|
|
|
|
def _component_paths() -> list[Path]:
|
|
return sorted(COMPONENTS_DIR.rglob("*.py"))
|
|
|
|
|
|
def _imports(tree: ast.Module) -> tuple[set[str], set[str]]:
|
|
"""Return (imported names, imported root modules)."""
|
|
names: set[str] = set()
|
|
modules: set[str] = set()
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.ImportFrom):
|
|
if node.module and node.level == 0:
|
|
modules.add(node.module.split(".")[0])
|
|
names.update(alias.name for alias in node.names)
|
|
elif isinstance(node, ast.Import):
|
|
for alias in node.names:
|
|
modules.add(alias.name.split(".")[0])
|
|
names.add(alias.name)
|
|
return names, modules
|
|
|
|
|
|
def test_page_modules_are_discovered():
|
|
"""Guard the guard: the rules below are meaningless if nothing is scanned."""
|
|
discovered = {path.stem for path in _page_paths()}
|
|
assert discovered >= {"documents_page", "jobs_page", "people_page", "sources_page"}
|
|
|
|
|
|
def test_no_page_imports_persistence_or_process_globals():
|
|
"""HIGH-07: pages orchestrate services; they do not own sessions or settings."""
|
|
violations: dict[str, list[str]] = {}
|
|
for path in _page_paths():
|
|
names, modules = _imports(ast.parse(path.read_text(encoding="utf-8")))
|
|
found = sorted((names & FORBIDDEN_PAGE_IMPORTS) | (modules & FORBIDDEN_PAGE_MODULES))
|
|
if found:
|
|
violations[path.stem] = found
|
|
assert violations == {}
|
|
|
|
|
|
def test_no_component_resolves_request_or_application_state():
|
|
"""`ui.instructions.md:31`: components render, they do not resolve app state."""
|
|
violations: dict[str, list[str]] = {}
|
|
for path in _component_paths():
|
|
names, modules = _imports(ast.parse(path.read_text(encoding="utf-8")))
|
|
found = sorted((names & FORBIDDEN_PAGE_IMPORTS) | (modules & (FORBIDDEN_PAGE_MODULES | {"fastapi"})))
|
|
if found:
|
|
violations[str(path.relative_to(COMPONENTS_DIR))] = found
|
|
assert violations == {}
|
|
|
|
|
|
# `build_table` owns the interactive table styling; `print_preview_page` owns the
|
|
# print-only table, which must never paginate or expose a search box.
|
|
TABLE_OWNERS = frozenset({"components/table/common.py", "pages/print_preview_page.py"})
|
|
|
|
|
|
def _calls_ui_table(tree: ast.Module) -> bool:
|
|
for node in ast.walk(tree):
|
|
if not isinstance(node, ast.Call):
|
|
continue
|
|
func = node.func
|
|
if (
|
|
isinstance(func, ast.Attribute)
|
|
and func.attr == "table"
|
|
and isinstance(func.value, ast.Name)
|
|
and func.value.id == "ui"
|
|
):
|
|
return True
|
|
return False
|
|
|
|
|
|
def test_only_the_designated_owners_construct_a_raw_table():
|
|
"""Review section 4: table styling lives in one place, not in every page."""
|
|
offenders = sorted(
|
|
str(path.relative_to(UI_DIR)).replace("\\", "/")
|
|
for path in UI_DIR.rglob("*.py")
|
|
if _calls_ui_table(ast.parse(path.read_text(encoding="utf-8")))
|
|
)
|
|
assert set(offenders) == TABLE_OWNERS
|