From ea74a962f30f96dcaf63311931a7301dc642d79d Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Fri, 11 Sep 2026 14:54:05 +0100 Subject: [PATCH 01/28] updated doc string --- flows/models/browse_models_flow.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/flows/models/browse_models_flow.py b/flows/models/browse_models_flow.py index e4e7e4f..f1bd766 100644 --- a/flows/models/browse_models_flow.py +++ b/flows/models/browse_models_flow.py @@ -1,35 +1,28 @@ -"""Browse models in the catalog. +"""Browse models in the catalog . Run from the flows directory with: python models/browse_models_flow.py --environment=fast-bakery run --with kubernetes """ from metaflow import FlowSpec, anaconda_models, step - from testdata.model_catalog_data import BROWSE_LIMIT -from utils.model_validators import validate_models 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)}" ) - 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") @@ -37,6 +30,7 @@ def start(self): @step def end(self): + """Report successful catalog validation.""" print("BROWSE MODELS FLOW PASSED") From b1c52faa14d22415552efeb05c5d6662b5fded44 Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Fri, 11 Sep 2026 14:55:31 +0100 Subject: [PATCH 02/28] update to the error message --- e2e/tests/pages/models/ob-page.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/pages/models/ob-page.ts b/e2e/tests/pages/models/ob-page.ts index 3182f60..2a7b05d 100644 --- a/e2e/tests/pages/models/ob-page.ts +++ b/e2e/tests/pages/models/ob-page.ts @@ -9,7 +9,7 @@ export class OBPage { public async verifyDashboardURL(): Promise { await AssertUtils.expectPageToHaveURL(new RegExp(`^${escapeRegExp(BASE_URL)}(?:/|$|\\?)`), { - message: 'Authenticated user should remain on the configured dashboard route', + message: 'Authenticated user should remain on the configured dashboard route ', }); } } From 9770184aacdf26903b58c61ad66d04d0cabb7798 Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Fri, 11 Sep 2026 15:00:34 +0100 Subject: [PATCH 03/28] delete main --- e2e/tests/pages/models/ob-page.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/pages/models/ob-page.ts b/e2e/tests/pages/models/ob-page.ts index 2a7b05d..3182f60 100644 --- a/e2e/tests/pages/models/ob-page.ts +++ b/e2e/tests/pages/models/ob-page.ts @@ -9,7 +9,7 @@ export class OBPage { public async verifyDashboardURL(): Promise { await AssertUtils.expectPageToHaveURL(new RegExp(`^${escapeRegExp(BASE_URL)}(?:/|$|\\?)`), { - message: 'Authenticated user should remain on the configured dashboard route ', + message: 'Authenticated user should remain on the configured dashboard route', }); } } From f345e595d65752ac967a240b542a4c13d90d279b Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Fri, 11 Sep 2026 15:49:34 +0100 Subject: [PATCH 04/28] testing hook --- .pre-commit-config.yaml | 30 ++++++++++++++++++++ flows/models/browse_models_flow.py | 4 +-- flows/requirements.txt | 2 +- flows/tests/test_tool_versions.py | 44 ++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 .pre-commit-config.yaml create mode 100644 flows/tests/test_tool_versions.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..bb412b1 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,30 @@ +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-flows + name: Validate Metaflow definitions + entry: python flows/scripts/check_flows.py + language: python + additional_dependencies: + - outerbounds==0.12.44 + files: ^flows/.*\.py$ + pass_filenames: false + + - id: test-flow-tools + name: Test flow utilities + entry: pytest + args: [flows/tests] + language: python + additional_dependencies: + - pytest==9.1.1 + files: ^flows/.*\.py$ + pass_filenames: false diff --git a/flows/models/browse_models_flow.py b/flows/models/browse_models_flow.py index f1bd766..3362f9d 100644 --- a/flows/models/browse_models_flow.py +++ b/flows/models/browse_models_flow.py @@ -1,4 +1,4 @@ -"""Browse models in the catalog . +"""Browse models in the catalog. Run from the flows directory with: python models/browse_models_flow.py --environment=fast-bakery run --with kubernetes @@ -31,7 +31,7 @@ def start(self): @step def end(self): """Report successful catalog validation.""" - print("BROWSE MODELS FLOW PASSED") + print("BROWSE MODELS FLOW PASSE") if __name__ == "__main__": diff --git a/flows/requirements.txt b/flows/requirements.txt index e668b0f..81b4438 100644 --- a/flows/requirements.txt +++ b/flows/requirements.txt @@ -1 +1 @@ -outerbounds \ No newline at end of file +outerbounds==0.12.44 diff --git a/flows/tests/test_tool_versions.py b/flows/tests/test_tool_versions.py new file mode 100644 index 0000000..8071dea --- /dev/null +++ b/flows/tests/test_tool_versions.py @@ -0,0 +1,44 @@ +import re +from pathlib import Path + +REPOSITORY_ROOT = Path(__file__).resolve().parents[2] +REQUIREMENTS_PATH = REPOSITORY_ROOT / "flows" / "requirements.txt" +DEV_REQUIREMENTS_PATH = REPOSITORY_ROOT / "flows" / "requirements-dev.txt" +PRE_COMMIT_CONFIG_PATH = REPOSITORY_ROOT / ".pre-commit-config.yaml" + + +def requirement_version(package: str, requirements_path: Path = DEV_REQUIREMENTS_PATH) -> str: + """Read one strictly pinned package version from a requirements file.""" + pattern = re.compile(rf"^{re.escape(package)}==([^\s#]+)", re.MULTILINE) + match = pattern.search(requirements_path.read_text(encoding="utf-8")) + # Under pre-commit only staged content is on disk, so an unstaged pin reads + # as missing here: stage the requirements and hook changes together. + assert match, f"Missing pinned {package} version in {requirements_path}" + return match.group(1) + + +def test_ruff_versions_match() -> None: + """Keep the Ruff CLI version aligned with the isolated pre-commit hook.""" + config = PRE_COMMIT_CONFIG_PATH.read_text(encoding="utf-8") + match = re.search( + r"repo: https://github\.com/astral-sh/ruff-pre-commit\s+rev: v([^\s]+)", + config, + ) + assert match, f"Missing pinned Ruff revision in {PRE_COMMIT_CONFIG_PATH}" + assert match.group(1) == requirement_version("ruff") + + +def test_pytest_versions_match() -> None: + """Keep the pytest CLI version aligned with its pre-commit environment.""" + config = PRE_COMMIT_CONFIG_PATH.read_text(encoding="utf-8") + match = re.search(r"additional_dependencies:\s+- pytest==([^\s]+)", config) + assert match, f"Missing pinned pytest dependency in {PRE_COMMIT_CONFIG_PATH}" + assert match.group(1) == requirement_version("pytest") + + +def test_outerbounds_versions_match() -> None: + """Validate flows against the same Outerbounds release that CI installs.""" + config = PRE_COMMIT_CONFIG_PATH.read_text(encoding="utf-8") + match = re.search(r"additional_dependencies:\s+- outerbounds==([^\s]+)", config) + assert match, f"Missing pinned outerbounds dependency in {PRE_COMMIT_CONFIG_PATH}" + assert match.group(1) == requirement_version("outerbounds", REQUIREMENTS_PATH) From 1858546d024bc2b3b7432a3563e93b9bcf9ab60e Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Fri, 11 Sep 2026 15:50:01 +0100 Subject: [PATCH 05/28] testing hook --- flows/models/browse_models_flow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flows/models/browse_models_flow.py b/flows/models/browse_models_flow.py index 3362f9d..85fa810 100644 --- a/flows/models/browse_models_flow.py +++ b/flows/models/browse_models_flow.py @@ -31,7 +31,7 @@ def start(self): @step def end(self): """Report successful catalog validation.""" - print("BROWSE MODELS FLOW PASSE") + print("BROWSE MODELS FLOW PASSED") if __name__ == "__main__": From e2a2f29f94c074a2ea27c3ff8a1fcd38589def09 Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Fri, 11 Sep 2026 17:48:39 +0100 Subject: [PATCH 06/28] add precommit hooks --- .gitignore | 3 + .husky/pre-commit | 53 ++++ .pre-commit-config.yaml | 7 +- README.md | 32 +++ e2e/.husky/pre-commit | 4 - flows/README.md | 103 +++++++- flows/models/browse_models_flow.py | 3 + flows/models/download_gguf_model_flow.py | 18 +- .../models/download_safetensors_model_flow.py | 36 +-- flows/models/local_cache_model_flow.py | 48 ++-- flows/models/utils/model_validators.py | 34 +-- flows/requirements-dev.txt | 6 + flows/scripts/check_flows.py | 240 ++++++++++++++++++ flows/tests/test_check_flows.py | 170 +++++++++++++ flows/tests/test_tool_versions.py | 54 ++-- pyproject.toml | 11 + 16 files changed, 700 insertions(+), 122 deletions(-) create mode 100755 .husky/pre-commit delete mode 100644 e2e/.husky/pre-commit create mode 100644 flows/requirements-dev.txt create mode 100644 flows/scripts/check_flows.py create mode 100644 flows/tests/test_check_flows.py create mode 100644 pyproject.toml diff --git a/.gitignore b/.gitignore index a4cd4f0..892c9ae 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,6 @@ .venv/ __pycache__/ *.py[cod] + +# Keep repository tooling visible when a user globally ignores YAML files. +!.pre-commit-config.yaml diff --git a/.husky/pre-commit b/.husky/pre-commit new file mode 100755 index 0000000..e8d3e69 --- /dev/null +++ b/.husky/pre-commit @@ -0,0 +1,53 @@ +#!/usr/bin/env sh +# Single Git entry point for both project areas. +# +# Git honors one hook path, and Husky owns it so the Playwright checks keep +# running through lint-staged, which auto-fixes and re-stages files. `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 +staged=$(git diff --cached --name-only --diff-filter=ACMRD) +deleted=$(git diff --cached --name-only --diff-filter=D) + +# 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 "^flows/|$config_pattern"; then + if command -v pre-commit >/dev/null 2>&1; then + pre_commit=pre-commit + elif [ -x flows/.venv/bin/pre-commit ]; then + pre_commit=flows/.venv/bin/pre-commit + else + echo "pre-commit not found. Install it with:" >&2 + echo " python -m pip install -r flows/requirements-dev.txt" >&2 + exit 1 + fi + + # 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 + else + # `files:` patterns in .pre-commit-config.yaml keep this to flows/. + "$pre_commit" run || status=1 + fi +fi + +if printf '%s\n' "$staged" | grep -Eq '^(e2e/|\.husky/pre-commit$)'; then + npm --prefix e2e run precommit || status=1 +fi + +exit "$status" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bb412b1..4ce619c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -17,7 +17,6 @@ repos: additional_dependencies: - outerbounds==0.12.44 files: ^flows/.*\.py$ - pass_filenames: false - id: test-flow-tools name: Test flow utilities @@ -26,5 +25,9 @@ repos: language: python additional_dependencies: - pytest==9.1.1 - files: ^flows/.*\.py$ + - pyyaml + # Includes flows/requirements*.txt and this file so the pinned-version + # consistency checks in flows/tests/test_tool_versions.py run whenever + # a version changes, not only when a flows/*.py file is also staged. + files: ^(flows/.*\.py|flows/requirements(-dev)?\.txt|\.pre-commit-config\.yaml)$ pass_filenames: false diff --git a/README.md b/README.md index d9fce32..579c5d5 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,38 @@ 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 and runs each area only when that area has staged changes. A +commit touching both directories 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 +e2e/node_modules/.bin/husky +``` + +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 +``` + +Python changes run Ruff, focused pytest tests, and native Metaflow definition +checks. E2E changes run the npm `precommit` command, which applies lint-staged +fixes and then the Playwright quality report. No hook authenticates to +Outerbounds or starts remote workloads. + ## Repository Layout ```text diff --git a/e2e/.husky/pre-commit b/e2e/.husky/pre-commit deleted file mode 100644 index 60b037d..0000000 --- a/e2e/.husky/pre-commit +++ /dev/null @@ -1,4 +0,0 @@ -if git diff --cached --name-only --diff-filter=ACMR | grep -qE '^(tests|test-setup)/'; then - npm run precommit || exit 1 -fi - diff --git a/flows/README.md b/flows/README.md index 409ae8c..6c1349e 100644 --- a/flows/README.md +++ b/flows/README.md @@ -7,19 +7,108 @@ 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 +e2e/node_modules/.bin/husky +``` + +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-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 +Python file under `flows/` activates all four flow hooks, and staged files +under `e2e/` separately activate the Playwright checks; a commit touching both +runs both. `test-flow-tools` also runs on its own when only +`flows/requirements.txt`, `flows/requirements-dev.txt`, or +`.pre-commit-config.yaml` is staged, so a version bump alone still triggers the +tool-version consistency checks. `check-flows` validates all discovered flows, +and `test-flow-tools` runs the complete local test suite. Neither hook +authenticates to Outerbounds or starts a Kubernetes workload. + +If Ruff modifies a file, review and stage the change before committing again: + +```bash +git add +git commit +``` + +## Flow Validation + +Every executable QA flow must: + +- Use a `*_flow.py` filename. +- 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. diff --git a/flows/models/browse_models_flow.py b/flows/models/browse_models_flow.py index 85fa810..563c260 100644 --- a/flows/models/browse_models_flow.py +++ b/flows/models/browse_models_flow.py @@ -6,6 +6,7 @@ from metaflow import FlowSpec, anaconda_models, step from testdata.model_catalog_data import BROWSE_LIMIT +from utils.model_validators import validate_models class BrowseModelsFlow(FlowSpec): @@ -21,6 +22,8 @@ def start(self): f"Expected at most {BROWSE_LIMIT} models, got {len(models)}" ) + validate_models(models) + names = [model["name"] for model in models] assert len(names) == len(set(names)), f"Expected unique model names, got {names}" diff --git a/flows/models/download_gguf_model_flow.py b/flows/models/download_gguf_model_flow.py index 2d10aa0..e2f92f1 100644 --- a/flows/models/download_gguf_model_flow.py +++ b/flows/models/download_gguf_model_flow.py @@ -7,7 +7,6 @@ import os from metaflow import FlowSpec, anaconda_models, step - from testdata.model_catalog_data import GGUF_MODEL @@ -15,6 +14,7 @@ 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") diff --git a/flows/models/download_safetensors_model_flow.py b/flows/models/download_safetensors_model_flow.py index f700334..3864334 100644 --- a/flows/models/download_safetensors_model_flow.py +++ b/flows/models/download_safetensors_model_flow.py @@ -7,7 +7,6 @@ import os from metaflow import FlowSpec, anaconda_models, step - from testdata.model_catalog_data import SAFETENSORS_MODEL @@ -15,6 +14,7 @@ class DownloadSafetensorsModelFlow(FlowSpec): @anaconda_models @step def start(self): + """Download and validate the configured Safetensors collection.""" model = self.anaconda_models.model( SAFETENSORS_MODEL["name"], format=SAFETENSORS_MODEL["format"], @@ -23,9 +23,7 @@ def start(self): assert model is not None, "Expected a model handle" 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.name == SAFETENSORS_MODEL["name"], ( f"Expected model {SAFETENSORS_MODEL['name']}, got {model.name}" ) @@ -39,12 +37,8 @@ def start(self): assert isinstance(model.path, str) and model.path.strip(), ( f"Expected a non-empty collection path, got {model.path!r}" ) - assert os.path.isdir(model.path), ( - f"Collection directory does not exist: {model.path}" - ) - assert os.access(model.path, os.R_OK), ( - f"Collection directory is not readable: {model.path}" - ) + assert os.path.isdir(model.path), f"Collection directory does not exist: {model.path}" + assert os.access(model.path, os.R_OK), f"Collection directory is not readable: {model.path}" assert isinstance(model.files, list) and model.files, ( "Expected a non-empty list of collection files" ) @@ -59,8 +53,7 @@ def start(self): for file_info in model.files: assert isinstance(file_info, dict), ( - f"Expected each file entry to be a dictionary, got " - f"{type(file_info).__name__}" + f"Expected each file entry to be a dictionary, got {type(file_info).__name__}" ) filename = file_info.get("filename") @@ -73,36 +66,28 @@ def start(self): f"Collection file resolves outside its directory: {filename}" ) assert os.path.isfile(file_path), f"Missing collection file: {filename}" - assert os.access(file_path, os.R_OK), ( - f"Collection file is not readable: {filename}" - ) + assert os.access(file_path, os.R_OK), f"Collection file is not readable: {filename}" actual_size = os.path.getsize(file_path) expected_size = file_info.get("size_bytes") if expected_size is not None: - assert isinstance(expected_size, int) and not isinstance( - expected_size, bool - ), ( + assert isinstance(expected_size, int) and not isinstance(expected_size, bool), ( f"Expected size_bytes for {filename} to be an integer, " f"got {type(expected_size).__name__}: {expected_size!r}" ) assert expected_size >= 0, ( - f"Expected size_bytes for {filename} to be non-negative, " - f"got {expected_size}" + f"Expected size_bytes for {filename} to be non-negative, got {expected_size}" ) if expected_size > 0: assert actual_size == expected_size, ( - f"Size mismatch for {filename}: " - f"expected {expected_size}, got {actual_size}" + f"Size mismatch for {filename}: expected {expected_size}, got {actual_size}" ) filenames.append(filename) total_size += actual_size - assert len(filenames) == len(set(filenames)), ( - f"Expected unique filenames, got {filenames}" - ) + assert len(filenames) == len(set(filenames)), f"Expected unique filenames, got {filenames}" assert total_size > 0, "Expected total collection size to be greater than zero" print(f"Downloaded collection: {model.name}") @@ -114,6 +99,7 @@ def start(self): @step def end(self): + """Report successful Safetensors collection validation.""" print("SAFETENSORS MODEL DOWNLOAD FLOW PASSED") diff --git a/flows/models/local_cache_model_flow.py b/flows/models/local_cache_model_flow.py index 6939e4c..2b57467 100644 --- a/flows/models/local_cache_model_flow.py +++ b/flows/models/local_cache_model_flow.py @@ -15,7 +15,6 @@ import uuid from metaflow import FlowSpec, anaconda_models, step - from testdata.model_catalog_data import GGUF_MODEL CACHE_ROOT = os.path.join( @@ -28,18 +27,15 @@ class LocalCacheModelFlow(FlowSpec): @anaconda_models(temp_dir_root=CACHE_ROOT) @step def start(self): + """Verify cold download and warm reuse in the task-local model cache.""" try: cold_model = self.anaconda_models.model( GGUF_MODEL["name"], format=GGUF_MODEL["format"], quant_method=GGUF_MODEL["quant_method"], ) - access_denied_reason = getattr( - cold_model, "access_denied_reason", None - ) - assert not access_denied_reason, ( - f"Model access was denied: {access_denied_reason}" - ) + access_denied_reason = getattr(cold_model, "access_denied_reason", None) + assert not access_denied_reason, f"Model access was denied: {access_denied_reason}" assert cold_model.name == GGUF_MODEL["name"], ( f"Expected model {GGUF_MODEL['name']}, got {cold_model.name}" ) @@ -47,8 +43,7 @@ def start(self): f"Expected format {GGUF_MODEL['format']}, got {cold_model.format}" ) assert cold_model.quant_method == GGUF_MODEL["quant_method"], ( - f"Expected quantization {GGUF_MODEL['quant_method']}, " - f"got {cold_model.quant_method}" + f"Expected quantization {GGUF_MODEL['quant_method']}, got {cold_model.quant_method}" ) assert not cold_model.is_collection, "Expected a single-file model" @@ -65,21 +60,16 @@ def start(self): cold_path = os.path.realpath(cold_model.path) assert cold_path == expected_path, ( - f"Model path changed after pull: expected {expected_path}, " - f"got {cold_path}" + f"Model path changed after pull: expected {expected_path}, got {cold_path}" ) assert cold_model.download_status == "downloaded", ( - f"Expected first pull to download the model, got " - f"{cold_model.download_status!r}" - ) - assert os.path.isfile(cold_path), ( - f"Downloaded model is missing: {cold_path}" + f"Expected first pull to download the model, got {cold_model.download_status!r}" ) + assert os.path.isfile(cold_path), f"Downloaded model is missing: {cold_path}" assert cold_model.files, "Cold pull did not report any model files" for file_info in cold_model.files: assert file_info["status"] == "downloaded", ( - "Expected downloaded file status 'downloaded', got " - f"{file_info['status']!r}" + f"Expected downloaded file status 'downloaded', got {file_info['status']!r}" ) cold_size = os.path.getsize(cold_path) @@ -99,32 +89,23 @@ def start(self): quant_method=GGUF_MODEL["quant_method"], ) assert warm_model is not cold_model, "Expected a new model handle" - access_denied_reason = getattr( - warm_model, "access_denied_reason", None - ) - assert not access_denied_reason, ( - f"Model access was denied: {access_denied_reason}" - ) + access_denied_reason = getattr(warm_model, "access_denied_reason", None) + assert not access_denied_reason, f"Model access was denied: {access_denied_reason}" warm_model.pull() warm_path = os.path.realpath(warm_model.path) - assert warm_path == cold_path, ( - f"Expected warm model path {cold_path}, got {warm_path}" - ) + assert warm_path == cold_path, f"Expected warm model path {cold_path}, got {warm_path}" assert warm_model.download_status == "skipped", ( - f"Expected second pull to hit the local cache, got " - f"{warm_model.download_status!r}" + f"Expected second pull to hit the local cache, got {warm_model.download_status!r}" ) assert warm_model.files, "Warm pull did not report any model files" assert len(warm_model.files) == len(cold_model.files), ( - f"Expected {len(cold_model.files)} cached files, got " - f"{len(warm_model.files)}" + f"Expected {len(cold_model.files)} cached files, got {len(warm_model.files)}" ) for file_info in warm_model.files: assert file_info["status"] == "skipped", ( - f"Expected cached file status 'skipped', got " - f"{file_info['status']!r}" + f"Expected cached file status 'skipped', got {file_info['status']!r}" ) warm_stat = os.stat(warm_path) @@ -144,6 +125,7 @@ def start(self): @step def end(self): + """Report successful task-local cache validation.""" print("LOCAL CACHE MODEL FLOW PASSED") diff --git a/flows/models/utils/model_validators.py b/flows/models/utils/model_validators.py index bda6952..470ce22 100644 --- a/flows/models/utils/model_validators.py +++ b/flows/models/utils/model_validators.py @@ -7,12 +7,10 @@ def validate_non_empty_string(value: Any, field_name: str, context: str) -> None: """Validate that a value is a non-empty string.""" assert isinstance(value, str), ( - f"{context}: field '{field_name}' must be a string; " - f"got {type(value).__name__}: {value!r}" + f"{context}: field '{field_name}' must be a string; got {type(value).__name__}: {value!r}" ) assert value.strip(), ( - f"{context}: field '{field_name}' must contain non-whitespace text; " - f"got {value!r}" + f"{context}: field '{field_name}' must contain non-whitespace text; got {value!r}" ) @@ -24,25 +22,21 @@ def validate_model(model: Mapping[str, Any], index: int) -> None: ) for field_name in ("name", "license", "source", "tags"): - assert field_name in model, ( - f"{context}: missing required field '{field_name}'" - ) + assert field_name in model, f"{context}: missing required field '{field_name}'" for field_name in ("name", "license"): validate_non_empty_string(model[field_name], field_name, context) source = model["source"] assert isinstance(source, Mapping), ( - f"{context}: field 'source' must be an object; " - f"got {type(source).__name__}: {source!r}" + f"{context}: field 'source' must be an object; got {type(source).__name__}: {source!r}" ) assert "name" in source, f"{context}: missing required field 'source.name'" validate_non_empty_string(source["name"], "source.name", context) tags = model["tags"] assert isinstance(tags, list), ( - f"{context}: field 'tags' must be a list of objects; " - f"got {type(tags).__name__}: {tags!r}" + f"{context}: field 'tags' must be a list of objects; got {type(tags).__name__}: {tags!r}" ) assert tags, f"{context}: field 'tags' must not be empty; got {tags!r}" for tag_index, tag in enumerate(tags): @@ -50,12 +44,8 @@ def validate_model(model: Mapping[str, Any], index: int) -> None: f"{context}: field 'tags[{tag_index}]' must be an object; " f"got {type(tag).__name__}: {tag!r}" ) - assert "name" in tag, ( - f"{context}: missing required field 'tags[{tag_index}].name'" - ) - validate_non_empty_string( - tag["name"], f"tags[{tag_index}].name", context - ) + assert "name" in tag, f"{context}: missing required field 'tags[{tag_index}].name'" + validate_non_empty_string(tag["name"], f"tags[{tag_index}].name", context) if "quantized_files" in model: quantized_files = model["quantized_files"] @@ -66,20 +56,16 @@ def validate_model(model: Mapping[str, Any], index: int) -> None: for file_index, file_info in enumerate(quantized_files): file_context = f"{context}, quantized file at index {file_index}" assert isinstance(file_info, Mapping), ( - f"{file_context}: expected an object, " - f"got {type(file_info).__name__}: {file_info!r}" + f"{file_context}: expected an object, got {type(file_info).__name__}: {file_info!r}" ) size_bytes = file_info.get("size_bytes") if size_bytes is not None: - assert isinstance(size_bytes, (int, float)) and not isinstance( - size_bytes, bool - ), ( + assert isinstance(size_bytes, (int, float)) and not isinstance(size_bytes, bool), ( f"{file_context}: field 'size_bytes' must be numeric, " f"not boolean; got {type(size_bytes).__name__}: {size_bytes!r}" ) assert size_bytes >= 0, ( - f"{file_context}: field 'size_bytes' must be at least zero; " - f"got {size_bytes!r}" + f"{file_context}: field 'size_bytes' must be at least zero; got {size_bytes!r}" ) diff --git a/flows/requirements-dev.txt b/flows/requirements-dev.txt new file mode 100644 index 0000000..95d1ba1 --- /dev/null +++ b/flows/requirements-dev.txt @@ -0,0 +1,6 @@ +-r requirements.txt + +pre-commit==4.6.2 +pytest==9.1.1 +pyyaml +ruff==0.16.7 \ No newline at end of file diff --git a/flows/scripts/check_flows.py b/flows/scripts/check_flows.py new file mode 100644 index 0000000..19ce8eb --- /dev/null +++ b/flows/scripts/check_flows.py @@ -0,0 +1,240 @@ +"""Discover QA flows, enforce repository conventions, and run Metaflow checks.""" + +from __future__ import annotations + +import argparse +import ast +import json +import subprocess +import sys +from collections.abc import Sequence +from dataclasses import dataclass +from pathlib import Path + +FLOWS_ROOT = Path(__file__).resolve().parents[1] +REPOSITORY_ROOT = FLOWS_ROOT.parent + + +@dataclass(frozen=True) +class FlowDefinition: + path: Path + class_name: str + + +class FlowCheckError(Exception): + """Raised when repository flow validation fails.""" + + +def _is_flowspec_base(base: ast.expr) -> bool: + """Recognize direct FlowSpec inheritance with or without module qualification.""" + return (isinstance(base, ast.Name) and base.id == "FlowSpec") or ( + isinstance(base, ast.Attribute) and base.attr == "FlowSpec" + ) + + +def _parse_flow(path: Path) -> ast.Module: + """Parse a flow file and surface readable syntax or file errors.""" + try: + return ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + except (OSError, SyntaxError) as error: + raise FlowCheckError(f"Unable to parse {path}: {error}") from error + + +def _flow_class_names(tree: ast.Module) -> list[str]: + """Return top-level FlowSpec subclass names from a parsed flow.""" + return [ + node.name + for node in tree.body + if isinstance(node, ast.ClassDef) and any(_is_flowspec_base(base) for base in node.bases) + ] + + +def _is_main_guard(test: ast.expr) -> bool: + """Recognize the standard `if __name__ == "__main__"` expression.""" + return ( + isinstance(test, ast.Compare) + and isinstance(test.left, ast.Name) + and test.left.id == "__name__" + and len(test.ops) == 1 + and isinstance(test.ops[0], ast.Eq) + and len(test.comparators) == 1 + and isinstance(test.comparators[0], ast.Constant) + and test.comparators[0].value == "__main__" + ) + + +def _has_main_guard_call(tree: ast.Module, class_name: str) -> bool: + """Return whether the main guard directly instantiates the discovered flow.""" + for node in tree.body: + if not isinstance(node, ast.If) or not _is_main_guard(node.test): + continue + return any( + isinstance(statement, ast.Expr) + and isinstance(statement.value, ast.Call) + and isinstance(statement.value.func, ast.Name) + and statement.value.func.id == class_name + for statement in node.body + ) + return False + + +def _resolve_selected_path(path: str, flows_root: Path) -> Path: + """Resolve CLI paths relative to either the repository or flows directory.""" + candidate = Path(path) + if candidate.is_absolute(): + return candidate.resolve() + + repository_candidate = (flows_root.parent / candidate).resolve() + if repository_candidate.is_file(): + return repository_candidate + return (flows_root / candidate).resolve() + + +def discover_flows( + flows_root: Path = FLOWS_ROOT, + selected_paths: Sequence[str] = (), +) -> list[FlowDefinition]: + """Discover flows and enforce repository-wide file and naming conventions.""" + root = flows_root.resolve() + + # Validate requested paths first, but do not use them as the repository + # inventory: duplicate flow names must still be detected in unselected files. + if selected_paths: + selected = sorted({_resolve_selected_path(path, root) for path in selected_paths}) + for path in selected: + try: + path.relative_to(root) + except ValueError as error: + raise FlowCheckError(f"Flow path is outside {root}: {path}") from error + if not path.is_file(): + raise FlowCheckError(f"Flow file does not exist: {path}") + if not path.name.endswith("_flow.py"): + raise FlowCheckError(f"Flow file must end with '_flow.py': {path}") + else: + selected = [] + + # Hidden directories and Python caches can contain third-party files whose + # names happen to end in `_flow.py`; they are not repository QA flows. + paths = sorted( + path.resolve() + for path in root.glob("**/*_flow.py") + if not any( + part.startswith(".") or part == "__pycache__" for part in path.relative_to(root).parts + ) + ) + + definitions: list[FlowDefinition] = [] + for path in paths: + # One FlowSpec per file keeps discovery and per-flow CI reporting + # unambiguous. Python parsing also catches syntax errors before Metaflow. + tree = _parse_flow(path) + class_names = _flow_class_names(tree) + if len(class_names) != 1: + raise FlowCheckError( + f"{path} must define exactly one top-level FlowSpec subclass; " + f"found {len(class_names)}" + ) + class_name = class_names[0] + + # Running `python flow.py check` requires the file's main guard to + # instantiate its FlowSpec; otherwise the command can exit without checking. + if not _has_main_guard_call(tree, class_name): + raise FlowCheckError( + f'{path} must instantiate {class_name} under `if __name__ == "__main__"`' + ) + definitions.append(FlowDefinition(path=path, class_name=class_name)) + + # An empty matrix would make CI appear successful without testing anything. + if not definitions: + raise FlowCheckError(f"No *_flow.py files found under {root}") + + # Metaflow identifies flows by class name, so duplicate names across product + # domains would make runs and metadata ambiguous. + paths_by_name: dict[str, Path] = {} + for definition in definitions: + previous_path = paths_by_name.get(definition.class_name) + if previous_path is not None: + raise FlowCheckError( + f"Duplicate flow class {definition.class_name}: " + f"{previous_path} and {definition.path}" + ) + paths_by_name[definition.class_name] = definition.path + + if not selected: + return definitions + + # A selected path can pass the checks above and still be excluded from + # discovery (hidden directory, Python cache), so report it instead of + # failing with a KeyError. + definitions_by_path = {definition.path: definition for definition in definitions} + missing = [path for path in selected if path not in definitions_by_path] + if missing: + raise FlowCheckError( + "Flow file is not a discoverable repository flow: " + + ", ".join(str(path) for path in missing) + ) + return [definitions_by_path[path] for path in selected] + + +def run_metaflow_checks( + definitions: Sequence[FlowDefinition], flows_root: Path = FLOWS_ROOT +) -> None: + """Run Metaflow's built-in definition and DAG validation for every flow.""" + for definition in definitions: + relative_path = definition.path.relative_to(flows_root.resolve()) + print(f"Checking {relative_path}", file=sys.stderr) + # Native `check` owns step, transition, decorator, parameter, import, and + # graph validation. It validates definitions without executing flow steps. + result = subprocess.run( + [sys.executable, str(relative_path), "check"], + cwd=flows_root, + check=False, + ) + if result.returncode: + raise FlowCheckError( + f"Metaflow validation failed for {relative_path} with exit code {result.returncode}" + ) + + +def _parse_args(arguments: Sequence[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("paths", nargs="*", help="Flow files to validate") + parser.add_argument( + "--discovery-only", + action="store_true", + help="Discover flows without invoking Metaflow", + ) + parser.add_argument( + "--format", + choices=("paths", "json"), + default="paths", + help="Discovery output format", + ) + return parser.parse_args(arguments) + + +def main(arguments: Sequence[str] | None = None) -> int: + args = _parse_args(arguments) + # pre-commit passes every staged file matching the hook's `files:` pattern, + # which includes non-flow modules (e.g. shared utils); only treat the + # `*_flow.py` ones as an explicit selection, otherwise fall back to + # validating every flow so changes to shared code still get checked. + flow_paths = [path for path in args.paths if path.endswith("_flow.py")] + try: + definitions = discover_flows(selected_paths=flow_paths) + if not args.discovery_only: + run_metaflow_checks(definitions) + except FlowCheckError as error: + print(f"Flow validation failed: {error}", file=sys.stderr) + return 1 + + paths = [definition.path.relative_to(REPOSITORY_ROOT).as_posix() for definition in definitions] + if args.format == "json": + print(json.dumps({"flow": paths})) + else: + print("\n".join(paths)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/flows/tests/test_check_flows.py b/flows/tests/test_check_flows.py new file mode 100644 index 0000000..14c5ca6 --- /dev/null +++ b/flows/tests/test_check_flows.py @@ -0,0 +1,170 @@ +import subprocess +import sys +from pathlib import Path + +import pytest +from scripts.check_flows import ( + FlowCheckError, + FlowDefinition, + discover_flows, + run_metaflow_checks, +) + + +def write_flow(path: Path, class_name: str = "ExampleFlow") -> None: + """Create the smallest flow definition needed by checker tests.""" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + f"from metaflow import FlowSpec\n\n" + f"class {class_name}(FlowSpec):\n pass\n\n" + f'if __name__ == "__main__":\n {class_name}()\n', + encoding="utf-8", + ) + + +def test_discovers_flows_recursively_in_stable_order(tmp_path: Path) -> None: + """Discover nested flows deterministically while ignoring support files.""" + write_flow(tmp_path / "zeta" / "second_flow.py", "SecondFlow") + write_flow(tmp_path / "alpha" / "first_flow.py", "FirstFlow") + write_flow(tmp_path / ".venv" / "ignored_flow.py", "IgnoredFlow") + (tmp_path / "alpha" / "helper.py").write_text("VALUE = 1\n", encoding="utf-8") + + definitions = discover_flows(tmp_path) + + assert [definition.class_name for definition in definitions] == [ + "FirstFlow", + "SecondFlow", + ] + + +@pytest.mark.parametrize("filename", ["missing_flow.py", "helper.py"]) +def test_rejects_invalid_selected_paths(tmp_path: Path, filename: str) -> None: + """Reject missing paths and existing files without the flow suffix.""" + if filename == "helper.py": + (tmp_path / filename).write_text("VALUE = 1\n", encoding="utf-8") + + with pytest.raises(FlowCheckError): + discover_flows(tmp_path, [filename]) + + +def test_rejects_file_without_one_flowspec_subclass(tmp_path: Path) -> None: + """Require each discovered flow file to contain one FlowSpec subclass.""" + path = tmp_path / "invalid_flow.py" + path.write_text("class NotAFlow:\n pass\n", encoding="utf-8") + + with pytest.raises(FlowCheckError, match="exactly one"): + discover_flows(tmp_path) + + +def test_rejects_flow_without_executable_main_guard(tmp_path: Path) -> None: + """Reject a flow file that native `python flow.py check` would not execute.""" + path = tmp_path / "invalid_flow.py" + path.write_text( + "from metaflow import FlowSpec\n\nclass InvalidFlow(FlowSpec):\n pass\n", + encoding="utf-8", + ) + + with pytest.raises(FlowCheckError, match="must instantiate InvalidFlow"): + discover_flows(tmp_path) + + +def test_rejects_main_guard_that_instantiates_another_class(tmp_path: Path) -> None: + """Require the main guard to instantiate the FlowSpec declared in that file.""" + path = tmp_path / "invalid_flow.py" + path.write_text( + "from metaflow import FlowSpec\n\n" + "class InvalidFlow(FlowSpec):\n pass\n\n" + 'if __name__ == "__main__":\n OtherFlow()\n', + encoding="utf-8", + ) + + with pytest.raises(FlowCheckError, match="must instantiate InvalidFlow"): + discover_flows(tmp_path) + + +def test_rejects_empty_flow_directory(tmp_path: Path) -> None: + """Fail instead of allowing CI to produce an empty flow matrix.""" + with pytest.raises(FlowCheckError, match=r"No \*_flow.py files"): + discover_flows(tmp_path) + + +def test_reports_python_syntax_errors(tmp_path: Path) -> None: + """Surface invalid Python before invoking Metaflow validation.""" + path = tmp_path / "invalid_flow.py" + path.write_text("class InvalidFlow(FlowSpec)\n pass\n", encoding="utf-8") + + with pytest.raises(FlowCheckError, match="Unable to parse"): + discover_flows(tmp_path) + + +def test_rejects_duplicate_flow_class_names(tmp_path: Path) -> None: + """Prevent ambiguous Metaflow identities across domain directories.""" + write_flow(tmp_path / "one" / "first_flow.py", "DuplicateFlow") + write_flow(tmp_path / "two" / "second_flow.py", "DuplicateFlow") + + with pytest.raises(FlowCheckError, match="Duplicate flow class"): + discover_flows(tmp_path) + + +def test_selected_flow_still_checks_global_name_uniqueness(tmp_path: Path) -> None: + """Detect duplicate names outside the flow selected for native validation.""" + selected = tmp_path / "one" / "first_flow.py" + write_flow(selected, "DuplicateFlow") + write_flow(tmp_path / "two" / "second_flow.py", "DuplicateFlow") + + with pytest.raises(FlowCheckError, match="Duplicate flow class"): + discover_flows(tmp_path, [str(selected)]) + + +def test_selected_flow_limits_returned_definitions(tmp_path: Path) -> None: + """Return only selected flows after validating the complete repository inventory.""" + selected = tmp_path / "one" / "first_flow.py" + write_flow(selected, "FirstFlow") + write_flow(tmp_path / "two" / "second_flow.py", "SecondFlow") + + definitions = discover_flows(tmp_path, [str(selected)]) + + assert definitions == [FlowDefinition(selected.resolve(), "FirstFlow")] + + +def test_rejects_selected_path_excluded_from_discovery(tmp_path: Path) -> None: + """Report a selected path that discovery skips instead of raising KeyError.""" + write_flow(tmp_path / "one" / "first_flow.py", "FirstFlow") + hidden = tmp_path / ".venv" / "hidden_flow.py" + write_flow(hidden, "HiddenFlow") + + with pytest.raises(FlowCheckError, match="not a discoverable repository flow"): + discover_flows(tmp_path, [str(hidden)]) + + +def test_invokes_native_metaflow_check(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Delegate DAG validation to the native `python flow.py check` command.""" + path = tmp_path / "example_flow.py" + write_flow(path) + calls: list[tuple[list[str], Path]] = [] + + def fake_run(command: list[str], cwd: Path, check: bool) -> subprocess.CompletedProcess: + calls.append((command, cwd)) + return subprocess.CompletedProcess(command, 0) + + monkeypatch.setattr(subprocess, "run", fake_run) + + run_metaflow_checks([FlowDefinition(path, "ExampleFlow")], tmp_path) + + assert calls == [([sys.executable, "example_flow.py", "check"], tmp_path)] + + +def test_reports_native_metaflow_check_failure( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Turn a nonzero native Metaflow result into a checker failure.""" + path = tmp_path / "example_flow.py" + write_flow(path) + + def fake_run(command: list[str], cwd: Path, check: bool) -> subprocess.CompletedProcess: + return subprocess.CompletedProcess(command, 2) + + monkeypatch.setattr(subprocess, "run", fake_run) + + with pytest.raises(FlowCheckError, match="exit code 2"): + run_metaflow_checks([FlowDefinition(path, "ExampleFlow")], tmp_path) diff --git a/flows/tests/test_tool_versions.py b/flows/tests/test_tool_versions.py index 8071dea..aba0c64 100644 --- a/flows/tests/test_tool_versions.py +++ b/flows/tests/test_tool_versions.py @@ -1,6 +1,8 @@ import re from pathlib import Path +import yaml + REPOSITORY_ROOT = Path(__file__).resolve().parents[2] REQUIREMENTS_PATH = REPOSITORY_ROOT / "flows" / "requirements.txt" DEV_REQUIREMENTS_PATH = REPOSITORY_ROOT / "flows" / "requirements-dev.txt" @@ -17,28 +19,50 @@ def requirement_version(package: str, requirements_path: Path = DEV_REQUIREMENTS return match.group(1) +def load_pre_commit_config() -> dict: + return yaml.safe_load(PRE_COMMIT_CONFIG_PATH.read_text(encoding="utf-8")) + + +def hook_config(hook_id: str) -> dict: + """Return the hook block for `hook_id` from `.pre-commit-config.yaml`.""" + for repo in load_pre_commit_config()["repos"]: + for hook in repo["hooks"]: + if hook["id"] == hook_id: + return hook + raise AssertionError(f"Missing hook {hook_id!r} in {PRE_COMMIT_CONFIG_PATH}") + + +def additional_dependency_version(hook_id: str, package: str) -> str: + """Read a pinned `package==version` entry from a hook's additional_dependencies.""" + dependencies = hook_config(hook_id).get("additional_dependencies", []) + for dependency in dependencies: + if dependency.startswith(f"{package}=="): + return dependency.split("==", 1)[1] + raise AssertionError( + f"Missing pinned {package} dependency for hook {hook_id!r} in {PRE_COMMIT_CONFIG_PATH}" + ) + + def test_ruff_versions_match() -> None: """Keep the Ruff CLI version aligned with the isolated pre-commit hook.""" - config = PRE_COMMIT_CONFIG_PATH.read_text(encoding="utf-8") - match = re.search( - r"repo: https://github\.com/astral-sh/ruff-pre-commit\s+rev: v([^\s]+)", - config, - ) - assert match, f"Missing pinned Ruff revision in {PRE_COMMIT_CONFIG_PATH}" - assert match.group(1) == requirement_version("ruff") + for repo in load_pre_commit_config()["repos"]: + if repo["repo"] == "https://github.com/astral-sh/ruff-pre-commit": + rev = repo["rev"] + break + else: + raise AssertionError(f"Missing ruff-pre-commit repo in {PRE_COMMIT_CONFIG_PATH}") + assert rev.lstrip("v") == requirement_version("ruff") def test_pytest_versions_match() -> None: """Keep the pytest CLI version aligned with its pre-commit environment.""" - config = PRE_COMMIT_CONFIG_PATH.read_text(encoding="utf-8") - match = re.search(r"additional_dependencies:\s+- pytest==([^\s]+)", config) - assert match, f"Missing pinned pytest dependency in {PRE_COMMIT_CONFIG_PATH}" - assert match.group(1) == requirement_version("pytest") + assert additional_dependency_version("test-flow-tools", "pytest") == requirement_version( + "pytest" + ) def test_outerbounds_versions_match() -> None: """Validate flows against the same Outerbounds release that CI installs.""" - config = PRE_COMMIT_CONFIG_PATH.read_text(encoding="utf-8") - match = re.search(r"additional_dependencies:\s+- outerbounds==([^\s]+)", config) - assert match, f"Missing pinned outerbounds dependency in {PRE_COMMIT_CONFIG_PATH}" - assert match.group(1) == requirement_version("outerbounds", REQUIREMENTS_PATH) + assert additional_dependency_version("check-flows", "outerbounds") == requirement_version( + "outerbounds", REQUIREMENTS_PATH + ) diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..0e1243a --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,11 @@ +[tool.ruff] +target-version = "py312" +line-length = 100 + +[tool.ruff.lint] +select = ["B", "D102", "E", "F", "I", "UP"] + +[tool.pytest.ini_options] +addopts = "-q" +pythonpath = ["flows"] +testpaths = ["flows/tests"] \ No newline at end of file From 42045b7742b7dc95903520635f12551beb3cf1bb Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Fri, 11 Sep 2026 18:12:19 +0100 Subject: [PATCH 07/28] add precommit hooks --- flows/scripts/check_flows.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/flows/scripts/check_flows.py b/flows/scripts/check_flows.py index 19ce8eb..4f17634 100644 --- a/flows/scripts/check_flows.py +++ b/flows/scripts/check_flows.py @@ -216,10 +216,15 @@ def _parse_args(arguments: Sequence[str] | None = None) -> argparse.Namespace: def main(arguments: Sequence[str] | None = None) -> int: args = _parse_args(arguments) # pre-commit passes every staged file matching the hook's `files:` pattern, - # which includes non-flow modules (e.g. shared utils); only treat the - # `*_flow.py` ones as an explicit selection, otherwise fall back to - # validating every flow so changes to shared code still get checked. - flow_paths = [path for path in args.paths if path.endswith("_flow.py")] + # which includes non-flow modules (e.g. shared utils). Only treat the + # staged paths as an explicit selection when they are all `*_flow.py` + # files; if any non-flow file is staged too, fall back to validating + # every flow so changes to shared code still get checked. + flow_paths = ( + list(args.paths) + if args.paths and all(path.endswith("_flow.py") for path in args.paths) + else [] + ) try: definitions = discover_flows(selected_paths=flow_paths) if not args.discovery_only: From 73d7f824f5d8acb87147be07a3f81574c6d415a7 Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 11:15:52 +0100 Subject: [PATCH 08/28] add precommit hooks --- .husky/pre-commit | 2 +- flows/scripts/check_flows.py | 41 ++++++++++++++++++++++++--------- flows/tests/test_check_flows.py | 36 +++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index e8d3e69..478a1d4 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -46,7 +46,7 @@ if printf '%s\n' "$staged" | grep -Eq "^flows/|$config_pattern"; then fi fi -if printf '%s\n' "$staged" | grep -Eq '^(e2e/|\.husky/pre-commit$)'; then +if git diff --cached --name-only --diff-filter=ACMR | grep -Eq '^e2e/(tests|test-setup)/'; then npm --prefix e2e run precommit || status=1 fi diff --git a/flows/scripts/check_flows.py b/flows/scripts/check_flows.py index 4f17634..9c9d8ed 100644 --- a/flows/scripts/check_flows.py +++ b/flows/scripts/check_flows.py @@ -87,7 +87,14 @@ def _resolve_selected_path(path: str, flows_root: Path) -> Path: repository_candidate = (flows_root.parent / candidate).resolve() if repository_candidate.is_file(): return repository_candidate - return (flows_root / candidate).resolve() + + flows_candidate = (flows_root / candidate).resolve() + if flows_candidate.is_file(): + return flows_candidate + + # Neither interpretation exists; report the repository-relative path since + # every caller (pre-commit, CI) passes repository-relative paths. + return repository_candidate def discover_flows( @@ -210,21 +217,33 @@ def _parse_args(arguments: Sequence[str] | None = None) -> argparse.Namespace: default="paths", help="Discovery output format", ) + parser.add_argument( + "--require-selection", + action="store_true", + help=( + "Treat every positional path as an explicit selection and fail if any " + "is invalid, instead of guessing from filename suffixes. Use this for " + "callers that pass a single human-provided path (e.g. CI workflow_dispatch)." + ), + ) return parser.parse_args(arguments) def main(arguments: Sequence[str] | None = None) -> int: args = _parse_args(arguments) - # pre-commit passes every staged file matching the hook's `files:` pattern, - # which includes non-flow modules (e.g. shared utils). Only treat the - # staged paths as an explicit selection when they are all `*_flow.py` - # files; if any non-flow file is staged too, fall back to validating - # every flow so changes to shared code still get checked. - flow_paths = ( - list(args.paths) - if args.paths and all(path.endswith("_flow.py") for path in args.paths) - else [] - ) + if args.require_selection: + flow_paths = list(args.paths) + else: + # pre-commit passes every staged file matching the hook's `files:` pattern, + # which includes non-flow modules (e.g. shared utils). Only treat the + # staged paths as an explicit selection when they are all `*_flow.py` + # files; if any non-flow file is staged too, fall back to validating + # every flow so changes to shared code still get checked. + flow_paths = ( + list(args.paths) + if args.paths and all(path.endswith("_flow.py") for path in args.paths) + else [] + ) try: definitions = discover_flows(selected_paths=flow_paths) if not args.discovery_only: diff --git a/flows/tests/test_check_flows.py b/flows/tests/test_check_flows.py index 14c5ca6..445b5e4 100644 --- a/flows/tests/test_check_flows.py +++ b/flows/tests/test_check_flows.py @@ -1,8 +1,10 @@ import subprocess import sys +from collections.abc import Sequence from pathlib import Path import pytest +import scripts.check_flows as check_flows from scripts.check_flows import ( FlowCheckError, FlowDefinition, @@ -137,6 +139,40 @@ def test_rejects_selected_path_excluded_from_discovery(tmp_path: Path) -> None: discover_flows(tmp_path, [str(hidden)]) +@pytest.mark.parametrize( + ("paths", "expected_selection"), + [ + (["flows/models/browse_models_flow.py"], ["flows/models/browse_models_flow.py"]), + (["flows/models/browse_models_flow.py", "flows/utils/model_validators.py"], []), + (["flows/utils/model_validators.py"], []), + ], +) +def test_main_filters_pre_commit_paths( + paths: list[str], + expected_selection: list[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Expand validation to all flows when pre-commit includes support files.""" + observed_selections: list[list[str]] = [] + definition = FlowDefinition( + check_flows.FLOWS_ROOT / "models" / "browse_models_flow.py", + "BrowseModelsFlow", + ) + + def fake_discover_flows( + flows_root: Path = check_flows.FLOWS_ROOT, + selected_paths: Sequence[str] = (), + ) -> list[FlowDefinition]: + del flows_root + observed_selections.append(list(selected_paths)) + return [definition] + + monkeypatch.setattr(check_flows, "discover_flows", fake_discover_flows) + + assert check_flows.main(["--discovery-only", *paths]) == 0 + assert observed_selections == [expected_selection] + + def test_invokes_native_metaflow_check(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: """Delegate DAG validation to the native `python flow.py check` command.""" path = tmp_path / "example_flow.py" From 27ebeff3698f748edef6921f84a7ebbff957cbfa Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 12:18:35 +0100 Subject: [PATCH 09/28] fix pre-commit hook and simplify flow selection logic - husky pre-commit no longer exits before running the e2e check when the pre-commit binary is missing, so a commit touching both flows/ and e2e/ still runs both checks. - narrow the check-flows hook's files pattern to *_flow.py so check_flows.py no longer needs to guess whether staged paths are an explicit selection, and drop the now-redundant --require-selection flag and the untested flows-relative path fallback. --- .github/workflows/metaflow-tests.yml | 142 +++++++++++++++++++++++++++ .husky/pre-commit | 23 +++-- .pre-commit-config.yaml | 2 +- flows/scripts/check_flows.py | 41 ++------ flows/tests/test_check_flows.py | 17 +--- 5 files changed, 166 insertions(+), 59 deletions(-) create mode 100644 .github/workflows/metaflow-tests.yml diff --git a/.github/workflows/metaflow-tests.yml b/.github/workflows/metaflow-tests.yml new file mode 100644 index 0000000..a34f122 --- /dev/null +++ b/.github/workflows/metaflow-tests.yml @@ -0,0 +1,142 @@ +name: Metaflow QA + +on: + schedule: + - cron: '0 6 * * 1-5' + workflow_dispatch: + inputs: + environment: + description: Target environment + required: true + default: dev + type: choice + options: + - dev + - staging + - production + flow: + description: Repository-relative flow path, or "all" + required: true + default: all + type: string + +permissions: + contents: read + +concurrency: + group: metaflow-qa-${{ inputs.environment || 'dev' }} + cancel-in-progress: false + +jobs: + quality: + name: Quality and discovery + runs-on: ubuntu-latest + timeout-minutes: 10 + outputs: + matrix: ${{ steps.discover.outputs.matrix }} + steps: + - name: Check out repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: '3.12' + cache: pip + cache-dependency-path: | + flows/requirements.txt + flows/requirements-dev.txt + + - name: Install dependencies + run: python -m pip install -r flows/requirements-dev.txt + + - name: Check formatting + run: ruff format --check flows + + - name: Lint flows + run: ruff check flows + + - name: Test flow utilities + run: pytest + + - name: Validate flows and build matrix + id: discover + env: + REQUESTED_FLOW: ${{ inputs.flow || 'all' }} + run: | + if [[ "$REQUESTED_FLOW" == "all" ]]; then + matrix="$(python flows/scripts/check_flows.py --format json)" + else + matrix="$(python flows/scripts/check_flows.py --format json "$REQUESTED_FLOW")" + fi + echo "matrix=$matrix" >> "$GITHUB_OUTPUT" + + run-flow: + name: Run ${{ matrix.flow }} + needs: quality + runs-on: ubuntu-latest + timeout-minutes: 35 + environment: ${{ inputs.environment || 'dev' }} + permissions: + contents: read + id-token: write + strategy: + fail-fast: false + max-parallel: 2 + matrix: ${{ fromJson(needs.quality.outputs.matrix) }} + steps: + - name: Check out repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: '3.12' + cache: pip + cache-dependency-path: flows/requirements.txt + + - name: Install runtime dependencies + run: python -m pip install -r flows/requirements.txt + + - name: Configure Outerbounds access + env: + OBP_MACHINE_USER: ${{ vars.OBP_MACHINE_USER }} + OBP_DEPLOYMENT_DOMAIN: ${{ vars.OBP_DEPLOYMENT_DOMAIN }} + OBP_PERIMETER: ${{ vars.OBP_PERIMETER }} + run: | + outerbounds service-principal-configure \ + --name "$OBP_MACHINE_USER" \ + --deployment-domain "$OBP_DEPLOYMENT_DOMAIN" \ + --perimeter "$OBP_PERIMETER" \ + --github-actions + + - name: Check Outerbounds connectivity + run: outerbounds check -v + + - name: Run flow on Kubernetes + env: + FLOW_PATH: ${{ matrix.flow }} + run: | + mkdir -p flow-logs + set -o pipefail + python "$FLOW_PATH" --environment=fast-bakery run --with kubernetes \ + 2>&1 | tee "flow-logs/$(basename "$FLOW_PATH" .py).log" + + - name: Add job summary + if: always() + env: + FLOW_PATH: ${{ matrix.flow }} + TARGET_ENVIRONMENT: ${{ inputs.environment || 'dev' }} + run: | + echo "### Metaflow QA" >> "$GITHUB_STEP_SUMMARY" + echo "- Environment: $TARGET_ENVIRONMENT" >> "$GITHUB_STEP_SUMMARY" + echo "- Flow: $FLOW_PATH" >> "$GITHUB_STEP_SUMMARY" + + - name: Upload flow log + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: metaflow-${{ inputs.environment || 'dev' }}-${{ strategy.job-index }} + path: flow-logs/ + if-no-files-found: warn + retention-days: 14 diff --git a/.husky/pre-commit b/.husky/pre-commit index 478a1d4..a25f2a8 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -31,18 +31,21 @@ if printf '%s\n' "$staged" | grep -Eq "^flows/|$config_pattern"; then else echo "pre-commit not found. Install it with:" >&2 echo " python -m pip install -r flows/requirements-dev.txt" >&2 - exit 1 + pre_commit="" + status=1 fi - # 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 - else - # `files:` patterns in .pre-commit-config.yaml keep this to flows/. - "$pre_commit" run || status=1 + 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 + else + # `files:` patterns in .pre-commit-config.yaml keep this to flows/. + "$pre_commit" run || status=1 + fi fi fi diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4ce619c..75a0c3a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -16,7 +16,7 @@ repos: language: python additional_dependencies: - outerbounds==0.12.44 - files: ^flows/.*\.py$ + files: ^flows/.*_flow\.py$ - id: test-flow-tools name: Test flow utilities diff --git a/flows/scripts/check_flows.py b/flows/scripts/check_flows.py index 9c9d8ed..0126d74 100644 --- a/flows/scripts/check_flows.py +++ b/flows/scripts/check_flows.py @@ -79,22 +79,14 @@ def _has_main_guard_call(tree: ast.Module, class_name: str) -> bool: def _resolve_selected_path(path: str, flows_root: Path) -> Path: - """Resolve CLI paths relative to either the repository or flows directory.""" + """Resolve a CLI path relative to the repository root. + + Every caller (pre-commit, CI) passes repository-relative paths. + """ candidate = Path(path) if candidate.is_absolute(): return candidate.resolve() - - repository_candidate = (flows_root.parent / candidate).resolve() - if repository_candidate.is_file(): - return repository_candidate - - flows_candidate = (flows_root / candidate).resolve() - if flows_candidate.is_file(): - return flows_candidate - - # Neither interpretation exists; report the repository-relative path since - # every caller (pre-commit, CI) passes repository-relative paths. - return repository_candidate + return (flows_root.parent / candidate).resolve() def discover_flows( @@ -217,33 +209,12 @@ def _parse_args(arguments: Sequence[str] | None = None) -> argparse.Namespace: default="paths", help="Discovery output format", ) - parser.add_argument( - "--require-selection", - action="store_true", - help=( - "Treat every positional path as an explicit selection and fail if any " - "is invalid, instead of guessing from filename suffixes. Use this for " - "callers that pass a single human-provided path (e.g. CI workflow_dispatch)." - ), - ) return parser.parse_args(arguments) def main(arguments: Sequence[str] | None = None) -> int: args = _parse_args(arguments) - if args.require_selection: - flow_paths = list(args.paths) - else: - # pre-commit passes every staged file matching the hook's `files:` pattern, - # which includes non-flow modules (e.g. shared utils). Only treat the - # staged paths as an explicit selection when they are all `*_flow.py` - # files; if any non-flow file is staged too, fall back to validating - # every flow so changes to shared code still get checked. - flow_paths = ( - list(args.paths) - if args.paths and all(path.endswith("_flow.py") for path in args.paths) - else [] - ) + flow_paths = list(args.paths) try: definitions = discover_flows(selected_paths=flow_paths) if not args.discovery_only: diff --git a/flows/tests/test_check_flows.py b/flows/tests/test_check_flows.py index 445b5e4..b58be06 100644 --- a/flows/tests/test_check_flows.py +++ b/flows/tests/test_check_flows.py @@ -139,20 +139,10 @@ def test_rejects_selected_path_excluded_from_discovery(tmp_path: Path) -> None: discover_flows(tmp_path, [str(hidden)]) -@pytest.mark.parametrize( - ("paths", "expected_selection"), - [ - (["flows/models/browse_models_flow.py"], ["flows/models/browse_models_flow.py"]), - (["flows/models/browse_models_flow.py", "flows/utils/model_validators.py"], []), - (["flows/utils/model_validators.py"], []), - ], -) -def test_main_filters_pre_commit_paths( - paths: list[str], - expected_selection: list[str], +def test_main_passes_positional_paths_through_as_selection( monkeypatch: pytest.MonkeyPatch, ) -> None: - """Expand validation to all flows when pre-commit includes support files.""" + """Forward CLI paths to discovery unchanged; pre-commit only ever stages flow files.""" observed_selections: list[list[str]] = [] definition = FlowDefinition( check_flows.FLOWS_ROOT / "models" / "browse_models_flow.py", @@ -169,8 +159,9 @@ def fake_discover_flows( monkeypatch.setattr(check_flows, "discover_flows", fake_discover_flows) + paths = ["flows/models/browse_models_flow.py"] assert check_flows.main(["--discovery-only", *paths]) == 0 - assert observed_selections == [expected_selection] + assert observed_selections == [paths] def test_invokes_native_metaflow_check(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: From d3ce3a04ae1ff7ae8000cab89535d30b379e2d66 Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 12:19:48 +0100 Subject: [PATCH 10/28] untrack GitHub workflow file Not part of this change; leaving it as an untracked working-tree file as it was before. --- .github/workflows/metaflow-tests.yml | 142 --------------------------- 1 file changed, 142 deletions(-) delete mode 100644 .github/workflows/metaflow-tests.yml diff --git a/.github/workflows/metaflow-tests.yml b/.github/workflows/metaflow-tests.yml deleted file mode 100644 index a34f122..0000000 --- a/.github/workflows/metaflow-tests.yml +++ /dev/null @@ -1,142 +0,0 @@ -name: Metaflow QA - -on: - schedule: - - cron: '0 6 * * 1-5' - workflow_dispatch: - inputs: - environment: - description: Target environment - required: true - default: dev - type: choice - options: - - dev - - staging - - production - flow: - description: Repository-relative flow path, or "all" - required: true - default: all - type: string - -permissions: - contents: read - -concurrency: - group: metaflow-qa-${{ inputs.environment || 'dev' }} - cancel-in-progress: false - -jobs: - quality: - name: Quality and discovery - runs-on: ubuntu-latest - timeout-minutes: 10 - outputs: - matrix: ${{ steps.discover.outputs.matrix }} - steps: - - name: Check out repository - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - - name: Set up Python - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - with: - python-version: '3.12' - cache: pip - cache-dependency-path: | - flows/requirements.txt - flows/requirements-dev.txt - - - name: Install dependencies - run: python -m pip install -r flows/requirements-dev.txt - - - name: Check formatting - run: ruff format --check flows - - - name: Lint flows - run: ruff check flows - - - name: Test flow utilities - run: pytest - - - name: Validate flows and build matrix - id: discover - env: - REQUESTED_FLOW: ${{ inputs.flow || 'all' }} - run: | - if [[ "$REQUESTED_FLOW" == "all" ]]; then - matrix="$(python flows/scripts/check_flows.py --format json)" - else - matrix="$(python flows/scripts/check_flows.py --format json "$REQUESTED_FLOW")" - fi - echo "matrix=$matrix" >> "$GITHUB_OUTPUT" - - run-flow: - name: Run ${{ matrix.flow }} - needs: quality - runs-on: ubuntu-latest - timeout-minutes: 35 - environment: ${{ inputs.environment || 'dev' }} - permissions: - contents: read - id-token: write - strategy: - fail-fast: false - max-parallel: 2 - matrix: ${{ fromJson(needs.quality.outputs.matrix) }} - steps: - - name: Check out repository - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - - name: Set up Python - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - with: - python-version: '3.12' - cache: pip - cache-dependency-path: flows/requirements.txt - - - name: Install runtime dependencies - run: python -m pip install -r flows/requirements.txt - - - name: Configure Outerbounds access - env: - OBP_MACHINE_USER: ${{ vars.OBP_MACHINE_USER }} - OBP_DEPLOYMENT_DOMAIN: ${{ vars.OBP_DEPLOYMENT_DOMAIN }} - OBP_PERIMETER: ${{ vars.OBP_PERIMETER }} - run: | - outerbounds service-principal-configure \ - --name "$OBP_MACHINE_USER" \ - --deployment-domain "$OBP_DEPLOYMENT_DOMAIN" \ - --perimeter "$OBP_PERIMETER" \ - --github-actions - - - name: Check Outerbounds connectivity - run: outerbounds check -v - - - name: Run flow on Kubernetes - env: - FLOW_PATH: ${{ matrix.flow }} - run: | - mkdir -p flow-logs - set -o pipefail - python "$FLOW_PATH" --environment=fast-bakery run --with kubernetes \ - 2>&1 | tee "flow-logs/$(basename "$FLOW_PATH" .py).log" - - - name: Add job summary - if: always() - env: - FLOW_PATH: ${{ matrix.flow }} - TARGET_ENVIRONMENT: ${{ inputs.environment || 'dev' }} - run: | - echo "### Metaflow QA" >> "$GITHUB_STEP_SUMMARY" - echo "- Environment: $TARGET_ENVIRONMENT" >> "$GITHUB_STEP_SUMMARY" - echo "- Flow: $FLOW_PATH" >> "$GITHUB_STEP_SUMMARY" - - - name: Upload flow log - if: always() - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - with: - name: metaflow-${{ inputs.environment || 'dev' }}-${{ strategy.job-index }} - path: flow-logs/ - if-no-files-found: warn - retention-days: 14 From 9eb9af8fb60cddc4eb4ed22c008b0e30e2fe2d02 Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 12:30:25 +0100 Subject: [PATCH 11/28] fix check-flows README wording and invalid-path test README claimed check-flows validates all discovered flows, but pre-commit only passes staged filenames. Also fixed test_rejects_invalid_selected_paths, which resolved relative paths against tmp_path.parent and accidentally tripped the outside-root check instead of the missing-file/wrong-suffix checks it meant to test. Co-Authored-By: Claude Sonnet 5 --- flows/README.md | 4 ++-- flows/tests/test_check_flows.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/flows/README.md b/flows/README.md index 6c1349e..006fa79 100644 --- a/flows/README.md +++ b/flows/README.md @@ -79,8 +79,8 @@ under `e2e/` separately activate the Playwright checks; a commit touching both runs both. `test-flow-tools` also runs on its own when only `flows/requirements.txt`, `flows/requirements-dev.txt`, or `.pre-commit-config.yaml` is staged, so a version bump alone still triggers the -tool-version consistency checks. `check-flows` validates all discovered flows, -and `test-flow-tools` runs the complete local test suite. Neither hook +tool-version consistency checks. `check-flows` validates only the staged +flow files, and `test-flow-tools` runs the complete local test suite. Neither hook authenticates to Outerbounds or starts a Kubernetes workload. If Ruff modifies a file, review and stage the change before committing again: diff --git a/flows/tests/test_check_flows.py b/flows/tests/test_check_flows.py index b58be06..a9337ee 100644 --- a/flows/tests/test_check_flows.py +++ b/flows/tests/test_check_flows.py @@ -46,7 +46,7 @@ def test_rejects_invalid_selected_paths(tmp_path: Path, filename: str) -> None: (tmp_path / filename).write_text("VALUE = 1\n", encoding="utf-8") with pytest.raises(FlowCheckError): - discover_flows(tmp_path, [filename]) + discover_flows(tmp_path, [tmp_path / filename]) def test_rejects_file_without_one_flowspec_subclass(tmp_path: Path) -> None: From fc58d86c3be79c1df8b4b6d9f63fc60a3eb0b551 Mon Sep 17 00:00:00 2001 From: Sri Vidya Vemulapalli Date: Mon, 14 Sep 2026 12:51:42 +0100 Subject: [PATCH 12/28] Update README with quality check instructions Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 579c5d5..f83d072 100644 --- a/README.md +++ b/README.md @@ -41,8 +41,8 @@ pre-commit run --all-files npm --prefix e2e run quality:full ``` -Python changes run Ruff, focused pytest tests, and native Metaflow definition -checks. E2E changes run the npm `precommit` command, which applies lint-staged +Staged `*_flow.py` changes run Ruff, focused pytest tests, and native Metaflow +definition checks; other staged Python changes run Ruff and the focused tests. fixes and then the Playwright quality report. No hook authenticates to Outerbounds or starts remote workloads. From 4045ba4ea0f50e7aa6db93b9c2123184fd311bf5 Mon Sep 17 00:00:00 2001 From: Sri Vidya Vemulapalli Date: Mon, 14 Sep 2026 12:51:57 +0100 Subject: [PATCH 13/28] Update README with flow hooks activation details Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- flows/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flows/README.md b/flows/README.md index 006fa79..6400486 100644 --- a/flows/README.md +++ b/flows/README.md @@ -74,9 +74,9 @@ The flow hooks perform these checks: consistency. During a normal `git commit`, staged files determine which hooks run. A staged -Python file under `flows/` activates all four flow hooks, and staged files -under `e2e/` separately activate the Playwright checks; a commit touching both -runs both. `test-flow-tools` also runs on its own when only +A staged `*_flow.py` file under `flows/` activates all four flow hooks; other +staged Python files activate Ruff and `test-flow-tools`. Staged files under +`e2e/` separately activate the Playwright checks; a commit touching both runs both. `flows/requirements.txt`, `flows/requirements-dev.txt`, or `.pre-commit-config.yaml` is staged, so a version bump alone still triggers the tool-version consistency checks. `check-flows` validates only the staged From d2311717e454f2682840e23059c3f6f71257221a Mon Sep 17 00:00:00 2001 From: Sri Vidya Vemulapalli Date: Mon, 14 Sep 2026 12:52:09 +0100 Subject: [PATCH 14/28] Update README with Playwright and flow checks details Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- flows/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/flows/README.md b/flows/README.md index 6400486..91c9634 100644 --- a/flows/README.md +++ b/flows/README.md @@ -79,8 +79,9 @@ staged Python files activate Ruff and `test-flow-tools`. Staged files under `e2e/` separately activate the Playwright checks; a commit touching both runs both. `flows/requirements.txt`, `flows/requirements-dev.txt`, or `.pre-commit-config.yaml` is staged, so a version bump alone still triggers the -tool-version consistency checks. `check-flows` validates only the staged -flow files, and `test-flow-tools` runs the complete local test suite. Neither hook +tool-version consistency checks. The native Metaflow check runs only for staged +flow files, while discovery and repository-wide convention checks still inspect all +repository flows. `test-flow-tools` runs the complete local test suite. Neither hook authenticates to Outerbounds or starts a Kubernetes workload. If Ruff modifies a file, review and stage the change before committing again: From 5d8ad7522d98ba28720bba6607e3a7efb4b38185 Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 12:56:33 +0100 Subject: [PATCH 15/28] fix Husky root hook initialization --- README.md | 1 - e2e/package.json | 2 +- flows/README.md | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index f83d072..db09123 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,6 @@ root: ```bash python -m pip install -r flows/requirements-dev.txt npm --prefix e2e install -e2e/node_modules/.bin/husky ``` Do not run `pre-commit install`; it refuses to write a hook while Husky owns diff --git a/e2e/package.json b/e2e/package.json index c4c25a7..c0bf342 100644 --- a/e2e/package.json +++ b/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", diff --git a/flows/README.md b/flows/README.md index 91c9634..e202903 100644 --- a/flows/README.md +++ b/flows/README.md @@ -32,7 +32,6 @@ Install the shared hook from the repository root: ```bash npm --prefix e2e install -e2e/node_modules/.bin/husky ``` Confirm that Husky owns the hook path and that `pre-commit` is callable from it: From a467d8727ce5c21f55f3c86bf849356bd832ba2a Mon Sep 17 00:00:00 2001 From: Sri Vidya Vemulapalli Date: Mon, 14 Sep 2026 13:04:14 +0100 Subject: [PATCH 16/28] Update README with staging process details Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index db09123..3e2cdf1 100644 --- a/README.md +++ b/README.md @@ -42,8 +42,8 @@ npm --prefix e2e run quality:full Staged `*_flow.py` changes run Ruff, focused pytest tests, and native Metaflow definition checks; other staged Python changes run Ruff and the focused tests. -fixes and then the Playwright quality report. No hook authenticates to -Outerbounds or starts remote workloads. +Ruff fixes are applied first, followed by the Playwright quality report. No hook +authenticates to Outerbounds or starts remote workloads. ## Repository Layout From c774ecde6e44f146e2402baf7ffc1dd25da83abe Mon Sep 17 00:00:00 2001 From: Sri Vidya Vemulapalli Date: Mon, 14 Sep 2026 13:04:25 +0100 Subject: [PATCH 17/28] Update README with git commit hook details Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- flows/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/flows/README.md b/flows/README.md index e202903..c1b7d79 100644 --- a/flows/README.md +++ b/flows/README.md @@ -72,10 +72,10 @@ The flow hooks perform these checks: - `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 -A staged `*_flow.py` file under `flows/` activates all four flow hooks; other -staged Python files activate Ruff and `test-flow-tools`. Staged files under -`e2e/` separately activate the Playwright checks; a commit touching both runs both. +During a normal `git commit`, a staged `*_flow.py` file under `flows/` activates +all four flow hooks; other staged Python files activate Ruff and `test-flow-tools`. +Staged files under `e2e/` separately activate the Playwright checks; a commit +touching both runs both. `flows/requirements.txt`, `flows/requirements-dev.txt`, or `.pre-commit-config.yaml` is staged, so a version bump alone still triggers the tool-version consistency checks. The native Metaflow check runs only for staged From 08f563046f7f7c903b978228077ed727b174afec Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 13:17:10 +0100 Subject: [PATCH 18/28] fix pre-commit routing edge cases --- .husky/pre-commit | 3 +++ .pre-commit-config.yaml | 15 +++++++++++---- README.md | 11 +++++++++-- flows/README.md | 14 ++++++++++++++ 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index a25f2a8..c8fd101 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -42,6 +42,9 @@ if printf '%s\n' "$staged" | grep -Eq "^flows/|$config_pattern"; then if printf '%s\n' "$staged" | grep -Eq "$config_pattern" \ || printf '%s\n' "$deleted" | grep -Eq '^flows/'; then "$pre_commit" run --all-files || status=1 + if printf '%s\n' "$deleted" | grep -Eq '^flows/.*_flow\.py$'; then + "$pre_commit" run check-flow-inventory --hook-stage manual || status=1 + fi else # `files:` patterns in .pre-commit-config.yaml keep this to flows/. "$pre_commit" run || status=1 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 75a0c3a..f70f14a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -18,6 +18,14 @@ repos: - outerbounds==0.12.44 files: ^flows/.*_flow\.py$ + - id: check-flow-inventory + name: Validate Metaflow flow inventory + entry: python flows/scripts/check_flows.py --discovery-only + language: python + always_run: true + pass_filenames: false + stages: [manual] + - id: test-flow-tools name: Test flow utilities entry: pytest @@ -26,8 +34,7 @@ repos: additional_dependencies: - pytest==9.1.1 - pyyaml - # Includes flows/requirements*.txt and this file so the pinned-version - # consistency checks in flows/tests/test_tool_versions.py run whenever - # a version changes, not only when a flows/*.py file is also staged. - files: ^(flows/.*\.py|flows/requirements(-dev)?\.txt|\.pre-commit-config\.yaml)$ + # Includes tool configuration so focused tests run whenever behavior or + # a pinned version changes, not only when a flows/*.py file is staged. + files: ^(flows/.*\.py|flows/requirements(-dev)?\.txt|\.pre-commit-config\.yaml|pyproject\.toml)$ pass_filenames: false diff --git a/README.md b/README.md index 3e2cdf1..901ee59 100644 --- a/README.md +++ b/README.md @@ -19,8 +19,9 @@ See [`e2e/tests/README.md`](e2e/tests/README.md) for the Playwright test structu 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 and runs each area only when that area has staged changes. A -commit touching both directories runs both. +is the entry point. Flow files and tooling configuration activate the Python +checks; files under `e2e/tests/` and `e2e/test-setup/` activate the Playwright +checks. A commit touching both scopes runs both. Install the Python development requirements and the hook from the repository root: @@ -42,8 +43,14 @@ npm --prefix e2e run quality:full Staged `*_flow.py` changes run Ruff, focused pytest tests, and native Metaflow definition checks; other staged Python changes run Ruff and the focused tests. +<<<<<<< HEAD Ruff fixes are applied first, followed by the Playwright quality report. No hook authenticates to Outerbounds or starts remote workloads. +======= +Staged files under `e2e/tests/` and `e2e/test-setup/` run lint-staged fixes and +then the Playwright quality report. No hook authenticates to Outerbounds or +starts remote workloads. +>>>>>>> bd3baf4 (fix pre-commit routing edge cases) ## Repository Layout diff --git a/flows/README.md b/flows/README.md index c1b7d79..6b9f2ba 100644 --- a/flows/README.md +++ b/flows/README.md @@ -72,6 +72,7 @@ The flow hooks perform these checks: - `test-flow-tools`: unit tests for the custom checker and tool-version consistency. +<<<<<<< HEAD During a normal `git commit`, a staged `*_flow.py` file under `flows/` activates all four flow hooks; other staged Python files activate Ruff and `test-flow-tools`. Staged files under `e2e/` separately activate the Playwright checks; a commit @@ -81,6 +82,19 @@ touching both runs both. tool-version consistency checks. The native Metaflow check runs only for staged flow files, while discovery and repository-wide convention checks still inspect all repository flows. `test-flow-tools` runs the complete local test suite. Neither hook +======= +During a normal `git commit`, staged files determine which hooks run. A staged +`*_flow.py` file under `flows/` activates all four flow hooks; other staged +Python files activate Ruff and `test-flow-tools`. The flow requirements files, +`.pre-commit-config.yaml`, and `pyproject.toml` activate the complete Python +suite, so configuration changes cannot bypass its checks. The native Metaflow +check runs only for staged flow files during normal runs, while deletion checks +still inspect the repository-wide flow inventory. `test-flow-tools` runs the +complete local test suite. + +Staged files under `e2e/tests/` and `e2e/test-setup/` separately activate the +Playwright checks; a commit touching both scopes runs both. Neither check +>>>>>>> bd3baf4 (fix pre-commit routing edge cases) authenticates to Outerbounds or starts a Kubernetes workload. If Ruff modifies a file, review and stage the change before committing again: From 4d3092ec49598f8af29d79f3042aff9f2f5d17de Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 13:33:36 +0100 Subject: [PATCH 19/28] simplify pre-commit checks --- .husky/pre-commit | 17 +++++++++++------ .pre-commit-config.yaml | 5 ++--- README.md | 14 +++++--------- flows/README.md | 37 +++++++++---------------------------- 4 files changed, 27 insertions(+), 46 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index c8fd101..018ecf9 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,11 +1,10 @@ #!/usr/bin/env sh # Single Git entry point for both project areas. # -# Git honors one hook path, and Husky owns it so the Playwright checks keep -# running through lint-staged, which auto-fixes and re-stages files. `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. +# 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 @@ -18,6 +17,9 @@ status=0 staged=$(git diff --cached --name-only --diff-filter=ACMRD) deleted=$(git diff --cached --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. @@ -42,7 +44,10 @@ if printf '%s\n' "$staged" | grep -Eq "^flows/|$config_pattern"; then if printf '%s\n' "$staged" | grep -Eq "$config_pattern" \ || printf '%s\n' "$deleted" | grep -Eq '^flows/'; then "$pre_commit" run --all-files || status=1 - if printf '%s\n' "$deleted" | grep -Eq '^flows/.*_flow\.py$'; then + # check-flows validates the remaining inventory. The manual hook is only + # needed when deleting the final flow leaves no matching file to trigger it. + if printf '%s\n' "$deleted" | grep -Eq '^flows/.*_flow\.py$' \ + && ! git ls-files --cached 'flows/*_flow.py' 'flows/**/*_flow.py' | grep -q .; then "$pre_commit" run check-flow-inventory --hook-stage manual || status=1 fi else diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f70f14a..e638ef1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -34,7 +34,6 @@ repos: additional_dependencies: - pytest==9.1.1 - pyyaml - # Includes tool configuration so focused tests run whenever behavior or - # a pinned version changes, not only when a flows/*.py file is staged. - files: ^(flows/.*\.py|flows/requirements(-dev)?\.txt|\.pre-commit-config\.yaml|pyproject\.toml)$ + # 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 diff --git a/README.md b/README.md index 901ee59..6c5e1c3 100644 --- a/README.md +++ b/README.md @@ -41,16 +41,12 @@ pre-commit run --all-files npm --prefix e2e run quality:full ``` -Staged `*_flow.py` changes run Ruff, focused pytest tests, and native Metaflow -definition checks; other staged Python changes run Ruff and the focused tests. -<<<<<<< HEAD -Ruff fixes are applied first, followed by the Playwright quality report. No hook -authenticates to Outerbounds or starts remote workloads. -======= -Staged files under `e2e/tests/` and `e2e/test-setup/` run lint-staged fixes and -then the Playwright quality report. No hook authenticates to Outerbounds or +Staged `*_flow.py` changes run Ruff and native Metaflow definition checks; +checker tests run only when checker tooling or configuration changes. Staged +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. ->>>>>>> bd3baf4 (fix pre-commit routing edge cases) ## Repository Layout diff --git a/flows/README.md b/flows/README.md index 6b9f2ba..f97a869 100644 --- a/flows/README.md +++ b/flows/README.md @@ -72,37 +72,18 @@ The flow hooks perform these checks: - `test-flow-tools`: unit tests for the custom checker and tool-version consistency. -<<<<<<< HEAD -During a normal `git commit`, a staged `*_flow.py` file under `flows/` activates -all four flow hooks; other staged Python files activate Ruff and `test-flow-tools`. -Staged files under `e2e/` separately activate the Playwright checks; a commit -touching both runs both. -`flows/requirements.txt`, `flows/requirements-dev.txt`, or -`.pre-commit-config.yaml` is staged, so a version bump alone still triggers the -tool-version consistency checks. The native Metaflow check runs only for staged -flow files, while discovery and repository-wide convention checks still inspect all -repository flows. `test-flow-tools` runs the complete local test suite. Neither hook -======= During a normal `git commit`, staged files determine which hooks run. A staged -`*_flow.py` file under `flows/` activates all four flow hooks; other staged -Python files activate Ruff and `test-flow-tools`. The flow requirements files, -`.pre-commit-config.yaml`, and `pyproject.toml` activate the complete Python -suite, so configuration changes cannot bypass its checks. The native Metaflow -check runs only for staged flow files during normal runs, while deletion checks -still inspect the repository-wide flow inventory. `test-flow-tools` runs the -complete local test suite. +`*_flow.py` file under `flows/` activates Ruff and native Metaflow validation; +other staged Python files activate Ruff. 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 +deletion checks still inspect the repository-wide flow inventory. Staged files under `e2e/tests/` and `e2e/test-setup/` separately activate the -Playwright checks; a commit touching both scopes runs both. Neither check ->>>>>>> bd3baf4 (fix pre-commit routing edge cases) -authenticates to Outerbounds or starts a Kubernetes workload. - -If Ruff modifies a file, review and stage the change before committing again: - -```bash -git add -git commit -``` +Playwright format, lint, and quality checks; 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 From 6d843c9c86b6311b0aa2349869c1722f572ba4a9 Mon Sep 17 00:00:00 2001 From: Sri Vidya Vemulapalli Date: Mon, 14 Sep 2026 13:45:15 +0100 Subject: [PATCH 20/28] Fix string formatting in test_check_flows.py Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- flows/tests/test_check_flows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flows/tests/test_check_flows.py b/flows/tests/test_check_flows.py index a9337ee..8da2c00 100644 --- a/flows/tests/test_check_flows.py +++ b/flows/tests/test_check_flows.py @@ -17,7 +17,7 @@ def write_flow(path: Path, class_name: str = "ExampleFlow") -> None: """Create the smallest flow definition needed by checker tests.""" path.parent.mkdir(parents=True, exist_ok=True) path.write_text( - f"from metaflow import FlowSpec\n\n" + "from metaflow import FlowSpec\n\n" f"class {class_name}(FlowSpec):\n pass\n\n" f'if __name__ == "__main__":\n {class_name}()\n', encoding="utf-8", From f0d9f2f359bdff3b7553356beebf351708db02e1 Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 13:52:15 +0100 Subject: [PATCH 21/28] validate renamed flow filenames --- .husky/pre-commit | 18 ++++++++++++++++-- flows/README.md | 1 + 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index 018ecf9..4913251 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -14,8 +14,22 @@ # spanning both reports every problem instead of only the first. status=0 -staged=$(git diff --cached --name-only --diff-filter=ACMRD) -deleted=$(git diff --cached --name-only --diff-filter=D) +# 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) + +invalid_flow_renames=$( + git diff --cached --name-status --find-renames --diff-filter=R \ + | awk -F '\t' '$2 ~ /^flows\/.*_flow\.py$/ && $3 !~ /^flows\/.*_flow\.py$/ { + print " " $2 " -> " $3 + }' +) +if [ -n "$invalid_flow_renames" ]; then + echo "Flow renames must keep the *_flow.py suffix:" >&2 + printf '%s\n' "$invalid_flow_renames" >&2 + status=1 +fi # Catch conflict markers and whitespace errors in staged changes for both areas. git diff --cached --check || status=1 diff --git a/flows/README.md b/flows/README.md index f97a869..beb78c4 100644 --- a/flows/README.md +++ b/flows/README.md @@ -90,6 +90,7 @@ check authenticates to Outerbounds or starts a Kubernetes workload. 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__"`. From 5899e0ad1dec8f93f2a3ec3b2d37ca1dd4f6b062 Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 13:59:57 +0100 Subject: [PATCH 22/28] fix pre-commit package and inventory scope --- .husky/pre-commit | 11 ++++++----- .pre-commit-config.yaml | 8 -------- flows/README.md | 2 +- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index 4913251..28ae677 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -58,11 +58,12 @@ if printf '%s\n' "$staged" | grep -Eq "^flows/|$config_pattern"; then if printf '%s\n' "$staged" | grep -Eq "$config_pattern" \ || printf '%s\n' "$deleted" | grep -Eq '^flows/'; then "$pre_commit" run --all-files || status=1 - # check-flows validates the remaining inventory. The manual hook is only - # needed when deleting the final flow leaves no matching file to trigger it. + # 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 'flows/*_flow.py' 'flows/**/*_flow.py' | grep -q .; then - "$pre_commit" run check-flow-inventory --hook-stage manual || status=1 + && ! 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/. @@ -72,7 +73,7 @@ if printf '%s\n' "$staged" | grep -Eq "^flows/|$config_pattern"; then fi if git diff --cached --name-only --diff-filter=ACMR | grep -Eq '^e2e/(tests|test-setup)/'; then - npm --prefix e2e run precommit || status=1 + (cd e2e && npm run precommit) || status=1 fi exit "$status" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e638ef1..62629b9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -18,14 +18,6 @@ repos: - outerbounds==0.12.44 files: ^flows/.*_flow\.py$ - - id: check-flow-inventory - name: Validate Metaflow flow inventory - entry: python flows/scripts/check_flows.py --discovery-only - language: python - always_run: true - pass_filenames: false - stages: [manual] - - id: test-flow-tools name: Test flow utilities entry: pytest diff --git a/flows/README.md b/flows/README.md index beb78c4..8b0161b 100644 --- a/flows/README.md +++ b/flows/README.md @@ -77,7 +77,7 @@ During a normal `git commit`, staged files determine which hooks run. A staged other staged Python files activate Ruff. 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 -deletion checks still inspect the repository-wide flow inventory. +deletion checks require the staged index to retain at least one tracked flow. Staged files under `e2e/tests/` and `e2e/test-setup/` separately activate the Playwright format, lint, and quality checks; a commit touching both scopes runs From 83acdeec2b3ce8b4d8d37d2f75c1782444fe23f5 Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 14:13:46 +0100 Subject: [PATCH 23/28] validate flow filenames from git index --- .husky/pre-commit | 12 --------- .pre-commit-config.yaml | 7 ++++++ flows/README.md | 6 ++++- flows/scripts/check_flows.py | 43 +++++++++++++++++++++++++++++++++ flows/tests/test_check_flows.py | 23 ++++++++++++++++++ 5 files changed, 78 insertions(+), 13 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index 28ae677..1e66706 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -19,18 +19,6 @@ status=0 staged=$(git diff --cached --no-renames --name-only --diff-filter=ACMRD) deleted=$(git diff --cached --no-renames --name-only --diff-filter=D) -invalid_flow_renames=$( - git diff --cached --name-status --find-renames --diff-filter=R \ - | awk -F '\t' '$2 ~ /^flows\/.*_flow\.py$/ && $3 !~ /^flows\/.*_flow\.py$/ { - print " " $2 " -> " $3 - }' -) -if [ -n "$invalid_flow_renames" ]; then - echo "Flow renames must keep the *_flow.py suffix:" >&2 - printf '%s\n' "$invalid_flow_renames" >&2 - status=1 -fi - # Catch conflict markers and whitespace errors in staged changes for both areas. git diff --cached --check || status=1 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 62629b9..60f672b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,6 +10,13 @@ repos: - 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: ^flows/.*\.py$ + pass_filenames: false + - id: check-flows name: Validate Metaflow definitions entry: python flows/scripts/check_flows.py diff --git a/flows/README.md b/flows/README.md index 8b0161b..ed44ca8 100644 --- a/flows/README.md +++ b/flows/README.md @@ -67,6 +67,8 @@ 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 @@ -77,7 +79,9 @@ During a normal `git commit`, staged files determine which hooks run. A staged other staged Python files activate Ruff. 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 -deletion checks require the staged index to retain at least one tracked flow. +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. Staged files under `e2e/tests/` and `e2e/test-setup/` separately activate the Playwright format, lint, and quality checks; a commit touching both scopes runs diff --git a/flows/scripts/check_flows.py b/flows/scripts/check_flows.py index 0126d74..b6edf30 100644 --- a/flows/scripts/check_flows.py +++ b/flows/scripts/check_flows.py @@ -49,6 +49,41 @@ def _flow_class_names(tree: ast.Module) -> list[str]: ] +def validate_tracked_flow_filenames(repository_root: Path = REPOSITORY_ROOT) -> None: + """Require every tracked Python file containing a FlowSpec to use the flow suffix.""" + listed = subprocess.run( + ["git", "ls-files", "--cached", "-z", "--", "*.py"], + cwd=repository_root, + check=False, + capture_output=True, + ) + if listed.returncode: + raise FlowCheckError("Unable to list tracked flow files from the Git index") + + for raw_path in listed.stdout.split(b"\0"): + if not raw_path: + continue + relative_path = raw_path.decode("utf-8") + path = Path(relative_path) + if path.suffix != ".py" or path.name.endswith("_flow.py"): + continue + + staged = subprocess.run( + ["git", "show", f":{relative_path}"], + cwd=repository_root, + check=False, + capture_output=True, + ) + if staged.returncode: + raise FlowCheckError(f"Unable to read staged file: {relative_path}") + try: + tree = ast.parse(staged.stdout, filename=relative_path) + except SyntaxError: + continue + if _flow_class_names(tree): + raise FlowCheckError(f"Tracked FlowSpec file must end with '_flow.py': {relative_path}") + + def _is_main_guard(test: ast.expr) -> bool: """Recognize the standard `if __name__ == "__main__"` expression.""" return ( @@ -203,6 +238,11 @@ def _parse_args(arguments: Sequence[str] | None = None) -> argparse.Namespace: action="store_true", help="Discover flows without invoking Metaflow", ) + parser.add_argument( + "--tracked-filenames-only", + action="store_true", + help="Validate tracked FlowSpec filenames from the Git index", + ) parser.add_argument( "--format", choices=("paths", "json"), @@ -216,6 +256,9 @@ def main(arguments: Sequence[str] | None = None) -> int: args = _parse_args(arguments) flow_paths = list(args.paths) try: + if args.tracked_filenames_only: + validate_tracked_flow_filenames() + return 0 definitions = discover_flows(selected_paths=flow_paths) if not args.discovery_only: run_metaflow_checks(definitions) diff --git a/flows/tests/test_check_flows.py b/flows/tests/test_check_flows.py index 8da2c00..2c8b0b5 100644 --- a/flows/tests/test_check_flows.py +++ b/flows/tests/test_check_flows.py @@ -10,6 +10,7 @@ FlowDefinition, discover_flows, run_metaflow_checks, + validate_tracked_flow_filenames, ) @@ -39,6 +40,28 @@ def test_discovers_flows_recursively_in_stable_order(tmp_path: Path) -> None: ] +@pytest.mark.parametrize("destination", ["flows/renamed.py", "renamed.py"]) +def test_rejects_rewritten_flow_moved_to_filename_without_suffix( + tmp_path: Path, destination: str +) -> None: + """Reject a rewritten flow that Git cannot detect as a rename.""" + subprocess.run(["git", "init", "--quiet"], cwd=tmp_path, check=True) + original = tmp_path / "flows" / "original_flow.py" + write_flow(original, "OriginalFlow") + subprocess.run(["git", "add", "flows/original_flow.py"], cwd=tmp_path, check=True) + + original.unlink() + replacement = tmp_path / destination + write_flow(replacement, "CompletelyRewrittenFlow") + replacement.write_text( + replacement.read_text(encoding="utf-8") + "VALUE = 1\n", encoding="utf-8" + ) + subprocess.run(["git", "add", "--all"], cwd=tmp_path, check=True) + + with pytest.raises(FlowCheckError, match=rf"must end with '_flow.py': {destination}"): + validate_tracked_flow_filenames(tmp_path) + + @pytest.mark.parametrize("filename", ["missing_flow.py", "helper.py"]) def test_rejects_invalid_selected_paths(tmp_path: Path, filename: str) -> None: """Reject missing paths and existing files without the flow suffix.""" From 3c465d1b51fde18121da58a565c22f680139d15d Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 14:18:11 +0100 Subject: [PATCH 24/28] prefer pinned pre-commit executable --- .husky/pre-commit | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index 1e66706..8aa50e7 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -28,10 +28,10 @@ git diff --cached --check || status=1 config_pattern='^(\.pre-commit-config\.yaml$|pyproject\.toml$|\.husky/pre-commit$|flows/requirements(-dev)?\.txt$)' if printf '%s\n' "$staged" | grep -Eq "^flows/|$config_pattern"; then - if command -v pre-commit >/dev/null 2>&1; then - pre_commit=pre-commit - elif [ -x flows/.venv/bin/pre-commit ]; 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 From 8d4bfbb26eada5274626716f4d3efdb96a8e715a Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 14:34:12 +0100 Subject: [PATCH 25/28] recognize FlowSpec import aliases --- flows/scripts/check_flows.py | 26 +++++++++++++++++++++----- flows/tests/test_check_flows.py | 30 ++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/flows/scripts/check_flows.py b/flows/scripts/check_flows.py index b6edf30..03455e2 100644 --- a/flows/scripts/check_flows.py +++ b/flows/scripts/check_flows.py @@ -25,10 +25,13 @@ class FlowCheckError(Exception): """Raised when repository flow validation fails.""" -def _is_flowspec_base(base: ast.expr) -> bool: - """Recognize direct FlowSpec inheritance with or without module qualification.""" - return (isinstance(base, ast.Name) and base.id == "FlowSpec") or ( - isinstance(base, ast.Attribute) and base.attr == "FlowSpec" +def _is_flowspec_base(base: ast.expr, flowspec_names: set[str], metaflow_modules: set[str]) -> bool: + """Recognize FlowSpec inheritance through direct and module imports.""" + return (isinstance(base, ast.Name) and base.id in flowspec_names) or ( + isinstance(base, ast.Attribute) + and base.attr == "FlowSpec" + and isinstance(base.value, ast.Name) + and base.value.id in metaflow_modules ) @@ -42,10 +45,23 @@ def _parse_flow(path: Path) -> ast.Module: def _flow_class_names(tree: ast.Module) -> list[str]: """Return top-level FlowSpec subclass names from a parsed flow.""" + flowspec_names = {"FlowSpec"} + metaflow_modules = {"metaflow"} + for node in tree.body: + if isinstance(node, ast.ImportFrom) and node.module == "metaflow": + flowspec_names.update( + alias.asname or alias.name for alias in node.names if alias.name == "FlowSpec" + ) + elif isinstance(node, ast.Import): + metaflow_modules.update( + alias.asname or alias.name for alias in node.names if alias.name == "metaflow" + ) + return [ node.name for node in tree.body - if isinstance(node, ast.ClassDef) and any(_is_flowspec_base(base) for base in node.bases) + if isinstance(node, ast.ClassDef) + and any(_is_flowspec_base(base, flowspec_names, metaflow_modules) for base in node.bases) ] diff --git a/flows/tests/test_check_flows.py b/flows/tests/test_check_flows.py index 2c8b0b5..cca9aed 100644 --- a/flows/tests/test_check_flows.py +++ b/flows/tests/test_check_flows.py @@ -40,6 +40,28 @@ def test_discovers_flows_recursively_in_stable_order(tmp_path: Path) -> None: ] +@pytest.mark.parametrize( + ("import_statement", "base_name"), + [ + ("from metaflow import FlowSpec as BaseFlow", "BaseFlow"), + ("import metaflow as mf", "mf.FlowSpec"), + ], +) +def test_discovers_flowspec_import_aliases( + tmp_path: Path, import_statement: str, base_name: str +) -> None: + """Recognize FlowSpec aliases in valid flow definitions.""" + path = tmp_path / "aliased_flow.py" + path.write_text( + f"{import_statement}\n\n" + f"class AliasedFlow({base_name}):\n pass\n\n" + 'if __name__ == "__main__":\n AliasedFlow()\n', + encoding="utf-8", + ) + + assert discover_flows(tmp_path) == [FlowDefinition(path.resolve(), "AliasedFlow")] + + @pytest.mark.parametrize("destination", ["flows/renamed.py", "renamed.py"]) def test_rejects_rewritten_flow_moved_to_filename_without_suffix( tmp_path: Path, destination: str @@ -52,9 +74,13 @@ def test_rejects_rewritten_flow_moved_to_filename_without_suffix( original.unlink() replacement = tmp_path / destination - write_flow(replacement, "CompletelyRewrittenFlow") + replacement.parent.mkdir(parents=True, exist_ok=True) replacement.write_text( - replacement.read_text(encoding="utf-8") + "VALUE = 1\n", encoding="utf-8" + "from metaflow import FlowSpec as BaseFlow\n\n" + "class CompletelyRewrittenFlow(BaseFlow):\n pass\n\n" + 'if __name__ == "__main__":\n CompletelyRewrittenFlow()\n\n' + "VALUE = 1\n", + encoding="utf-8", ) subprocess.run(["git", "add", "--all"], cwd=tmp_path, check=True) From 592c46313de4def0709aa7cdc1ee8490c3a3620e Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 14:41:18 +0100 Subject: [PATCH 26/28] validate FlowSpec filenames for all Python changes --- .husky/pre-commit | 2 +- .pre-commit-config.yaml | 2 +- README.md | 7 ++++--- flows/README.md | 9 +++++---- flows/tests/test_tool_versions.py | 9 +++++++++ 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index 8aa50e7..f283485 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -27,7 +27,7 @@ git diff --cached --check || status=1 # 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 "^flows/|$config_pattern"; then +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 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 60f672b..1820958 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,7 +14,7 @@ repos: name: Validate tracked FlowSpec filenames entry: python flows/scripts/check_flows.py --tracked-filenames-only language: python - files: ^flows/.*\.py$ + files: \.py$ pass_filenames: false - id: check-flows diff --git a/README.md b/README.md index 6c5e1c3..9b2e541 100644 --- a/README.md +++ b/README.md @@ -19,9 +19,10 @@ See [`e2e/tests/README.md`](e2e/tests/README.md) for the Playwright test structu 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. Flow files and tooling configuration activate the Python -checks; files under `e2e/tests/` and `e2e/test-setup/` activate the Playwright -checks. A commit touching both scopes runs both. +is the entry point. Staged Python files activate FlowSpec filename validation; +files under `flows/` also activate the flow checks. Files under `e2e/tests/` and +`e2e/test-setup/` activate the Playwright checks. A commit touching both scopes +runs both. Install the Python development requirements and the hook from the repository root: diff --git a/flows/README.md b/flows/README.md index ed44ca8..1fcb5aa 100644 --- a/flows/README.md +++ b/flows/README.md @@ -76,10 +76,11 @@ The flow hooks perform these checks: 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 activate Ruff. 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 +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. diff --git a/flows/tests/test_tool_versions.py b/flows/tests/test_tool_versions.py index aba0c64..7e38635 100644 --- a/flows/tests/test_tool_versions.py +++ b/flows/tests/test_tool_versions.py @@ -66,3 +66,12 @@ def test_outerbounds_versions_match() -> None: assert additional_dependency_version("check-flows", "outerbounds") == requirement_version( "outerbounds", REQUIREMENTS_PATH ) + + +def test_flow_filename_hook_matches_python_files_anywhere() -> None: + """Run staged FlowSpec filename validation outside the flows directory.""" + pattern = re.compile(hook_config("check-flow-filenames")["files"]) + + assert pattern.search("renamed.py") + assert pattern.search("flows/models/example.py") + assert not pattern.search("flows/README.md") From 912bd1fe928dec571dfbaa95c22b809f41fbb625 Mon Sep 17 00:00:00 2001 From: Sri Vidya Vemulapalli Date: Mon, 14 Sep 2026 14:53:52 +0100 Subject: [PATCH 27/28] Refactor flow class names to use set type annotations Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- flows/scripts/check_flows.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/flows/scripts/check_flows.py b/flows/scripts/check_flows.py index 03455e2..381e0ab 100644 --- a/flows/scripts/check_flows.py +++ b/flows/scripts/check_flows.py @@ -45,12 +45,14 @@ def _parse_flow(path: Path) -> ast.Module: def _flow_class_names(tree: ast.Module) -> list[str]: """Return top-level FlowSpec subclass names from a parsed flow.""" - flowspec_names = {"FlowSpec"} - metaflow_modules = {"metaflow"} + flowspec_names: set[str] = set() + metaflow_modules: set[str] = set() for node in tree.body: if isinstance(node, ast.ImportFrom) and node.module == "metaflow": flowspec_names.update( - alias.asname or alias.name for alias in node.names if alias.name == "FlowSpec" + alias.asname or "FlowSpec" + for alias in node.names + if alias.name in {"FlowSpec", "*"} ) elif isinstance(node, ast.Import): metaflow_modules.update( From b382bf166187984eb06d954735e463e18b90922c Mon Sep 17 00:00:00 2001 From: vvemulapalli11 Date: Mon, 14 Sep 2026 14:57:40 +0100 Subject: [PATCH 28/28] clarify e2e pre-commit deletion behavior --- README.md | 15 ++++++++------- flows/README.md | 12 +++++++----- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 9b2e541..0ee0c39 100644 --- a/README.md +++ b/README.md @@ -20,9 +20,10 @@ 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. Files under `e2e/tests/` and -`e2e/test-setup/` activate the Playwright checks. A commit touching both scopes -runs both. +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: @@ -44,10 +45,10 @@ 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 -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. +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 diff --git a/flows/README.md b/flows/README.md index 1fcb5aa..4468fb2 100644 --- a/flows/README.md +++ b/flows/README.md @@ -84,11 +84,13 @@ 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. -Staged files under `e2e/tests/` and `e2e/test-setup/` separately activate the -Playwright format, lint, and quality checks; 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. +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