From 05984337836ae973c7e3e4adce3349f29bd2d180 Mon Sep 17 00:00:00 2001 From: Peyton DeNiro <25550995+pydn@users.noreply.github.com> Date: Sat, 28 Mar 2026 16:02:40 -0500 Subject: [PATCH] Remove upstream smoke validation tier --- PR_DESCRIPTION.md | 35 +++ docs/runtime-validation.md | 99 +++++++ docs/specs/runtime-e2e-agent-spec.md | 128 ++++++++++ .../runtime-validation-environment-spec.md | 136 ++++++++++ .../runtime-validation-implementation-spec.md | 241 ++++++++++++++++++ tests/runtime/run_runtime_validation.py | 17 +- 6 files changed, 645 insertions(+), 11 deletions(-) create mode 100644 PR_DESCRIPTION.md create mode 100644 docs/runtime-validation.md create mode 100644 docs/specs/runtime-e2e-agent-spec.md create mode 100644 docs/specs/runtime-validation-environment-spec.md create mode 100644 docs/specs/runtime-validation-implementation-spec.md diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md new file mode 100644 index 0000000..674d348 --- /dev/null +++ b/PR_DESCRIPTION.md @@ -0,0 +1,35 @@ +## Summary + +This PR adds a runtime validation path for exported workflows so exporter changes can be checked at two levels: + +- fast exporter-only validation +- runtime validation against a pinned local ComfyUI checkout + +It includes committed fixtures for: +- `upscale-model-loader` +- `text-to-image` +- `unsafe-kwargs` +- `subgraph-identifiers` + +It also updates the exporter so generated scripts preserve workflow metadata and hidden runtime inputs needed for end-to-end execution. + +## Included + +- runtime validation harness in `tests/runtime/run_runtime_validation.py` +- runtime fixtures in `tests/fixtures/runtime/` +- exporter support changes in `comfyui_to_python.py` + +## Validation + +Planned validation commands: + +```bash +uv run python tests/runtime/run_runtime_validation.py --tier fast --fixture all +uv run python tests/runtime/run_runtime_validation.py --tier runtime --fixture all +``` + +Runtime-tier failures are classified as: +- `repo regression` +- `model provisioning failure` +- `fixture bug` +- `environment/setup failure` diff --git a/docs/runtime-validation.md b/docs/runtime-validation.md new file mode 100644 index 0000000..538712f --- /dev/null +++ b/docs/runtime-validation.md @@ -0,0 +1,99 @@ +# Runtime Validation + +This repo has two validation tiers: + +- `fast` +- `runtime` + +Use `uv` for repo commands. + +## Local Runtime Paths + +Pinned ComfyUI checkout: + +- `/home/peyton/code/ComfyUI` + +The runtime harness uses `COMFYUI_PATH` to point generated scripts and export logic at the target ComfyUI checkout. +For full E2E runtime execution, the harness prefers the ComfyUI checkout's own Python interpreter at `/home/peyton/code/ComfyUI/.venv/bin/python` when available. + +## Fixtures + +Committed runtime fixtures live in: + +- [tests/fixtures/runtime](/home/peyton/code/ComfyUI-to-Python-Extension/tests/fixtures/runtime) + +Current fixtures: + +- `upscale-model-loader` +- `text-to-image` +- `unsafe-kwargs` +- `subgraph-identifiers` + +Runtime-tier fixtures are based on official core ComfyUI workflow shapes: + +- the Image Upscale workflow +- the Text to Image workflow + +Sources: + +- https://docs.comfy.org/tutorials/basic/upscale +- https://docs.comfy.org/tutorials/basic/text-to-image + +## Commands + +### Fast + +Run exporter-only validation without a real ComfyUI runtime: + +```bash +uv run python tests/runtime/run_runtime_validation.py --tier fast --fixture all +``` + +### Runtime + +Run runtime-capable validation against the pinned baseline checkout. This now performs full E2E execution by default: + +```bash +uv run python tests/runtime/run_runtime_validation.py --tier runtime --fixture upscale-model-loader +``` + +Run both official runtime fixtures: + +```bash +uv run python tests/runtime/run_runtime_validation.py --tier runtime --fixture all +``` + +### Model Provisioning + +If a runtime fixture is missing required models, the harness will report `model provisioning failure`. + +To print the required download commands without executing them: + +```bash +uv run python tests/runtime/run_runtime_validation.py --tier runtime --fixture all --print-download-plan +``` + +Model downloads should be approval-gated and should target the existing `/home/peyton/code/ComfyUI/models/...` directories. + +## Failure Classification + +The runtime harness classifies failures as: + +- `repo regression` +- `model provisioning failure` +- `fixture bug` +- `environment/setup failure` + +This classification matters more than raw pass/fail. A failed run without classification is not actionable enough for maintenance work. + +## Notes + +- `unsafe-kwargs` and `subgraph-identifiers` are committed fast-tier regression fixtures. They are not runtime-tier fixtures. +- `upscale-model-loader` and `text-to-image` are the current runtime-tier fixtures and are based on official ComfyUI tutorials. +- Runtime-tier success now means: + - generated Python parses + - generated script executes + - a new output file is created + - output dimensions meet expectations + - output metadata contains expected markers +- `Save As Script` route validation is still phase 2 work. diff --git a/docs/specs/runtime-e2e-agent-spec.md b/docs/specs/runtime-e2e-agent-spec.md new file mode 100644 index 0000000..fb49060 --- /dev/null +++ b/docs/specs/runtime-e2e-agent-spec.md @@ -0,0 +1,128 @@ +# Runtime E2E Agent Spec + +## Purpose + +Own end-to-end runtime validation for this repo against a real ComfyUI runtime, using a pinned validation target and a small representative workflow corpus. + +## Use When + +- a change affects runtime/bootstrap behavior +- a merge recommendation needs stronger evidence than exporter-only tests +- a maintainer wants a runtime sanity check before or after a hotfix merge + +## Required Inputs + +- approved spec or target branch under review +- pinned ComfyUI runtime target +- runtime validation workflow corpus +- required runtime dependencies in the validation environment +- current repo worktree or PR branch + +## Expected Outputs + +- a short executive summary first +- a structured validation report with: + - Inputs + - Checks Performed + - Findings + - Fixture or Environment Issues + - Coverage Gaps + - Merge Recommendation + - Next Action + +## Reporting Rules + +- respect the maintainer's time +- keep the report decision-oriented +- distinguish repo regression from harness failure +- do not dump raw logs unless a short excerpt is needed to explain a failure + +## Validation Tiers + +### `fast` + +Use exporter-only checks that do not require a full ComfyUI runtime. This tier is not owned exclusively by this agent, but the agent should understand when `fast` is enough and when runtime validation is required. + +### `runtime` + +Run representative generated-script validations against a pinned local ComfyUI checkout with required runtime dependencies available. + +## Responsibilities + +- maintain awareness of the pinned ComfyUI runtime target +- run the `runtime` tier for runtime-sensitive changes +- classify failures into: + - repo bug + - fixture bug + - environment/setup failure +- provide a concise merge recommendation: + - `pass` + - `pass with risk` + - `fail` + +## Initial Workflow Corpus + +The first validation corpus should cover: + +- upscale loader workflow for `#143` +- unsafe-kwargs workflow for `#119` / `#96` +- subgraph identifier workflow for `#128` + +Future additions may include: + +- frontend save-flow validation fixtures +- runtime performance/bootstrap parity fixtures for issues like `#147` + +## Suggested Cadence + +- on demand for runtime-sensitive branches +- before merge for runtime/bootstrap fixes + +## Handoff Rules + +- report merge decisions to `maintainer-orchestrator` +- send runtime regressions to `runtime-compat-agent` +- send exporter-only failures back to `verification-agent` or `exporter-bug-agent` +- send upstream harness breakage to `upstream-watch-agent` + +## Branch and PR Conventions + +- validation results should reference the exact working branch and, when present, the PR title +- runtime-only harness changes should use focused branch names such as: + - `codex/runtime-e2e-` + - `codex/issue--runtime-e2e` + +## Ready-to-Paste Prompt + +```text +You are the runtime-e2e-agent for the ComfyUI-to-Python-Extension repo. + +Your job is to validate runtime-sensitive changes against a real ComfyUI runtime using a pinned validation target and a representative workflow corpus. Respect the maintainer's time. Be concise, strict, and decision-oriented. Do not write long test diaries. + +Repo rules: +- use uv for repo commands +- preserve a clear separation between exporter-only checks and real runtime validation +- classify failures accurately: repo bug, fixture bug, or environment failure +- do not call something "pass" if the runtime path was not actually exercised + +When you respond: +- Start with Executive Summary in 3 to 6 short lines. +- Then use: + - Inputs + - Checks Performed + - Findings + - Fixture or Environment Issues + - Coverage Gaps + - Merge Recommendation + - Next Action + +Recommendation labels: +- pass +- pass with risk +- fail + +Validation rules: +- run the pinned runtime suite when the change affects runtime/bootstrap behavior +- keep logs summarized unless a short excerpt is necessary +- make it obvious whether a failure belongs to this repo or the validation harness +``` diff --git a/docs/specs/runtime-validation-environment-spec.md b/docs/specs/runtime-validation-environment-spec.md new file mode 100644 index 0000000..8f56836 --- /dev/null +++ b/docs/specs/runtime-validation-environment-spec.md @@ -0,0 +1,136 @@ +# Runtime Validation Environment Spec + +## Purpose + +Define the environment, fixture layout, and execution contract for end-to-end runtime validation of `ComfyUI-to-Python-Extension`. + +## Problem + +Exporter-only regression tests are necessary but insufficient for runtime/bootstrap-sensitive changes. Local verification becomes unreliable when tests depend on an arbitrary ComfyUI installation or on environment details that are not controlled. The repo needs a repeatable runtime validation harness. + +## Goal + +Create a stable runtime validation setup that: + +- uses a pinned ComfyUI checkout +- validates generated scripts against a real runtime +- uses a small set of representative workflow fixtures +- clearly separates repo regressions from environment problems + +## Non-Goals + +- no image-quality benchmarking +- no full browser automation in the first phase +- no external CI/CD setup in this phase +- no attempt to cover every ComfyUI node or workflow shape + +## Validation Model + +### Tier 1: `fast` + +Purpose: + +- run quickly during local development +- validate exporter behavior without requiring a real ComfyUI runtime + +Typical checks: + +- focused unit or regression tests with injected mappings +- generated Python parse checks +- syntax compilation + +### Tier 2: `runtime` + +Purpose: + +- validate generated scripts against a real pinned ComfyUI runtime + +Typical checks: + +- export from API-format workflow to Python +- parse/import generated Python +- execute generated script through the targeted runtime path +- confirm expected bootstrap and node-loading behavior + +## Environment Requirements + +- a local ComfyUI checkout available through a documented path convention +- the ability to point this repo to that checkout, preferably with `COMFYUI_PATH` +- required runtime dependencies installed in the validation environment, including `torch` +- all repo command execution through `uv` + +## Proposed Path Convention + +The validation harness should assume: + +- this repo remains where it is +- the pinned ComfyUI checkout lives at a maintainer-chosen local path +- the runtime command surface uses `COMFYUI_PATH=/path/to/ComfyUI` + +The exact checkout location does not need to be committed into this repo, but the contract for supplying it must be documented and stable. + +## Workflow Fixture Corpus + +The first committed or documented fixture set should cover: + +- `upscale-model-loader` + for `#143`-style runtime/bootstrap validation +- `unsafe-kwargs` + for `#119` / `#96`-style generated-call validation against real workflows +- `subgraph-identifiers` + for `#128`-style identifier safety + +Fixture principles: + +- prefer real workflow shapes from official ComfyUI examples when possible +- keep fixtures minimal but representative +- name fixtures after behavior, not issue numbers, when practical + +## Assertions + +The first phase should assert: + +- export succeeds +- generated Python parses +- generated script can execute far enough to validate the targeted runtime path +- expected code patterns are present or absent when relevant + +The first phase should not require: + +- image-diff assertions +- performance benchmarking beyond obvious bootstrap regressions +- full frontend automation + +## Failure Classification + +Every runtime validation failure should be classified as one of: + +- repo regression +- fixture bug +- environment/setup failure + +This classification is mandatory because raw pass/fail is not enough for a maintenance workflow. + +## Likely Future Deliverables + +When implementation is approved, the runtime validation system will likely need: + +- a fixtures directory for runtime workflows +- a short operator document for setting `COMFYUI_PATH` +- one or more helper commands or scripts for running: + - `fast` + - `runtime` +- a `runtime-e2e-agent` config file under `.codex/agents/` + +## Acceptance Criteria + +- runtime validation is defined around a pinned ComfyUI target +- the initial workflow corpus covers the main known runtime-sensitive issue classes +- validation tiers are clearly separated +- pass/fail reporting includes failure classification +- the setup is small enough to maintain manually inside Codex/chat workflows + +## Open Questions + +- should the workflow corpus be committed into this repo or referenced from a local external fixture path? +- should the first implementation cover only CLI/runtime execution, or also a later `Save As Script` route check? diff --git a/docs/specs/runtime-validation-implementation-spec.md b/docs/specs/runtime-validation-implementation-spec.md new file mode 100644 index 0000000..deed264 --- /dev/null +++ b/docs/specs/runtime-validation-implementation-spec.md @@ -0,0 +1,241 @@ +# Runtime Validation Implementation Spec + +## Document Status + +- Date: March 28, 2026 +- Scope: implementation spec only +- Approval status: approved for spec creation, not yet approved for implementation + +## Problem + +The repo now has a runtime validation strategy, but not an implementation contract. Runtime-sensitive changes still rely too heavily on ad hoc local setups and narrow regression tests. The repo needs a small, repeatable runtime validation harness that agents and maintainers can run consistently. + +## Intent + +Implement a phase-based runtime validation harness and add a project-scoped `runtime-e2e-agent` that can operate against it. + +## Locked Decisions + +- Baseline pinned ComfyUI checkout: `/home/peyton/code/ComfyUI` +- Runtime workflow fixtures will be committed in this repo +- Implementation covers both CLI/runtime and `Save As Script`, but in phases +- Phase 1: CLI/runtime first +- Phase 2: `Save As Script` route validation + +## Goals + +- create a repeatable runtime validation harness +- keep the harness small enough to maintain manually +- make the workflow corpus reviewable and committed in-repo +- support pinned-runtime validation with a stable command surface +- give the future `runtime-e2e-agent` a stable command surface + +## Non-Goals + +- no full browser automation in phase 1 +- no image-quality regression testing +- no external CI/CD setup in this phase +- no vendoring of the ComfyUI repo into this repo +- no attempt to validate every possible custom-node workflow + +## Phase Plan + +### Phase 1: CLI / Runtime Harness + +Phase 1 must provide: + +- in-repo runtime workflow fixtures +- a command path that exports workflow JSON to Python +- a command path that validates generated Python parses +- a command path that executes generated Python against a pinned ComfyUI runtime +- clear pass/fail classification + +This phase should be enough to validate: + +- runtime/bootstrap regressions like `#143` +- generated-code validity issues when they must be checked against a real runtime +- runtime/bootstrap changes that affect node-loading or script execution + +### Phase 2: `Save As Script` Route Validation + +Phase 2 must build on the phase 1 harness and add: + +- route-level validation of the `Save As Script` request path +- coverage for server-side export behavior in [__init__.py](/home/peyton/code/ComfyUI-to-Python-Extension/__init__.py) +- coverage for the exported payload shape expected by the frontend flow + +Phase 2 should not start with full browser automation unless route-level checks prove insufficient. + +## Directory Layout + +The first implementation should add: + +- `tests/fixtures/runtime/` +- `tests/fixtures/runtime/upscale-model-loader.json` +- `tests/fixtures/runtime/unsafe-kwargs.json` +- `tests/fixtures/runtime/subgraph-identifiers.json` +- `tests/runtime/` +- a focused runtime validation runner or helper module inside `tests/runtime/` +- `.codex/agents/runtime-e2e-agent.toml` + +Optional supporting docs: + +- `docs/runtime-validation.md` + +## Fixture Rules + +- prefer real ComfyUI workflow shapes over synthetic-only fixtures +- keep fixtures as small as possible while still reproducing the target behavior +- name fixtures after behavior, not issue numbers, when practical +- commit fixtures to the repo so they are reviewable and stable + +## Initial Fixture Set + +### `upscale-model-loader.json` + +Purpose: + +- validate the `#143`/`#148` runtime/bootstrap path + +Assertions: + +- export succeeds +- generated Python parses +- generated script reaches the relevant loader and upscale path without the old missing-node failure + +### `unsafe-kwargs.json` + +Purpose: + +- validate mixed safe and unsafe keys in a real workflow shape + +Assertions: + +- export succeeds +- generated Python parses +- generated code remains safe when real workflow data contains symbol-heavy or invalid keys + +### `subgraph-identifiers.json` + +Purpose: + +- validate that subgraph-expanded identifiers do not break generated Python in real workflow shapes + +Assertions: + +- export succeeds +- generated Python parses +- generated variable names are safe enough for execution + +## Command Contract + +The harness should expose two stable command categories. + +### `fast` + +Purpose: + +- exporter-only validation without a real ComfyUI runtime + +Examples: + +- `uv run python -m unittest tests.test_upscale_model_loader_export -v` +- additional focused exporter tests as they are added + +### `runtime` + +Purpose: + +- run runtime validation against the pinned baseline ComfyUI checkout + +Environment: + +- `COMFYUI_PATH=/home/peyton/code/ComfyUI` + +Behavior: + +- export fixture workflow to Python +- parse or compile generated Python +- execute generated Python with the pinned runtime available + +## Runner Shape + +The implementation should prefer one small runner over many scattered commands. + +Recommended shape: + +- a small Python entry point that: + - selects a tier + - selects a fixture or fixture subset + - sets or reads `COMFYUI_PATH` + - runs export + - validates generated Python + - optionally executes the generated script + - reports failure classification + +The runner must stay small and repo-local. It should not introduce a large new framework. + +## Failure Classification Contract + +Every runtime validation result must classify failure as one of: + +- repo regression +- fixture bug +- environment/setup failure + +This classification is mandatory in both manual runs and future agent reports. + +## Runtime-E2E Agent Addition + +A future implementation step should add: + +- `.codex/agents/runtime-e2e-agent.toml` + +Its instructions should align with: + +- [runtime-e2e-agent-spec.md](/home/peyton/code/ComfyUI-to-Python-Extension/docs/specs/runtime-e2e-agent-spec.md) +- [runtime-validation-environment-spec.md](/home/peyton/code/ComfyUI-to-Python-Extension/docs/specs/runtime-validation-environment-spec.md) + +The agent should: + +- run the `runtime` tier +- report concise results +- avoid long logs +- return `pass`, `pass with risk`, or `fail` + +## Validation Expectations + +Phase 1 success means: + +- the pinned runtime can validate at least the three initial fixtures +- the runner can distinguish export failures from runtime execution failures +- the runtime harness can be invoked consistently by maintainers and agents + +Phase 2 success means: + +- route-level `Save As Script` validation exists +- frontend/server export behavior can be checked without relying on a full browser automation stack + +## Risks + +- fixture maintenance can drift if fixtures are too complex +- local ComfyUI checkout state can become stale if not periodically refreshed intentionally +- runtime validation may still fail for environment reasons if `torch` or ComfyUI dependencies are missing + +## Acceptance Criteria + +- the implementation plan is explicit enough to build without inventing workflow shape later +- fixture names and locations are defined +- command tiers are defined +- phase split is defined +- failure classification is mandatory +- the `runtime-e2e-agent` rollout target is defined + +## Next Step + +If this implementation spec is approved, the next step is to produce a concrete implementation plan and then request explicit approval before creating: + +- runtime fixture files +- the runner/helper module +- the `.codex/agents/runtime-e2e-agent.toml` file +- the short operator doc diff --git a/tests/runtime/run_runtime_validation.py b/tests/runtime/run_runtime_validation.py index 6312449..eed5397 100644 --- a/tests/runtime/run_runtime_validation.py +++ b/tests/runtime/run_runtime_validation.py @@ -18,7 +18,6 @@ ROOT = Path(__file__).resolve().parents[2] FIXTURE_DIR = ROOT / "tests" / "fixtures" / "runtime" GENERATED_DIR = ROOT / "tests" / "runtime" / "generated" BASELINE_COMFYUI_PATH = Path("/home/peyton/code/ComfyUI") -UPSTREAM_SMOKE_COMFYUI_PATH = Path("/home/peyton/code/ComfyUI-upstream-smoke") COMFYUI_OUTPUT_DIRNAME = "output" COMFYUI_INPUT_DIRNAME = "input" @@ -237,11 +236,11 @@ FIXTURES = { def parse_args() -> argparse.Namespace: parser = argparse.ArgumentParser( - description="Run fast, runtime, or upstream-smoke validation for committed fixtures." + description="Run fast or runtime validation for committed fixtures." ) parser.add_argument( "--tier", - choices=("fast", "runtime", "upstream-smoke"), + choices=("fast", "runtime"), help="Validation tier to run.", ) parser.add_argument( @@ -277,8 +276,6 @@ def parse_args() -> argparse.Namespace: def load_fixture_names(selection: str) -> list[str]: if selection == "all": return list(FIXTURES.keys()) - if selection == "smoke": - return ["upscale-model-loader", "text-to-image"] if selection not in FIXTURES: raise ValidationFailure("fixture bug", f"Unknown fixture '{selection}'.") return [selection] @@ -287,8 +284,6 @@ def load_fixture_names(selection: str) -> list[str]: def ensure_runtime_path(tier: str) -> str: if tier == "runtime": runtime_path = BASELINE_COMFYUI_PATH - elif tier == "upstream-smoke": - runtime_path = UPSTREAM_SMOKE_COMFYUI_PATH else: return os.environ.get("COMFYUI_PATH", "") @@ -596,8 +591,8 @@ def run_fixture(fixture: FixtureConfig, tier: str, execute: bool, runtime_path: generated_code = export_workflow_in_runtime_env(fixture, runtime_path) validate_generated_python(generated_code, fixture.name) - should_execute = execute or tier in {"runtime", "upstream-smoke"} - if should_execute and tier in {"runtime", "upstream-smoke"}: + should_execute = execute or tier == "runtime" + if should_execute and tier == "runtime": execute_generated_python(generated_code, fixture, runtime_path) return "pass" @@ -634,7 +629,7 @@ def main() -> int: "fixture bug", "No selected fixtures are fast-tier compatible.", ) - elif args.tier in {"runtime", "upstream-smoke"}: + elif args.tier == "runtime": requested = [fixture for fixture in requested if fixture.runtime_capable] if not requested: raise ValidationFailure( @@ -645,7 +640,7 @@ def main() -> int: failures: list[tuple[str, str, str]] = [] for fixture in requested: try: - if args.print_download_plan and args.tier in {"runtime", "upstream-smoke"}: + if args.print_download_plan and args.tier == "runtime": print_download_plan(fixture, runtime_path) status = run_fixture(fixture, args.tier, args.execute, runtime_path) print(f"{fixture.name}: {status}")