Add pre-commit hooks for flow validation

.gitignore

@@ -5,3 +5,6 @@
 .venv/
 __pycache__/
 *.py[cod]
+
+# Keep repository tooling visible when a user globally ignores YAML files.
+!.pre-commit-config.yaml

.husky/pre-commit

@@ -0,0 +1,67 @@
+#!/usr/bin/env sh
+# Single Git entry point for both project areas.
+#
+# Git honors one hook path, and Husky owns it so both project areas can run
+# their checks. `pre-commit install` refuses to write a hook while
+# core.hooksPath is set, so this script calls `pre-commit run` instead; that is
+# the same staged-file run the installed hook performs.
+#
+# Each area only runs when it has staged changes, and a commit touching both
+# directories runs both. Nothing is nested: pre-commit and lint-staged each own
+# their stash for the length of their own step.
+#
+# The areas are independent, so a failure in one still runs the other: a commit
+# spanning both reports every problem instead of only the first.
+
+status=0
+# Treat renames as a deletion plus an addition so moving a flow to a path that
+# no longer matches *_flow.py cannot bypass inventory validation.
+staged=$(git diff --cached --no-renames --name-only --diff-filter=ACMRD)
+deleted=$(git diff --cached --no-renames --name-only --diff-filter=D)
+
+# Catch conflict markers and whitespace errors in staged changes for both areas.
+git diff --cached --check || status=1
+
+# Config that governs how every hook behaves (ruff settings, hook definitions,
+# pinned tool versions): a change here can affect files that aren't otherwise
+# staged, so it forces `--all-files` below instead of the default scoped run.
+config_pattern='^(\.pre-commit-config\.yaml$|pyproject\.toml$|\.husky/pre-commit$|flows/requirements(-dev)?\.txt$)'
+
+if printf '%s\n' "$staged" | grep -Eq "\.py$|$config_pattern"; then
+  if [ -x flows/.venv/bin/pre-commit ]; then
+    pre_commit=flows/.venv/bin/pre-commit
+  elif command -v pre-commit >/dev/null 2>&1; then
+    pre_commit=pre-commit
+  else
+    echo "pre-commit not found. Install it with:" >&2
+    echo "  python -m pip install -r flows/requirements-dev.txt" >&2
+    pre_commit=""
+    status=1
+  fi
+
+  if [ -n "$pre_commit" ]; then
+    # A deletion under flows/ leaves no staged file on disk for pre-commit's own
+    # staged-file scan to match, so `pre-commit run` would silently skip
+    # check-flows; run --all-files instead so discovery still sees the deletion.
+    if printf '%s\n' "$staged" | grep -Eq "$config_pattern" \
+      || printf '%s\n' "$deleted" | grep -Eq '^flows/'; then
+      "$pre_commit" run --all-files || status=1
+      # Discovery reads the working tree, where an untracked flow may exist.
+      # Check the index directly so the commit itself must retain a flow.
+      if printf '%s\n' "$deleted" | grep -Eq '^flows/.*_flow\.py$' \
+        && ! git ls-files --cached | grep -Eq '^flows/.*_flow\.py$'; then
+        echo "At least one tracked *_flow.py file is required under flows/." >&2
+        status=1
+      fi
+    else
+      # `files:` patterns in .pre-commit-config.yaml keep this to flows/.
+      "$pre_commit" run || status=1
+    fi
+  fi
+fi
+
+if git diff --cached --name-only --diff-filter=ACMR | grep -Eq '^e2e/(tests|test-setup)/'; then
+  (cd e2e && npm run precommit) || status=1
+fi
+
+exit "$status"

.pre-commit-config.yaml

@@ -0,0 +1,38 @@
+repos:
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.16.7
+    hooks:
+      - id: ruff-check
+        args: [--fix]
+        files: ^flows/.*\.py$
+      - id: ruff-format
+        files: ^flows/.*\.py$
+
+  - repo: local
+    hooks:
+      - id: check-flow-filenames
+        name: Validate tracked FlowSpec filenames
+        entry: python flows/scripts/check_flows.py --tracked-filenames-only
+        language: python
+        files: \.py$
+        pass_filenames: false
+
+      - id: check-flows
+        name: Validate Metaflow definitions
+        entry: python flows/scripts/check_flows.py
+        language: python
+        additional_dependencies:
+          - outerbounds==0.12.44
+        files: ^flows/.*_flow\.py$
+
+      - id: test-flow-tools
+        name: Test flow utilities
+        entry: pytest
+        args: [flows/tests]
+        language: python
+        additional_dependencies:
+          - pytest==9.1.1
+          - pyyaml
+        # Run checker tests only when checker behavior, tests, or tool pins change.
+        files: ^(flows/scripts/check_flows\.py|flows/tests/.*\.py|flows/requirements(-dev)?\.txt|\.pre-commit-config\.yaml|pyproject\.toml)$
+        pass_filenames: false

README.md

@@ -14,6 +14,42 @@ The `e2e/` directory contains browser-based end-to-end tests built with Playwrig
 
 See [`e2e/tests/README.md`](e2e/tests/README.md) for the Playwright test structure, import conventions, and validation workflow.
 
+## Development Checks
+
+Each project area keeps its own tooling: `flows/` uses the root-level
+`pre-commit` configuration, and `e2e/` uses its existing npm pre-commit command.
+Git honors a single hook path, so a Husky hook at [`.husky/pre-commit`](.husky/pre-commit)
+is the entry point. Staged Python files activate FlowSpec filename validation;
+files under `flows/` also activate the flow checks. Added, copied, modified, or
+renamed files under `e2e/tests/` and `e2e/test-setup/` activate the Playwright
+checks. Deletion-only e2e changes skip that staged-file pipeline. A commit
+touching both scopes runs both.
+
+Install the Python development requirements and the hook from the repository
+root:
+
+```bash
+python -m pip install -r flows/requirements-dev.txt
+npm --prefix e2e install
+```
+
+Do not run `pre-commit install`; it refuses to write a hook while Husky owns
+`core.hooksPath`, and the Husky hook already invokes `pre-commit run`.
+
+Run every local check explicitly with:
+
+```bash
+pre-commit run --all-files
+npm --prefix e2e run quality:full
+```
+
+Staged `*_flow.py` changes run Ruff and native Metaflow definition checks;
+checker tests run only when checker tooling or configuration changes. Staged
+existing files under `e2e/tests/` and `e2e/test-setup/` are formatted and linted
+before the Playwright quality report runs. The hook also rejects conflict
+markers and whitespace errors in staged changes. No hook authenticates to
+Outerbounds or starts remote workloads.
+
 ## Repository Layout
 
 ```text

e2e/package.json

@@ -33,7 +33,7 @@
     "trace": "playwright show-trace --port 0",
     "lint": "cross-env eslint 'tests/**/*.ts' 'test-setup/**/*.ts' 'playwright.config.ts'",
     "lint:fix": "cross-env eslint 'tests/**/*.ts' 'test-setup/**/*.ts' 'playwright.config.ts' --fix",
-    "prepare": "husky",
+    "prepare": "cd .. && husky",
     "format": "cross-env prettier --write --no-error-on-unmatched-pattern 'tests/**/*.ts' 'test-setup/**/*.ts' 'playwright.config.ts' '**/*.json' '**/*.md' '!package-lock.json' '!dist/**/*' '!build/**/*'",
     "postinstall": "playwright install chromium",
     "ncu:check": "npx npm-check-updates",

flows/README.md

@@ -7,19 +7,111 @@ independent Metaflow flows and their supporting test data and utilities. Each
 flow validates a focused scenario and can be run independently on an
 Outerbounds cluster.
 
-### Setup
+## Pre-Commit Setup
 
-Use an Outerbounds-supported Python version and install the client package in
-an isolated environment:
+Run all setup commands from the repository root. Use an Outerbounds-supported
+Python version to create an isolated environment:
 
 ```bash
-cd flows
-python3 -m venv .venv
-source .venv/bin/activate
+cd /path/to/ob-quality
+python3 -m venv flows/.venv
+source flows/.venv/bin/activate
 python -m pip install --upgrade pip
-python -m pip install -r requirements.txt
+python -m pip install -r flows/requirements-dev.txt
 ```
 
+`requirements-dev.txt` includes the runtime requirements, so a separate
+installation of `flows/requirements.txt` is not required for development.
+
+Git honors a single hook path, and the repository gives it to Husky so the
+Playwright checks keep their lint-staged auto-fixes. `.husky/pre-commit` calls
+`pre-commit run` when a commit stages files under `flows/`, so do not run
+`pre-commit install` — it refuses to write a hook while `core.hooksPath` is set.
+
+Install the shared hook from the repository root:
+
+```bash
+npm --prefix e2e install
+```
+
+Confirm that Husky owns the hook path and that `pre-commit` is callable from it:
+
+```bash
+test "$(git config --get core.hooksPath)" = ".husky/_" && echo "husky installed"
+pre-commit --version
+```
+
+The hook falls back to `flows/.venv/bin/pre-commit` when the virtual environment
+is not active, so a commit from an editor or GUI client still runs the flow
+checks.
+
+### Test the Hook
+
+Run all flow hooks against all tracked files:
+
+```bash
+pre-commit run --all-files
+```
+
+Run only the flow-related hooks:
+
+```bash
+pre-commit run ruff-check --all-files
+pre-commit run ruff-format --all-files
+pre-commit run check-flows --all-files
+pre-commit run test-flow-tools --all-files
+```
+
+The flow hooks perform these checks:
+
+- `ruff-check`: Python linting, import ordering, common bug checks, and public
+  method docstrings.
+- `ruff-format`: Python formatting.
+- `check-flow-filenames`: staged-index validation that every tracked Python file
+  containing a `FlowSpec` uses the `*_flow.py` suffix.
+- `check-flows`: flow discovery, repository conventions, and native Metaflow
+  definition and DAG validation.
+- `test-flow-tools`: unit tests for the custom checker and tool-version
+  consistency.
+
+During a normal `git commit`, staged files determine which hooks run. A staged
+`*_flow.py` file under `flows/` activates Ruff and native Metaflow validation;
+other staged Python files under `flows/` activate Ruff. Any staged Python file
+activates repository-wide FlowSpec filename validation. Checker tests run only
+when `check_flows.py`, its tests, requirements, or tool configuration changes.
+The native Metaflow check runs only for staged flow files during normal runs,
+while filename and deletion checks use the staged index. This catches rewritten
+delete/add pairs without depending on Git rename detection and requires the
+index to retain at least one tracked flow.
+
+Added, copied, modified, or renamed files under `e2e/tests/` and
+`e2e/test-setup/` separately activate the Playwright format, lint, and quality
+checks; deletion-only e2e changes skip that staged-file pipeline. A commit
+touching both scopes runs both. Ruff also fixes staged Python lint and formatting
+issues. All staged changes are checked for conflict markers and whitespace
+errors. Neither project check authenticates to Outerbounds or starts a
+Kubernetes workload.
+
+## Flow Validation
+
+Every executable QA flow must:
+
+- Use a `*_flow.py` filename.
+- Retain the `*_flow.py` suffix when renamed or moved.
+- Define exactly one top-level `FlowSpec` subclass.
+- Use a flow class name that is unique across all domains.
+- Instantiate that class under `if __name__ == "__main__"`.
+
+To validate only flow discovery and Metaflow definitions:
+
+```bash
+python flows/scripts/check_flows.py
+```
+
+This command runs Metaflow's native `check` command for every discovered flow.
+It validates definitions and DAG structure without authenticating, downloading
+models, or starting local or remote flow runs.
+
 Configure a Metaflow profile for the target Outerbounds cluster before running
 a flow. Remote task pods must receive `OBP_API_SERVER`, `OBP_PERIMETER`, and
 `METAFLOW_SERVICE_HEADERS`; the platform normally injects these values.

flows/models/browse_models_flow.py

@@ -5,7 +5,6 @@
 """
 
 from metaflow import FlowSpec, anaconda_models, step
-
 from testdata.model_catalog_data import BROWSE_LIMIT
 from utils.model_validators import validate_models
 
@@ -14,11 +13,10 @@ class BrowseModelsFlow(FlowSpec):
     @anaconda_models
     @step
     def start(self):
+        """Browse the model catalog and validate returned metadata."""
         models = self.anaconda_models.list_models(limit=BROWSE_LIMIT)
 
-        assert isinstance(models, list), (
-            f"Expected a list of models, got {type(models).__name__}"
-        )
+        assert isinstance(models, list), f"Expected a list of models, got {type(models).__name__}"
         assert models, "Expected at least one model"
         assert len(models) <= BROWSE_LIMIT, (
             f"Expected at most {BROWSE_LIMIT} models, got {len(models)}"
@@ -27,16 +25,15 @@ def start(self):
         validate_models(models)
 
         names = [model["name"] for model in models]
-        assert len(names) == len(set(names)), (
-            f"Expected unique model names, got {names}"
-        )
+        assert len(names) == len(set(names)), f"Expected unique model names, got {names}"
 
         print(f"Validated {len(models)} models")
 
         self.next(self.end)
 
     @step
     def end(self):
+        """Report successful catalog validation."""
         print("BROWSE MODELS FLOW PASSED")
 
 

flows/models/download_gguf_model_flow.py

@@ -7,14 +7,14 @@
 import os
 
 from metaflow import FlowSpec, anaconda_models, step
-
 from testdata.model_catalog_data import GGUF_MODEL
 
 
 class DownloadGgufModelFlow(FlowSpec):
     @anaconda_models
     @step
     def start(self):
+        """Download the configured GGUF model and validate its local artifact."""
         model = self.anaconda_models.model(
             GGUF_MODEL["name"],
             format=GGUF_MODEL["format"],
@@ -26,30 +26,23 @@ def start(self):
             f"Expected model {GGUF_MODEL['name']}, got {model.name}"
         )
         access_denied_reason = getattr(model, "access_denied_reason", None)
-        assert not access_denied_reason, (
-            f"Model access was denied: {access_denied_reason}"
-        )
+        assert not access_denied_reason, f"Model access was denied: {access_denied_reason}"
         assert model.format == GGUF_MODEL["format"], (
             f"Expected format {GGUF_MODEL['format']}, got {model.format}"
         )
         assert model.quant_method == GGUF_MODEL["quant_method"], (
-            f"Expected quantization {GGUF_MODEL['quant_method']}, "
-            f"got {model.quant_method}"
+            f"Expected quantization {GGUF_MODEL['quant_method']}, got {model.quant_method}"
         )
         pulled_path = model.pull()
 
-        assert pulled_path == model.path, (
-            f"Expected pull to return {model.path}, got {pulled_path}"
-        )
+        assert pulled_path == model.path, f"Expected pull to return {model.path}, got {pulled_path}"
         assert model.download_status in ("downloaded", "skipped"), (
             f"Unexpected download status: {model.download_status}"
         )
         assert isinstance(pulled_path, str) and pulled_path.strip(), (
             f"Expected a non-empty model path, got {pulled_path!r}"
         )
-        assert os.path.isfile(pulled_path), (
-            f"Downloaded model file does not exist: {pulled_path}"
-        )
+        assert os.path.isfile(pulled_path), f"Downloaded model file does not exist: {pulled_path}"
         assert os.access(pulled_path, os.R_OK), (
             f"Downloaded model file is not readable: {pulled_path}"
         )
@@ -75,6 +68,7 @@ def start(self):
 
     @step
     def end(self):
+        """Report successful GGUF model validation."""
         print("GGUF MODEL DOWNLOAD FLOW PASSED")