From 5020abac30d662157bc14a08b6892959a89313e2 Mon Sep 17 00:00:00 2001 From: Alexander Nguyen Date: Sat, 30 May 2026 13:04:45 -0700 Subject: [PATCH 1/2] feat(templates): GUI template authoring + per-instance template folders Lets operators author Copier templates entirely in the GUI and scopes templates per instance. Approved design: docs/superpowers/specs/2026-05-30-template-authoring-gui-design.md. Authoring (new template/ service layer, no NiceGUI): - manifest.py: typed TemplateManifest with round-tripping from_yaml/to_yaml (hosts TemplateQuestion/template_questions; templates.py re-exports them). - lint.py: single-source validation (LintFinding error/warn) factored out of TemplateEngine.resolve + a Jinja2 .jinja parse check; resolve re-raises the same TemplateLoadError/TemplateCoreFieldRedeclaredError on the same codes. - authoring.py: atomic writes via atomic_write_bytes, one _safe_target path-traversal chokepoint (reuses paths.project_name_violations), upload size/count caps, opt-in .jinja, stale-edit guard. - ui/pages/template_editor.py + /templates/edit route + manager Edit action and scope/location selector. Per-instance template folders (.exlab-wizard/templates/{equipment|project|run}/): - resolution.py: nearest-wins chain (project -> equipment -> global), run-scope narrowing, TemplateChoices bundle, reconcile_selection. - provenance.py: post-CACHE_WRITE frozen copy of the rendering template into the instance's own typed subfolder; creation.json gains template .provenance_path (CREATION_JSON_VERSION 1.9 -> 1.10); POST_VALIDATE skips .exlab-wizard/ so the copy isn't validated as run output. Reactive wizard wiring (Phase 5): - Project/run wizards re-resolve their template list from the operator's equipment/project selection and submit the resolved absolute path (not templates_dir/name), so a per-instance template renders the right source. Tests: lint/authoring/manifest/resolution/provenance units, editor + mount units, and a creation-provenance integration test. Full feature suite green; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/exlab_wizard/api/schemas.py | 5 + src/exlab_wizard/constants/__init__.py | 6 + src/exlab_wizard/constants/filenames.py | 3 + src/exlab_wizard/constants/limits.py | 6 + src/exlab_wizard/constants/schema_versions.py | 2 +- src/exlab_wizard/controller/creation.py | 19 + .../controller/metadata_assembly.py | 6 + src/exlab_wizard/template/authoring.py | 674 ++++++++++++++++++ src/exlab_wizard/template/copier_driver.py | 85 ++- src/exlab_wizard/template/lint.py | 399 +++++++++++ src/exlab_wizard/template/manifest.py | 275 +++++++ src/exlab_wizard/template/provenance.py | 66 ++ src/exlab_wizard/template/resolution.py | 297 ++++++++ src/exlab_wizard/ui/mount.py | 335 ++++++++- src/exlab_wizard/ui/pages/template_editor.py | 419 +++++++++++ src/exlab_wizard/ui/pages/templates.py | 184 ++--- src/exlab_wizard/ui/pages/wizard_project.py | 107 ++- src/exlab_wizard/ui/pages/wizard_run.py | 126 +++- .../controller/test_creation_provenance.py | 181 +++++ tests/unit/constants/test_schema_versions.py | 4 +- .../unit/controller/test_metadata_assembly.py | 14 +- tests/unit/template/test_authoring.py | 350 +++++++++ tests/unit/template/test_lint.py | 260 +++++++ tests/unit/template/test_manifest.py | 208 ++++++ tests/unit/template/test_provenance.py | 98 +++ tests/unit/template/test_resolution.py | 424 +++++++++++ tests/unit/ui/test_mount.py | 121 +++- tests/unit/ui/test_template_editor.py | 161 +++++ 28 files changed, 4576 insertions(+), 259 deletions(-) create mode 100644 src/exlab_wizard/template/authoring.py create mode 100644 src/exlab_wizard/template/lint.py create mode 100644 src/exlab_wizard/template/manifest.py create mode 100644 src/exlab_wizard/template/provenance.py create mode 100644 src/exlab_wizard/template/resolution.py create mode 100644 src/exlab_wizard/ui/pages/template_editor.py create mode 100644 tests/integration/controller/test_creation_provenance.py create mode 100644 tests/unit/template/test_authoring.py create mode 100644 tests/unit/template/test_lint.py create mode 100644 tests/unit/template/test_manifest.py create mode 100644 tests/unit/template/test_provenance.py create mode 100644 tests/unit/template/test_resolution.py create mode 100644 tests/unit/ui/test_template_editor.py diff --git a/src/exlab_wizard/api/schemas.py b/src/exlab_wizard/api/schemas.py index df1c0c7..a5f4ed9 100644 --- a/src/exlab_wizard/api/schemas.py +++ b/src/exlab_wizard/api/schemas.py @@ -113,6 +113,11 @@ class TemplateBlock( # (legal for project / equipment templates per Spec §5.2). Persisted as # an omitted field thanks to ``omit_defaults=True``. run_scope: RunScope | None = None + # Path (relative to the instance dir, POSIX) of the frozen verbatim copy + # of the template source written under ``.exlab-wizard/templates/...`` at + # creation time (added in schema 1.10). Empty when the provenance copy was + # not made; omitted on serialize thanks to ``omit_defaults=True``. + provenance_path: str = "" class PluginIsolation( diff --git a/src/exlab_wizard/constants/__init__.py b/src/exlab_wizard/constants/__init__.py index 04ea264..bd1955f 100644 --- a/src/exlab_wizard/constants/__init__.py +++ b/src/exlab_wizard/constants/__init__.py @@ -59,6 +59,7 @@ SERVER_STATE_FILE, SYNC_QUEUE_DB_NAME, SYNC_STATE_FILENAME, + TEMPLATES_SUBDIR, TEST_RUNS_JSON_NAME, ) @@ -89,6 +90,8 @@ QUIT_DRAIN_TIMEOUT_SECONDS, SESSION_GC_AFTER_SECONDS, SIGTERM_DRAIN_TIMEOUT_SECONDS, + TEMPLATE_MAX_FILES, + TEMPLATE_UPLOAD_MAX_BYTES, TRAY_STATUS_REFRESH_SECONDS, VALIDATOR_BINARY_DETECT_BYTES, WINDOW_DEFAULT_HEIGHT, @@ -215,8 +218,11 @@ "SYNC_QUEUE_DB_NAME", "SYNC_STATE_FILENAME", "SYNC_STATE_JSON_VERSION", + "TEMPLATES_SUBDIR", + "TEMPLATE_MAX_FILES", "TEMPLATE_QUESTION_ID_PATTERN", "TEMPLATE_QUESTION_ID_REGEX", + "TEMPLATE_UPLOAD_MAX_BYTES", "TEST_MODE_ENV", "TEST_MODE_PREFIX", "TEST_RUNS_DIR_NAME", diff --git a/src/exlab_wizard/constants/filenames.py b/src/exlab_wizard/constants/filenames.py index 1b14d66..c7b05c6 100644 --- a/src/exlab_wizard/constants/filenames.py +++ b/src/exlab_wizard/constants/filenames.py @@ -31,6 +31,9 @@ # wizard against an existing directory. Backend Spec §5.3. ANSWERS_FILE_NAME: str = ".exlab-answers.yml" +# Subdirectory name (under a cache dir) holding per-instance template stores. Spec §5.0. +TEMPLATES_SUBDIR: str = "templates" + # Per-host log filename template inside the central log dir. Backend Spec §4.5. # Format with ``LOG_FILE_TEMPLATE.format(hostname=...)``. LOG_FILE_TEMPLATE: str = "wizard.{hostname}.log" diff --git a/src/exlab_wizard/constants/limits.py b/src/exlab_wizard/constants/limits.py index 852265d..f93d33b 100644 --- a/src/exlab_wizard/constants/limits.py +++ b/src/exlab_wizard/constants/limits.py @@ -106,3 +106,9 @@ # Pre-flight free-disk-space requirement, in MiB, on the run/project target # volume before the wizard will start a creation. Frontend Spec §4.6. DISK_SPACE_PREFLIGHT_MIB: int = 100 + +# Max size (bytes) of a single file uploaded into a template via the GUI. Spec §5.3. +TEMPLATE_UPLOAD_MAX_BYTES: int = 25 * 1024 * 1024 + +# Max number of files a single template directory may hold (GUI upload guard). +TEMPLATE_MAX_FILES: int = 500 diff --git a/src/exlab_wizard/constants/schema_versions.py b/src/exlab_wizard/constants/schema_versions.py index 543f7bc..308672e 100644 --- a/src/exlab_wizard/constants/schema_versions.py +++ b/src/exlab_wizard/constants/schema_versions.py @@ -9,7 +9,7 @@ # Version of the per-run ``creation.json`` cache file. See Backend Spec # section 11.3 (history table) for the schema and migration rules. -CREATION_JSON_VERSION: str = "1.9" +CREATION_JSON_VERSION: str = "1.10" # Version of the per-equipment ``readme_fields.json`` cache. Backend Spec §11.4. README_FIELDS_JSON_VERSION: str = "1.1" diff --git a/src/exlab_wizard/controller/creation.py b/src/exlab_wizard/controller/creation.py index 430f54a..716ab02 100644 --- a/src/exlab_wizard/controller/creation.py +++ b/src/exlab_wizard/controller/creation.py @@ -106,6 +106,7 @@ ResolvedTemplate, TemplateEngine, ) +from exlab_wizard.template.provenance import copy_template_into_instance from exlab_wizard.utils.time import utc_now, utc_now_iso from exlab_wizard.validator.engine import CreationValidationInput, Validator from exlab_wizard.validator.findings import Finding @@ -1002,6 +1003,17 @@ async def _write_cache( for entry in plugin_result.applied ] + # Freeze a verbatim copy of the template source into the instance's + # own typed provenance store (.exlab-wizard/templates///) + # and record its instance-relative path in creation.json. Best-effort: + # a provenance-copy failure must NOT fail the creation. + own_type = "run" if isinstance(req, RunCreateRequest) else "project" + try: + provenance_path = copy_template_into_instance(resolved, dst, own_type) + except Exception as exc: # best-effort: a copy failure is never fatal + _log.warning("provenance copy failed for %s: %s", dst, exc) + provenance_path = "" + # Redesign §3.1/§3.3: the orchestrator block, template/paths blocks, # and the CreationJson assembly are shared with the sample-data # seeder via ``build_creation_json`` so the two can never drift. @@ -1010,6 +1022,7 @@ async def _write_cache( version=resolved.exlab_version, source_path=str(resolved.path), run_scope=resolved.run_scope, + provenance_path=provenance_path, extra_readme_fields=resolved.extra_readme_fields, plugin_order=resolved.plugin_order, ) @@ -1090,6 +1103,12 @@ def _post_validate( file_names: list[str] = [] file_contents: dict[str, str] = {} for path in dst.rglob("*"): + # Skip the ``.exlab-wizard/`` cache subtree -- it holds the frozen + # provenance template copy (copier.yml / .jinja / placeholder + # files) and other wizard metadata, none of which is run output to + # be scanned for residual placeholders. + if CACHE_DIR_NAME in path.parts: + continue if not path.is_file(): continue file_names.append(path.name) diff --git a/src/exlab_wizard/controller/metadata_assembly.py b/src/exlab_wizard/controller/metadata_assembly.py index 691729a..83bd746 100644 --- a/src/exlab_wizard/controller/metadata_assembly.py +++ b/src/exlab_wizard/controller/metadata_assembly.py @@ -79,6 +79,10 @@ class TemplateDesc: ``str(ResolvedTemplate.path)``. run_scope: The run-scope tag persisted on ``creation.json``'s template block; ``None`` for project/equipment templates. + provenance_path: Path (relative to the instance dir, POSIX) of the + frozen verbatim template copy written under + ``.exlab-wizard/templates/...`` at creation time; empty when no + copy was made. extra_readme_fields: ``_exlab_readme.fields`` entries (free-form dicts) used to build the template-layer field declarations. plugin_order: Plugin slug ordering (unused by these helpers but @@ -89,6 +93,7 @@ class TemplateDesc: version: str source_path: str run_scope: RunScope | None = None + provenance_path: str = "" extra_readme_fields: list[dict[str, Any]] = field(default_factory=list) plugin_order: list[str] = field(default_factory=list) @@ -210,6 +215,7 @@ def build_creation_json( version=template.version, source_path=template.source_path, run_scope=template.run_scope, + provenance_path=template.provenance_path, ), variables=dict(variables), paths=PathsBlock( diff --git a/src/exlab_wizard/template/authoring.py b/src/exlab_wizard/template/authoring.py new file mode 100644 index 0000000..b8bc5d0 --- /dev/null +++ b/src/exlab_wizard/template/authoring.py @@ -0,0 +1,674 @@ +"""Author-time template service: scaffold, edit, and validate templates. + +This is the non-UI backend the GUI template authoring form (Frontend Spec +§5) drives. It owns every mutation of a template directory under +``config.paths.templates_dir`` so the NiceGUI page stays a thin view: + +* :func:`create_template_dir` -- scaffold a new minimal Copier template + (the same shape :func:`exlab_wizard.ui.pages.templates.create_template` + historically produced, now routed through here so the page can delegate). +* :func:`read_manifest` / :func:`write_manifest` -- round-trip the + structured :class:`~exlab_wizard.template.manifest.TemplateManifest` + through ``copier.yml``, gating every write on the shared + :mod:`exlab_wizard.template.lint` rule set. +* :func:`read_content` / :func:`write_content_file` / :func:`upload_file` + -- read and write the template's content files (``*.jinja`` and the + editable text allowlist), with a Jinja2 parse gate on ``*.jinja`` saves + and size / file-count caps on uploads. +* :func:`rename_path` / :func:`delete_path` -- move and remove files + within the template, never touching ``copier.yml`` on delete. +* :func:`list_files` -- a pure directory walk the GUI tree consumes. + +Two invariants run through the whole module: + +* **Every** new or edited path is resolved through :func:`_safe_target`, + the single chokepoint that rejects traversal (``..``), absolute paths, + path separators inside a segment, Windows-reserved / control / non-ASCII + segment names, and any resolved path that escapes the template root. + The per-segment rule reuses + :func:`exlab_wizard.paths.project_name_violations` so author-time + filenames obey the same filesystem-safety contract as project names. +* **Every** write goes through :func:`exlab_wizard.io.atomic_write_bytes` + -- the temp-file + ``fsync`` + ``os.replace`` recipe -- so a crash + mid-write never leaves a half-written ``copier.yml`` or content file. + +The :func:`write_manifest` / :func:`write_content_file` family also take an +optional ``expected_stat`` tuple ``(st_mtime, st_size)`` captured at read +time; if the on-disk file changed since, the write raises +:class:`StaleEditError` rather than clobbering a concurrent edit (the +optimistic-concurrency guard the GUI surfaces as "reload, your copy is +stale"). +""" + +from __future__ import annotations + +import os +import shutil +from pathlib import Path +from typing import TYPE_CHECKING + +import yaml +from jinja2 import Environment, TemplateSyntaxError + +from exlab_wizard.constants import ( + COPIER_MANIFEST_NAME, + TEMPLATE_MAX_FILES, + TEMPLATE_UPLOAD_MAX_BYTES, + RunScope, + TemplateType, +) +from exlab_wizard.errors import ExLabError +from exlab_wizard.io.atomic_write import atomic_write_bytes +from exlab_wizard.logging import get_logger +from exlab_wizard.paths import project_name_violations +from exlab_wizard.template import lint +from exlab_wizard.template.manifest import TemplateManifest + +if TYPE_CHECKING: + from collections.abc import Iterable + +__all__ = [ + "EDITABLE_SUFFIXES", + "StaleEditError", + "TemplateAuthoringError", + "UnsafePathError", + "create_template_dir", + "delete_path", + "is_editable", + "list_files", + "read_content", + "read_manifest", + "rename_path", + "upload_file", + "write_content_file", + "write_manifest", +] + +_log = get_logger(__name__) + + +# --------------------------------------------------------------------------- +# Exceptions (subclass the repo base so callers can catch ExLabError) +# --------------------------------------------------------------------------- + + +class TemplateAuthoringError(ExLabError): + """Raised on an author-time template-edit failure. + + Covers lint-rejected manifest saves, Jinja-syntax-rejected content + saves, over-cap uploads, and edits to non-editable / disallowed + paths. The two narrower failures below subclass this so callers can + catch the specific case or the whole family. + """ + + +class StaleEditError(TemplateAuthoringError): + """Raised when an optimistic-concurrency write loses a stat race. + + The file changed on disk between the caller's read (which captured + ``expected_stat``) and the write, so applying the edit would clobber + a concurrent change. The GUI surfaces this as "reload -- your copy is + stale" rather than silently overwriting. + """ + + +class UnsafePathError(TemplateAuthoringError): + """Raised when a requested path is not a safe in-template location. + + Covers traversal (``..``), absolute paths, path separators or + Windows-reserved / control / non-ASCII characters inside a segment, + and any resolved target that escapes the template root. + """ + + +# --------------------------------------------------------------------------- +# Editable-suffix classification +# --------------------------------------------------------------------------- + +# Suffixes the GUI may open in its text editor. Anything else (``.xlsx``, +# ``.png``, ...) is treated as opaque binary the operator can upload / +# rename / delete but not edit inline. +EDITABLE_SUFFIXES: frozenset[str] = frozenset( + {".md", ".txt", ".csv", ".yml", ".yaml", ".jinja", ".json"} +) + +# Content file every scaffolded template carries. ``.jinja`` so Copier +# renders it; the body has no variables so it renders verbatim. +_SCAFFOLD_CONTENT_NAME = "notes.md.jinja" +_SCAFFOLD_CONTENT_BODY = "# Notes\n\nScaffolded by ExLab-Wizard.\n" + +_JINJA_SUFFIX = ".jinja" + + +def is_editable(path: Path) -> bool: + """Return ``True`` if ``path`` is text the GUI may edit inline. + + The decision is purely by suffix against :data:`EDITABLE_SUFFIXES` + (case-insensitive); the file need not exist. A ``foo.md.jinja`` is + editable (its final suffix ``.jinja`` is in the set), as is a bare + ``.md`` / ``.csv`` / ``.json``; a ``.xlsx`` / ``.png`` is not. + """ + return path.suffix.lower() in EDITABLE_SUFFIXES + + +# --------------------------------------------------------------------------- +# The single path chokepoint +# --------------------------------------------------------------------------- + + +def _safe_target(template_dir: Path, rel: str) -> Path: + """Resolve ``rel`` to an absolute path proven to live inside ``template_dir``. + + Every new or edited path in this module flows through here. The + relative path is rejected outright if empty; otherwise it is split on + both ``/`` and ``\\`` and each non-empty segment is validated with + :func:`exlab_wizard.paths.project_name_violations` -- the same + filesystem-safety rule project names obey (no separators, no ``..`` / + trailing dot, no Windows-reserved name, no control / non-ASCII, not + over-length). The composed target is then ``resolve()``-d and required + to equal the resolved template root or sit beneath it, so a path that + escapes via symlink or a residual ``..`` is caught even though the + per-segment check already rejects literal ``..``. + + Args: + template_dir: The template root the path must stay within. + rel: A relative, in-template path (POSIX or Windows separators). + + Returns: + The resolved absolute path inside ``template_dir``. + + Raises: + UnsafePathError: ``rel`` is empty, contains an empty / unsafe + segment, or resolves outside ``template_dir``. + """ + if not rel: + msg = "empty path is not a valid template target" + raise UnsafePathError(msg) + + segments = rel.replace("\\", "/").split("/") + if any(seg == "" for seg in segments): + msg = f"path {rel!r} contains an empty segment" + raise UnsafePathError(msg) + + for seg in segments: + violations = project_name_violations(seg) + if violations: + _token, detail = violations[0] + msg = f"unsafe path segment {seg!r} in {rel!r}: {detail}" + raise UnsafePathError(msg) + + target = template_dir / rel + resolved = target.resolve() + base = template_dir.resolve() + if resolved != base and base not in resolved.parents: + msg = f"path {rel!r} escapes the template directory" + raise UnsafePathError(msg) + return resolved + + +# --------------------------------------------------------------------------- +# Scaffold +# --------------------------------------------------------------------------- + + +def create_template_dir( + base_dir: Path, + *, + name: str, + template_type: str, + description: str = "", + run_scope: str | None = None, +) -> Path: + """Scaffold a new minimal Copier template under ``base_dir``. + + Writes ``//copier.yml`` (serialised from a + :class:`~exlab_wizard.template.manifest.TemplateManifest` so the + author-time and structured-edit paths emit byte-identical YAML) plus + one ``notes.md.jinja`` content file. Both writes go through + :func:`atomic_write_bytes`. The result is immediately loadable by + :class:`~exlab_wizard.template.copier_driver.TemplateEngine`. + + Args: + base_dir: The ``templates_dir`` the new template is created under. + name: The template directory name. Stripped of surrounding + whitespace, then validated as a single safe filesystem + segment via :func:`project_name_violations`. + template_type: One of :class:`TemplateType` values. + description: Free-form ``_exlab_description`` text (stripped). + run_scope: Required for ``run`` templates; one of + :class:`RunScope` values. Must be ``None`` / unused otherwise. + + Returns: + The new template's root directory. + + Raises: + ValueError: Empty / duplicate ``name``, unknown ``template_type``, + or a run template missing / with an invalid ``run_scope``. + UnsafePathError: ``name`` is not a safe single filesystem segment. + """ + clean_name = name.strip() + if not clean_name: + msg = "template name must not be empty" + raise ValueError(msg) + if template_type not in {t.value for t in TemplateType}: + msg = f"unknown template type {template_type!r}" + raise ValueError(msg) + if template_type == TemplateType.RUN.value: + if run_scope is None: + msg = "run templates require a run_scope" + raise ValueError(msg) + if run_scope not in {s.value for s in RunScope}: + msg = f"unknown run_scope {run_scope!r}" + raise ValueError(msg) + + violations = project_name_violations(clean_name) + if violations: + _token, detail = violations[0] + msg = f"unsafe template name {clean_name!r}: {detail}" + raise UnsafePathError(msg) + + root = Path(base_dir) / clean_name + if root.exists(): + msg = f"a template named {clean_name!r} already exists" + raise ValueError(msg) + root.mkdir(parents=True) + + manifest = TemplateManifest( + exlab_type=template_type, + exlab_version="1.0", + exlab_run_scope=run_scope if template_type == TemplateType.RUN.value else None, + description=description.strip(), + ) + atomic_write_bytes(root / COPIER_MANIFEST_NAME, manifest.to_yaml().encode("utf-8")) + atomic_write_bytes(root / _SCAFFOLD_CONTENT_NAME, _SCAFFOLD_CONTENT_BODY.encode("utf-8")) + _log.info("scaffolded %s template %r at %s", template_type, clean_name, root) + return root + + +# --------------------------------------------------------------------------- +# Manifest round-trip +# --------------------------------------------------------------------------- + + +def _stat_tuple(path: Path) -> tuple[float, int]: + """Return the ``(st_mtime, st_size)`` stale-edit signature of ``path``.""" + st = path.stat() + return (st.st_mtime, st.st_size) + + +def _check_stale(path: Path, expected_stat: tuple[float, int] | None) -> None: + """Raise :class:`StaleEditError` if ``path``'s stat differs from expected. + + A ``None`` ``expected_stat`` skips the guard (the caller opted out of + optimistic concurrency). A missing file with a non-``None`` expectation + is itself a stale condition (the file the caller read is gone). + """ + if expected_stat is None: + return + try: + current = _stat_tuple(path) + except OSError as exc: + msg = f"{path} changed on disk since it was read (now missing): {exc}" + raise StaleEditError(msg) from exc + if current != expected_stat: + msg = ( + f"{path} changed on disk since it was read " + f"(expected stat {expected_stat}, found {current})" + ) + raise StaleEditError(msg) + + +def read_manifest(template_dir: Path) -> tuple[TemplateManifest, tuple[float, int]]: + """Read ``copier.yml`` and return the parsed manifest + its stat signature. + + The returned ``(st_mtime, st_size)`` tuple is passed back to + :func:`write_manifest` as ``expected_stat`` to detect a concurrent + edit. The manifest is parsed via + :meth:`TemplateManifest.from_yaml`, which is tolerant of missing + ``_exlab_*`` keys. + + Args: + template_dir: The template root. + + Returns: + ``(manifest, (st_mtime, st_size))``. + + Raises: + TemplateAuthoringError: ``copier.yml`` is missing or unreadable. + """ + copier_path = Path(template_dir) / COPIER_MANIFEST_NAME + try: + text = copier_path.read_text(encoding="utf-8") + stat = _stat_tuple(copier_path) + except OSError as exc: + msg = f"failed to read {copier_path}: {exc}" + raise TemplateAuthoringError(msg) from exc + return TemplateManifest.from_yaml(text), stat + + +def write_manifest( + template_dir: Path, + manifest: TemplateManifest, + *, + expected_stat: tuple[float, int] | None = None, +) -> None: + """Serialise ``manifest`` to ``copier.yml``, gated on the lint rule set. + + The manifest is rendered with :meth:`TemplateManifest.to_yaml`, then + re-parsed and run through :func:`exlab_wizard.template.lint.lint_manifest_dict`. + If any finding is an ERROR the file is **not** written and a + :class:`TemplateAuthoringError` carrying the joined error messages is + raised, so a manifest that ``TemplateEngine.resolve`` would reject can + never be saved. WARN findings do not block the save. On success the + bytes are written through :func:`atomic_write_bytes`. + + Args: + template_dir: The template root. + manifest: The manifest to serialise. + expected_stat: Optional ``(st_mtime, st_size)`` from + :func:`read_manifest`; if given and the on-disk file differs, + :class:`StaleEditError` is raised before any write. + + Raises: + StaleEditError: ``expected_stat`` given and the file changed. + TemplateAuthoringError: The serialised manifest has lint ERRORs. + """ + copier_path = Path(template_dir) / COPIER_MANIFEST_NAME + _check_stale(copier_path, expected_stat) + + text = manifest.to_yaml() + parsed = yaml.safe_load(text) + manifest_dict = parsed if isinstance(parsed, dict) else {} + findings = lint.lint_manifest_dict(manifest_dict, copier_path) + if lint.has_errors(findings): + errors = "; ".join(f.message for f in findings if f.severity == "error") + msg = f"manifest has lint errors, not written: {errors}" + raise TemplateAuthoringError(msg) + + atomic_write_bytes(copier_path, text.encode("utf-8")) + + +# --------------------------------------------------------------------------- +# Content read / write +# --------------------------------------------------------------------------- + + +def read_content(path: Path) -> tuple[str, tuple[float, int]]: + """Read an editable text file as UTF-8 and return its content + stat. + + Args: + path: The file to read. Its suffix must be in + :data:`EDITABLE_SUFFIXES`. + + Returns: + ``(text, (st_mtime, st_size))`` -- the stat is the + optimistic-concurrency signature for a later + :func:`write_content_file`. + + Raises: + TemplateAuthoringError: The suffix is not editable, or the file + is missing / unreadable / not valid UTF-8. + """ + p = Path(path) + if not is_editable(p): + msg = f"{p} is not an editable text file (suffix {p.suffix!r})" + raise TemplateAuthoringError(msg) + try: + text = p.read_text(encoding="utf-8") + stat = _stat_tuple(p) + except (OSError, UnicodeDecodeError) as exc: + msg = f"failed to read {p}: {exc}" + raise TemplateAuthoringError(msg) from exc + return text, stat + + +def write_content_file( + template_dir: Path, + rel: str, + text: str, + *, + expected_stat: tuple[float, int] | None = None, +) -> Path: + """Write ``text`` to an in-template content file at ``rel``. + + The target is resolved through :func:`_safe_target`. When ``rel`` ends + in ``.jinja`` the text is parsed with Jinja2 first; a syntax error + refuses the save with a :class:`TemplateAuthoringError` (so a broken + template never lands on disk). The write itself goes through + :func:`atomic_write_bytes`. + + Args: + template_dir: The template root. + rel: The in-template relative path to write. + text: The UTF-8 content to write. + expected_stat: Optional ``(st_mtime, st_size)`` from + :func:`read_content`; if given and the on-disk file differs, + :class:`StaleEditError` is raised before any write. + + Returns: + The resolved absolute path written. + + Raises: + UnsafePathError: ``rel`` is not a safe in-template path. + StaleEditError: ``expected_stat`` given and the file changed. + TemplateAuthoringError: A ``.jinja`` target with a syntax error. + """ + target = _safe_target(Path(template_dir), rel) + _check_stale(target, expected_stat) + + if target.suffix.lower() == _JINJA_SUFFIX: + _validate_jinja(text, rel) + + target.parent.mkdir(parents=True, exist_ok=True) + atomic_write_bytes(target, text.encode("utf-8")) + return target + + +def _validate_jinja(text: str, rel: str) -> None: + """Parse ``text`` as Jinja2; raise :class:`TemplateAuthoringError` on error. + + Parse-only (never renders), mirroring + :func:`exlab_wizard.template.lint._lint_jinja_files`. + """ + try: + Environment().parse(text) # parse-only; never renders untrusted input + except TemplateSyntaxError as exc: + msg = f"{rel}: Jinja syntax error on line {exc.lineno}: {exc.message}" + raise TemplateAuthoringError(msg) from exc + + +# --------------------------------------------------------------------------- +# Upload +# --------------------------------------------------------------------------- + + +def upload_file( + template_dir: Path, + filename: str, + data: bytes, + *, + render_as_template: bool = False, +) -> Path: + """Write uploaded ``data`` into the template as ``filename``. + + The filename is resolved through :func:`_safe_target` (so a traversal + or absolute path is rejected). When ``render_as_template`` is set and + the name is not already ``*.jinja``, a ``.jinja`` suffix is appended so + Copier renders the file. Two caps gate the write: + + * the upload may not exceed :data:`TEMPLATE_UPLOAD_MAX_BYTES`; + * the template may not already hold :data:`TEMPLATE_MAX_FILES` files. + + A ``.jinja`` upload that decodes as UTF-8 text is Jinja-parse-checked + (a binary ``.jinja`` -- unusual but possible -- skips the parse). The + write goes through :func:`atomic_write_bytes`. + + Args: + template_dir: The template root. + filename: The upload's in-template name (single path, may nest). + data: The raw bytes to write. + render_as_template: Append ``.jinja`` so Copier renders the file. + + Returns: + The resolved absolute path written. + + Raises: + UnsafePathError: ``filename`` is not a safe in-template path. + TemplateAuthoringError: The upload exceeds the size cap, the + template is at the file-count cap, or a UTF-8 ``.jinja`` upload + has a Jinja syntax error. + """ + if len(data) > TEMPLATE_UPLOAD_MAX_BYTES: + msg = ( + f"upload {filename!r} is {len(data)} bytes, exceeds the " + f"{TEMPLATE_UPLOAD_MAX_BYTES}-byte cap" + ) + raise TemplateAuthoringError(msg) + + root = Path(template_dir) + existing = _count_files(root) + if existing >= TEMPLATE_MAX_FILES: + msg = ( + f"template already holds {existing} files, at the " + f"{TEMPLATE_MAX_FILES}-file cap; cannot upload {filename!r}" + ) + raise TemplateAuthoringError(msg) + + name = filename + if render_as_template and not name.lower().endswith(_JINJA_SUFFIX): + name = f"{name}{_JINJA_SUFFIX}" + + target = _safe_target(root, name) + if target.suffix.lower() == _JINJA_SUFFIX: + try: + decoded = data.decode("utf-8") + except UnicodeDecodeError: + decoded = None # Binary .jinja: skip the parse, write verbatim. + if decoded is not None: + _validate_jinja(decoded, name) + + target.parent.mkdir(parents=True, exist_ok=True) + atomic_write_bytes(target, data) + return target + + +def _count_files(template_dir: Path) -> int: + """Count regular files (not directories) under ``template_dir``.""" + root = Path(template_dir) + if not root.is_dir(): + return 0 + return sum(1 for p in root.rglob("*") if p.is_file()) + + +# --------------------------------------------------------------------------- +# Rename / delete +# --------------------------------------------------------------------------- + + +def rename_path(template_dir: Path, src_rel: str, dst_rel: str) -> Path: + """Move an in-template path from ``src_rel`` to ``dst_rel``. + + Both ends are resolved through :func:`_safe_target`, so neither may + escape the template root. The move is ``os.replace`` (atomic on the + same filesystem); ``copier.yml`` may not be renamed away. + + Args: + template_dir: The template root. + src_rel: The existing in-template path. + dst_rel: The new in-template path. + + Returns: + The resolved absolute destination path. + + Raises: + UnsafePathError: Either end is not a safe in-template path. + TemplateAuthoringError: ``src_rel`` is ``copier.yml`` or does not + exist. + """ + root = Path(template_dir) + src = _safe_target(root, src_rel) + dst = _safe_target(root, dst_rel) + if src == (root / COPIER_MANIFEST_NAME).resolve(): + msg = "copier.yml cannot be renamed" + raise TemplateAuthoringError(msg) + if not src.exists(): + msg = f"cannot rename {src_rel!r}: it does not exist" + raise TemplateAuthoringError(msg) + dst.parent.mkdir(parents=True, exist_ok=True) + os.replace(src, dst) + return dst + + +def delete_path(template_dir: Path, rel: str) -> None: + """Delete an in-template file or directory at ``rel``. + + Resolved through :func:`_safe_target`. A file is ``unlink``-ed, a + directory is removed recursively with :func:`shutil.rmtree` (only ever + within the template root). ``copier.yml`` may not be deleted. + + Args: + template_dir: The template root. + rel: The in-template path to remove. + + Raises: + UnsafePathError: ``rel`` is not a safe in-template path. + TemplateAuthoringError: ``rel`` is ``copier.yml`` or does not + exist. + """ + root = Path(template_dir) + target = _safe_target(root, rel) + if target == (root / COPIER_MANIFEST_NAME).resolve(): + msg = "copier.yml cannot be deleted" + raise TemplateAuthoringError(msg) + if not target.exists(): + msg = f"cannot delete {rel!r}: it does not exist" + raise TemplateAuthoringError(msg) + if target.is_dir(): + shutil.rmtree(target) + else: + target.unlink() + + +# --------------------------------------------------------------------------- +# Directory listing (pure, GUI tree) +# --------------------------------------------------------------------------- + + +def list_files(template_dir: Path) -> list[dict]: + """Walk ``template_dir`` and return a sorted entry list for the GUI tree. + + Each entry is ``{"rel": str, "is_dir": bool, "editable": bool, + "size": int}`` -- ``rel`` is the POSIX-style path relative to the + template root, ``editable`` is :func:`is_editable` (always ``False`` + for directories), and ``size`` is the file size in bytes (``0`` for + directories). Entries are sorted by ``rel`` for a stable tree. + + Args: + template_dir: The template root. + + Returns: + The sorted entry list (empty if ``template_dir`` is not a + directory). + """ + root = Path(template_dir) + if not root.is_dir(): + return [] + entries: list[dict] = [] + for p in _iter_paths(root): + rel = p.relative_to(root).as_posix() + is_dir = p.is_dir() + entries.append( + { + "rel": rel, + "is_dir": is_dir, + "editable": (not is_dir) and is_editable(p), + "size": 0 if is_dir else p.stat().st_size, + } + ) + entries.sort(key=lambda e: e["rel"]) + return entries + + +def _iter_paths(root: Path) -> Iterable[Path]: + """Yield every path under ``root`` (files and directories).""" + yield from root.rglob("*") diff --git a/src/exlab_wizard/template/copier_driver.py b/src/exlab_wizard/template/copier_driver.py index ed7b8fc..e0b359a 100644 --- a/src/exlab_wizard/template/copier_driver.py +++ b/src/exlab_wizard/template/copier_driver.py @@ -149,62 +149,59 @@ def resolve(self, template_path: Path, scope: TemplateType) -> ResolvedTemplate: f"failed to parse {manifest_path}: {exc}", ) from exc - # _exlab_type: must be present, valid, and match the caller scope. - raw_type = manifest.get("_exlab_type") - if not isinstance(raw_type, str) or not raw_type: - raise TemplateLoadError( - f"{manifest_path}: _exlab_type missing or empty", - ) - try: - parsed_type = TemplateType(raw_type) - except ValueError as exc: - raise TemplateLoadError( - f"{manifest_path}: _exlab_type must be one of " - f"{sorted(t.value for t in TemplateType)}, got {raw_type!r}", - ) from exc + # Manifest validation is delegated to ``template.lint`` so the + # resolve-time and author-time rule sets never drift (§5.1). Imported + # lazily to avoid a circular import (``lint`` imports this module's + # :data:`CORE_README_FIELD_IDS`). ``lint_manifest_dict`` produces the + # same ERROR messages this method used to raise inline; we re-raise + # each as the historical exception type. + from exlab_wizard.template import lint as _lint + + findings = _lint.lint_manifest_dict(manifest, manifest_path) + errors = [f for f in findings if f.severity == "error"] + + # Raise the type ERRORs (missing / invalid) first -- the scope-match + # check below cannot run without a valid type, matching the original + # raise order. + for finding in errors: + if finding.code in {"template_type_missing", "template_type_invalid"}: + raise TemplateLoadError(finding.message) + + # _exlab_type matches the caller scope. This is resolve-specific (the + # author-time lint has no caller scope) so it is not a lint finding. + parsed_type = TemplateType(manifest["_exlab_type"]) if parsed_type is not scope: raise TemplateLoadError( f"{manifest_path}: _exlab_type {parsed_type.value!r} does not " f"match requested scope {scope.value!r}", ) - # _exlab_version: required non-empty string per §5.7. - exlab_version = manifest.get("_exlab_version") - if not isinstance(exlab_version, str) or not exlab_version.strip(): - raise TemplateLoadError( - f"{manifest_path}: _exlab_version is required and must be a " - f"non-empty string (§5.7)", - ) - - # _exlab_run_scope: required for run templates, optional otherwise. - run_scope: RunScope | None = None - if parsed_type is TemplateType.RUN: - raw_scope = manifest.get("_exlab_run_scope") - if not isinstance(raw_scope, str) or not raw_scope: - raise TemplateLoadError( - f"{manifest_path}: _exlab_run_scope is required for run " - f"templates and must be one of " - f"{sorted(s.value for s in RunScope)}", - ) - try: - run_scope = RunScope(raw_scope) - except ValueError as exc: - raise TemplateLoadError( - f"{manifest_path}: _exlab_run_scope must be one of " - f"{sorted(s.value for s in RunScope)}, got {raw_scope!r}", - ) from exc - - # _exlab_readme.fields: reject redeclaration of core fields. - extra_fields = self._extract_readme_fields(manifest, manifest_path) - - # _tasks: silently ignored per §5.5; warn so authors know. - if "_tasks" in manifest: + # Remaining ERRORs (version / run_scope / core-field redeclaration). + for finding in errors: + if finding.code == "core_field_redeclared": + raise TemplateCoreFieldRedeclaredError(finding.message) + if finding.code in {"template_type_missing", "template_type_invalid"}: + continue # already raised above + raise TemplateLoadError(finding.message) + + # _tasks: silently ignored per §5.5; warn so authors know. The lint + # surfaces this as a WARN finding; we keep the resolve-time log. + if any(f.code == "tasks_present" for f in findings): _log.warning( "template %s declares _tasks; silently ignored " "(unsafe=False, see Backend Spec §5.5)", template_path, ) + # All ERROR gates passed -- build the resolved view. + exlab_version = manifest["_exlab_version"] + run_scope: RunScope | None = None + if parsed_type is TemplateType.RUN: + run_scope = RunScope(manifest["_exlab_run_scope"]) + + # _exlab_readme.fields: tolerated/normalised the same way as before. + extra_fields = self._extract_readme_fields(manifest, manifest_path) + # _exlab_plugins: optional ordered list (§6.2.3). raw_plugins = manifest.get("_exlab_plugins") plugin_order = list(raw_plugins) if isinstance(raw_plugins, list) else [] diff --git a/src/exlab_wizard/template/lint.py b/src/exlab_wizard/template/lint.py new file mode 100644 index 0000000..711dd05 --- /dev/null +++ b/src/exlab_wizard/template/lint.py @@ -0,0 +1,399 @@ +"""Single source of truth for template (``copier.yml``) validation. + +The §5.1 checks that used to live inline in +:meth:`exlab_wizard.template.copier_driver.TemplateEngine.resolve` are +factored out here so resolve-time and author-time validation share one +rule set. The GUI authoring form calls :func:`lint_template` / +:func:`lint_manifest_dict` to gate saves; ``TemplateEngine.resolve`` +calls :func:`lint_manifest_dict` and re-raises its ERROR findings as the +existing ``TemplateLoadError`` / ``TemplateCoreFieldRedeclaredError``. + +Findings are returned as :class:`LintFinding` (a small, lint-specific +shape) rather than the validator's run-output ``Finding`` -- the latter +carries ``rule`` / ``run_path`` / ``offending_path`` / ``offending_kind`` +fields that are meaningless for a ``copier.yml``. + +Two tiers: + +* ``error`` -- the manifest/template is unusable; a save is refused and + ``resolve`` raises. +* ``warn`` -- the manifest is usable but deviates from convention; a + save proceeds with a banner. + +:func:`lint_manifest_dict` validates an already-parsed manifest mapping +(no file I/O). :func:`lint_template` adds the file-level checks +(``copier.yml`` existence / readability / YAML-parse) and a Jinja2 +syntax check across every ``*.jinja`` file under the template root. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Literal + +import yaml +from jinja2 import Environment, TemplateSyntaxError + +from exlab_wizard.constants import ( + COPIER_MANIFEST_NAME, + TEMPLATE_QUESTION_ID_PATTERN, + RunScope, + TemplateType, +) +from exlab_wizard.template.copier_driver import CORE_README_FIELD_IDS + +__all__ = [ + "LintFinding", + "LintSeverity", + "has_errors", + "lint_manifest_dict", + "lint_template", +] + +LintSeverity = Literal["error", "warn"] + +# Baseline ``_min_copier_version`` the app supports (Backend §5). A +# missing or lower value is a WARN. +_MIN_COPIER_VERSION_BASELINE: str = "9.0" + +# Leading dotted-integer run of a version string, e.g. "10.0" in "10.0.1rc2". +_VERSION_NUMERIC_PREFIX = re.compile(r"\d+(?:\.\d+)*") + + +def _version_tuple(value: str) -> tuple[int, ...]: + """Parse the leading dotted-integer run of ``value`` into a tuple. + + Returns ``()`` when ``value`` has no leading numeric component, which + sorts below every real version. Used so ``_min_copier_version`` + comparison is numeric (``"10.0" > "9.0"``) rather than lexicographic + (where ``"10.0" < "9.0"``). + """ + match = _VERSION_NUMERIC_PREFIX.match(value.strip()) + if match is None: + return () + return tuple(int(part) for part in match.group().split(".")) + +# Conventional ``_answers_file`` value (Backend §5.3). A deviation is a WARN. +_CONVENTIONAL_ANSWERS_FILE: str = ".exlab-answers.yml" + +# Valid Copier long-form ``type`` strings (Backend §5). A question whose +# declared long-form ``type`` is outside this set is a WARN. +_VALID_QUESTION_TYPES: frozenset[str] = frozenset({"str", "int", "float", "bool", "yaml", "json"}) + + +@dataclass(frozen=True) +class LintFinding: + """One template-validation finding. + + Attributes: + code: Stable machine code (e.g. ``"template_type_missing"``). + message: Human-readable description (carries the same wording + ``TemplateEngine.resolve`` historically raised, so the + re-raised exceptions are message-identical). + severity: ``"error"`` (refuse / raise) or ``"warn"`` (proceed). + path: Relative path of the offending file for file-scoped + findings (e.g. a ``*.jinja`` with a syntax error); ``None`` + for manifest-level findings. + """ + + code: str + message: str + severity: LintSeverity + path: str | None = None + + def to_dict(self) -> dict[str, Any]: + """Return a JSON-serialisable mapping of this finding.""" + return { + "code": self.code, + "message": self.message, + "severity": self.severity, + "path": self.path, + } + + +def lint_manifest_dict(manifest: dict[str, Any], manifest_path: Path) -> list[LintFinding]: + """Validate an already-parsed ``copier.yml`` mapping. + + Ports the §5.1 metadata checks out of ``TemplateEngine.resolve`` and + ``_extract_readme_fields``. Does **not** check file existence or + YAML-parse the file -- those are file-level and handled by + :func:`lint_template`. Messages match the wording + ``TemplateEngine.resolve`` historically raised so re-raised + exceptions are byte-identical. + + Args: + manifest: The parsed ``copier.yml`` body. + manifest_path: Path to the manifest, used only to prefix messages. + + Returns: + All findings (ERROR + WARN), in a stable order. + """ + findings: list[LintFinding] = [] + + # ---- _exlab_type: present, valid ------------------------------------- + raw_type = manifest.get("_exlab_type") + type_ok = False + if not isinstance(raw_type, str) or not raw_type: + findings.append( + LintFinding( + code="template_type_missing", + message=f"{manifest_path}: _exlab_type missing or empty", + severity="error", + ) + ) + elif raw_type not in {t.value for t in TemplateType}: + findings.append( + LintFinding( + code="template_type_invalid", + message=( + f"{manifest_path}: _exlab_type must be one of " + f"{sorted(t.value for t in TemplateType)}, got {raw_type!r}" + ), + severity="error", + ) + ) + else: + type_ok = True + + # ---- _exlab_version: required non-empty string (§5.7) ---------------- + exlab_version = manifest.get("_exlab_version") + if not isinstance(exlab_version, str) or not exlab_version.strip(): + findings.append( + LintFinding( + code="exlab_version_missing", + message=( + f"{manifest_path}: _exlab_version is required and must be a " + f"non-empty string (§5.7)" + ), + severity="error", + ) + ) + + # ---- _exlab_run_scope: required + valid for run templates ------------ + if type_ok and raw_type == TemplateType.RUN.value: + raw_scope = manifest.get("_exlab_run_scope") + if not isinstance(raw_scope, str) or not raw_scope: + findings.append( + LintFinding( + code="run_scope_missing", + message=( + f"{manifest_path}: _exlab_run_scope is required for run " + f"templates and must be one of " + f"{sorted(s.value for s in RunScope)}" + ), + severity="error", + ) + ) + elif raw_scope not in {s.value for s in RunScope}: + findings.append( + LintFinding( + code="run_scope_invalid", + message=( + f"{manifest_path}: _exlab_run_scope must be one of " + f"{sorted(s.value for s in RunScope)}, got {raw_scope!r}" + ), + severity="error", + ) + ) + + # ---- _exlab_readme.fields: must not redeclare core fields (§10.3) ---- + # Tolerate malformed _exlab_readme / fields shapes exactly like + # ``_extract_readme_fields`` (skip silently, never crash). + readme_block = manifest.get("_exlab_readme") + if isinstance(readme_block, dict): + raw_fields = readme_block.get("fields") + if isinstance(raw_fields, list): + for entry in raw_fields: + if not isinstance(entry, dict): + continue + field_id = entry.get("id") + if isinstance(field_id, str) and field_id in CORE_README_FIELD_IDS: + findings.append( + LintFinding( + code="core_field_redeclared", + message=( + f"{manifest_path}: _exlab_readme.fields redeclares " + f"core field {field_id!r}; core fields (label / " + f"operator / objective) are backend-managed (§10.3)" + ), + severity="error", + ) + ) + + # ---- _tasks: present (silently ignored under unsafe=False, §5.5) ----- + if "_tasks" in manifest: + findings.append( + LintFinding( + code="tasks_present", + message=( + f"{manifest_path}: _tasks declared; silently ignored " + f"(unsafe=False, see Backend Spec §5.5)" + ), + severity="warn", + ) + ) + + # ---- _min_copier_version: missing or below the "9.0" baseline -------- + raw_min = manifest.get("_min_copier_version") + if not isinstance(raw_min, str) or _version_tuple(raw_min) < _version_tuple( + _MIN_COPIER_VERSION_BASELINE + ): + findings.append( + LintFinding( + code="min_copier_version_low", + message=( + f"{manifest_path}: _min_copier_version should be >= " + f"{_MIN_COPIER_VERSION_BASELINE!r}" + ), + severity="warn", + ) + ) + + # ---- _answers_file: deviates from convention ------------------------- + raw_answers = manifest.get("_answers_file") + if isinstance(raw_answers, str) and raw_answers != _CONVENTIONAL_ANSWERS_FILE: + findings.append( + LintFinding( + code="answers_file_deviation", + message=( + f"{manifest_path}: _answers_file {raw_answers!r} deviates from " + f"the convention {_CONVENTIONAL_ANSWERS_FILE!r}" + ), + severity="warn", + ) + ) + + # ---- question keys: grammar + long-form type ------------------------- + for key, spec in manifest.items(): + if key.startswith("_"): + continue + if not TEMPLATE_QUESTION_ID_PATTERN.match(key): + findings.append( + LintFinding( + code="question_id_invalid", + message=( + f"{manifest_path}: question id {key!r} must match " + f"{TEMPLATE_QUESTION_ID_PATTERN.pattern!r}" + ), + severity="warn", + ) + ) + if isinstance(spec, dict) and "type" in spec: + q_type = str(spec.get("type")) + if q_type not in _VALID_QUESTION_TYPES: + findings.append( + LintFinding( + code="question_type_invalid", + message=( + f"{manifest_path}: question {key!r} has type {q_type!r}; " + f"expected one of {sorted(_VALID_QUESTION_TYPES)}" + ), + severity="warn", + ) + ) + + return findings + + +def lint_template(template_dir: Path) -> list[LintFinding]: + """Validate a template directory end-to-end. + + Runs the file-level checks (``copier.yml`` existence, readability, + YAML-parse), then delegates the manifest checks to + :func:`lint_manifest_dict`, then parses every ``*.jinja`` file under + ``template_dir`` with Jinja2 and reports syntax errors. + + Args: + template_dir: The template root (directory containing + ``copier.yml``). + + Returns: + All findings (ERROR + WARN). A fatal ``copier.yml`` problem + short-circuits the manifest checks (but the Jinja scan still runs). + """ + findings: list[LintFinding] = [] + manifest_path = template_dir / COPIER_MANIFEST_NAME + + if not manifest_path.is_file(): + findings.append( + LintFinding( + code="copier_yml_missing", + message=f"copier.yml not found at {manifest_path}", + severity="error", + ) + ) + else: + manifest: dict[str, Any] | None = None + try: + text = manifest_path.read_text(encoding="utf-8") + parsed = yaml.safe_load(text) + except OSError as exc: + findings.append( + LintFinding( + code="copier_yml_unreadable", + message=f"failed to read {manifest_path}: {exc}", + severity="error", + ) + ) + except yaml.YAMLError as exc: + findings.append( + LintFinding( + code="copier_yml_parse_error", + message=f"failed to parse {manifest_path}: {exc}", + severity="error", + ) + ) + else: + manifest = parsed if isinstance(parsed, dict) else {} + if manifest is not None: + findings.extend(lint_manifest_dict(manifest, manifest_path)) + + findings.extend(_lint_jinja_files(template_dir)) + return findings + + +def _lint_jinja_files(template_dir: Path) -> list[LintFinding]: + """Parse every ``*.jinja`` file under ``template_dir`` with Jinja2. + + Emits an ERROR ``jinja_syntax_error`` (with the offending file's + path relative to ``template_dir`` and the failing line number in the + message) for each file Jinja2 cannot parse. Unreadable files are + reported the same way so the author sees the problem. + """ + findings: list[LintFinding] = [] + env = Environment() # parse-only; never renders untrusted input. + for jinja_path in sorted(template_dir.rglob("*.jinja")): + if not jinja_path.is_file(): + continue + rel = str(jinja_path.relative_to(template_dir)) + try: + text = jinja_path.read_text(encoding="utf-8") + except OSError as exc: + findings.append( + LintFinding( + code="jinja_syntax_error", + message=f"{rel}: failed to read: {exc}", + severity="error", + path=rel, + ) + ) + continue + try: + env.parse(text) + except TemplateSyntaxError as exc: + findings.append( + LintFinding( + code="jinja_syntax_error", + message=f"{rel}: Jinja syntax error on line {exc.lineno}: {exc.message}", + severity="error", + path=rel, + ) + ) + return findings + + +def has_errors(findings: list[LintFinding]) -> bool: + """Return ``True`` if any finding has ``severity == "error"``.""" + return any(f.severity == "error" for f in findings) diff --git a/src/exlab_wizard/template/manifest.py b/src/exlab_wizard/template/manifest.py new file mode 100644 index 0000000..f7becd4 --- /dev/null +++ b/src/exlab_wizard/template/manifest.py @@ -0,0 +1,275 @@ +"""Typed, round-trippable model of a Copier ``copier.yml`` manifest. + +This module owns the two primitives the GUI authoring form and the +wizard consume questions through: + +* :class:`TemplateQuestion` -- one Copier question normalised to the + widget family the wizard renders (``str`` / ``int`` / ``float`` / + ``bool`` / ``choice``). +* :func:`template_questions` -- parse the operator-answerable questions + out of a raw ``copier.yml`` body (both Copier long- and short-form). + +These two used to live in :mod:`exlab_wizard.ui.pages.templates`. They +were moved here so the (non-UI) :class:`TemplateManifest` model can +reuse them without importing a NiceGUI page module (an import cycle). +``ui.pages.templates`` re-exports them so existing callers keep working. + +:class:`TemplateManifest` mirrors the ``_exlab_*`` metadata keys plus +the parsed questions and round-trips ``copier.yml`` so the structured +authoring form never hand-writes YAML: +``TemplateManifest.from_yaml(m.to_yaml()) == m`` holds for manifests +built from every supported question kind. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Any + +import yaml + +__all__ = [ + "TemplateManifest", + "TemplateQuestion", + "template_questions", +] + + +@dataclass(frozen=True) +class TemplateQuestion: + """One Copier question parsed from a template's ``copier.yml``. + + ``kind`` is normalised to the widget family the wizard renders: + ``str`` / ``int`` / ``float`` / ``bool`` / ``choice``. ``choices`` + is populated only for ``choice`` questions. ``secret`` flags a + password-style ``str`` input. + """ + + key: str + kind: str + default: Any = None + choices: tuple[Any, ...] = () + help: str = "" + secret: bool = False + + +# Copier reserves ``_``-prefixed manifest keys for itself; everything +# else under the top level is an operator-answerable question. +_COPIER_TYPE_TO_KIND: dict[str, str] = { + "str": "str", + "int": "int", + "float": "float", + "bool": "bool", + "yaml": "str", + "json": "str", +} + +# Reverse map for emitting a Copier ``type`` string from a normalised +# :class:`TemplateQuestion.kind`. ``choice`` is special-cased (emits +# ``type: str`` + a ``choices`` block) in :meth:`TemplateManifest.to_yaml`. +_KIND_TO_COPIER_TYPE: dict[str, str] = { + "str": "str", + "int": "int", + "float": "float", + "bool": "bool", + "choice": "str", +} + + +def template_questions(raw_manifest: dict[str, Any]) -> list[TemplateQuestion]: + """Parse the operator-answerable questions out of a ``copier.yml`` body. + + Handles both Copier question forms: + + * **long form** -- ``key: {type: ..., default: ..., choices: ...}`` + * **short form** -- ``key: `` (the scalar is the default; + the type is inferred from it) + + ``_``-prefixed keys (Copier / ``_exlab_*`` metadata) are skipped. + Questions carrying a ``when`` clause are still returned -- the + wizard renders them unconditionally for v1. + """ + questions: list[TemplateQuestion] = [] + for key, spec in raw_manifest.items(): + if key.startswith("_"): + continue + if isinstance(spec, dict): + raw_type = str(spec.get("type", "str")) + raw_choices = spec.get("choices") + choices: tuple[Any, ...] = () + if isinstance(raw_choices, dict): + choices = tuple(raw_choices.values()) + elif isinstance(raw_choices, list): + choices = tuple(raw_choices) + kind = "choice" if choices else _COPIER_TYPE_TO_KIND.get(raw_type, "str") + questions.append( + TemplateQuestion( + key=key, + kind=kind, + default=spec.get("default"), + choices=choices, + help=str(spec.get("help", "")), + secret=bool(spec.get("secret", False)), + ) + ) + else: + # Short form: the scalar is the default; infer the kind. + if isinstance(spec, bool): + kind = "bool" + elif isinstance(spec, int): + kind = "int" + elif isinstance(spec, float): + kind = "float" + else: + kind = "str" + questions.append(TemplateQuestion(key=key, kind=kind, default=spec)) + return questions + + +@dataclass(frozen=True) +class TemplateManifest: + """A typed, round-trippable model of a Copier ``copier.yml``. + + Mirrors the ``_exlab_*`` metadata keys plus the parsed + :class:`TemplateQuestion` list. Built so the structured authoring + form never hand-writes YAML: :meth:`from_yaml` parses a manifest + (string or already-parsed dict) and :meth:`to_yaml` emits a + deterministic, long-form ``copier.yml`` such that + ``TemplateManifest.from_yaml(m.to_yaml()) == m``. + + Attributes: + exlab_type: One of ``"project"`` / ``"equipment"`` / ``"run"`` + (``_exlab_type``). Not validated here -- see + :mod:`exlab_wizard.template.lint`. + exlab_version: The required ``_exlab_version`` string (§5.7). + exlab_run_scope: ``_exlab_run_scope`` for run templates; ``None`` + otherwise (and then never emitted by :meth:`to_yaml`). + description: ``_exlab_description`` free-form text. + plugins: Ordered ``_exlab_plugins`` slug list (§6.2.3); emitted + only when non-empty. + readme_fields: ``_exlab_readme.fields`` field-extension list + (§10.3), each a free-form dict. + questions: Parsed operator-answerable questions, emitted in + long form. + min_copier_version: ``_min_copier_version`` (defaults ``"9.0"``). + answers_file: ``_answers_file`` (defaults ``".exlab-answers.yml"``). + """ + + exlab_type: str + exlab_version: str + exlab_run_scope: str | None = None + description: str = "" + plugins: list[str] = field(default_factory=list) + readme_fields: list[dict[str, Any]] = field(default_factory=list) + questions: list[TemplateQuestion] = field(default_factory=list) + min_copier_version: str = "9.0" + answers_file: str = ".exlab-answers.yml" + + @classmethod + def from_yaml(cls, raw: str | dict[str, Any]) -> TemplateManifest: + """Build a :class:`TemplateManifest` from a manifest body. + + Args: + raw: Either a raw ``copier.yml`` string (parsed with + :func:`yaml.safe_load`) or an already-parsed mapping. + + Returns: + A :class:`TemplateManifest`. Missing ``_exlab_*`` / ``_*`` + keys fall back to the dataclass defaults; questions are + parsed via :func:`template_questions`. + """ + data: dict[str, Any] + if isinstance(raw, str): + parsed = yaml.safe_load(raw) + data = parsed if isinstance(parsed, dict) else {} + else: + data = raw + + raw_type = data.get("_exlab_type") + exlab_type = raw_type if isinstance(raw_type, str) else "" + + raw_version = data.get("_exlab_version") + exlab_version = raw_version if isinstance(raw_version, str) else "" + + raw_scope = data.get("_exlab_run_scope") + exlab_run_scope = raw_scope if isinstance(raw_scope, str) else None + + raw_description = data.get("_exlab_description") + description = raw_description if isinstance(raw_description, str) else "" + + raw_plugins = data.get("_exlab_plugins") + plugins = list(raw_plugins) if isinstance(raw_plugins, list) else [] + + readme_fields: list[dict[str, Any]] = [] + readme_block = data.get("_exlab_readme") + if isinstance(readme_block, dict): + raw_fields = readme_block.get("fields") + if isinstance(raw_fields, list): + readme_fields = [e for e in raw_fields if isinstance(e, dict)] + + raw_min = data.get("_min_copier_version") + min_copier_version = raw_min if isinstance(raw_min, str) else "9.0" + + raw_answers = data.get("_answers_file") + answers_file = raw_answers if isinstance(raw_answers, str) else ".exlab-answers.yml" + + return cls( + exlab_type=exlab_type, + exlab_version=exlab_version, + exlab_run_scope=exlab_run_scope, + description=description, + plugins=plugins, + readme_fields=readme_fields, + questions=template_questions(data), + min_copier_version=min_copier_version, + answers_file=answers_file, + ) + + def to_yaml(self) -> str: + """Emit a deterministic, long-form ``copier.yml`` string. + + Keys are emitted in a fixed order (Copier metadata first, then + each question in long form) with ``sort_keys=False`` so the + ordering is preserved. ``_exlab_run_scope`` is emitted only when + not ``None`` and ``_exlab_plugins`` only when non-empty. Each + question emits ``{type, help, default, choices, secret}`` with + empty / ``None`` sub-keys omitted, so + :meth:`from_yaml` reconstructs the identical model. + """ + body: dict[str, Any] = { + "_min_copier_version": self.min_copier_version, + "_answers_file": self.answers_file, + "_exlab_type": self.exlab_type, + "_exlab_version": self.exlab_version, + } + if self.exlab_run_scope is not None: + body["_exlab_run_scope"] = self.exlab_run_scope + body["_exlab_description"] = self.description + body["_exlab_readme"] = {"fields": self.readme_fields} + if self.plugins: + body["_exlab_plugins"] = list(self.plugins) + + for question in self.questions: + body[question.key] = _question_to_long_form(question) + + return yaml.safe_dump(body, sort_keys=False) + + +def _question_to_long_form(question: TemplateQuestion) -> dict[str, Any]: + """Render one :class:`TemplateQuestion` as a Copier long-form spec. + + Maps ``kind`` back to a Copier ``type`` string; ``choice`` questions + emit ``type: str`` plus a ``choices`` list. Empty / ``None`` sub-keys + (``help``, ``default``, ``choices``, ``secret``) are omitted so the + round-trip through :func:`template_questions` is lossless. + """ + spec: dict[str, Any] = {"type": _KIND_TO_COPIER_TYPE.get(question.kind, "str")} + if question.help: + spec["help"] = question.help + if question.default is not None: + spec["default"] = question.default + if question.choices: + spec["choices"] = list(question.choices) + if question.secret: + spec["secret"] = True + return spec diff --git a/src/exlab_wizard/template/provenance.py b/src/exlab_wizard/template/provenance.py new file mode 100644 index 0000000..0fc9bff --- /dev/null +++ b/src/exlab_wizard/template/provenance.py @@ -0,0 +1,66 @@ +"""Frozen template provenance copy. Design spec (Phase 3b). + +After a template renders into a destination folder, the exact template +source is copied -- verbatim, including ``copier.yml`` and ``.jinja`` +files -- into that instance's own typed template store under +``/.exlab-wizard/templates///``. This freezes the +provenance so a later reader can see precisely which template produced +the instance, even if the shared templates directory drifts. + +The copy path is recorded in ``creation.json`` (the ``template`` block's +``provenance_path``). This module is intentionally pure and dependency +light: it reads only ``resolved.name`` / ``resolved.path`` and performs a +single ``copytree``. +""" + +from __future__ import annotations + +import shutil +from typing import TYPE_CHECKING + +from exlab_wizard.constants import TEMPLATES_SUBDIR +from exlab_wizard.paths import cache_dir + +if TYPE_CHECKING: + from pathlib import Path + + from exlab_wizard.template.copier_driver import ResolvedTemplate + +__all__ = ["copy_template_into_instance"] + + +def copy_template_into_instance( + resolved: ResolvedTemplate, + dst: Path, + own_type: str, +) -> str: + """Copy the resolved template root into ``dst``'s own typed provenance store. + + Writes ``/.exlab-wizard/templates///`` as a + verbatim copy (incl. ``copier.yml`` and ``.jinja`` files). The copy is a + frozen snapshot of the exact template source that produced this instance. + + Args: + resolved: The resolved template whose ``name`` / ``path`` (template + root directory) are copied. Only these two attributes are read. + dst: The instance destination directory the template rendered into. + own_type: The instance's own template type segment -- ``"run"`` for a + run, ``"project"`` for a project. + + Returns: + The provenance copy's path RELATIVE to ``dst`` as a POSIX string, + e.g. ``".exlab-wizard/templates/run/confocal_run"``. + + Raises: + FileNotFoundError: ``resolved.path`` does not exist (should not happen + after a successful render). The caller wraps this best-effort. + """ + source = resolved.path + if not source.is_dir(): + raise FileNotFoundError( + f"template source {source} is missing; cannot copy provenance", + ) + + target = cache_dir(dst) / TEMPLATES_SUBDIR / own_type / resolved.name + shutil.copytree(source, target, dirs_exist_ok=True) + return target.relative_to(dst).as_posix() diff --git a/src/exlab_wizard/template/resolution.py b/src/exlab_wizard/template/resolution.py new file mode 100644 index 0000000..d339f7a --- /dev/null +++ b/src/exlab_wizard/template/resolution.py @@ -0,0 +1,297 @@ +"""Read-side template resolver (generalizes Backend Spec §5.0). + +The project / run wizards do not offer a single flat template list: a +template defined nearer the work (per-project, then per-equipment) should +override a same-named template defined further out (the global +``paths.templates_dir``). This module turns a config plus a wizard context +(``template_type`` + optional ``equipment_id`` / ``project_path``) into the +ordered, de-duplicated list of templates the wizard should offer, nearest +scope first, nearest scope winning on a name collision. + +Directory layout searched (design spec §2): + +* **Global** -- ``paths.templates_dir`` (flat: templates sit directly + inside, all types mixed, filtered by ``_exlab_type``). +* **Per-equipment** -- ``//.exlab-wizard/templates//`` + (type-segregated). +* **Per-project** -- ``/.exlab-wizard/templates//`` + (type-segregated). + +The per-instance directory scanning + manifest parsing is delegated to +:func:`exlab_wizard.ui.pages.templates.list_templates`; this module only +composes the search chain and merges the results. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import TYPE_CHECKING + +from exlab_wizard.constants import TEMPLATES_SUBDIR, RunScope, TemplateType +from exlab_wizard.paths import cache_dir + +if TYPE_CHECKING: + from pathlib import Path + + from exlab_wizard.config.models import Config + from exlab_wizard.ui.pages.templates import TemplateQuestion, TemplateSummary + +__all__ = [ + "TemplateChoices", + "instance_template_dir", + "project_dir", + "reconcile_selection", + "resolve_template_chain", + "search_dirs", +] + + +@dataclass(frozen=True) +class TemplateChoices: + """The templates a wizard offers for one resolution context. + + Bundles the three parallel views the wizard's template + variables + steps need so a single ``on_resolve(equipment_id, project_name)`` call + returns everything for the current selection: + + Attributes: + names: Offered template names, nearest scope first. + questions: Per-template parsed ``copier.yml`` questions (drives the + dynamic Variables step). Missing entry == no variables. + paths: Per-template absolute source path of the *resolved* template, + so ``on_submit`` renders the exact file the wizard listed rather + than re-deriving ``templates_dir / name`` (which would ignore a + per-instance override). Keyed by template name. + """ + + names: list[str] = field(default_factory=list) + questions: dict[str, list[TemplateQuestion]] = field(default_factory=dict) + paths: dict[str, Path] = field(default_factory=dict) + + +def instance_template_dir(instance_dir: Path, template_type: str) -> Path: + """Return the per-instance template directory for one scope. + + A per-instance (per-equipment / per-project) template store is + ``/.exlab-wizard/templates//`` -- the + ``.exlab-wizard`` cache dir, a ``templates`` sub-dir, then a child + sub-dir per template type so the project / run stores never collide. + + Args: + instance_dir: The equipment or project root directory. + template_type: The child template type (``"project"`` / ``"run"``) + whose sub-dir is returned. + + Returns: + The (possibly non-existent) per-instance template directory. + """ + return cache_dir(instance_dir) / TEMPLATES_SUBDIR / template_type + + +def search_dirs( + config: Config, + *, + template_type: str, + equipment_id: str | None = None, + project_path: Path | None = None, +) -> list[Path]: + """Return the template search directories, highest precedence first. + + The precedence chain narrows from the work outwards: a per-project + store beats a per-equipment store, which beats the global + ``paths.templates_dir``. Which scopes apply depends on the template + type the wizard is offering: + + * ``"run"`` -- per-project, then per-equipment, then global. + * ``"project"`` -- per-equipment, then global (a project has no + children of its own to scope project templates by). + * ``"equipment"`` -- global only. + + A scope is included only when its backing config value is present: + the global dir is skipped when ``paths.templates_dir`` is empty, the + per-equipment dir when ``equipment_id`` is ``None`` (or ``local_root`` + is empty), and the per-project dir when ``project_path`` is ``None``. + Returned directories need not exist -- :func:`list_templates` + tolerates a missing directory by returning no templates. + + Args: + config: The loaded config (supplies ``paths.templates_dir`` and + ``paths.local_root``). + template_type: One of ``"project"`` / ``"run"`` / ``"equipment"``. + equipment_id: The equipment the wizard runs under, if any. Gates + the per-equipment scope. + project_path: The absolute project directory, if any. Gates the + per-project scope (run wizard only). + + Returns: + The ordered list of search directories, nearest scope first. + """ + dirs: list[Path] = [] + + if template_type == TemplateType.RUN.value: + if project_path is not None: + dirs.append(instance_template_dir(project_path, template_type)) + equipment_dir = _equipment_dir(config, equipment_id) + if equipment_dir is not None: + dirs.append(instance_template_dir(equipment_dir, template_type)) + elif template_type == TemplateType.PROJECT.value: + equipment_dir = _equipment_dir(config, equipment_id) + if equipment_dir is not None: + dirs.append(instance_template_dir(equipment_dir, template_type)) + + global_dir = _global_dir(config) + if global_dir is not None: + dirs.append(global_dir) + return dirs + + +def resolve_template_chain( + config: Config, + *, + template_type: str, + equipment_id: str | None = None, + project_path: Path | None = None, + run_scope: str | None = None, +) -> list[TemplateSummary]: + """Resolve the merged template list a wizard should offer. + + Walks :func:`search_dirs` nearest-first, scanning each directory with + :func:`list_templates`, and merges the results by template name keeping + the **first** (nearest-scope) occurrence -- so a per-project template + shadows a same-named per-equipment or global one. Nearest-first order is + preserved in the result. + + ``template_type`` is always passed to :func:`list_templates`: the global + dir is flat (mixed types) so the filter is required, and per-instance + ``/`` dirs pass it defensively so a misfiled template of the wrong + type is skipped rather than offered. + + For run templates, ``run_scope`` additionally narrows by the + template's declared scope: when given (``"experimental"`` / ``"test"``) + a run template is kept only when its ``run_scope`` equals that scope or + is ``"both"``. ``run_scope=None`` keeps every run template. The + parameter is ignored for non-run template types. + + Args: + config: The loaded config. + template_type: One of ``"project"`` / ``"run"`` / ``"equipment"``. + equipment_id: The equipment the wizard runs under, if any. + project_path: The absolute project directory, if any (run wizard). + run_scope: Optional run-scope filter (run templates only). + + Returns: + The merged, de-duplicated list of templates, nearest scope first. + """ + # Lazy import to avoid an import cycle: ``ui.pages.templates`` is part of + # the UI package whose ``mount`` indirectly imports this resolver. + from exlab_wizard.ui.pages.templates import list_templates + + merged: list[TemplateSummary] = [] + seen: set[str] = set() + for directory in search_dirs( + config, + template_type=template_type, + equipment_id=equipment_id, + project_path=project_path, + ): + for summary in list_templates(directory, template_type=template_type): + if summary.name in seen: + continue + if not _run_scope_matches(template_type, run_scope, summary.run_scope): + continue + seen.add(summary.name) + merged.append(summary) + return merged + + +def reconcile_selection( + choices: TemplateChoices, selected_name: str | None +) -> tuple[str | None, Path | None, bool]: + """Reconcile a prior template selection against freshly-resolved choices. + + Called after a wizard re-resolves its template chain (the operator + changed equipment / project). Returns ``(name, path, dropped)``: + + * The selected name **survives** the new context -> its resolved path is + **re-derived** from ``choices`` and returned with ``dropped=False``. + This is the critical case: a same-named per-instance template shadows + the global one at a *different* path, and a wizard ``ui.select`` that + keeps its value does not re-fire its change handler -- so the stored + path must be refreshed here or submit would render the stale source. + * The selected name is **gone** (or was ``None``) -> returns + ``(None, None, True)`` so the caller clears the selection and its + now-orphaned variables. + + Args: + choices: The freshly-resolved templates for the new context. + selected_name: The template name selected under the old context. + + Returns: + ``(name, path, dropped)`` -- the reconciled selection name, its + re-derived absolute path (or ``None``), and whether the prior + selection was dropped. + """ + if selected_name is not None and selected_name in choices.names: + return selected_name, choices.paths.get(selected_name), False + return None, None, True + + +def project_dir(config: Config, equipment_id: str | None, project_name: str | None) -> Path | None: + """Return the absolute project directory, or ``None`` when not derivable. + + A run's per-project template store lives under + ``///`` (Backend Spec §3.2). The + wizard knows the equipment id and the parent project's folder name, so + this composes the path the run-wizard resolver passes as + ``project_path``. Returns ``None`` when ``local_root``, ``equipment_id``, + or ``project_name`` is missing -- the caller then resolves without the + per-project scope (per-equipment + global only). + """ + from pathlib import Path + + local_root = config.paths.local_root + if not local_root or not equipment_id or not project_name: + return None + return Path(local_root) / equipment_id / project_name + + +def _global_dir(config: Config) -> Path | None: + """Return the global ``paths.templates_dir`` as a ``Path``, or ``None``. + + ``None`` when the config value is empty, so the caller skips the scope. + """ + from pathlib import Path + + templates_dir = config.paths.templates_dir + return Path(templates_dir) if templates_dir else None + + +def _equipment_dir(config: Config, equipment_id: str | None) -> Path | None: + """Return ``/`` as a ``Path``, or ``None``. + + ``None`` when ``equipment_id`` is unset or ``local_root`` is empty, so + the caller skips the per-equipment scope. + """ + from pathlib import Path + + local_root = config.paths.local_root + if not equipment_id or not local_root: + return None + return Path(local_root) / equipment_id + + +def _run_scope_matches( + template_type: str, + requested_scope: str | None, + template_scope: str | None, +) -> bool: + """Return whether a template passes the run-scope filter. + + Only run templates with a requested scope are narrowed. A run + template is kept when its scope equals the requested scope or is + :attr:`RunScope.BOTH`. Non-run types and an unset ``requested_scope`` + always pass. + """ + if template_type != TemplateType.RUN.value or requested_scope is None: + return True + return template_scope in (requested_scope, RunScope.BOTH.value) diff --git a/src/exlab_wizard/ui/mount.py b/src/exlab_wizard/ui/mount.py index eea287f..0bec918 100644 --- a/src/exlab_wizard/ui/mount.py +++ b/src/exlab_wizard/ui/mount.py @@ -43,6 +43,8 @@ if TYPE_CHECKING: from fastapi import FastAPI + from exlab_wizard.template.resolution import TemplateChoices + __all__ = ["MOUNT_PATH", "mount_ui"] @@ -106,6 +108,9 @@ def _register_pages(app: FastAPI, ui: Any) -> None: from exlab_wizard.ui.pages import ( settings as settings_page, ) + from exlab_wizard.ui.pages import ( + template_editor as template_editor_page, + ) from exlab_wizard.ui.pages import ( templates as templates_page, ) @@ -270,10 +275,20 @@ def _on_file_context_action(entry: Any, action: str) -> None: @ui.page("/wizard/project") async def _wizard_project() -> Any: deps = _deps() + initial = _resolve_template_choices(deps, "project") + + def _resolve_project_templates(equipment_id: str | None) -> Any: + # Per-equipment project templates layer over the global store + # once the operator picks equipment (step 3, after the template + # step -- so this reflects on a step-back). Backend Spec §5.0. + return _resolve_template_choices(deps, "project", equipment_id=equipment_id) + return wizard_project_page.render_project_wizard( - templates=_template_names(deps, "project"), + templates=initial.names, equipment_ids=_equipment_ids(deps), - template_questions=_template_questions_map(deps, "project"), + template_questions=initial.questions, + template_paths=initial.paths, + on_resolve=_resolve_project_templates, lims_projects=await _lims_projects(deps), on_submit=lambda state: _submit_project(deps, state, ui), on_cancel=lambda: ui.navigate.to("/main"), @@ -343,9 +358,12 @@ def _on_confirm(eq: Any) -> None: ) @ui.page("/templates") - def _templates() -> Any: + def _templates(loc: str = "global") -> Any: deps = _deps() + # Global scaffolding always targets the flat templates_dir; the + # location selector only changes which directories are *listed*. templates_dir = _templates_dir(deps) + scan_dirs = _location_scan_dirs(deps, loc) def _on_create( name: str, template_type: str, description: str, run_scope: str | None @@ -367,13 +385,106 @@ def _on_create( _show_toast(ui, f"Template {name!r} created", positive=True) ui.navigate.to("/templates") - summaries = ( - templates_page.list_templates(templates_dir) if templates_dir is not None else [] - ) + # Merge the listings across the location's scan dirs (the global + # location has one flat dir; an equipment location has its + # type-segregated project/ + run/ stores). ``rel_by_name`` records + # the on-disk path so the Edit link can deep-link straight to it. + summaries: list[Any] = [] + dir_by_name: dict[str, Path] = {} + for directory in scan_dirs: + for summary in templates_page.list_templates(directory): + if summary.name in dir_by_name: + continue + dir_by_name[summary.name] = summary.path + summaries.append(summary) + + def _on_edit(name: str) -> None: + target = dir_by_name.get(name) + if target is None: + _show_toast(ui, f"Template {name!r} not found", positive=False) + return + ui.navigate.to(f"/templates/edit?dir={target}") + return templates_page.render_template_manager( templates=summaries, on_create=_on_create, on_back=lambda: ui.navigate.to("/main"), + on_edit=_on_edit, + locations=_template_locations(deps), + on_location_change=lambda value: ui.navigate.to(f"/templates?loc={value}"), + ) + + @ui.page("/templates/edit") + def _templates_edit(dir: str = "") -> Any: # query-param name (binds NiceGUI ?dir=) + from exlab_wizard.template import authoring + + template_dir = Path(dir) if dir else None + if template_dir is None or not template_dir.is_dir(): + _show_toast(ui, "Template not found; returning to templates", positive=False) + ui.navigate.to("/templates") + return None + + def _reload() -> None: + ui.navigate.to(f"/templates/edit?dir={template_dir}") + + def _on_save_manifest(manifest: Any) -> None: + try: + _, expected = authoring.read_manifest(template_dir) + authoring.write_manifest(template_dir, manifest, expected_stat=expected) + except ( + authoring.StaleEditError, + authoring.UnsafePathError, + authoring.TemplateAuthoringError, + ) as exc: + _show_toast(ui, f"Questions not saved: {exc}", positive=False) + return + _show_toast(ui, "Questions saved", positive=True) + _reload() + + def _on_save_content(rel: str, text: str) -> None: + try: + existing = template_dir / rel + expected = None + if existing.is_file(): + _, expected = authoring.read_content(existing) + authoring.write_content_file(template_dir, rel, text, expected_stat=expected) + except ( + authoring.StaleEditError, + authoring.UnsafePathError, + authoring.TemplateAuthoringError, + ) as exc: + _show_toast(ui, f"File not saved: {exc}", positive=False) + return + _show_toast(ui, f"Saved {rel}", positive=True) + _reload() + + def _on_upload(filename: str, data: bytes, render_as_template: bool) -> None: + try: + authoring.upload_file( + template_dir, filename, data, render_as_template=render_as_template + ) + except (authoring.UnsafePathError, authoring.TemplateAuthoringError) as exc: + _show_toast(ui, f"Upload failed: {exc}", positive=False) + return + _show_toast(ui, f"Uploaded {filename}", positive=True) + _reload() + + def _on_delete(rel: str) -> None: + try: + authoring.delete_path(template_dir, rel) + except (authoring.UnsafePathError, authoring.TemplateAuthoringError) as exc: + _show_toast(ui, f"Delete failed: {exc}", positive=False) + return + _show_toast(ui, f"Deleted {rel}", positive=True) + _reload() + + return template_editor_page.render_template_editor( + template_dir=template_dir, + on_save_manifest=_on_save_manifest, + on_save_content=_on_save_content, + on_upload=_on_upload, + on_delete=_on_delete, + on_back=lambda: ui.navigate.to("/templates"), ) @ui.page("/settings") @@ -1741,54 +1852,185 @@ def _templates_dir(deps: Any) -> Path | None: return Path(config.paths.templates_dir) -def _template_names(deps: Any, template_type: str) -> list[str]: - """List template directory names of ``template_type`` under templates_dir.""" +def _selected_template_path(deps: Any, state: Any) -> Path | None: + """Return the absolute path of the template the wizard state selected. + + Prefers the resolved ``state.selected_template_path`` the template step + stored -- this honours a per-instance (per-project / per-equipment) + override the resolver picked, so the pipeline renders the exact file the + operator saw (Backend Spec §5.0; design §4.3). Falls back to + ``templates_dir / selected_template`` when the state carries no resolved + path (e.g. the resolver was not wired), preserving the prior behaviour. + Returns ``None`` when no template is selected and no fallback is + derivable. + """ + resolved = getattr(state, "selected_template_path", None) + if resolved is not None: + return Path(resolved) + name = getattr(state, "selected_template", None) + if not name: + return None templates_dir = _templates_dir(deps) - if templates_dir is None: + return templates_dir / name if templates_dir is not None else None + + +def _template_locations(deps: Any) -> list[tuple[str, str]]: + """Return the manager's scope/location options as ``[(label, value)]``. + + Always offers ``("Global", "global")``; appends one + ``("Equipment ", "equipment:")`` per configured equipment so + the operator can browse / edit per-equipment template stores. The + global option is first so it stays the manager's default scope. + """ + locations: list[tuple[str, str]] = [("Global", "global")] + for equipment_id in _equipment_ids(deps): + locations.append((f"Equipment {equipment_id}", f"equipment:{equipment_id}")) + return locations + + +def _location_scan_dirs(deps: Any, loc: str) -> list[Path]: + """Return the template directories the manager lists for ``loc``. + + ``"global"`` (or anything unrecognised) lists the flat + ``paths.templates_dir``. ``"equipment:"`` lists that equipment's + type-segregated per-instance stores (``project/`` + ``run/`` under + ``//.exlab-wizard/templates/``) so editing reaches the + per-equipment templates the resolver layers in. Missing directories are + tolerated -- :func:`list_templates` returns nothing for them. + """ + from exlab_wizard.constants import TemplateType + from exlab_wizard.template.resolution import instance_template_dir + + if loc.startswith("equipment:"): + equipment_id = loc.split(":", 1)[1] + config = getattr(deps, "config", None) if deps is not None else None + local_root = config.paths.local_root if config is not None else "" + if equipment_id and local_root: + equipment_dir = Path(local_root) / equipment_id + return [ + instance_template_dir(equipment_dir, TemplateType.PROJECT.value), + instance_template_dir(equipment_dir, TemplateType.RUN.value), + ] + return [] + + templates_dir = _templates_dir(deps) + return [templates_dir] if templates_dir is not None else [] + + +def _template_names( + deps: Any, + template_type: str, + *, + equipment_id: str | None = None, + project_path: Path | None = None, +) -> list[str]: + """List the resolved template names a wizard should offer for ``template_type``. + + Resolves through :func:`exlab_wizard.template.resolution.resolve_template_chain` + so per-instance (per-equipment / per-project) templates layer over the + global ``templates_dir`` when an equipment / project context is known. + The wizard page handlers call this at render time before the operator + has picked equipment / project, so ``equipment_id`` / ``project_path`` + are typically ``None`` -- the resolver then returns the global templates + (identical to the pre-resolver behaviour), degrading gracefully. + """ + config = getattr(deps, "config", None) if deps is not None else None + if config is None: return [] try: - from exlab_wizard.ui.pages import templates as templates_page + from exlab_wizard.template.resolution import resolve_template_chain return [ summary.name - for summary in templates_page.list_templates(templates_dir, template_type=template_type) + for summary in resolve_template_chain( + config, + template_type=template_type, + equipment_id=equipment_id, + project_path=project_path, + ) ] except Exception as exc: _log.warning("template scan failed: %s", exc) return [] -def _template_questions_map(deps: Any, template_type: str) -> dict[str, Any]: - """Map each ``template_type`` template name to its parsed copier questions. +def _template_questions_map( + deps: Any, + template_type: str, + *, + equipment_id: str | None = None, + project_path: Path | None = None, +) -> dict[str, Any]: + """Map each resolved template name to its parsed copier questions. - Resolves every template through the real ``TemplateEngine`` so the - wizard's dynamic Variables step is driven by the actual - ``copier.yml`` question definitions. A template that fails to - resolve is skipped with a WARN -- its wizard entry simply shows no - variables. + Thin wrapper over :func:`_resolve_template_choices` that returns only the + questions map (kept as a stable name for existing callers / tests). """ - templates_dir = _templates_dir(deps) - if templates_dir is None: - return {} + return _resolve_template_choices( + deps, + template_type, + equipment_id=equipment_id, + project_path=project_path, + ).questions + + +def _resolve_template_choices( + deps: Any, + template_type: str, + *, + equipment_id: str | None = None, + project_path: Path | None = None, + run_scope: str | None = None, +) -> TemplateChoices: + """Resolve the templates a wizard should offer for one context. + + Resolves the chain through + :func:`exlab_wizard.template.resolution.resolve_template_chain` (so + per-instance templates layer over the global store when an equipment / + project context is known), then resolves each template through the real + ``TemplateEngine`` to extract its ``copier.yml`` questions. Returns a + :class:`TemplateChoices` bundling the offered names (nearest scope + first), the per-template questions (driving the dynamic Variables step), + and the per-template **absolute resolved path** (so the wizard's submit + renders the exact file listed, honouring a per-instance override rather + than re-deriving ``templates_dir / name``). A template that fails to + resolve is skipped with a WARN. + """ + from exlab_wizard.template.resolution import TemplateChoices + + config = getattr(deps, "config", None) if deps is not None else None + if config is None: + return TemplateChoices() try: from exlab_wizard.constants import TemplateType from exlab_wizard.template.copier_driver import TemplateEngine + from exlab_wizard.template.resolution import resolve_template_chain from exlab_wizard.ui.pages import templates as templates_page engine = TemplateEngine() scope = TemplateType(template_type) - result: dict[str, Any] = {} - for summary in templates_page.list_templates(templates_dir, template_type=template_type): + names: list[str] = [] + questions: dict[str, Any] = {} + paths: dict[str, Path] = {} + for summary in resolve_template_chain( + config, + template_type=template_type, + equipment_id=equipment_id, + project_path=project_path, + run_scope=run_scope, + ): try: resolved = engine.resolve(summary.path, scope) except Exception as exc: _log.warning("template %s failed to resolve: %s", summary.name, exc) continue - result[summary.name] = templates_page.template_questions(resolved.raw_manifest) - return result + names.append(summary.name) + questions[summary.name] = templates_page.template_questions(resolved.raw_manifest) + paths[summary.name] = summary.path + return TemplateChoices(names=names, questions=questions, paths=paths) except Exception as exc: _log.warning("template question scan failed: %s", exc) - return {} + return TemplateChoices() def _lims_catalogue_projects(deps: Any) -> list[dict[str, Any]]: @@ -1939,8 +2181,11 @@ async def _submit_project(deps: Any, state: Any, ui: Any) -> None: if controller is None: _show_toast(ui, "Project creation unavailable: controller not initialized", positive=False) return - templates_dir = _templates_dir(deps) - if templates_dir is None or not state.selected_template: + if not state.selected_template: + _show_toast(ui, "Pick a template before creating the project", positive=False) + return + template_path = _selected_template_path(deps, state) + if template_path is None: _show_toast(ui, "Pick a template before creating the project", positive=False) return @@ -1949,7 +2194,7 @@ async def _submit_project(deps: Any, state: Any, ui: Any) -> None: readme = state.readme_fields request = ProjectCreateRequest( equipment_id=state.selected_equipment or "", - template_path=templates_dir / state.selected_template, + template_path=template_path, lims_project={ "uid": str(uuid.uuid4()), "short_id": state.selected_lims_short_id or "", @@ -1972,8 +2217,11 @@ async def _submit_run(deps: Any, state: Any, run_kind: RunKind, ui: Any) -> None if controller is None: _show_toast(ui, "Run creation unavailable: controller not initialized", positive=False) return - templates_dir = _templates_dir(deps) - if templates_dir is None or not state.selected_template: + if not state.selected_template: + _show_toast(ui, "Pick a template before creating the run", positive=False) + return + template_path = _selected_template_path(deps, state) + if template_path is None: _show_toast(ui, "Pick a template before creating the run", positive=False) return @@ -1986,7 +2234,7 @@ async def _submit_run(deps: Any, state: Any, run_kind: RunKind, ui: Any) -> None request = RunCreateRequest( equipment_id=state.selected_equipment or "", project_name=state.selected_project_name or "", - template_path=templates_dir / state.selected_template, + template_path=template_path, run_kind=run_kind, variables=dict(state.template_variables), label=readme.get("label", ""), @@ -2037,14 +2285,33 @@ async def _run_creation( def _render_run_wizard(deps: Any, run_kind: RunKind, ui: Any) -> Any: + from exlab_wizard.constants import RunScope + from exlab_wizard.template.resolution import project_dir from exlab_wizard.ui.pages import wizard_run as wizard_run_page state = wizard_run_page.RunWizardState(run_kind=run_kind) + # Run templates are narrowed to the run kind's scope (a "test" run only + # sees test/both templates; experimental sees experimental/both). + scope = RunScope.TEST.value if run_kind is RunKind.TEST else RunScope.EXPERIMENTAL.value + config = getattr(deps, "config", None) if deps is not None else None + initial = _resolve_template_choices(deps, "run", run_scope=scope) + + def _resolve_run_templates(equipment_id: str | None, project_name: str | None) -> Any: + # Per-project then per-equipment run templates layer over the global + # store once the operator picks project + equipment (step 1, before + # the template step). Backend Spec §5.0 / §3.2. + proj_path = project_dir(config, equipment_id, project_name) if config is not None else None + return _resolve_template_choices( + deps, "run", equipment_id=equipment_id, project_path=proj_path, run_scope=scope + ) + return wizard_run_page.render_run_wizard( state=state, - templates=_template_names(deps, "run"), + templates=initial.names, equipment_ids=_equipment_ids(deps), - template_questions=_template_questions_map(deps, "run"), + template_questions=initial.questions, + template_paths=initial.paths, + on_resolve=_resolve_run_templates, on_submit=lambda submitted: _submit_run(deps, submitted, run_kind, ui), on_cancel=lambda: ui.navigate.to("/main"), ) diff --git a/src/exlab_wizard/ui/pages/template_editor.py b/src/exlab_wizard/ui/pages/template_editor.py new file mode 100644 index 0000000..93b7442 --- /dev/null +++ b/src/exlab_wizard/ui/pages/template_editor.py @@ -0,0 +1,419 @@ +"""Template authoring editor page (Frontend Spec §5 GUI). + +The author-time counterpart to :mod:`exlab_wizard.ui.pages.templates`: where +the manager lists / scaffolds templates, this page opens *one* template +directory and lets the operator edit its ``copier.yml`` (questions), edit its +inline text files, upload binary / rendered files, and delete files. Every +disk mutation lives in the callbacks (wired in :mod:`exlab_wizard.ui.mount`) +that delegate to the :mod:`exlab_wizard.template.authoring` service; this +module only *reads* the template (manifest + file list + lint findings) to +render and routes operator actions back to those callbacks. + +:func:`render_template_editor` follows the same pure-view-with-payload +contract as :func:`exlab_wizard.ui.pages.templates.render_template_manager`: +when NiceGUI is unavailable it returns a plain payload dict (so the view is +unit-testable headless); otherwise it builds the NiceGUI widget tree. Every +widget carries a ``data-testid`` for the browser e2e suite. +""" + +from __future__ import annotations + +from collections.abc import Callable +from typing import TYPE_CHECKING, Any + +from exlab_wizard.logging import get_logger +from exlab_wizard.template.authoring import list_files, read_content, read_manifest +from exlab_wizard.template.lint import has_errors, lint_template +from exlab_wizard.template.manifest import TemplateManifest, TemplateQuestion + +if TYPE_CHECKING: + from pathlib import Path + +__all__ = ["render_template_editor"] + +_log = get_logger(__name__) + +# The Copier question kinds the editor's type ``ui.select`` offers. ``choice`` +# is the editor-level pseudo-kind that ``TemplateQuestion`` normalises to (it +# round-trips through ``copier.yml`` as ``type: str`` + a ``choices`` block). +_QUESTION_KINDS: tuple[str, ...] = ("str", "int", "float", "bool", "choice") + + +def _build_payload(template_dir: Path) -> dict[str, Any]: + """Read ``template_dir`` and return the headless render payload. + + Loads the manifest + file list + lint findings via the authoring / + lint services. Used directly as the NiceGUI-unavailable return value + and as the data source for the NiceGUI branch, so the two stay in + lock-step. Tolerant of an unreadable manifest (falls back to empty + metadata) so a broken template still renders its lint banner. + """ + try: + manifest, _stat = read_manifest(template_dir) + except Exception as exc: # a broken manifest must still render its lint banner + _log.warning("template editor: failed to read manifest for %s: %s", template_dir, exc) + manifest = None + + files = list_files(template_dir) + findings = lint_template(template_dir) + return { + "template": template_dir.name, + "files": [entry["rel"] for entry in files], + "questions": [q.key for q in (manifest.questions if manifest else [])], + "findings": [f.code for f in findings], + "exlab_type": manifest.exlab_type if manifest else "", + "run_scope": manifest.exlab_run_scope if manifest else None, + } + + +def render_template_editor( + *, + template_dir: Path, + on_save_manifest: Callable[[TemplateManifest], None] | None = None, + on_save_content: Callable[[str, str], None] | None = None, + on_upload: Callable[[str, bytes, bool], None] | None = None, + on_delete: Callable[[str], None] | None = None, + on_back: Callable[[], None] | None = None, +) -> Any: + """Render the single-template authoring editor. + + The view loads ``template_dir`` (manifest, file list, lint findings) + and renders a header + lint banner, a file list with per-row edit / + delete actions, an inline text editor, a question form, and an upload + widget. All disk mutation is delegated to the callbacks: + + * ``on_save_manifest(manifest)`` -- persist a rebuilt + :class:`~exlab_wizard.template.manifest.TemplateManifest` (the + question form preserves the loaded manifest's non-question + ``_exlab_*`` fields and replaces only its questions). + * ``on_save_content(rel, text)`` -- write an edited text file. + * ``on_upload(filename, data, render_as_template)`` -- store an upload. + * ``on_delete(rel)`` -- remove a file. + * ``on_back()`` -- leave the editor. + + Every callback is optional; when ``None`` the corresponding action is + a no-op so the function still renders (and returns its payload + headless) without raising. + + Returns: + The NiceGUI root card, or -- when NiceGUI is unavailable -- the + headless payload dict from :func:`_build_payload`. + """ + payload = _build_payload(template_dir) + try: + from nicegui import ui + except Exception: + return payload + + # Re-read structured data for the rich render (the payload only carries + # the headless projection). A broken manifest falls back to an empty one + # so the lint banner + file list still render. + try: + manifest, _stat = read_manifest(template_dir) + except Exception as exc: # render the banner even if the manifest is unreadable + _log.warning("template editor: manifest unreadable for render: %s", exc) + manifest = TemplateManifest(exlab_type="", exlab_version="") + findings = lint_template(template_dir) + entries = list_files(template_dir) + + card = ( + ui.card() + .props('data-testid="te-card"') + .style( + "min-width: 820px; margin: 2rem auto; padding: var(--sp-6); " + "background: var(--color-surface); border-radius: var(--radius-md);" + ) + ) + with card: + _render_header(ui, template_dir.name) + _render_lint_banner(ui, findings) + _render_file_list(ui, template_dir, entries, on_save_content, on_delete) + _render_question_form(ui, manifest, on_save_manifest) + _render_upload(ui, on_upload) + if on_back is not None: + with ( + ui.row() + .classes("items-center w-full justify-end") + .style("gap: var(--sp-3); padding-top: var(--sp-4);") + ): + ui.button("Back", on_click=lambda _evt: on_back()).props( + 'flat data-testid="te-back"' + ) + return card + + +# --------------------------------------------------------------------------- +# NiceGUI section renderers (only reached when NiceGUI imports cleanly) +# --------------------------------------------------------------------------- + + +def _render_header(ui: Any, name: str) -> None: + """Render the template-name header.""" + ui.label(f"Editing template: {name}").props('data-testid="te-title"').style( + "font-family: var(--font-display); font-size: var(--text-lg); " + "font-weight: 600; color: var(--color-heading);" + ) + + +def _render_lint_banner(ui: Any, findings: list[Any]) -> None: + """Render the lint banner: errors red, warnings amber, else a clean note.""" + errors = [f for f in findings if f.severity == "error"] + warns = [f for f in findings if f.severity == "warn"] + if has_errors(findings): + banner = ui.column().props('data-testid="te-lint-errors"') + with banner: + ui.label(f"{len(errors)} error(s) -- template will not load:").style( + "color: var(--color-danger, #d33); font-weight: 600;" + ) + for finding in errors: + ui.label(f"• {finding.message}").style("color: var(--color-danger, #d33);") + if warns: + banner = ui.column().props('data-testid="te-lint-warns"') + with banner: + ui.label(f"{len(warns)} warning(s):").style( + "color: var(--color-warning, #b8860b); font-weight: 600;" + ) + for finding in warns: + ui.label(f"• {finding.message}").style("color: var(--color-warning, #b8860b);") + if not findings: + ui.label("No lint findings.").props('data-testid="te-lint-clean"').style( + "color: var(--color-muted);" + ) + + +def _render_file_list( + ui: Any, + template_dir: Path, + entries: list[dict], + on_save_content: Callable[[str, str], None] | None, + on_delete: Callable[[str], None] | None, +) -> None: + """Render the file list with per-row edit / delete actions + inline editor. + + Each editable text file gets an Edit button that loads its content into + a shared inline editor (``ui.codemirror`` if available, else + ``ui.textarea``); the editor's Save button calls ``on_save_content``. + Every non-``copier.yml`` row gets a Delete button calling ``on_delete``. + """ + ui.label("Files").style("font-weight: 600; padding-top: var(--sp-3);") + + # Shared inline editor + its state, declared up front so the per-row + # Edit handlers can target it. The editor widget is whichever of + # codemirror / textarea NiceGUI provides. + editor_state: dict[str, Any] = {"rel": None} + editor_factory = getattr(ui, "codemirror", None) or ui.textarea + editor = editor_factory(value="").props('data-testid="te-editor"') + editor.style("width: 100%; min-height: 12rem; display: none;") + + def _open(rel: str) -> None: + path = template_dir / rel + try: + text, _stat = read_content(path) + except Exception as exc: # surface unreadable files instead of crashing the page + _log.warning("template editor: cannot open %s: %s", rel, exc) + return + editor_state["rel"] = rel + editor.value = text + editor.style("width: 100%; min-height: 12rem; display: block;") + + def _save() -> None: + rel = editor_state["rel"] + if rel is None or on_save_content is None: + return + on_save_content(rel, editor.value or "") + + for entry in entries: + rel = entry["rel"] + with ( + ui.row() + .classes("items-center w-full") + .props(f'data-testid="te-file-{rel}"') + .style("gap: var(--sp-2);") + ): + label = rel + ("/" if entry["is_dir"] else "") + ui.label(label).style("color: var(--color-body); flex: 1;") + if entry["editable"]: + ui.button("Edit", on_click=lambda _evt, r=rel: _open(r)).props( + f'flat dense data-testid="te-edit-{rel}"' + ) + if not entry["is_dir"]: + ui.button( + "Delete", + on_click=lambda _evt, r=rel: on_delete and on_delete(r), + ).props(f'flat dense color=negative data-testid="te-delete-{rel}"') + + ui.button("Save file", on_click=lambda _evt: _save()).props( + 'color=primary data-testid="te-save-content"' + ) + + +def _render_question_form( + ui: Any, + manifest: TemplateManifest, + on_save_manifest: Callable[[TemplateManifest], None] | None, +) -> None: + """Render the editable question form bound to the manifest's questions. + + Each question is one row of widgets (key / kind / default / help / + choices / secret). Add / Remove buttons grow / shrink the row list; + Save Questions rebuilds a :class:`TemplateManifest` -- preserving every + non-question ``_exlab_*`` field from ``manifest`` and replacing only its + questions -- and calls ``on_save_manifest``. + """ + ui.label("Questions").style("font-weight: 600; padding-top: var(--sp-3);") + + # Each row's widgets are stashed in a dict so Save can read their values + # back. ``rows`` is the live working set; Remove drops a row's widgets. + rows: list[dict[str, Any]] = [] + container = ui.column().props('data-testid="te-questions"').style("width: 100%;") + + def _add_row(question: TemplateQuestion | None = None) -> None: + with container: + row = ui.row().classes("items-center w-full").style("gap: var(--sp-2);") + with row: + key_w = ui.input(label="Key", value=(question.key if question else "")).props( + 'dense data-testid="te-q-key"' + ) + kind_w = ui.select( + list(_QUESTION_KINDS), + value=(question.kind if question else "str"), + label="Type", + ).props('dense data-testid="te-q-kind"') + default_w = ui.input( + label="Default", + value=( + "" if question is None or question.default is None else str(question.default) + ), + ).props('dense data-testid="te-q-default"') + help_w = ui.input(label="Help", value=(question.help if question else "")).props( + 'dense data-testid="te-q-help"' + ) + choices_w = ui.input( + label="Choices (comma-separated)", + value=(", ".join(str(c) for c in question.choices) if question else ""), + ).props('dense data-testid="te-q-choices"') + secret_w = ui.checkbox( + "Secret", value=(bool(question.secret) if question else False) + ).props('data-testid="te-q-secret"') + record = { + "row": row, + "key": key_w, + "kind": kind_w, + "default": default_w, + "help": help_w, + "choices": choices_w, + "secret": secret_w, + } + + def _remove(_evt: Any = None, rec: dict[str, Any] = record) -> None: + rec["row"].delete() + if rec in rows: + rows.remove(rec) + + with row: + ui.button("Remove", on_click=_remove).props( + 'flat dense color=negative data-testid="te-q-remove"' + ) + rows.append(record) + + for existing in manifest.questions: + _add_row(existing) + + def _save() -> None: + if on_save_manifest is None: + return + questions = [_question_from_row(rec) for rec in rows] + questions = [q for q in questions if q.key] + rebuilt = TemplateManifest( + exlab_type=manifest.exlab_type, + exlab_version=manifest.exlab_version, + exlab_run_scope=manifest.exlab_run_scope, + description=manifest.description, + plugins=list(manifest.plugins), + readme_fields=list(manifest.readme_fields), + questions=questions, + min_copier_version=manifest.min_copier_version, + answers_file=manifest.answers_file, + ) + on_save_manifest(rebuilt) + + with ( + ui.row().classes("items-center w-full").style("gap: var(--sp-3); padding-top: var(--sp-2);") + ): + ui.button("Add question", on_click=lambda _evt: _add_row()).props( + 'flat data-testid="te-q-add"' + ) + ui.button("Save questions", on_click=lambda _evt: _save()).props( + 'color=primary data-testid="te-save-questions"' + ) + + +def _question_from_row(rec: dict[str, Any]) -> TemplateQuestion: + """Build a :class:`TemplateQuestion` from one form row's widget values. + + The default string is coerced toward the selected kind (int / float / + bool) so a round-trip through ``copier.yml`` keeps the declared type; + an uncoercible value falls back to the raw string. ``choice`` questions + split the comma-text choices field; other kinds carry no choices. + """ + key = (rec["key"].value or "").strip() + kind = rec["kind"].value or "str" + raw_default = rec["default"].value or "" + help_text = rec["help"].value or "" + secret = bool(rec["secret"].value) + + choices: tuple[Any, ...] = () + if kind == "choice": + choices = tuple( + piece.strip() for piece in str(rec["choices"].value or "").split(",") if piece.strip() + ) + + default = _coerce_default(raw_default, kind) + return TemplateQuestion( + key=key, kind=kind, default=default, choices=choices, help=help_text, secret=secret + ) + + +def _coerce_default(raw: str, kind: str) -> Any: + """Coerce a string default toward ``kind`` (``None`` for an empty string).""" + text = raw.strip() + if not text: + return None + try: + if kind == "int": + return int(text) + if kind == "float": + return float(text) + if kind == "bool": + return text.lower() in {"1", "true", "yes", "on"} + except ValueError: + return text + return text + + +def _render_upload(ui: Any, on_upload: Callable[[str, bytes, bool], None] | None) -> None: + """Render the upload widget + a "render as template" checkbox. + + ``ui.upload`` is guarded (not yet a hard dependency in this codebase); + when absent the section is skipped. On upload the handler reads the + NiceGUI upload event's bytes and calls ``on_upload(name, data, flag)``. + """ + ui.label("Upload file").style("font-weight: 600; padding-top: var(--sp-3);") + render_flag = ui.checkbox("Render as template (.jinja)", value=False).props( + 'data-testid="te-upload-render"' + ) + + upload_widget = getattr(ui, "upload", None) + if upload_widget is None: + ui.label("Upload unavailable in this build.").props('data-testid="te-upload-unavailable"') + return + + def _on_event(event: Any) -> None: + if on_upload is None: + return + name = getattr(event, "name", "") or "" + content = getattr(event, "content", None) + data = content.read() if content is not None and hasattr(content, "read") else b"" + on_upload(name, data, bool(render_flag.value)) + + upload_widget(on_upload=_on_event, auto_upload=True).props('data-testid="te-upload"') diff --git a/src/exlab_wizard/ui/pages/templates.py b/src/exlab_wizard/ui/pages/templates.py index f708795..dd3c2c9 100644 --- a/src/exlab_wizard/ui/pages/templates.py +++ b/src/exlab_wizard/ui/pages/templates.py @@ -27,6 +27,13 @@ from exlab_wizard.constants import COPIER_MANIFEST_NAME, RunScope, TemplateType from exlab_wizard.logging import get_logger +# ``TemplateQuestion`` / ``template_questions`` live in ``template.manifest`` +# (a non-UI module) so the typed manifest model can reuse them without +# importing this NiceGUI page. Re-exported here so existing callers +# (``from exlab_wizard.ui.pages.templates import TemplateQuestion``) keep +# working unchanged. +from exlab_wizard.template.manifest import TemplateQuestion, template_questions + __all__ = [ "TemplateQuestion", "TemplateSummary", @@ -62,86 +69,6 @@ class TemplateSummary: description: str -@dataclass(frozen=True) -class TemplateQuestion: - """One Copier question parsed from a template's ``copier.yml``. - - ``kind`` is normalised to the widget family the wizard renders: - ``str`` / ``int`` / ``float`` / ``bool`` / ``choice``. ``choices`` - is populated only for ``choice`` questions. ``secret`` flags a - password-style ``str`` input. - """ - - key: str - kind: str - default: Any = None - choices: tuple[Any, ...] = () - help: str = "" - secret: bool = False - - -# Copier reserves ``_``-prefixed manifest keys for itself; everything -# else under the top level is an operator-answerable question. -_COPIER_TYPE_TO_KIND: dict[str, str] = { - "str": "str", - "int": "int", - "float": "float", - "bool": "bool", - "yaml": "str", - "json": "str", -} - - -def template_questions(raw_manifest: dict[str, Any]) -> list[TemplateQuestion]: - """Parse the operator-answerable questions out of a ``copier.yml`` body. - - Handles both Copier question forms: - - * **long form** -- ``key: {type: ..., default: ..., choices: ...}`` - * **short form** -- ``key: `` (the scalar is the default; - the type is inferred from it) - - ``_``-prefixed keys (Copier / ``_exlab_*`` metadata) are skipped. - Questions carrying a ``when`` clause are still returned -- the - wizard renders them unconditionally for v1. - """ - questions: list[TemplateQuestion] = [] - for key, spec in raw_manifest.items(): - if key.startswith("_"): - continue - if isinstance(spec, dict): - raw_type = str(spec.get("type", "str")) - raw_choices = spec.get("choices") - choices: tuple[Any, ...] = () - if isinstance(raw_choices, dict): - choices = tuple(raw_choices.values()) - elif isinstance(raw_choices, list): - choices = tuple(raw_choices) - kind = "choice" if choices else _COPIER_TYPE_TO_KIND.get(raw_type, "str") - questions.append( - TemplateQuestion( - key=key, - kind=kind, - default=spec.get("default"), - choices=choices, - help=str(spec.get("help", "")), - secret=bool(spec.get("secret", False)), - ) - ) - else: - # Short form: the scalar is the default; infer the kind. - if isinstance(spec, bool): - kind = "bool" - elif isinstance(spec, int): - kind = "int" - elif isinstance(spec, float): - kind = "float" - else: - kind = "str" - questions.append(TemplateQuestion(key=key, kind=kind, default=spec)) - return questions - - def list_templates( templates_dir: Path, *, @@ -202,43 +129,23 @@ def create_template( Returns the new template's root directory. Raises ``ValueError`` on an empty / duplicate name, an unknown ``template_type``, or a run template missing its ``run_scope``. + + The scaffold logic lives in + :func:`exlab_wizard.template.authoring.create_template_dir` (the + non-UI authoring service); this thin wrapper preserves the historical + page-level signature and is imported lazily to avoid an import cycle. """ - clean_name = name.strip() - if not clean_name: - msg = "template name must not be empty" - raise ValueError(msg) - if template_type not in {t.value for t in TemplateType}: - msg = f"unknown template type {template_type!r}" - raise ValueError(msg) - if template_type == TemplateType.RUN.value: - if run_scope is None: - msg = "run templates require a run_scope" - raise ValueError(msg) - if run_scope not in {s.value for s in RunScope}: - msg = f"unknown run_scope {run_scope!r}" - raise ValueError(msg) - - root = Path(templates_dir) / clean_name - if root.exists(): - msg = f"a template named {clean_name!r} already exists" - raise ValueError(msg) - root.mkdir(parents=True) - - manifest: dict[str, Any] = { - "_min_copier_version": "9.0", - "_exlab_type": template_type, - "_exlab_version": "1.0", - "_exlab_description": description.strip(), - } - if template_type == TemplateType.RUN.value: - manifest["_exlab_run_scope"] = run_scope - (root / COPIER_MANIFEST_NAME).write_text( - yaml.safe_dump(manifest, sort_keys=False), - encoding="utf-8", + # Imported lazily so this NiceGUI page module does not pull in the + # authoring service (and its deps) at import time. + from exlab_wizard.template.authoring import create_template_dir + + return create_template_dir( + Path(templates_dir), + name=name, + template_type=template_type, + description=description, + run_scope=run_scope, ) - (root / _SCAFFOLD_CONTENT_NAME).write_text(_SCAFFOLD_CONTENT_BODY, encoding="utf-8") - _log.info("scaffolded %s template %r at %s", template_type, clean_name, root) - return root def render_question_field( @@ -298,17 +205,34 @@ def render_template_manager( templates: list[TemplateSummary], on_create: Callable[[str, str, str, str | None], None] | None = None, on_back: Callable[[], None] | None = None, + on_edit: Callable[[str], None] | None = None, + locations: list[tuple[str, str]] | None = None, + on_location_change: Callable[[str], None] | None = None, ) -> Any: """Render the template manager: existing-template list + create form. ``on_create`` is invoked with ``(name, template_type, description, run_scope)`` when the operator submits the create form; ``run_scope`` is ``None`` for non-run templates. + + ``on_edit`` (optional) is invoked with a template name when the + operator clicks that row's Edit button -- the caller routes it to the + template editor page. + + ``locations`` (optional) is a ``[(label, value), ...]`` scope list (at + minimum ``("Global", ...)`` plus one entry per configured equipment / + project); when given, a selector is rendered at the top and + ``on_location_change(value)`` is invoked when the operator switches + scope (the caller re-renders the manager scoped to that location). All + three new parameters are optional and default ``None`` so existing + callers keep working unchanged. """ payload = { "templates": [t.name for t in templates], "count": len(templates), } + if locations is not None: + payload["locations"] = list(locations) try: from nicegui import ui except Exception: @@ -328,13 +252,37 @@ def render_template_manager( "font-weight: 600; color: var(--color-heading);" ) + # Scope / location selector ----------------------------------------- + if locations: + label_by_value = {value: label for label, value in locations} + location_select = ui.select( + {value: label for label, value in locations}, + value=locations[0][1], + label="Location", + ).props('data-testid="templates-location"') + if on_location_change is not None: + location_select.on_value_change( + lambda e: on_location_change(e.value) if e.value in label_by_value else None + ) + # Existing templates ------------------------------------------------ if templates: for summary in templates: scope = f" [{summary.run_scope}]" if summary.run_scope else "" - ui.label(f"{summary.name} -- {summary.template_type}{scope}").props( - 'data-testid="template-row"' - ).style("color: var(--color-body);") + with ( + ui.row() + .classes("items-center w-full") + .props('data-testid="template-row"') + .style("gap: var(--sp-2);") + ): + ui.label(f"{summary.name} -- {summary.template_type}{scope}").style( + "color: var(--color-body); flex: 1;" + ) + if on_edit is not None: + ui.button( + "Edit", + on_click=lambda _evt, n=summary.name: on_edit(n), + ).props(f'flat dense data-testid="template-edit-{summary.name}"') else: ui.label("No templates yet. Create one below.").props( 'data-testid="templates-empty"' diff --git a/src/exlab_wizard/ui/pages/wizard_project.py b/src/exlab_wizard/ui/pages/wizard_project.py index b5479c2..e1e762b 100644 --- a/src/exlab_wizard/ui/pages/wizard_project.py +++ b/src/exlab_wizard/ui/pages/wizard_project.py @@ -24,9 +24,12 @@ from typing import TYPE_CHECKING, Any from exlab_wizard.logging import get_logger +from exlab_wizard.template.resolution import TemplateChoices, reconcile_selection from exlab_wizard.ui.components import session_progress if TYPE_CHECKING: + from pathlib import Path + from exlab_wizard.ui.pages.templates import TemplateQuestion _log = get_logger(__name__) @@ -66,6 +69,11 @@ class ProjectWizardState: lims_project_name: str = "" selected_lims_source: str = "manual" selected_template: str | None = None + # Absolute path of the resolved template the operator picked, so + # ``on_submit`` renders the exact file the wizard listed -- a + # per-equipment project template wins over a same-named global one and + # the pipeline must not re-pick (Backend Spec §5.0; design §4.3). + selected_template_path: Path | None = None selected_equipment: str | None = None template_variables: dict[str, Any] = field(default_factory=dict) readme_fields: dict[str, str] = field(default_factory=dict) @@ -134,6 +142,8 @@ def render_project_wizard( templates: list[str] | None = None, equipment_ids: list[str] | None = None, template_questions: dict[str, list[TemplateQuestion]] | None = None, + template_paths: dict[str, Path] | None = None, + on_resolve: Callable[[str | None], TemplateChoices] | None = None, lims_projects: list[dict[str, Any]] | None = None, on_submit: Callable[[ProjectWizardState], Any] | None = None, on_cancel: Callable[[], None] | None = None, @@ -144,28 +154,44 @@ def render_project_wizard( operator can pick from; ``equipment_ids`` is the configured equipment list; ``template_questions`` maps each template name to its parsed ``copier.yml`` questions (drives the dynamic Variables - step); ``lims_projects`` is the cache / offline-catalogue project + step); ``template_paths`` maps each name to the resolved absolute + source path; ``lims_projects`` is the cache / offline-catalogue project list backing the LIMS project picker. Each step binds real inputs into ``state`` so the confirm step's ``on_submit`` sees a fully populated :class:`ProjectWizardState`. + ``on_resolve`` enables per-instance template resolution: when supplied, + the template step re-resolves the offered project templates from the + operator's chosen equipment, so a per-equipment project template + shadows a same-named global one. It is called with ``(equipment_id)`` + and returns a :class:`TemplateChoices`. Because the equipment step + (step 3) follows the template step (step 2), the template panel + re-resolves whenever equipment changes (and reflects it on a step-back). + When ``on_resolve`` is ``None`` the static ``templates`` list is used + unchanged (the pre-resolver behaviour). + Returns the NiceGUI dialog (or, in tests, a payload describing the rendered steps). """ s = state or ProjectWizardState() - template_choices = list(templates or []) + initial = TemplateChoices( + names=list(templates or []), + questions=dict(template_questions or {}), + paths=dict(template_paths or {}), + ) equipment_choices = list(equipment_ids or []) - questions_map = template_questions or {} project_rows = list(lims_projects or []) + # Mutable holder so the refreshable panels read the latest resolution. + choices = {"current": initial} payload = { "steps": PROJECT_WIZARD_STEPS, "active": s.active_step, "can_advance": can_advance(s), - "templates": template_choices, + "templates": initial.names, "equipment_ids": equipment_choices, "lims_projects": [row.get("short_id") for row in project_rows], - "template_questions": {k: [q.key for q in v] for k, v in questions_map.items()}, + "template_questions": {k: [q.key for q in v] for k, v in initial.questions.items()}, } try: @@ -175,10 +201,30 @@ def render_project_wizard( from exlab_wizard.ui.pages.templates import render_question_field + def _reresolve() -> None: + """Refresh the offered templates from the current equipment. + + A surviving selection has its resolved path **re-derived** from the + new context: a same-named per-equipment template shadows the global + one at a *different* path, and the rebuilt ``ui.select`` keeps the + value without re-firing ``on_value_change`` -- so without this the + stored path (and thus what submit renders) would go stale. A + selection whose name no longer appears is cleared outright, along + with its now-orphaned variables. + """ + if on_resolve is None: + return + choices["current"] = on_resolve(s.selected_equipment) + name, path, dropped = reconcile_selection(choices["current"], s.selected_template) + s.selected_template = name + s.selected_template_path = path + if dropped: + s.template_variables.clear() + @ui.refreshable def _variables_panel() -> None: """Dynamic Copier-variable form for the currently-picked template.""" - questions = questions_map.get(s.selected_template or "", []) + questions = choices["current"].questions.get(s.selected_template or "", []) if not questions: ui.label("This template declares no variables; Copier defaults are used.").props( 'data-testid="wizard-project-variables-empty"' @@ -189,6 +235,25 @@ def _variables_panel() -> None: question, s.template_variables, testid_prefix="wizard-project-var" ) + @ui.refreshable + def _template_panel() -> None: + """Project-template select, re-resolved from the chosen equipment.""" + _reresolve() + names = choices["current"].names + + def _on_template(event: Any) -> None: + s.selected_template = event.value or None + s.selected_template_path = ( + choices["current"].paths.get(event.value) if event.value else None + ) + _variables_panel.refresh() + + ui.select( + names, + value=s.selected_template if s.selected_template in names else None, + label="Project template", + ).props('data-testid="wizard-project-template"').on_value_change(_on_template) + card = ( ui.card() .props('data-testid="wizard-project-card"') @@ -218,14 +283,15 @@ def _variables_panel() -> None: ui.label(_step_helper_text(step_id, s)).style("color: var(--color-body);") if step_id == "variables": _variables_panel() + elif step_id == "template": + _template_panel() else: _render_project_step_fields( step_id, s, - template_choices, equipment_choices, project_rows, - on_template_change=_variables_panel.refresh, + on_equipment_change=_template_panel.refresh, ) if step_id == "confirm": @@ -285,18 +351,19 @@ async def _on_primary( def _render_project_step_fields( step_id: str, state: ProjectWizardState, - templates: list[str], equipment_ids: list[str], lims_projects: list[dict[str, Any]], *, - on_template_change: Callable[..., Any], + on_equipment_change: Callable[..., Any], ) -> None: """Render the bound input fields for one project-wizard step. Each widget two-way binds into ``state`` so values entered on an earlier step survive while the operator moves through the stepper. - The "variables" step is rendered by the caller's refreshable panel, - not here. + The "variables" and "template" steps are rendered by the caller's + refreshable panels, not here. ``on_equipment_change`` is called when the + operator changes equipment so the template panel re-resolves its + per-equipment chain. """ from nicegui import ui @@ -361,24 +428,16 @@ def _reveal_manual(_evt: Any) -> None: gate.set_visibility(False) gate.on_click(_reveal_manual) - elif step_id == "template": - - def _on_template(event: Any) -> None: - state.selected_template = event.value or None - on_template_change() - - ui.select( - templates, - value=state.selected_template if state.selected_template in templates else None, - label="Project template", - ).props('data-testid="wizard-project-template"').on_value_change(_on_template) elif step_id == "equipment": ui.select( equipment_ids, value=(state.selected_equipment if state.selected_equipment in equipment_ids else None), label="Equipment", ).props('data-testid="wizard-project-equipment"').on_value_change( - lambda e: setattr(state, "selected_equipment", e.value or None) + lambda e: ( + setattr(state, "selected_equipment", e.value or None), + on_equipment_change(), + ) ) elif step_id == "readme": for field_id, label in ( diff --git a/src/exlab_wizard/ui/pages/wizard_run.py b/src/exlab_wizard/ui/pages/wizard_run.py index eddcf6a..88a6c35 100644 --- a/src/exlab_wizard/ui/pages/wizard_run.py +++ b/src/exlab_wizard/ui/pages/wizard_run.py @@ -24,9 +24,12 @@ from exlab_wizard.constants import RunKind from exlab_wizard.logging import get_logger from exlab_wizard.paths import run_dir_stem +from exlab_wizard.template.resolution import TemplateChoices, reconcile_selection from exlab_wizard.ui.components import mode_badge, session_progress if TYPE_CHECKING: + from pathlib import Path + from exlab_wizard.ui.pages.templates import TemplateQuestion _log = get_logger(__name__) @@ -60,6 +63,13 @@ class RunWizardState: selected_project_name: str | None = None selected_equipment: str | None = None selected_template: str | None = None + # Absolute path of the resolved template the operator picked. The + # template select stores it here so ``on_submit`` renders the exact + # file the wizard listed -- a per-instance (per-project / per-equipment) + # template wins over a same-named global one, and the pipeline must not + # re-pick (Backend Spec §5.0; design §4.3). ``None`` until a template + # is chosen. + selected_template_path: Path | None = None template_variables: dict[str, Any] = field(default_factory=dict) readme_fields: dict[str, str] = field(default_factory=dict) validator_findings: list[dict[str, Any]] = field(default_factory=list) @@ -145,6 +155,8 @@ def render_run_wizard( templates: list[str] | None = None, equipment_ids: list[str] | None = None, template_questions: dict[str, list[TemplateQuestion]] | None = None, + template_paths: dict[str, Path] | None = None, + on_resolve: Callable[[str | None, str | None], TemplateChoices] | None = None, on_submit: Callable[[RunWizardState], Any] | None = None, on_cancel: Callable[[], None] | None = None, ) -> Any: @@ -153,14 +165,32 @@ def render_run_wizard( ``templates`` lists run-scope template names appropriate to the run kind; ``equipment_ids`` is the configured equipment list; ``template_questions`` maps each template name to its parsed - ``copier.yml`` questions (drives the dynamic Variables step). Each - step binds real inputs into ``state`` so the confirm step's - ``on_submit`` sees a fully-populated :class:`RunWizardState`. + ``copier.yml`` questions (drives the dynamic Variables step); + ``template_paths`` maps each template name to the absolute path of the + resolved source. Each step binds real inputs into ``state`` so the + confirm step's ``on_submit`` sees a fully-populated + :class:`RunWizardState`. + + ``on_resolve`` enables per-instance template resolution: when supplied, + the template step re-resolves the offered templates from the operator's + current project + equipment selection (step 1 precedes the template + step, so both are known). It is called with + ``(equipment_id, project_name)`` and returns a :class:`TemplateChoices` + of the names / questions / absolute paths for that context, so a + per-project or per-equipment run template shadows a same-named global + one. When ``on_resolve`` is ``None`` the static ``templates`` list is + used unchanged (the pre-resolver behaviour). """ - template_choices = list(templates or []) + initial = TemplateChoices( + names=list(templates or []), + questions=dict(template_questions or {}), + paths=dict(template_paths or {}), + ) equipment_choices = list(equipment_ids or []) - questions_map = template_questions or {} + # Mutable holder so the refreshable template / variables panels read the + # latest resolution after the operator changes project / equipment. + choices = {"current": initial} payload = { "title": title_text(state), "mode_badge": mode_badge.mode_badge_props(state.run_kind), @@ -168,9 +198,9 @@ def render_run_wizard( "active": state.active_step, "primary_label": primary_button_label(state), "primary_color": primary_button_color(state), - "templates": template_choices, + "templates": initial.names, "equipment_ids": equipment_choices, - "template_questions": {k: [q.key for q in v] for k, v in questions_map.items()}, + "template_questions": {k: [q.key for q in v] for k, v in initial.questions.items()}, } try: @@ -180,10 +210,30 @@ def render_run_wizard( from exlab_wizard.ui.pages.templates import render_question_field + def _reresolve() -> None: + """Refresh the offered templates from the current project/equipment. + + A surviving selection has its resolved path **re-derived** from the + new context: a same-named per-project/per-equipment template shadows + the global one at a *different* path, and the rebuilt ``ui.select`` + keeps the value without re-firing ``on_value_change`` -- so without + this the stored path (and thus what submit renders) would go stale. + A selection whose name no longer appears is cleared outright, along + with its now-orphaned variables. + """ + if on_resolve is None: + return + choices["current"] = on_resolve(state.selected_equipment, state.selected_project_name) + name, path, dropped = reconcile_selection(choices["current"], state.selected_template) + state.selected_template = name + state.selected_template_path = path + if dropped: + state.template_variables.clear() + @ui.refreshable def _variables_panel() -> None: """Dynamic Copier-variable form for the currently-picked template.""" - questions = questions_map.get(state.selected_template or "", []) + questions = choices["current"].questions.get(state.selected_template or "", []) if not questions: ui.label("This template declares no variables; Copier defaults are used.").props( 'data-testid="wizard-run-variables-empty"' @@ -194,6 +244,31 @@ def _variables_panel() -> None: question, state.template_variables, testid_prefix="wizard-run-var" ) + @ui.refreshable + def _template_panel() -> None: + """Run-template select, re-resolved from the current project/equipment. + + Refreshed when the operator changes project or equipment on step 1 + so a per-project / per-equipment run template appears (and shadows a + same-named global one). Picking a template stores its absolute + resolved path on the state so submit renders the exact file listed. + """ + _reresolve() + names = choices["current"].names + + def _on_template(event: Any) -> None: + state.selected_template = event.value or None + state.selected_template_path = ( + choices["current"].paths.get(event.value) if event.value else None + ) + _variables_panel.refresh() + + ui.select( + names, + value=state.selected_template if state.selected_template in names else None, + label="Run template", + ).props('data-testid="wizard-run-template"').on_value_change(_on_template) + card = ( ui.card() .props(f'data-testid="wizard-run-card-{state.run_kind}"') @@ -224,13 +299,14 @@ def _variables_panel() -> None: ui.label(_step_helper_text(step_id, state)).style("color: var(--color-body);") if step_id == "variables": _variables_panel() + elif step_id == "template": + _template_panel() else: _render_run_step_fields( step_id, state, - template_choices, equipment_choices, - on_template_change=_variables_panel.refresh, + on_project_equipment_change=_template_panel.refresh, ) if step_id == "confirm": @@ -294,15 +370,16 @@ async def _on_primary( def _render_run_step_fields( step_id: str, state: RunWizardState, - templates: list[str], equipment_ids: list[str], *, - on_template_change: Callable[..., Any], + on_project_equipment_change: Callable[..., Any], ) -> None: """Render the bound input fields for one run-wizard step. - The "variables" step is rendered by the caller's refreshable panel, - not here. + The "variables" and "template" steps are rendered by the caller's + refreshable panels, not here. ``on_project_equipment_change`` is called + when the operator changes the parent project or equipment so the + template panel re-resolves its per-instance chain. """ from nicegui import ui @@ -311,26 +388,21 @@ def _render_run_step_fields( label="Parent project name", value=state.selected_project_name or "", ).props('data-testid="wizard-run-project-name"').on_value_change( - lambda e: setattr(state, "selected_project_name", e.value or None) + lambda e: ( + setattr(state, "selected_project_name", e.value or None), + on_project_equipment_change(), + ) ) ui.select( equipment_ids, value=(state.selected_equipment if state.selected_equipment in equipment_ids else None), label="Equipment", ).props('data-testid="wizard-run-equipment"').on_value_change( - lambda e: setattr(state, "selected_equipment", e.value or None) + lambda e: ( + setattr(state, "selected_equipment", e.value or None), + on_project_equipment_change(), + ) ) - elif step_id == "template": - - def _on_template(event: Any) -> None: - state.selected_template = event.value or None - on_template_change() - - ui.select( - templates, - value=state.selected_template if state.selected_template in templates else None, - label="Run template", - ).props('data-testid="wizard-run-template"').on_value_change(_on_template) elif step_id == "readme": for field_id, label in ( ("label", "Label"), diff --git a/tests/integration/controller/test_creation_provenance.py b/tests/integration/controller/test_creation_provenance.py new file mode 100644 index 0000000..e75161e --- /dev/null +++ b/tests/integration/controller/test_creation_provenance.py @@ -0,0 +1,181 @@ +"""Integration test for the frozen template provenance copy (Phase 3b). + +Drives a full project + run creation through :class:`CreationController` +and asserts that the run's instance directory carries a verbatim copy of +the run template source under ``.exlab-wizard/templates/run//``, +that ``creation.json`` records the instance-relative provenance path, and +that the copied ``copier.yml`` / ``.jinja`` files do NOT trip the +post-validate gate (the session reaches DONE and is not +``BLOCKED_BY_VALIDATION``). +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from pathlib import Path +from typing import Any + +import msgspec + +from exlab_wizard.api.schemas import CreationJson +from exlab_wizard.cache.creation_writer import CreationWriter +from exlab_wizard.cache.equipment import EquipmentCacheWriter +from exlab_wizard.config.models import ( + Config, + EquipmentConfig, + OperatorsConfig, + PathsConfig, + READMEConfig, +) +from exlab_wizard.constants import ( + CACHE_DIR_NAME, + CREATION_JSON_NAME, + CREATION_JSON_VERSION, + RunKind, + SyncStatus, +) +from exlab_wizard.controller import ( + CreationController, + NoOpNASSync, + NoOpReadmeGenerator, + ProjectCreateRequest, + RunCreateRequest, + SessionState, + SessionStore, +) +from exlab_wizard.template.copier_driver import TemplateEngine +from exlab_wizard.validator.engine import Validator + +FIXTURE_TEMPLATES = Path(__file__).parent.parent.parent / "fixtures" / "templates" +FIXTURE_PLUGINS = Path(__file__).parent.parent.parent / "fixtures" / "plugins" + +RUN_TEMPLATE = FIXTURE_TEMPLATES / "run_basic_experimental" + + +# --------------------------------------------------------------------------- +# Helpers (mirror tests/integration/controller/test_creation_flow.py) +# --------------------------------------------------------------------------- + + +def _build_config(local_root: Path) -> Config: + return Config( + paths=PathsConfig( + templates_dir=str(FIXTURE_TEMPLATES), + plugin_dir=str(FIXTURE_PLUGINS), + local_root=str(local_root), + ), + equipment=[ + EquipmentConfig( + id="EQ1", + label="Equipment 1", + local_root=str(local_root), + nas_root="/srv/nas", + ) + ], + operators=OperatorsConfig(allowlist=[]), + readme=READMEConfig(defaults=[]), + ) + + +def _build_controller(config: Config) -> CreationController: + return CreationController( + config=config, + validator=Validator(), + template_engine=TemplateEngine(), + plugin_host=None, + cache_creation=CreationWriter(), + cache_equipment=EquipmentCacheWriter(), + readme_generator=NoOpReadmeGenerator(), + nas_sync=NoOpNASSync(), + session_store=SessionStore(), + ) + + +def _project_request() -> ProjectCreateRequest: + return ProjectCreateRequest( + equipment_id="EQ1", + template_path=FIXTURE_TEMPLATES / "project_basic", + lims_project={ + "uid": "8c7e9d2f-1a4b-4e6c-9b3d-7f2a1e5d8c4b", + "short_id": "PROJ-0042", + "name_at_creation": "Cortex Q3 Pilot", + "source": "live", + }, + variables={"_exlab_proj": "PROJ-0042"}, + label="Cortex Q3 calibration", + operator="asmith", + objective="First-pass calibration.", + readme_extra={}, + ) + + +def _run_request(run_date: datetime) -> RunCreateRequest: + return RunCreateRequest( + equipment_id="EQ1", + project_name="Cortex Q3 Pilot", + template_path=RUN_TEMPLATE, + run_kind=RunKind.EXPERIMENTAL, + variables={"run_id": "run_001"}, + label="calibration sweep", + operator="asmith", + objective="Sweep the laser wavelengths.", + readme_extra={}, + run_date=run_date, + ) + + +async def _drain_to_done(controller: CreationController, session_id: str) -> dict[str, Any]: + task = controller._tasks.get(session_id) + if task is not None: + await task + handle = await controller.status(session_id) + return {"state": handle.state, "current_phase": handle.current_phase} + + +# --------------------------------------------------------------------------- +# Test +# --------------------------------------------------------------------------- + + +async def test_run_creation_freezes_template_provenance_copy(tmp_path: Path) -> None: + """A run's instance dir carries a frozen verbatim copy of its run + template, creation.json records the relative path, and the copied + copier.yml/.jinja files do not block sync via post-validate.""" + local_root = tmp_path / "data" + local_root.mkdir() + config = _build_config(local_root) + controller = _build_controller(config) + + # Parent project first so the run inherits a real LIMS block. + project_handle = await controller.create_project(_project_request()) + project_final = await _drain_to_done(controller, project_handle.session_id) + assert project_final["state"] is SessionState.DONE + + run_date = datetime(2026, 4, 17, 14, 32, 0, tzinfo=UTC) + run_handle = await controller.create_run(_run_request(run_date)) + run_final = await _drain_to_done(controller, run_handle.session_id) + assert run_final["state"] is SessionState.DONE + + run_dir = local_root / "EQ1" / "Cortex Q3 Pilot" / "Runs" / "Run_2026-04-17T14-32" + assert run_dir.is_dir() + + # The frozen provenance copy is present, verbatim (copier.yml + .jinja). + template_name = RUN_TEMPLATE.name + provenance_root = run_dir / CACHE_DIR_NAME / "templates" / "run" / template_name + assert (provenance_root / "copier.yml").is_file() + jinja_copy = provenance_root / "run_data.txt.jinja" + assert jinja_copy.is_file() + # The .jinja is frozen unrendered (still contains the Jinja placeholder). + assert "{{ run_id }}" in jinja_copy.read_text(encoding="utf-8") + + # creation.json records the instance-relative provenance path. + cache_path = run_dir / CACHE_DIR_NAME / CREATION_JSON_NAME + decoded = msgspec.json.decode(cache_path.read_bytes(), type=CreationJson) + assert decoded.schema_version == CREATION_JSON_VERSION + assert decoded.template.provenance_path.startswith(".exlab-wizard/templates/run/") + assert decoded.template.provenance_path == f".exlab-wizard/templates/run/{template_name}" + + # The copied copier.yml / .jinja are NOT scanned as run output, so the + # session reached DONE without being blocked by post-validate. + assert decoded.sync_status != SyncStatus.BLOCKED_BY_VALIDATION.value + assert decoded.sync_status == SyncStatus.PENDING.value diff --git a/tests/unit/constants/test_schema_versions.py b/tests/unit/constants/test_schema_versions.py index 4cf0066..03741d6 100644 --- a/tests/unit/constants/test_schema_versions.py +++ b/tests/unit/constants/test_schema_versions.py @@ -12,7 +12,7 @@ def test_creation_json_version_is_pinned() -> None: # Backend Spec §11.3 history table. - assert schema_versions.CREATION_JSON_VERSION == "1.9" + assert schema_versions.CREATION_JSON_VERSION == "1.10" def test_readme_fields_json_version_is_pinned() -> None: @@ -65,7 +65,7 @@ def test_schema_versions_re_exported_from_package() -> None: # ``from exlab_wizard.constants import CREATION_JSON_VERSION``. from exlab_wizard import constants - assert constants.CREATION_JSON_VERSION == "1.9" + assert constants.CREATION_JSON_VERSION == "1.10" assert constants.README_FIELDS_JSON_VERSION == "1.1" assert constants.SYNC_STATE_JSON_VERSION == "1.0" assert constants.EQUIPMENT_JSON_VERSION == "1.0" diff --git a/tests/unit/controller/test_metadata_assembly.py b/tests/unit/controller/test_metadata_assembly.py index 0ad2240..d8afc61 100644 --- a/tests/unit/controller/test_metadata_assembly.py +++ b/tests/unit/controller/test_metadata_assembly.py @@ -429,7 +429,13 @@ async def test_parity_build_creation_json(tmp_path: Path) -> None: name_at_creation="Cortex Q3 Pilot", source=LIMSProjectSource.LIVE, ), - template=_resolved_desc_from(resolved), + template=dataclasses.replace( + _resolved_desc_from(resolved), + # The controller's ``_write_cache`` freezes a verbatim template + # copy into ``dst`` and records its instance-relative path; mirror + # that here so the parity comparison covers every other field. + provenance_path=controller_payload.template.provenance_path, + ), variables=req.variables, dst=dst, nas_root="/srv/nas", @@ -438,6 +444,12 @@ async def test_parity_build_creation_json(tmp_path: Path) -> None: created_at_iso=FIXED_CREATED_AT_ISO, ) + # The controller actually performed the provenance copy, so its path is + # non-empty -- sanity-check that before the encoded-form comparison. + assert controller_payload.template.provenance_path == ( + f".exlab-wizard/templates/project/{resolved.name}" + ) + # Normalize the injected timestamp before comparing the encoded form. norm_controller = msgspec.structs.replace(controller_payload, created_at=FIXED_CREATED_AT_ISO) assert msgspec.json.encode(norm_controller) == msgspec.json.encode(helper_payload) diff --git a/tests/unit/template/test_authoring.py b/tests/unit/template/test_authoring.py new file mode 100644 index 0000000..8e17abb --- /dev/null +++ b/tests/unit/template/test_authoring.py @@ -0,0 +1,350 @@ +"""Tests for the author-time template service. + +The authoring service is the non-UI backend the GUI template editor +drives. The load-bearing assertions here are the security and durability +invariants: every path flows through :func:`_safe_target` (no traversal, +no escape), every gated write refuses an invalid manifest / Jinja before +it touches disk, uploads honour the size / count caps, and an +optimistic-concurrency write loses the stat race rather than clobbering a +concurrent edit. +""" + +from __future__ import annotations + +import os +import time +from pathlib import Path + +import pytest + +from exlab_wizard.constants import COPIER_MANIFEST_NAME, TemplateType +from exlab_wizard.template import authoring +from exlab_wizard.template.authoring import ( + StaleEditError, + TemplateAuthoringError, + UnsafePathError, + create_template_dir, + delete_path, + is_editable, + list_files, + read_content, + read_manifest, + rename_path, + upload_file, + write_content_file, + write_manifest, +) +from exlab_wizard.template.manifest import TemplateManifest + + +@pytest.fixture +def template(tmp_path: Path) -> Path: + """Return a freshly scaffolded project template directory.""" + return create_template_dir(tmp_path, name="tpl", template_type="project") + + +# --------------------------------------------------------------------------- +# _safe_target -- the path chokepoint attack table +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "rel", + [ + "../escape", + "/abs/path", + "a/../../b", + "CON", + "foo.", # trailing dot + "a\x00b", # control char + "café.md", # non-ASCII + "", # empty + "a//b", # empty segment + ], +) +def test_safe_target_rejects_unsafe(template: Path, rel: str) -> None: + with pytest.raises(UnsafePathError): + authoring._safe_target(template, rel) + + +def test_safe_target_accepts_legit_nested(template: Path) -> None: + resolved = authoring._safe_target(template, "sub/dir/file.md") + assert resolved == (template / "sub" / "dir" / "file.md").resolve() + assert template.resolve() in resolved.parents + + +# --------------------------------------------------------------------------- +# create_template_dir -- scaffold + name validation +# --------------------------------------------------------------------------- + + +def test_create_template_dir_scaffolds_manifest_and_content(tmp_path: Path) -> None: + root = create_template_dir(tmp_path, name="proj", template_type="project") + assert root == tmp_path / "proj" + assert (root / COPIER_MANIFEST_NAME).is_file() + content = [p for p in root.iterdir() if p.name != COPIER_MANIFEST_NAME] + assert len(content) == 1 + assert content[0].read_text(encoding="utf-8") + + +def test_create_template_dir_rejects_unsafe_name(tmp_path: Path) -> None: + with pytest.raises(UnsafePathError): + create_template_dir(tmp_path, name="bad/name", template_type="project") + + +def test_create_template_dir_rejects_duplicate(tmp_path: Path) -> None: + create_template_dir(tmp_path, name="dup", template_type="project") + with pytest.raises(ValueError, match="already exists"): + create_template_dir(tmp_path, name="dup", template_type="project") + + +def test_create_run_template_requires_scope(tmp_path: Path) -> None: + with pytest.raises(ValueError, match="run_scope"): + create_template_dir(tmp_path, name="r", template_type="run") + + +# --------------------------------------------------------------------------- +# manifest round-trip + lint gate +# --------------------------------------------------------------------------- + + +def test_manifest_round_trip(template: Path) -> None: + manifest, stat = read_manifest(template) + assert manifest.exlab_type == TemplateType.PROJECT.value + assert isinstance(stat, tuple) and len(stat) == 2 + + +def test_write_manifest_lint_error_not_written(template: Path) -> None: + copier_path = template / COPIER_MANIFEST_NAME + before = copier_path.read_bytes() + # An empty _exlab_type is a lint ERROR (template_type_missing). + bad = TemplateManifest(exlab_type="", exlab_version="1.0") + with pytest.raises(TemplateAuthoringError, match="lint errors"): + write_manifest(template, bad) + # The original file is unchanged -- the bad manifest never landed. + assert copier_path.read_bytes() == before + + +def test_write_manifest_valid_persists(template: Path) -> None: + manifest, stat = read_manifest(template) + updated = TemplateManifest( + exlab_type=manifest.exlab_type, + exlab_version=manifest.exlab_version, + description="now described", + ) + write_manifest(template, updated, expected_stat=stat) + reread, _ = read_manifest(template) + assert reread.description == "now described" + + +# --------------------------------------------------------------------------- +# content read / write -- atomic round-trip + jinja gate +# --------------------------------------------------------------------------- + + +def test_content_round_trip(template: Path) -> None: + written = write_content_file(template, "doc.md", "hello\nworld\n") + assert written == (template / "doc.md").resolve() + text, stat = read_content(written) + assert text == "hello\nworld\n" + assert isinstance(stat, tuple) and len(stat) == 2 + + +def test_write_content_nested_creates_dirs(template: Path) -> None: + written = write_content_file(template, "a/b/c.txt", "x") + assert written.is_file() + assert written.read_text(encoding="utf-8") == "x" + + +def test_write_content_rejects_unsafe(template: Path) -> None: + with pytest.raises(UnsafePathError): + write_content_file(template, "../escape.md", "x") + + +def test_write_content_jinja_syntax_error_refused(template: Path) -> None: + with pytest.raises(TemplateAuthoringError, match="Jinja syntax error"): + write_content_file(template, "broken.md.jinja", "{% if %}") + assert not (template / "broken.md.jinja").exists() + + +def test_write_content_jinja_valid_persists(template: Path) -> None: + written = write_content_file(template, "ok.md.jinja", "{{ name }}\n") + assert written.read_text(encoding="utf-8") == "{{ name }}\n" + + +# --------------------------------------------------------------------------- +# upload -- verbatim, render-as-template, caps +# --------------------------------------------------------------------------- + + +def test_upload_verbatim_keeps_bytes(template: Path) -> None: + data = b"\x00\x01binary\xff" + written = upload_file(template, "data.bin", data) + assert written.read_bytes() == data + assert written.suffix == ".bin" + + +def test_upload_render_as_template_appends_jinja(template: Path) -> None: + written = upload_file(template, "report.md", b"# Report\n", render_as_template=True) + assert written.name == "report.md.jinja" + + +def test_upload_render_as_template_idempotent_on_jinja(template: Path) -> None: + written = upload_file(template, "x.md.jinja", b"{{ a }}", render_as_template=True) + assert written.name == "x.md.jinja" + + +def test_upload_oversize_rejected(template: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(authoring, "TEMPLATE_UPLOAD_MAX_BYTES", 8) + with pytest.raises(TemplateAuthoringError, match="cap"): + upload_file(template, "big.bin", b"0123456789") + + +def test_upload_count_cap_rejected(template: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(authoring, "TEMPLATE_MAX_FILES", 1) + # The scaffold already holds copier.yml + notes.md.jinja (>= 1). + with pytest.raises(TemplateAuthoringError, match="cap"): + upload_file(template, "another.txt", b"x") + + +def test_upload_traversal_rejected(template: Path) -> None: + with pytest.raises(UnsafePathError): + upload_file(template, "../evil.txt", b"x") + + +def test_upload_jinja_bad_syntax_rejected(template: Path) -> None: + with pytest.raises(TemplateAuthoringError, match="Jinja syntax error"): + upload_file(template, "t.txt", b"{% for %}", render_as_template=True) + + +def test_upload_binary_jinja_skips_parse(template: Path) -> None: + # Non-UTF-8 bytes in a .jinja upload: the parse is skipped, bytes land. + data = b"\xff\xfe\x00binary" + written = upload_file(template, "blob", data, render_as_template=True) + assert written.name == "blob.jinja" + assert written.read_bytes() == data + + +# --------------------------------------------------------------------------- +# stale-edit guard +# --------------------------------------------------------------------------- + + +def test_stale_edit_content_raises(template: Path) -> None: + write_content_file(template, "note.md", "v1\n") + _, stat = read_content(template / "note.md") + # Modify out-of-band with a different size + bumped mtime. + target = template / "note.md" + target.write_text("a much longer second version\n", encoding="utf-8") + new_mtime = stat[0] + 5 + os.utime(target, (new_mtime, new_mtime)) + with pytest.raises(StaleEditError): + write_content_file(template, "note.md", "v3\n", expected_stat=stat) + + +def test_stale_edit_manifest_raises(template: Path) -> None: + manifest, stat = read_manifest(template) + # Mutate copier.yml out-of-band so the stat no longer matches. + copier_path = template / COPIER_MANIFEST_NAME + time.sleep(0.01) + copier_path.write_text( + copier_path.read_text(encoding="utf-8") + "\n# touched\n", encoding="utf-8" + ) + new = stat[0] + 5 + os.utime(copier_path, (new, new)) + with pytest.raises(StaleEditError): + write_manifest(template, manifest, expected_stat=stat) + + +def test_stale_edit_none_skips_guard(template: Path) -> None: + # No expected_stat -> no guard, even after an out-of-band change. + write_content_file(template, "f.md", "x") + written = write_content_file(template, "f.md", "y", expected_stat=None) + assert written.read_text(encoding="utf-8") == "y" + + +# --------------------------------------------------------------------------- +# rename / delete +# --------------------------------------------------------------------------- + + +def test_rename_moves_file(template: Path) -> None: + write_content_file(template, "old.md", "x") + dst = rename_path(template, "old.md", "new.md") + assert dst.name == "new.md" + assert not (template / "old.md").exists() + assert (template / "new.md").read_text(encoding="utf-8") == "x" + + +def test_rename_copier_yml_refused(template: Path) -> None: + with pytest.raises(TemplateAuthoringError, match=r"copier\.yml"): + rename_path(template, COPIER_MANIFEST_NAME, "elsewhere.yml") + + +def test_delete_file(template: Path) -> None: + write_content_file(template, "gone.md", "x") + delete_path(template, "gone.md") + assert not (template / "gone.md").exists() + + +def test_delete_dir_recursive(template: Path) -> None: + write_content_file(template, "d/inner.md", "x") + delete_path(template, "d") + assert not (template / "d").exists() + + +def test_delete_copier_yml_refused(template: Path) -> None: + with pytest.raises(TemplateAuthoringError, match=r"copier\.yml"): + delete_path(template, COPIER_MANIFEST_NAME) + + +# --------------------------------------------------------------------------- +# editable classification + list_files +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("name", "expected"), + [ + ("a.md", True), + ("a.jinja", True), + ("a.md.jinja", True), + ("a.csv", True), + ("a.json", True), + ("a.xlsx", False), + ("a.png", False), + ("a", False), + ], +) +def test_is_editable(name: str, expected: bool) -> None: + assert is_editable(Path(name)) is expected + + +def test_list_files_shape_and_sort(template: Path) -> None: + write_content_file(template, "sub/b.md", "b") + write_content_file(template, "a.txt", "a") + entries = list_files(template) + rels = [e["rel"] for e in entries] + assert rels == sorted(rels) + by_rel = {e["rel"]: e for e in entries} + assert by_rel["a.txt"]["editable"] is True + assert by_rel["a.txt"]["is_dir"] is False + assert by_rel["a.txt"]["size"] == 1 + assert by_rel["sub"]["is_dir"] is True + assert by_rel["sub"]["editable"] is False + + +def test_list_files_empty_for_missing_dir(tmp_path: Path) -> None: + assert list_files(tmp_path / "nope") == [] + + +# --------------------------------------------------------------------------- +# read_content rejects non-editable suffix +# --------------------------------------------------------------------------- + + +def test_read_content_rejects_binary_suffix(template: Path) -> None: + blob = template / "sheet.xlsx" + blob.write_bytes(b"\x00\x01") + with pytest.raises(TemplateAuthoringError, match="editable"): + read_content(blob) diff --git a/tests/unit/template/test_lint.py b/tests/unit/template/test_lint.py new file mode 100644 index 0000000..9852428 --- /dev/null +++ b/tests/unit/template/test_lint.py @@ -0,0 +1,260 @@ +"""Tests for the factored-out template linter (Backend Spec §5.1). + +One test per ERROR code and per WARN code, plus the file-level checks +(``copier.yml`` existence / readability / parse) and the Jinja2 syntax +scan. A clean template yields no errors; :func:`has_errors` reflects the +ERROR tier. +""" + +from __future__ import annotations + +from pathlib import Path + +from exlab_wizard.constants import COPIER_MANIFEST_NAME +from exlab_wizard.template.lint import ( + LintFinding, + has_errors, + lint_manifest_dict, + lint_template, +) + +_MANIFEST_PATH = Path("/tpl/copier.yml") + + +def _codes(findings: list[LintFinding]) -> set[str]: + return {f.code for f in findings} + + +def _valid_manifest() -> dict: + """A manifest dict that triggers no findings at all.""" + return { + "_min_copier_version": "9.0", + "_answers_file": ".exlab-answers.yml", + "_exlab_type": "project", + "_exlab_version": "1.0", + } + + +def _write_template(root: Path, manifest_body: str) -> Path: + root.mkdir(parents=True, exist_ok=True) + (root / COPIER_MANIFEST_NAME).write_text(manifest_body, encoding="utf-8") + return root + + +# --------------------------------------------------------------------------- +# clean manifest +# --------------------------------------------------------------------------- + + +def test_valid_manifest_dict_has_no_findings() -> None: + assert lint_manifest_dict(_valid_manifest(), _MANIFEST_PATH) == [] + + +def test_valid_run_manifest_dict_has_no_findings() -> None: + manifest = _valid_manifest() + manifest["_exlab_type"] = "run" + manifest["_exlab_run_scope"] = "experimental" + assert lint_manifest_dict(manifest, _MANIFEST_PATH) == [] + + +# --------------------------------------------------------------------------- +# ERROR codes (manifest-dict) +# --------------------------------------------------------------------------- + + +def test_error_template_type_missing() -> None: + manifest = _valid_manifest() + del manifest["_exlab_type"] + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "template_type_missing" in _codes(findings) + assert has_errors(findings) + + +def test_error_template_type_invalid() -> None: + manifest = _valid_manifest() + manifest["_exlab_type"] = "nonsense" + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "template_type_invalid" in _codes(findings) + assert has_errors(findings) + + +def test_error_exlab_version_missing() -> None: + manifest = _valid_manifest() + del manifest["_exlab_version"] + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "exlab_version_missing" in _codes(findings) + + +def test_error_exlab_version_blank() -> None: + manifest = _valid_manifest() + manifest["_exlab_version"] = " " + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "exlab_version_missing" in _codes(findings) + + +def test_error_run_scope_missing() -> None: + manifest = _valid_manifest() + manifest["_exlab_type"] = "run" + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "run_scope_missing" in _codes(findings) + + +def test_error_run_scope_invalid() -> None: + manifest = _valid_manifest() + manifest["_exlab_type"] = "run" + manifest["_exlab_run_scope"] = "bogus" + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "run_scope_invalid" in _codes(findings) + + +def test_error_core_field_redeclared() -> None: + manifest = _valid_manifest() + manifest["_exlab_readme"] = {"fields": [{"id": "operator"}]} + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "core_field_redeclared" in _codes(findings) + + +def test_malformed_readme_does_not_crash() -> None: + manifest = _valid_manifest() + manifest["_exlab_readme"] = "not a dict" + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "core_field_redeclared" not in _codes(findings) + manifest["_exlab_readme"] = {"fields": "not a list"} + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "core_field_redeclared" not in _codes(findings) + + +# --------------------------------------------------------------------------- +# WARN codes +# --------------------------------------------------------------------------- + + +def test_warn_tasks_present() -> None: + manifest = _valid_manifest() + manifest["_tasks"] = ["echo hi"] + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "tasks_present" in _codes(findings) + assert not has_errors(findings) + + +def test_warn_min_copier_version_low() -> None: + manifest = _valid_manifest() + manifest["_min_copier_version"] = "8.0" + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "min_copier_version_low" in _codes(findings) + + +def test_warn_min_copier_version_missing() -> None: + manifest = _valid_manifest() + del manifest["_min_copier_version"] + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "min_copier_version_low" in _codes(findings) + + +def test_min_copier_version_double_digit_major_not_flagged() -> None: + """A future major (e.g. "10.0") is >= the "9.0" baseline. + + Guards against the lexicographic-compare regression where + ``"10.0" < "9.0"`` is ``True`` (string order), which would spuriously + flag every template once Copier reaches v10. + """ + manifest = _valid_manifest() + manifest["_min_copier_version"] = "10.0" + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "min_copier_version_low" not in _codes(findings) + + +def test_warn_answers_file_deviation() -> None: + manifest = _valid_manifest() + manifest["_answers_file"] = ".custom-answers.yml" + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "answers_file_deviation" in _codes(findings) + + +def test_warn_question_id_invalid() -> None: + manifest = _valid_manifest() + manifest["Bad-Key"] = {"type": "str"} + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "question_id_invalid" in _codes(findings) + + +def test_warn_question_type_invalid() -> None: + manifest = _valid_manifest() + manifest["good_key"] = {"type": "weirdtype"} + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "question_type_invalid" in _codes(findings) + + +def test_valid_question_does_not_warn() -> None: + manifest = _valid_manifest() + manifest["sample_id"] = {"type": "str", "default": "x"} + findings = lint_manifest_dict(manifest, _MANIFEST_PATH) + assert "question_id_invalid" not in _codes(findings) + assert "question_type_invalid" not in _codes(findings) + + +# --------------------------------------------------------------------------- +# file-level checks (lint_template) +# --------------------------------------------------------------------------- + + +def test_lint_template_copier_yml_missing(tmp_path: Path) -> None: + (tmp_path / "tpl").mkdir() + findings = lint_template(tmp_path / "tpl") + assert "copier_yml_missing" in _codes(findings) + assert has_errors(findings) + + +def test_lint_template_copier_yml_parse_error(tmp_path: Path) -> None: + root = _write_template(tmp_path / "tpl", "key: [unclosed\n") + findings = lint_template(root) + assert "copier_yml_parse_error" in _codes(findings) + + +def test_lint_template_valid_template_no_errors(tmp_path: Path) -> None: + root = _write_template( + tmp_path / "tpl", + "_min_copier_version: '9.0'\n" + "_answers_file: .exlab-answers.yml\n" + "_exlab_type: project\n" + "_exlab_version: '1.0'\n", + ) + (root / "notes.md.jinja").write_text("# {{ project_name }}\n", encoding="utf-8") + findings = lint_template(root) + assert not has_errors(findings) + + +def test_lint_template_jinja_syntax_error(tmp_path: Path) -> None: + root = _write_template( + tmp_path / "tpl", + "_min_copier_version: '9.0'\n_exlab_type: project\n_exlab_version: '1.0'\n", + ) + broken = root / "broken.md.jinja" + broken.write_text("{{ broken\n", encoding="utf-8") + findings = lint_template(root) + jinja_findings = [f for f in findings if f.code == "jinja_syntax_error"] + assert len(jinja_findings) == 1 + assert jinja_findings[0].path == "broken.md.jinja" + assert jinja_findings[0].severity == "error" + assert has_errors(findings) + + +# --------------------------------------------------------------------------- +# has_errors + to_dict +# --------------------------------------------------------------------------- + + +def test_has_errors_true_and_false() -> None: + assert has_errors([LintFinding(code="x", message="m", severity="error")]) + assert not has_errors([LintFinding(code="x", message="m", severity="warn")]) + assert not has_errors([]) + + +def test_finding_to_dict() -> None: + finding = LintFinding(code="x", message="m", severity="warn", path="a/b.jinja") + assert finding.to_dict() == { + "code": "x", + "message": "m", + "severity": "warn", + "path": "a/b.jinja", + } diff --git a/tests/unit/template/test_manifest.py b/tests/unit/template/test_manifest.py new file mode 100644 index 0000000..a9fb84d --- /dev/null +++ b/tests/unit/template/test_manifest.py @@ -0,0 +1,208 @@ +"""Tests for the typed, round-trippable :class:`TemplateManifest` model. + +The load-bearing invariant is round-trip stability: +``TemplateManifest.from_yaml(m.to_yaml()) == m`` for manifests built +from every supported question kind. Frozen-dataclass equality (field by +field, ``TemplateQuestion`` is itself frozen) makes the comparison exact. +""" + +from __future__ import annotations + +import yaml + +from exlab_wizard.template.manifest import ( + TemplateManifest, + TemplateQuestion, + template_questions, +) + + +def _roundtrip(manifest: TemplateManifest) -> TemplateManifest: + return TemplateManifest.from_yaml(manifest.to_yaml()) + + +# --------------------------------------------------------------------------- +# round-trip per question kind +# --------------------------------------------------------------------------- + + +def test_roundtrip_str_question() -> None: + m = TemplateManifest( + exlab_type="project", + exlab_version="1.0", + questions=[TemplateQuestion(key="sample_id", kind="str", default="abc", help="ID")], + ) + assert _roundtrip(m) == m + + +def test_roundtrip_int_question() -> None: + m = TemplateManifest( + exlab_type="project", + exlab_version="1.0", + questions=[TemplateQuestion(key="count", kind="int", default=7)], + ) + assert _roundtrip(m) == m + + +def test_roundtrip_float_question() -> None: + m = TemplateManifest( + exlab_type="project", + exlab_version="1.0", + questions=[TemplateQuestion(key="ratio", kind="float", default=1.5)], + ) + assert _roundtrip(m) == m + + +def test_roundtrip_bool_question() -> None: + m = TemplateManifest( + exlab_type="project", + exlab_version="1.0", + questions=[TemplateQuestion(key="enabled", kind="bool", default=True)], + ) + assert _roundtrip(m) == m + + +def test_roundtrip_choice_question() -> None: + m = TemplateManifest( + exlab_type="project", + exlab_version="1.0", + questions=[ + TemplateQuestion( + key="mode", + kind="choice", + default="fast", + choices=("fast", "slow"), + ) + ], + ) + assert _roundtrip(m) == m + + +def test_roundtrip_secret_question() -> None: + m = TemplateManifest( + exlab_type="project", + exlab_version="1.0", + questions=[TemplateQuestion(key="token", kind="str", secret=True)], + ) + assert _roundtrip(m) == m + + +def test_roundtrip_all_kinds_together() -> None: + m = TemplateManifest( + exlab_type="run", + exlab_version="3.2", + exlab_run_scope="experimental", + description="A mixed-question template", + plugins=["plugin_a", "plugin_b"], + readme_fields=[{"id": "instrument", "label": "Instrument"}], + questions=[ + TemplateQuestion(key="sample_id", kind="str", default="s1", help="Sample"), + TemplateQuestion(key="count", kind="int", default=3), + TemplateQuestion(key="ratio", kind="float", default=0.25), + TemplateQuestion(key="enabled", kind="bool", default=False), + TemplateQuestion(key="mode", kind="choice", default="b", choices=("a", "b")), + TemplateQuestion(key="token", kind="str", secret=True), + ], + ) + assert _roundtrip(m) == m + + +# --------------------------------------------------------------------------- +# _-prefixed metadata keys preserved +# --------------------------------------------------------------------------- + + +def test_underscore_prefixed_keys_preserved() -> None: + m = TemplateManifest( + exlab_type="project", + exlab_version="2.0", + description="desc", + plugins=["p1"], + min_copier_version="9.0", + answers_file=".exlab-answers.yml", + ) + body = yaml.safe_load(m.to_yaml()) + assert body["_min_copier_version"] == "9.0" + assert body["_answers_file"] == ".exlab-answers.yml" + assert body["_exlab_type"] == "project" + assert body["_exlab_version"] == "2.0" + assert body["_exlab_description"] == "desc" + assert body["_exlab_plugins"] == ["p1"] + assert body["_exlab_readme"] == {"fields": []} + assert _roundtrip(m) == m + + +def test_emitted_key_order_metadata_first() -> None: + m = TemplateManifest( + exlab_type="project", + exlab_version="1.0", + questions=[TemplateQuestion(key="sample_id", kind="str")], + ) + keys = list(yaml.safe_load(m.to_yaml()).keys()) + # Metadata keys come before the question key, in the documented order. + assert keys[:5] == [ + "_min_copier_version", + "_answers_file", + "_exlab_type", + "_exlab_version", + "_exlab_description", + ] + assert keys[-1] == "sample_id" + + +# --------------------------------------------------------------------------- +# run-scope emission +# --------------------------------------------------------------------------- + + +def test_run_scope_emitted_only_for_run_type() -> None: + run_m = TemplateManifest( + exlab_type="run", + exlab_version="1.0", + exlab_run_scope="test", + ) + project_m = TemplateManifest(exlab_type="project", exlab_version="1.0") + + assert "_exlab_run_scope" in yaml.safe_load(run_m.to_yaml()) + assert "_exlab_run_scope" not in yaml.safe_load(project_m.to_yaml()) + assert _roundtrip(run_m) == run_m + assert _roundtrip(project_m) == project_m + + +def test_plugins_omitted_when_empty() -> None: + m = TemplateManifest(exlab_type="project", exlab_version="1.0") + assert "_exlab_plugins" not in yaml.safe_load(m.to_yaml()) + + +# --------------------------------------------------------------------------- +# from_yaml accepts string OR dict; tolerates missing keys +# --------------------------------------------------------------------------- + + +def test_from_yaml_accepts_string() -> None: + m = TemplateManifest.from_yaml("_exlab_type: project\n_exlab_version: '1.0'\n") + assert m.exlab_type == "project" + assert m.exlab_version == "1.0" + # Defaults filled for missing keys. + assert m.min_copier_version == "9.0" + assert m.answers_file == ".exlab-answers.yml" + assert m.questions == [] + + +def test_from_yaml_accepts_dict() -> None: + m = TemplateManifest.from_yaml({"_exlab_type": "equipment", "_exlab_version": "9"}) + assert m.exlab_type == "equipment" + assert m.exlab_version == "9" + + +def test_from_yaml_tolerates_empty_and_non_mapping() -> None: + assert TemplateManifest.from_yaml("").exlab_type == "" + assert TemplateManifest.from_yaml("just a string").exlab_type == "" + + +def test_template_questions_reexport_parses_questions() -> None: + questions = template_questions( + {"_exlab_type": "project", "name": {"type": "str", "default": "x"}} + ) + assert [q.key for q in questions] == ["name"] + assert questions[0].kind == "str" diff --git a/tests/unit/template/test_provenance.py b/tests/unit/template/test_provenance.py new file mode 100644 index 0000000..c48a959 --- /dev/null +++ b/tests/unit/template/test_provenance.py @@ -0,0 +1,98 @@ +"""Unit tests for :func:`copy_template_into_instance`. + +Verify the frozen verbatim template copy lands under the instance's typed +provenance store (``/.exlab-wizard/templates///``), +that ``.jinja`` / ``copier.yml`` / nested files are copied unchanged, and +that the returned path is the instance-relative POSIX string. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path + +import pytest + +from exlab_wizard.template.provenance import copy_template_into_instance + + +@dataclass(frozen=True) +class _StubResolved: + """Lightweight stand-in -- the function only reads ``name`` / ``path``.""" + + name: str + path: Path + + +def _make_template(root: Path) -> None: + """Write a minimal template tree (copier.yml + .jinja + nested file).""" + root.mkdir(parents=True) + (root / "copier.yml").write_text( + '_exlab_type: "run"\n_exlab_version: "1.0"\n', + encoding="utf-8", + ) + (root / "body.txt.jinja").write_text("hello {{ run_id }}\n", encoding="utf-8") + sub = root / "sub" + sub.mkdir() + (sub / "nested.txt").write_text("nested\n", encoding="utf-8") + + +def test_copy_template_into_instance_copies_tree_and_returns_rel_path( + tmp_path: Path, +) -> None: + src = tmp_path / "templates" / "confocal_run" + _make_template(src) + dst = tmp_path / "instance" + dst.mkdir() + resolved = _StubResolved(name="confocal_run", path=src) + + rel = copy_template_into_instance(resolved, dst, "run") + + assert rel == ".exlab-wizard/templates/run/confocal_run" + + copy_root = dst / ".exlab-wizard" / "templates" / "run" / "confocal_run" + assert (copy_root / "copier.yml").is_file() + # The .jinja source is copied verbatim (not rendered). + jinja = copy_root / "body.txt.jinja" + assert jinja.is_file() + assert jinja.read_text(encoding="utf-8") == "hello {{ run_id }}\n" + # Nested subdirectory files come along too. + assert (copy_root / "sub" / "nested.txt").read_text(encoding="utf-8") == "nested\n" + + +def test_copy_template_into_instance_uses_project_segment(tmp_path: Path) -> None: + src = tmp_path / "templates" / "project_basic" + _make_template(src) + dst = tmp_path / "instance" + dst.mkdir() + resolved = _StubResolved(name="project_basic", path=src) + + rel = copy_template_into_instance(resolved, dst, "project") + + assert rel == ".exlab-wizard/templates/project/project_basic" + assert ( + dst / ".exlab-wizard" / "templates" / "project" / "project_basic" / "copier.yml" + ).is_file() + + +def test_copy_template_into_instance_is_idempotent(tmp_path: Path) -> None: + """A second copy over an existing target succeeds (dirs_exist_ok).""" + src = tmp_path / "templates" / "run_a" + _make_template(src) + dst = tmp_path / "instance" + dst.mkdir() + resolved = _StubResolved(name="run_a", path=src) + + first = copy_template_into_instance(resolved, dst, "run") + second = copy_template_into_instance(resolved, dst, "run") + + assert first == second == ".exlab-wizard/templates/run/run_a" + + +def test_copy_template_into_instance_missing_source_raises(tmp_path: Path) -> None: + dst = tmp_path / "instance" + dst.mkdir() + resolved = _StubResolved(name="ghost", path=tmp_path / "does-not-exist") + + with pytest.raises(FileNotFoundError): + copy_template_into_instance(resolved, dst, "run") diff --git a/tests/unit/template/test_resolution.py b/tests/unit/template/test_resolution.py new file mode 100644 index 0000000..6a78e1f --- /dev/null +++ b/tests/unit/template/test_resolution.py @@ -0,0 +1,424 @@ +"""Tests for the read-side template resolver (Backend Spec §5.0). + +:func:`resolve_template_chain` merges a per-project -> per-equipment -> +global search chain, nearest scope winning on a name collision, optionally +narrowing run templates by scope. These tests build real ``copier.yml`` +template stores on disk (the same minimal ``_exlab_*`` manifest shape the +template-manager page emits) and assert the merge precedence, the +type-specific search chains, run-scope narrowing, and graceful handling of +missing per-instance directories. +""" + +from __future__ import annotations + +from pathlib import Path + +import yaml + +# Prime the api package before importing ui.pages so the pre-existing +# orchestrator <-> api import order resolves cleanly (see test_templates_page). +import exlab_wizard.api.app # noqa: F401 -- import order matters +from exlab_wizard.config.models import Config, EquipmentConfig, PathsConfig +from exlab_wizard.constants import COPIER_MANIFEST_NAME, RunScope, TemplateType +from exlab_wizard.paths import cache_dir +from exlab_wizard.template.resolution import ( + TemplateChoices, + instance_template_dir, + reconcile_selection, + resolve_template_chain, + search_dirs, +) + +EQUIPMENT_ID = "MICROSCOPE_01" + + +# --------------------------------------------------------------------------- +# reconcile_selection -- post-re-resolution path refresh (Phase 5) +# --------------------------------------------------------------------------- + + +def test_reconcile_selection_refreshes_path_for_same_name_override() -> None: + """A surviving same-named selection re-derives its (possibly new) path. + + The regression guard for the per-equipment-override bug: the operator + picked ``lab_default`` at the global path, then chose an equipment whose + own ``lab_default`` shadows it at a different path. The select keeps the + name, so the stored path must be refreshed to the per-equipment one -- + otherwise submit would render the global template. + """ + global_path = Path("/global/lab_default") + per_eq_path = Path("/local/EQ/.exlab-wizard/templates/project/lab_default") + # Selection was made under the global context... + assert global_path != per_eq_path + # ...now re-resolved with the per-equipment override at a new path. + new_choices = TemplateChoices(names=["lab_default"], paths={"lab_default": per_eq_path}) + name, path, dropped = reconcile_selection(new_choices, "lab_default") + assert name == "lab_default" + assert path == per_eq_path + assert dropped is False + + +def test_reconcile_selection_drops_vanished_name() -> None: + """A selection whose name no longer appears is dropped (clears variables).""" + new_choices = TemplateChoices(names=["other"], paths={"other": Path("/x/other")}) + name, path, dropped = reconcile_selection(new_choices, "gone") + assert name is None + assert path is None + assert dropped is True + + +def test_reconcile_selection_none_selection_is_dropped() -> None: + """No prior selection reconciles to a cleared, dropped state.""" + name, path, dropped = reconcile_selection(TemplateChoices(), None) + assert (name, path, dropped) == (None, None, True) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _write_template( + parent: Path, + *, + name: str, + template_type: str, + run_scope: str | None = None, + description: str = "", +) -> Path: + """Write a minimal ``copier.yml`` template under ``parent/``.""" + root = parent / name + root.mkdir(parents=True, exist_ok=True) + manifest: dict[str, object] = { + "_exlab_type": template_type, + "_exlab_version": "1.0", + "_exlab_description": description, + } + if run_scope is not None: + manifest["_exlab_run_scope"] = run_scope + (root / COPIER_MANIFEST_NAME).write_text( + yaml.safe_dump(manifest, sort_keys=False), + encoding="utf-8", + ) + return root + + +def _build_config(tmp_path: Path) -> Config: + """A config whose global templates dir + local_root live under ``tmp_path``.""" + return Config( + paths=PathsConfig( + templates_dir=str(tmp_path / "global-templates"), + plugin_dir=str(tmp_path / "plugins"), + local_root=str(tmp_path / "local"), + ), + equipment=[ + EquipmentConfig( + id=EQUIPMENT_ID, + label="Microscope 01", + local_root=str(tmp_path / "local"), + nas_root="nas-root", + ) + ], + ) + + +def _project_path(tmp_path: Path) -> Path: + """The absolute project dir under ``local_root//``.""" + return tmp_path / "local" / EQUIPMENT_ID / "ProjectA" + + +# --------------------------------------------------------------------------- +# instance_template_dir +# --------------------------------------------------------------------------- + + +def test_instance_template_dir_composes_cache_templates_type(tmp_path: Path) -> None: + result = instance_template_dir(tmp_path, TemplateType.RUN.value) + assert result == cache_dir(tmp_path) / "templates" / "run" + + +# --------------------------------------------------------------------------- +# search_dirs -- which scopes each template type searches +# --------------------------------------------------------------------------- + + +def test_search_dirs_run_orders_project_equipment_global(tmp_path: Path) -> None: + config = _build_config(tmp_path) + project = _project_path(tmp_path) + dirs = search_dirs( + config, + template_type=TemplateType.RUN.value, + equipment_id=EQUIPMENT_ID, + project_path=project, + ) + assert dirs == [ + instance_template_dir(project, TemplateType.RUN.value), + instance_template_dir(tmp_path / "local" / EQUIPMENT_ID, TemplateType.RUN.value), + Path(config.paths.templates_dir), + ] + + +def test_search_dirs_project_orders_equipment_global(tmp_path: Path) -> None: + config = _build_config(tmp_path) + dirs = search_dirs( + config, + template_type=TemplateType.PROJECT.value, + equipment_id=EQUIPMENT_ID, + ) + assert dirs == [ + instance_template_dir(tmp_path / "local" / EQUIPMENT_ID, TemplateType.PROJECT.value), + Path(config.paths.templates_dir), + ] + + +def test_search_dirs_equipment_is_global_only(tmp_path: Path) -> None: + config = _build_config(tmp_path) + dirs = search_dirs( + config, + template_type=TemplateType.EQUIPMENT.value, + equipment_id=EQUIPMENT_ID, + ) + assert dirs == [Path(config.paths.templates_dir)] + + +def test_search_dirs_skips_empty_global(tmp_path: Path) -> None: + config = _build_config(tmp_path) + config = config.model_copy( + update={"paths": config.paths.model_copy(update={"templates_dir": ""})} + ) + dirs = search_dirs( + config, + template_type=TemplateType.PROJECT.value, + equipment_id=EQUIPMENT_ID, + ) + # Only the per-equipment dir survives; the empty global dir is dropped. + assert dirs == [ + instance_template_dir(tmp_path / "local" / EQUIPMENT_ID, TemplateType.PROJECT.value) + ] + + +def test_search_dirs_skips_equipment_when_id_missing(tmp_path: Path) -> None: + config = _build_config(tmp_path) + dirs = search_dirs(config, template_type=TemplateType.PROJECT.value, equipment_id=None) + assert dirs == [Path(config.paths.templates_dir)] + + +# --------------------------------------------------------------------------- +# resolve_template_chain -- merge precedence +# --------------------------------------------------------------------------- + + +def test_run_chain_nearest_scope_wins_on_name_collision(tmp_path: Path) -> None: + config = _build_config(tmp_path) + project = _project_path(tmp_path) + global_dir = Path(config.paths.templates_dir) + equip_run_dir = instance_template_dir(tmp_path / "local" / EQUIPMENT_ID, TemplateType.RUN.value) + project_run_dir = instance_template_dir(project, TemplateType.RUN.value) + + # Same name "shared" in all three scopes; distinguish by description. + _write_template( + global_dir, + name="shared", + template_type="run", + run_scope=RunScope.BOTH.value, + description="global", + ) + _write_template( + equip_run_dir, + name="shared", + template_type="run", + run_scope=RunScope.BOTH.value, + description="equipment", + ) + _write_template( + project_run_dir, + name="shared", + template_type="run", + run_scope=RunScope.BOTH.value, + description="project", + ) + + result = resolve_template_chain( + config, + template_type=TemplateType.RUN.value, + equipment_id=EQUIPMENT_ID, + project_path=project, + ) + + names = [s.name for s in result] + assert names == ["shared"] + # Nearest scope (project) wins. + assert result[0].description == "project" + + +def test_run_chain_distinct_names_ordered_project_equipment_global(tmp_path: Path) -> None: + config = _build_config(tmp_path) + project = _project_path(tmp_path) + global_dir = Path(config.paths.templates_dir) + equip_run_dir = instance_template_dir(tmp_path / "local" / EQUIPMENT_ID, TemplateType.RUN.value) + project_run_dir = instance_template_dir(project, TemplateType.RUN.value) + + _write_template(global_dir, name="g-run", template_type="run", run_scope=RunScope.BOTH.value) + _write_template(equip_run_dir, name="e-run", template_type="run", run_scope=RunScope.BOTH.value) + _write_template( + project_run_dir, name="p-run", template_type="run", run_scope=RunScope.BOTH.value + ) + + result = resolve_template_chain( + config, + template_type=TemplateType.RUN.value, + equipment_id=EQUIPMENT_ID, + project_path=project, + ) + + assert [s.name for s in result] == ["p-run", "e-run", "g-run"] + + +def test_project_chain_equipment_beats_global_on_collision(tmp_path: Path) -> None: + config = _build_config(tmp_path) + global_dir = Path(config.paths.templates_dir) + equip_proj_dir = instance_template_dir( + tmp_path / "local" / EQUIPMENT_ID, TemplateType.PROJECT.value + ) + + _write_template(global_dir, name="layout", template_type="project", description="global") + _write_template(equip_proj_dir, name="layout", template_type="project", description="equipment") + # A distinct global-only project template still surfaces. + _write_template(global_dir, name="global-only", template_type="project") + + result = resolve_template_chain( + config, + template_type=TemplateType.PROJECT.value, + equipment_id=EQUIPMENT_ID, + ) + + by_name = {s.name: s for s in result} + assert by_name["layout"].description == "equipment" + assert "global-only" in by_name + # Equipment scope first, then the global-only distinct name. + assert [s.name for s in result] == ["layout", "global-only"] + + +def test_equipment_chain_searches_only_global(tmp_path: Path) -> None: + config = _build_config(tmp_path) + global_dir = Path(config.paths.templates_dir) + # A per-equipment "equipment"-type store should be ignored (never searched). + equip_equip_dir = instance_template_dir( + tmp_path / "local" / EQUIPMENT_ID, TemplateType.EQUIPMENT.value + ) + _write_template(global_dir, name="rig", template_type="equipment") + _write_template(equip_equip_dir, name="ignored", template_type="equipment") + + result = resolve_template_chain( + config, + template_type=TemplateType.EQUIPMENT.value, + equipment_id=EQUIPMENT_ID, + ) + + assert [s.name for s in result] == ["rig"] + + +# --------------------------------------------------------------------------- +# resolve_template_chain -- run-scope narrowing +# --------------------------------------------------------------------------- + + +def test_run_scope_excludes_test_when_experimental_requested(tmp_path: Path) -> None: + config = _build_config(tmp_path) + global_dir = Path(config.paths.templates_dir) + _write_template( + global_dir, name="exp-only", template_type="run", run_scope=RunScope.EXPERIMENTAL.value + ) + _write_template( + global_dir, name="test-only", template_type="run", run_scope=RunScope.TEST.value + ) + _write_template(global_dir, name="both", template_type="run", run_scope=RunScope.BOTH.value) + + result = resolve_template_chain( + config, + template_type=TemplateType.RUN.value, + run_scope=RunScope.EXPERIMENTAL.value, + ) + + names = {s.name for s in result} + assert names == {"exp-only", "both"} + assert "test-only" not in names + + +def test_run_scope_test_keeps_test_and_both(tmp_path: Path) -> None: + config = _build_config(tmp_path) + global_dir = Path(config.paths.templates_dir) + _write_template( + global_dir, name="exp-only", template_type="run", run_scope=RunScope.EXPERIMENTAL.value + ) + _write_template( + global_dir, name="test-only", template_type="run", run_scope=RunScope.TEST.value + ) + _write_template(global_dir, name="both", template_type="run", run_scope=RunScope.BOTH.value) + + result = resolve_template_chain( + config, + template_type=TemplateType.RUN.value, + run_scope=RunScope.TEST.value, + ) + + assert {s.name for s in result} == {"test-only", "both"} + + +def test_run_scope_none_keeps_all_run_templates(tmp_path: Path) -> None: + config = _build_config(tmp_path) + global_dir = Path(config.paths.templates_dir) + _write_template( + global_dir, name="exp-only", template_type="run", run_scope=RunScope.EXPERIMENTAL.value + ) + _write_template( + global_dir, name="test-only", template_type="run", run_scope=RunScope.TEST.value + ) + + result = resolve_template_chain(config, template_type=TemplateType.RUN.value) + + assert {s.name for s in result} == {"exp-only", "test-only"} + + +# --------------------------------------------------------------------------- +# resolve_template_chain -- missing dirs are skipped gracefully +# --------------------------------------------------------------------------- + + +def test_missing_per_instance_dirs_are_skipped(tmp_path: Path) -> None: + config = _build_config(tmp_path) + project = _project_path(tmp_path) + global_dir = Path(config.paths.templates_dir) + # Only the global dir has templates; no per-equipment / per-project folders. + _write_template(global_dir, name="g-run", template_type="run", run_scope=RunScope.BOTH.value) + + result = resolve_template_chain( + config, + template_type=TemplateType.RUN.value, + equipment_id=EQUIPMENT_ID, + project_path=project, + ) + + assert [s.name for s in result] == ["g-run"] + + +def test_no_dirs_exist_returns_empty(tmp_path: Path) -> None: + config = _build_config(tmp_path) + result = resolve_template_chain( + config, + template_type=TemplateType.PROJECT.value, + equipment_id=EQUIPMENT_ID, + ) + assert result == [] + + +def test_global_misfiled_type_is_filtered_out(tmp_path: Path) -> None: + """A project template in the flat global dir is skipped for a run search.""" + config = _build_config(tmp_path) + global_dir = Path(config.paths.templates_dir) + _write_template(global_dir, name="a-project", template_type="project") + _write_template(global_dir, name="a-run", template_type="run", run_scope=RunScope.BOTH.value) + + result = resolve_template_chain(config, template_type=TemplateType.RUN.value) + + assert [s.name for s in result] == ["a-run"] diff --git a/tests/unit/ui/test_mount.py b/tests/unit/ui/test_mount.py index 438be70..ad5836a 100644 --- a/tests/unit/ui/test_mount.py +++ b/tests/unit/ui/test_mount.py @@ -773,8 +773,8 @@ def test_template_names_lists_summary_names(monkeypatch: pytest.MonkeyPatch) -> from exlab_wizard.ui.pages import templates as templates_page summaries = [ - SimpleNamespace(name="proj_a", path=Path("/tmp/tpl/proj_a")), - SimpleNamespace(name="proj_b", path=Path("/tmp/tpl/proj_b")), + SimpleNamespace(name="proj_a", path=Path("/tmp/tpl/proj_a"), run_scope=None), + SimpleNamespace(name="proj_b", path=Path("/tmp/tpl/proj_b"), run_scope=None), ] monkeypatch.setattr(templates_page, "list_templates", lambda _d, template_type=None: summaries) deps = _deps(config=_config()) @@ -934,7 +934,9 @@ def test_template_questions_map_resolves_questions( from exlab_wizard.template import copier_driver from exlab_wizard.ui.pages import templates as templates_page - summaries = [SimpleNamespace(name="proj_basic", path=Path("/tmp/tpl/proj_basic"))] + summaries = [ + SimpleNamespace(name="proj_basic", path=Path("/tmp/tpl/proj_basic"), run_scope=None) + ] monkeypatch.setattr(templates_page, "list_templates", lambda _d, template_type=None: summaries) class _Engine: @@ -955,7 +957,7 @@ def test_template_questions_map_skips_unresolvable_template( from exlab_wizard.template import copier_driver from exlab_wizard.ui.pages import templates as templates_page - summaries = [SimpleNamespace(name="broken", path=Path("/tmp/tpl/broken"))] + summaries = [SimpleNamespace(name="broken", path=Path("/tmp/tpl/broken"), run_scope=None)] monkeypatch.setattr(templates_page, "list_templates", lambda _d, template_type=None: summaries) class _Engine: @@ -969,6 +971,98 @@ def resolve(self, _path: Any, _scope: Any) -> Any: assert any("failed to resolve" in r.message for r in caplog.records) +# --------------------------------------------------------------------------- +# _resolve_template_choices -- per-instance resolution (Phase 5) +# --------------------------------------------------------------------------- + + +def _write_min_template( + parent: Path, *, name: str, template_type: str, run_scope: str | None = None +) -> Path: + """Write a minimal valid copier.yml template under ``parent/``.""" + import yaml + + from exlab_wizard.constants import COPIER_MANIFEST_NAME + + root = parent / name + root.mkdir(parents=True, exist_ok=True) + manifest: dict[str, Any] = {"_exlab_type": template_type, "_exlab_version": "1.0"} + if run_scope is not None: + manifest["_exlab_run_scope"] = run_scope + (root / COPIER_MANIFEST_NAME).write_text(yaml.safe_dump(manifest), encoding="utf-8") + return root + + +def test_resolve_template_choices_prefers_per_equipment_and_captures_path( + tmp_path: Path, +) -> None: + """A per-equipment project template shadows a same-named global one. + + Proves Phase 5 end to end at the mount layer: the resolver merges the + per-equipment store over the global store nearest-wins, and the returned + ``paths`` point at the *resolved* (per-equipment) source -- which is what + the wizard stores on the state and submits, so the pipeline renders the + override rather than ``templates_dir / name``. + """ + from exlab_wizard.constants import CACHE_DIR_NAME + + global_dir = tmp_path / "global" + local_root = tmp_path / "local" + equipment_id = "MICROSCOPE_01" + # Same template name in both scopes; the per-equipment copy must win. + _write_min_template(global_dir, name="lab_default", template_type="project") + per_eq_dir = local_root / equipment_id / CACHE_DIR_NAME / "templates" / "project" + per_eq_template = _write_min_template(per_eq_dir, name="lab_default", template_type="project") + # A global-only template still surfaces, ranked after the per-equipment one. + _write_min_template(global_dir, name="global_only", template_type="project") + + deps = _deps( + config=_config( + templates_dir=str(global_dir), + local_root=str(local_root), + equipment=(SimpleNamespace(id=equipment_id),), + ) + ) + + # No equipment context -> global only (degrades gracefully, prior behaviour). + bare = mount._resolve_template_choices(deps, "project") + assert set(bare.names) == {"lab_default", "global_only"} + assert bare.paths["lab_default"] == global_dir / "lab_default" + + # With the equipment context the per-equipment copy shadows the global one. + scoped = mount._resolve_template_choices(deps, "project", equipment_id=equipment_id) + assert scoped.names[0] == "lab_default" + assert "global_only" in scoped.names + assert scoped.paths["lab_default"] == per_eq_template + + +async def test_submit_run_uses_resolved_template_path(tmp_path: Path) -> None: + """``_submit_run`` renders the resolved path the wizard stored, not name-join. + + The fix that makes per-instance selection real: when the wizard state + carries ``selected_template_path`` (the absolute resolved source), submit + must use it verbatim rather than ``templates_dir / selected_template``. + """ + nav = _NavSpy() + controller = _FakeController(final_state=SessionState.DONE) + deps = _deps(controller=controller, config=_config(templates_dir=str(tmp_path / "global"))) + resolved = ( + tmp_path / "local" / "MICROSCOPE_01" / "ProjA" / ".exlab-wizard" / "templates" / "run" + ) + resolved.mkdir(parents=True) + state = SimpleNamespace( + selected_template="confocal", + selected_template_path=resolved, + selected_equipment="MICROSCOPE_01", + selected_project_name="ProjA", + template_variables={}, + readme_fields={"label": "L", "operator": "op", "objective": "obj"}, + ) + await mount._submit_run(deps, state, RunKind.EXPERIMENTAL, nav) + assert len(controller.created) == 1 + assert controller.created[0].template_path == resolved + + # --------------------------------------------------------------------------- # _await_session # --------------------------------------------------------------------------- @@ -2652,26 +2746,37 @@ async def test_submit_run_toasts_without_template() -> None: def test_render_run_wizard_builds_page(monkeypatch: pytest.MonkeyPatch) -> None: - """``_render_run_wizard`` wires templates/equipment into the run-wizard page.""" + """``_render_run_wizard`` wires resolved templates/equipment + ``on_resolve``.""" + from exlab_wizard.template.resolution import TemplateChoices from exlab_wizard.ui.pages import wizard_run as wizard_run_page captured: dict[str, Any] = {} - def _fake_render(*, state: Any, templates: Any, equipment_ids: Any, **kwargs: Any) -> Any: + def _fake_render( + *, state: Any, templates: Any, equipment_ids: Any, on_resolve: Any = None, **kwargs: Any + ) -> Any: captured["run_kind"] = state.run_kind captured["templates"] = templates captured["equipment_ids"] = equipment_ids + captured["on_resolve"] = on_resolve return "PAGE" + # The run wizard resolves its templates through ``_resolve_template_choices`` + # (names + questions + absolute paths) rather than the legacy producers. monkeypatch.setattr(wizard_run_page, "render_run_wizard", _fake_render) - monkeypatch.setattr(mount, "_template_names", lambda _deps, _t: ["run_basic"]) - monkeypatch.setattr(mount, "_template_questions_map", lambda _deps, _t: {}) + monkeypatch.setattr( + mount, + "_resolve_template_choices", + lambda _deps, _t, **_kw: TemplateChoices(names=["run_basic"]), + ) deps = _deps(config=_config(equipment=(SimpleNamespace(id="EQ1"),))) out = mount._render_run_wizard(deps, RunKind.TEST, _UiSpy()) assert out == "PAGE" assert captured["run_kind"] is RunKind.TEST assert captured["templates"] == ["run_basic"] assert captured["equipment_ids"] == ["EQ1"] + # ``on_resolve`` is wired so the template step re-resolves per-instance. + assert callable(captured["on_resolve"]) # --------------------------------------------------------------------------- diff --git a/tests/unit/ui/test_template_editor.py b/tests/unit/ui/test_template_editor.py new file mode 100644 index 0000000..0fb5674 --- /dev/null +++ b/tests/unit/ui/test_template_editor.py @@ -0,0 +1,161 @@ +"""Tests for the template-editor page (headless payload contract). + +``render_template_editor`` builds NiceGUI widgets when NiceGUI is +importable, but -- like ``render_template_manager`` -- falls back to a +plain payload dict when the ``from nicegui import ui`` inside the function +raises. That headless payload carries the data-shaping logic, so these +tests force the fallback (``sys.modules["nicegui"] = None``) and assert on +the dict: the template's files, question keys, exlab_type, run_scope, and +the lint finding codes for the directory. A companion test exercises the +new manager parameters (``on_edit`` / ``locations``) through the same +headless path. +""" + +from __future__ import annotations + +import sys +from collections.abc import Iterator +from pathlib import Path + +import pytest + +# Prime the api package before importing ui.pages so the pre-existing +# orchestrator <-> api import order resolves cleanly (see test_mount.py). +import exlab_wizard.api.app # noqa: F401 -- import order matters +from exlab_wizard.constants import COPIER_MANIFEST_NAME, RunScope +from exlab_wizard.template.authoring import create_template_dir, write_content_file +from exlab_wizard.ui.pages.template_editor import render_template_editor +from exlab_wizard.ui.pages.templates import TemplateSummary, render_template_manager + + +@pytest.fixture +def _headless(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]: + """Force the ``from nicegui import ui`` inside the views to fail. + + Setting ``sys.modules["nicegui"]`` to ``None`` makes the import raise + ``ImportError``, so the render functions return their headless payload + dict regardless of whether NiceGUI is installed in the test env. + """ + monkeypatch.setitem(sys.modules, "nicegui", None) + yield + + +# --------------------------------------------------------------------------- +# render_template_editor -- headless payload +# --------------------------------------------------------------------------- + + +def test_editor_payload_lists_files_and_metadata(_headless: None, tmp_path: Path) -> None: + root = create_template_dir( + tmp_path, + name="proj", + template_type="project", + description="layout", + ) + + payload = render_template_editor(template_dir=root) + + assert isinstance(payload, dict) + assert payload["template"] == "proj" + assert payload["exlab_type"] == "project" + assert payload["run_scope"] is None + # copier.yml + the scaffolded notes.md.jinja are listed. + assert COPIER_MANIFEST_NAME in payload["files"] + assert any(rel.endswith(".jinja") for rel in payload["files"]) + # A clean scaffold has no jinja-syntax ERROR findings. + assert "jinja_syntax_error" not in payload["findings"] + + +def test_editor_payload_reports_run_scope(_headless: None, tmp_path: Path) -> None: + root = create_template_dir( + tmp_path, + name="runtpl", + template_type="run", + run_scope=RunScope.TEST.value, + ) + + payload = render_template_editor(template_dir=root) + + assert payload["exlab_type"] == "run" + assert payload["run_scope"] == RunScope.TEST.value + + +def test_editor_payload_lists_question_keys(_headless: None, tmp_path: Path) -> None: + root = create_template_dir(tmp_path, name="q", template_type="project") + # Append a question to the scaffolded manifest. + manifest_path = root / COPIER_MANIFEST_NAME + manifest_path.write_text( + manifest_path.read_text(encoding="utf-8") + + "\nspecimen_id:\n type: str\n help: Specimen identifier\n", + encoding="utf-8", + ) + + payload = render_template_editor(template_dir=root) + + assert "specimen_id" in payload["questions"] + + +def test_editor_payload_surfaces_jinja_syntax_error(_headless: None, tmp_path: Path) -> None: + root = create_template_dir(tmp_path, name="broken", template_type="project") + # Write a syntactically broken Jinja file directly (bypassing the + # authoring Jinja gate, which would refuse it). + (root / "broken.txt.jinja").write_text("{% if %}", encoding="utf-8") + + payload = render_template_editor(template_dir=root) + + assert "jinja_syntax_error" in payload["findings"] + + +def test_editor_callbacks_are_optional(_headless: None, tmp_path: Path) -> None: + root = create_template_dir(tmp_path, name="nocallbacks", template_type="project") + # All callbacks default to None -- the function must still return a + # payload without raising. + payload = render_template_editor(template_dir=root) + assert payload["template"] == "nocallbacks" + + +def test_editor_payload_includes_uploaded_content_file(_headless: None, tmp_path: Path) -> None: + root = create_template_dir(tmp_path, name="extra", template_type="project") + write_content_file(root, "data/values.csv", "a,b\n1,2\n") + + payload = render_template_editor(template_dir=root) + + assert "data/values.csv" in payload["files"] + + +# --------------------------------------------------------------------------- +# render_template_manager -- new on_edit / locations params +# --------------------------------------------------------------------------- + + +def test_manager_payload_includes_new_and_old_keys(_headless: None, tmp_path: Path) -> None: + summaries = [ + TemplateSummary( + name="alpha", + path=tmp_path / "alpha", + template_type="project", + run_scope=None, + description="", + ) + ] + + payload = render_template_manager( + templates=summaries, + on_edit=lambda _name: None, + locations=[("Global", "global")], + on_location_change=lambda _value: None, + ) + + assert isinstance(payload, dict) + # New key present. + assert payload["locations"] == [("Global", "global")] + # Old keys still present (regression: test_templates_page.py contract). + assert payload["templates"] == ["alpha"] + assert payload["count"] == 1 + + +def test_manager_payload_omits_locations_when_not_given(_headless: None) -> None: + payload = render_template_manager(templates=[]) + assert "locations" not in payload + assert payload["templates"] == [] + assert payload["count"] == 0 From 6e771ea9c905beb016501aa56105204615467ca2 Mon Sep 17 00:00:00 2001 From: Alexander Nguyen Date: Sat, 30 May 2026 13:26:06 -0700 Subject: [PATCH 2/2] test+fix(templates): satisfy CI lint + coverage gates CI feedback on PR #24: - lint job (ruff format --check): reformat template/lint.py. - lint job (mypy): the Phase-5 reactive handlers used a `lambda e: (setattr(...), cb())` tuple idiom; mypy rejects setattr in an expression (func-returns-value). Replaced with named nested handlers in both wizards (also clearer per review feedback). - coverage job (91% floor; was 90.02%): template_editor.py was only 17% covered (headless payload path only). Added a fake-NiceGUI `ui` surface that records widgets + click/upload handlers by data-testid, exercising the rich render branch end to end: lint banners (clean/warn/error), file list + inline edit/save/delete, codemirror vs textarea fallback, question add/remove/save with default coercion (int/float/bool/choice + uncoercible fallback), upload (with/without callback, unavailable build), unreadable-manifest and unreadable-file branches, and Back. Editor coverage 17% -> 99%. ruff check/format + mypy clean; affected suites green. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/exlab_wizard/template/lint.py | 1 + src/exlab_wizard/ui/pages/wizard_project.py | 12 +- src/exlab_wizard/ui/pages/wizard_run.py | 23 +- tests/unit/ui/test_template_editor.py | 453 +++++++++++++++++++- 4 files changed, 470 insertions(+), 19 deletions(-) diff --git a/src/exlab_wizard/template/lint.py b/src/exlab_wizard/template/lint.py index 711dd05..5331e3e 100644 --- a/src/exlab_wizard/template/lint.py +++ b/src/exlab_wizard/template/lint.py @@ -75,6 +75,7 @@ def _version_tuple(value: str) -> tuple[int, ...]: return () return tuple(int(part) for part in match.group().split(".")) + # Conventional ``_answers_file`` value (Backend §5.3). A deviation is a WARN. _CONVENTIONAL_ANSWERS_FILE: str = ".exlab-answers.yml" diff --git a/src/exlab_wizard/ui/pages/wizard_project.py b/src/exlab_wizard/ui/pages/wizard_project.py index e1e762b..c34d48d 100644 --- a/src/exlab_wizard/ui/pages/wizard_project.py +++ b/src/exlab_wizard/ui/pages/wizard_project.py @@ -429,16 +429,16 @@ def _reveal_manual(_evt: Any) -> None: gate.on_click(_reveal_manual) elif step_id == "equipment": + + def _on_equipment(event: Any) -> None: + state.selected_equipment = event.value or None + on_equipment_change() + ui.select( equipment_ids, value=(state.selected_equipment if state.selected_equipment in equipment_ids else None), label="Equipment", - ).props('data-testid="wizard-project-equipment"').on_value_change( - lambda e: ( - setattr(state, "selected_equipment", e.value or None), - on_equipment_change(), - ) - ) + ).props('data-testid="wizard-project-equipment"').on_value_change(_on_equipment) elif step_id == "readme": for field_id, label in ( ("label", "Label"), diff --git a/src/exlab_wizard/ui/pages/wizard_run.py b/src/exlab_wizard/ui/pages/wizard_run.py index 88a6c35..db8c7da 100644 --- a/src/exlab_wizard/ui/pages/wizard_run.py +++ b/src/exlab_wizard/ui/pages/wizard_run.py @@ -384,25 +384,24 @@ def _render_run_step_fields( from nicegui import ui if step_id == "project_equipment": + + def _on_project_name(event: Any) -> None: + state.selected_project_name = event.value or None + on_project_equipment_change() + + def _on_equipment(event: Any) -> None: + state.selected_equipment = event.value or None + on_project_equipment_change() + ui.input( label="Parent project name", value=state.selected_project_name or "", - ).props('data-testid="wizard-run-project-name"').on_value_change( - lambda e: ( - setattr(state, "selected_project_name", e.value or None), - on_project_equipment_change(), - ) - ) + ).props('data-testid="wizard-run-project-name"').on_value_change(_on_project_name) ui.select( equipment_ids, value=(state.selected_equipment if state.selected_equipment in equipment_ids else None), label="Equipment", - ).props('data-testid="wizard-run-equipment"').on_value_change( - lambda e: ( - setattr(state, "selected_equipment", e.value or None), - on_project_equipment_change(), - ) - ) + ).props('data-testid="wizard-run-equipment"').on_value_change(_on_equipment) elif step_id == "readme": for field_id, label in ( ("label", "Label"), diff --git a/tests/unit/ui/test_template_editor.py b/tests/unit/ui/test_template_editor.py index 0fb5674..9f0f1fb 100644 --- a/tests/unit/ui/test_template_editor.py +++ b/tests/unit/ui/test_template_editor.py @@ -14,8 +14,9 @@ from __future__ import annotations import sys -from collections.abc import Iterator +from collections.abc import Callable, Iterator from pathlib import Path +from typing import Any import pytest @@ -24,6 +25,7 @@ import exlab_wizard.api.app # noqa: F401 -- import order matters from exlab_wizard.constants import COPIER_MANIFEST_NAME, RunScope from exlab_wizard.template.authoring import create_template_dir, write_content_file +from exlab_wizard.template.manifest import TemplateManifest from exlab_wizard.ui.pages.template_editor import render_template_editor from exlab_wizard.ui.pages.templates import TemplateSummary, render_template_manager @@ -159,3 +161,452 @@ def test_manager_payload_omits_locations_when_not_given(_headless: None) -> None assert "locations" not in payload assert payload["templates"] == [] assert payload["count"] == 0 + + +# --------------------------------------------------------------------------- +# render_template_editor -- NiceGUI render branch (fake ``ui`` surface) +# --------------------------------------------------------------------------- +# +# The headless payload tests above cover ``_build_payload``; the rich render +# branch (header / lint banner / file list + inline editor / question form / +# upload) only runs when ``from nicegui import ui`` succeeds. We inject a fake +# ``ui`` module that records created widgets and the ``on_click`` / ``on_upload`` +# handlers keyed by their ``data-testid`` so the tests can both render the tree +# and *invoke* the action handlers (edit / save / delete / add-question / +# upload), mirroring the ``_Fluent`` pattern in ``test_mount.py``. + + +class _FakeWidget: + """Chainable NiceGUI element stand-in that records props + handlers.""" + + def __init__(self, registry: _Registry, kind: str, **attrs: Any) -> None: + self._registry = registry + self.kind = kind + self.attrs = attrs + self.value = attrs.get("value", "") + self.testid: str | None = None + self.on_click_handler: Callable[..., Any] | None = None + self.deleted = False + + def props(self, spec: str = "", **_kw: Any) -> _FakeWidget: + # Extract data-testid="..." so handlers can be looked up by it. + marker = 'data-testid="' + if marker in spec: + start = spec.index(marker) + len(marker) + self.testid = spec[start : spec.index('"', start)] + self._registry.by_testid[self.testid] = self + return self + + def style(self, *_a: Any, **_k: Any) -> _FakeWidget: + return self + + def classes(self, *_a: Any, **_k: Any) -> _FakeWidget: + return self + + def bind_value(self, *_a: Any, **_k: Any) -> _FakeWidget: + return self + + def on_value_change(self, *_a: Any, **_k: Any) -> _FakeWidget: + return self + + def delete(self) -> None: + self.deleted = True + + def __enter__(self) -> _FakeWidget: + return self + + def __exit__(self, *_a: Any) -> bool: + return False + + +class _Registry: + """Records every widget the fake ``ui`` creates, indexed by data-testid.""" + + def __init__(self) -> None: + self.widgets: list[_FakeWidget] = [] + self.by_testid: dict[str, _FakeWidget] = {} + + +class _FakeUI: + """Minimal NiceGUI ``ui`` surface covering the editor's widget calls. + + Exposes ``codemirror`` and ``upload`` so the editor takes its primary + branches (rich code editor + available upload widget). + """ + + def __init__(self, registry: _Registry) -> None: + self._registry = registry + + def _make(self, kind: str, on_click: Callable[..., Any] | None = None, **attrs: Any) -> Any: + w = _FakeWidget(self._registry, kind, **attrs) + w.on_click_handler = on_click + self._registry.widgets.append(w) + return w + + def card(self, *_a: Any, **_k: Any) -> Any: + return self._make("card") + + def column(self, *_a: Any, **_k: Any) -> Any: + return self._make("column") + + def row(self, *_a: Any, **_k: Any) -> Any: + return self._make("row") + + def label(self, text: str = "", *_a: Any, **_k: Any) -> Any: + return self._make("label", text=text) + + def input(self, *_a: Any, **kw: Any) -> Any: + return self._make("input", **kw) + + def textarea(self, *_a: Any, **kw: Any) -> Any: + return self._make("textarea", **kw) + + def codemirror(self, *_a: Any, **kw: Any) -> Any: + return self._make("codemirror", **kw) + + def select(self, *_a: Any, **kw: Any) -> Any: + return self._make("select", **kw) + + def checkbox(self, *_a: Any, **kw: Any) -> Any: + return self._make("checkbox", **kw) + + def button( + self, _text: str = "", *, on_click: Callable[..., Any] | None = None, **kw: Any + ) -> Any: + return self._make("button", on_click=on_click, **kw) + + def upload(self, *, on_upload: Callable[..., Any] | None = None, **_kw: Any) -> Any: + w = self._make("upload") + w.on_click_handler = on_upload # reuse the handler slot for the upload cb + return w + + +@pytest.fixture +def _fake_ui(monkeypatch: pytest.MonkeyPatch) -> Iterator[_Registry]: + """Inject a fake ``nicegui`` module so the rich render branch executes.""" + import types + + registry = _Registry() + fake_module = types.ModuleType("nicegui") + fake_module.ui = _FakeUI(registry) # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "nicegui", fake_module) + yield registry + + +def _click(registry: _Registry, testid: str, *args: Any) -> None: + """Invoke the recorded ``on_click`` handler for a widget by data-testid. + + The editor's button handlers are ``lambda _evt, ...`` (NiceGUI passes a + click event), so when the caller supplies no explicit args we pass a + single ``None`` event. Callers that drive a real payload (e.g. the upload + event) pass it explicitly. + """ + widget = registry.by_testid[testid] + assert widget.on_click_handler is not None, f"{testid} has no handler" + widget.on_click_handler(*(args or (None,))) + + +def test_editor_render_builds_widget_tree(_fake_ui: _Registry, tmp_path: Path) -> None: + """The NiceGUI branch renders header, file rows, question form, and upload.""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + write_content_file(root, "notes2.md", "# hi\n") + + card = render_template_editor(template_dir=root) + + assert card is not None # returns the root card, not the headless dict + ids = _fake_ui.by_testid + assert "te-title" in ids + assert "te-questions" in ids + assert "te-save-questions" in ids + assert "te-upload" in ids # upload widget present (fake ui exposes it) + # A per-file row + edit/delete controls for the editable content file. + assert "te-file-notes2.md" in ids + assert "te-edit-notes2.md" in ids + assert "te-delete-notes2.md" in ids + + +def test_editor_lint_banner_renders_errors(_fake_ui: _Registry, tmp_path: Path) -> None: + """A template with a broken .jinja renders the error lint banner branch.""" + root = create_template_dir(tmp_path, name="broken", template_type="project") + (root / "x.txt.jinja").write_text("{% if %}", encoding="utf-8") + + render_template_editor(template_dir=root) + + assert "te-lint-errors" in _fake_ui.by_testid + + +def test_editor_edit_then_save_content_invokes_callback( + _fake_ui: _Registry, tmp_path: Path +) -> None: + """Clicking Edit loads the file; Save file calls on_save_content(rel, text).""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + write_content_file(root, "data.csv", "a,b\n1,2\n") + saved: list[tuple[str, str]] = [] + + render_template_editor( + template_dir=root, + on_save_content=lambda rel, text: saved.append((rel, text)), + ) + + _click(_fake_ui, "te-edit-data.csv") # loads content into the shared editor + _click(_fake_ui, "te-save-content") + assert saved == [("data.csv", "a,b\n1,2\n")] + + +def test_editor_delete_invokes_callback(_fake_ui: _Registry, tmp_path: Path) -> None: + """The per-row Delete button calls on_delete(rel).""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + write_content_file(root, "drop.txt", "x") + deleted: list[str] = [] + + render_template_editor(template_dir=root, on_delete=deleted.append) + + _click(_fake_ui, "te-delete-drop.txt") + assert deleted == ["drop.txt"] + + +def test_editor_add_and_save_questions_rebuilds_manifest( + _fake_ui: _Registry, tmp_path: Path +) -> None: + """Add question -> set its widgets -> Save questions calls on_save_manifest.""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + saved: list[TemplateManifest] = [] + + render_template_editor(template_dir=root, on_save_manifest=saved.append) + + _click(_fake_ui, "te-q-add") # appends a blank question row + # Fill the freshly-added row's key/kind so it survives the empty-key filter. + _fake_ui.by_testid["te-q-key"].value = "specimen_id" + _fake_ui.by_testid["te-q-kind"].value = "int" + _fake_ui.by_testid["te-q-default"].value = "5" + _click(_fake_ui, "te-save-questions") + + assert len(saved) == 1 + manifest = saved[0] + assert manifest.exlab_type == "project" + assert [q.key for q in manifest.questions] == ["specimen_id"] + assert manifest.questions[0].kind == "int" + assert manifest.questions[0].default == 5 # coerced toward the int kind + + +def test_editor_remove_question_drops_row(_fake_ui: _Registry, tmp_path: Path) -> None: + """Adding then removing a question leaves no questions on save.""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + saved: list[TemplateManifest] = [] + + render_template_editor(template_dir=root, on_save_manifest=saved.append) + + _click(_fake_ui, "te-q-add") + _fake_ui.by_testid["te-q-key"].value = "temp" + _click(_fake_ui, "te-q-remove") + _click(_fake_ui, "te-save-questions") + + assert saved[0].questions == [] + + +def test_editor_upload_handler_invokes_callback(_fake_ui: _Registry, tmp_path: Path) -> None: + """The upload widget's event handler forwards (name, bytes, render_flag).""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + uploads: list[tuple[str, bytes, bool]] = [] + + render_template_editor( + template_dir=root, + on_upload=lambda name, data, flag: uploads.append((name, data, flag)), + ) + + _fake_ui.by_testid["te-upload-render"].value = True + + class _Content: + def read(self) -> bytes: + return b"payload" + + event = type("Evt", (), {"name": "proto.docx", "content": _Content()})() + _click(_fake_ui, "te-upload", event) + assert uploads == [("proto.docx", b"payload", True)] + + +def test_editor_back_button_invokes_callback(_fake_ui: _Registry, tmp_path: Path) -> None: + """The Back button calls on_back.""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + backs: list[bool] = [] + + render_template_editor(template_dir=root, on_back=lambda: backs.append(True)) + + _click(_fake_ui, "te-back", None) + assert backs == [True] + + +def test_editor_renders_warning_lint_banner(_fake_ui: _Registry, tmp_path: Path) -> None: + """A manifest that only trips WARN lint rules renders the warn banner.""" + root = create_template_dir(tmp_path, name="warn", template_type="project") + # A non-conforming question id is a WARN (not an ERROR), so the template + # still loads but the warn banner must render. + manifest_path = root / COPIER_MANIFEST_NAME + manifest_path.write_text( + manifest_path.read_text(encoding="utf-8") + "\nBadKey:\n type: str\n", + encoding="utf-8", + ) + + render_template_editor(template_dir=root) + + assert "te-lint-warns" in _fake_ui.by_testid + assert "te-lint-errors" not in _fake_ui.by_testid + + +def test_editor_save_content_without_open_is_noop(_fake_ui: _Registry, tmp_path: Path) -> None: + """Clicking Save file before opening any file does not call the callback.""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + saved: list[tuple[str, str]] = [] + + render_template_editor( + template_dir=root, on_save_content=lambda rel, text: saved.append((rel, text)) + ) + + _click(_fake_ui, "te-save-content") # no file opened -> editor_state["rel"] is None + assert saved == [] + + +def test_editor_edit_unreadable_file_does_not_crash(_fake_ui: _Registry, tmp_path: Path) -> None: + """Opening a file that disappears between listing and edit is swallowed.""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + write_content_file(root, "gone.txt", "x") + saved: list[tuple[str, str]] = [] + render_template_editor( + template_dir=root, on_save_content=lambda rel, text: saved.append((rel, text)) + ) + # Remove the file after render, then click its Edit button: _open hits the + # read failure branch and returns without setting editor state. + (root / "gone.txt").unlink() + _click(_fake_ui, "te-edit-gone.txt") + _click(_fake_ui, "te-save-content") + assert saved == [] # nothing was loaded, so save is a no-op + + +def test_editor_upload_event_noop_when_callback_missing( + _fake_ui: _Registry, tmp_path: Path +) -> None: + """The upload handler is safe when on_upload is None (no crash).""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + render_template_editor(template_dir=root) # on_upload defaults to None + event = type("Evt", (), {"name": "x.txt", "content": None})() + _click(_fake_ui, "te-upload", event) # must not raise + + +# A fake ``ui`` WITHOUT codemirror / upload to drive the fallback branches. +class _FakeUIMinimal(_FakeUI): + """Like ``_FakeUI`` but lacks ``codemirror`` and ``upload`` attributes.""" + + codemirror = None # type: ignore[assignment] + upload = None # type: ignore[assignment] + + +@pytest.fixture +def _fake_ui_minimal(monkeypatch: pytest.MonkeyPatch) -> Iterator[_Registry]: + import types + + registry = _Registry() + fake_module = types.ModuleType("nicegui") + fake_module.ui = _FakeUIMinimal(registry) # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "nicegui", fake_module) + yield registry + + +def test_editor_falls_back_to_textarea_and_marks_upload_unavailable( + _fake_ui_minimal: _Registry, tmp_path: Path +) -> None: + """Without codemirror/upload, the editor uses textarea + an unavailable note.""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + write_content_file(root, "n.md", "# n\n") + + card = render_template_editor(template_dir=root) + + assert card is not None + ids = _fake_ui_minimal.by_testid + # Inline editor still present (textarea fallback), upload flagged unavailable. + assert "te-editor" in ids + assert ids["te-editor"].kind == "textarea" + assert "te-upload-unavailable" in ids + assert "te-upload" not in ids + + +def test_editor_save_questions_noop_without_callback(_fake_ui: _Registry, tmp_path: Path) -> None: + """Save questions with no on_save_manifest is a no-op (early return).""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + render_template_editor(template_dir=root) # on_save_manifest defaults to None + _click(_fake_ui, "te-q-add") + _fake_ui.by_testid["te-q-key"].value = "k" + _click(_fake_ui, "te-save-questions") # must not raise + + +def test_editor_question_kinds_coerce_defaults(_fake_ui: _Registry, tmp_path: Path) -> None: + """choice/float/bool rows round-trip through _question_from_row + _coerce_default.""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + saved: list[TemplateManifest] = [] + render_template_editor(template_dir=root, on_save_manifest=saved.append) + + # Row 1: a choice question with a comma-separated choices field. + _click(_fake_ui, "te-q-add") + _fake_ui.by_testid["te-q-key"].value = "stain" + _fake_ui.by_testid["te-q-kind"].value = "choice" + _fake_ui.by_testid["te-q-choices"].value = "DAPI, GFP , RFP" + _click(_fake_ui, "te-save-questions") + + q = saved[-1].questions[0] + assert q.kind == "choice" + assert q.choices == ("DAPI", "GFP", "RFP") # split + stripped + + +def test_editor_bool_and_float_defaults_coerced(_fake_ui: _Registry, tmp_path: Path) -> None: + """A float row coerces its default to float; a bool row to bool.""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + saved: list[TemplateManifest] = [] + render_template_editor(template_dir=root, on_save_manifest=saved.append) + + _click(_fake_ui, "te-q-add") + _fake_ui.by_testid["te-q-key"].value = "exposure" + _fake_ui.by_testid["te-q-kind"].value = "float" + _fake_ui.by_testid["te-q-default"].value = "1.5" + _click(_fake_ui, "te-save-questions") + q = saved[-1].questions[0] + assert q.default == 1.5 + assert isinstance(q.default, float) + + +def test_editor_bool_default_and_uncoercible_int_fallback( + _fake_ui: _Registry, tmp_path: Path +) -> None: + """bool default coerces to True; an int kind with non-numeric default + falls back to the raw string (the _coerce_default ValueError branch).""" + root = create_template_dir(tmp_path, name="proj", template_type="project") + saved: list[TemplateManifest] = [] + render_template_editor(template_dir=root, on_save_manifest=saved.append) + + _click(_fake_ui, "te-q-add") + _fake_ui.by_testid["te-q-key"].value = "enabled" + _fake_ui.by_testid["te-q-kind"].value = "bool" + _fake_ui.by_testid["te-q-default"].value = "yes" + _click(_fake_ui, "te-save-questions") + assert saved[-1].questions[0].default is True + + _click(_fake_ui, "te-q-add") + # Second row added; its widgets are the latest te-q-* bound by the form. + _fake_ui.by_testid["te-q-key"].value = "count" + _fake_ui.by_testid["te-q-kind"].value = "int" + _fake_ui.by_testid["te-q-default"].value = "not-a-number" + _click(_fake_ui, "te-save-questions") + # The uncoercible int default falls back to the raw string, not a crash. + count_q = next(q for q in saved[-1].questions if q.key == "count") + assert count_q.default == "not-a-number" + + +def test_editor_renders_with_unreadable_manifest(_fake_ui: _Registry, tmp_path: Path) -> None: + """A corrupt copier.yml still renders (empty manifest + error lint banner).""" + root = create_template_dir(tmp_path, name="corrupt", template_type="project") + # Invalid YAML -> read_manifest raises -> both manifest-read branches fall back. + (root / COPIER_MANIFEST_NAME).write_text("{ this: is: not: yaml", encoding="utf-8") + + card = render_template_editor(template_dir=root) + + assert card is not None + # The error banner renders from the copier.yml parse failure. + assert "te-lint-errors" in _fake_ui.by_testid