Seven review findings of the P1 phase, each fixed at its root:
1. (high) The CI install list omitted PyYAML, but the vendored tree imports
yaml eagerly (modules/deps/basicsr/utils/options.py:5), so the CI job
failed on its first run. pyyaml is now installed and asserted by
tests/test_ci_workflow.py.
2. (medium) tests/test_ci_workflow.py skipped the whole module without
PyYAML. The module never skips now: a minimal indentation parser is the
fallback, and it is cross-checked against the PyYAML parse.
3. (medium) Guard (e)'s self-test asserted that the tree still contains
breakage (it would break exactly at P2.1). It now proves the guard
instead; P2.1's work list is the scanner output and
test_guard_e_flip_owner_covers_every_current_finding keeps plan and tree
in sync.
4. (medium) The mock-fidelity tripwire skipped the comfy_api.latest
symbols. Every mocked symbol is now checked against the real source
(re-export / definition / instance attribute), including io.ComfyNode,
ComfyExtension, ComfyAPI and ComfyAPISync, plus the reverse check that
the mock invents nothing.
5. (low) The phase review listed committed work as outstanding and
misstated the ruff ignore list; both are corrected.
6. (low) The reported whole-suite count was impossible; the phase review
now carries the measured numbers.
7. (low) The root conftest installed the mocks unconditionally. The
KEEP_NO_COMFY_MOCKS=1 opt-out lets the headless tier import the real
comfy/comfy_api modules in-process.
tools/ci_env_sim.py runs the gate in an environment without the packages
the CI install list omits.
- plan section 9: P1.1-P1.5 marked done (2026-10-23)
- docs/audit/2026-10-23-p1-test-infrastructure.md: the deviations of the
phase (the pytest collection artifact had to stay, the CI install list was
completed, the ruff scope, the shadowed tests package)
- docs/2026-10-23-comfyui-keep-p1-phase.md: the phase review with the gate
results
.github/workflows/tests.yml:
- lint job: pip install 'ruff==0.8.*' (pinned major) + ruff check .
- test job: matrix python 3.10/3.12/3.13, CPU torch + torchvision from the
pytorch CPU index, numpy<2.3 and tomli on 3.10 only, opencv-python-headless,
einops, scipy, Pillow, tqdm, pytest; runs
python -m pytest -q -m 'unit or mock_integration or headless' with
COMFYUI_ROOT empty so the env-dependent tests skip
- publish.yml untouched
pyproject.toml: [tool.ruff] scope (E9+F, F401/F541 ignored) and the vendored
tree modules/deps excluded - with the default rules the repo has 127 findings.
tests/test_ci_workflow.py: parses the workflow and asserts the jobs, the
matrix, the install steps, COMFYUI_ROOT='' and that the CI command equals the
local gate command.
tests/_headless.py resolves the ComfyUI tree from COMFYUI_ROOT and the weight
directory from KEEP_MODEL_DIR (no hardcoded paths, actionable errors), loads
extra_model_paths.yaml through utils.extra_config like the server does,
registers a package under an importable alias with __path__ set and can stub
only the ui preview helpers.
tests/test_headless_bootstrap.py proves the helper on a dummy package: alias
resolution, submodule import, the missing-environment-variable errors and the
absence of machine-specific paths.
tests/test_structure.py implements the five plan guards as pure functions over
a repository root, each xfail(strict=False) with the step that flips it
strict in the reason:
(a) nodes/ classes exported by nodes/__init__.py -> P3.1
(b) no io.Schema(/define_schema in src/ -> P3.2
(c) __init__.py exposes exactly comfy_entrypoint -> P4.4
(d) no sys.path mutation outside modules/deps/ -> P2.5
(e) no pdb/profiler/pip in the vendored closure -> P2.1
The self-tests copy the repository into tmp_path and prove that each guard
fails on a deliberately violating file.
- conftest.py: comfy_api.latest (io/ui as siblings, ComfyExtension,
ComfyAPI/ComfyAPISync with an instance-only progress recorder),
comfy.model_management, comfy.model_patcher, comfy.utils and folder_paths
are installed before collection; the package alias ComfyUI_KEEP keeps
__path__ + submodule_search_locations and provides the logger the
modules/* code imports
- fixtures: deterministic sample_image and an autouse per-test
models_dir/folder-registry reset
- tests/test_conftest_mocks.py: dummy V3 node end-to-end through the mock
io, folder_paths isolation, mock-fidelity tripwire against the real
ComfyUI source (skips cleanly without COMFYUI_ROOT)
- pyproject: [tool.pytest.ini_options] with testpaths=tests, the four plan
markers (unit/mock_integration/headless/gpu_acceptance), addopts
'-v --tb=short --import-mode=importlib', consider_namespace_packages and
pythonpath=['.']
- tests/test_sanity.py::test_python_and_torch pins the supported interpreter
and the torch import
- 'python -m pytest --collect-only -q' exits 0 (40 tests collected)
All 7 P0 review findings were verified by execution and fixed.
F1 (medium) - the compat scanner modelled only the basicsr/archs eager scan.
The vendored basicsr also scans data/, losses/ and models/ eagerly
(basicsr/__init__.py:3-11 imports all of them), so the closure missed 32
files that the repaired package really imports (incl. models/keep_model.py and
models/keep_gan_model.py, both with 'import pdb' at line 4). The scanner now
models all four scans plus 'from <package> import <submodule>': closure
100 -> 132 files, findings 8 -> 10, and 0 runtime-loaded vendored files are
missing from the closure (verified on a temp copy of modules/deps).
The audit doc's opposite claim is corrected and P2.4 in the plan now has to
rewrite basicsr/__init__.py and prove 'import basicsr' still succeeds after
pruning (otherwise ModuleNotFoundError is swallowed into DummyArchRegistry).
F2 (medium) - the Asian manifest entry is now decided (P0.4 section 4.1):
url 404, sha256 null, sha256_prefix '4765ebe0', unverified true, available
false, required FALSE (a required entry would break the default KEEP model
through P5.3's fail-fast rule). Plan P5.0/P5.1/P5.3 and the weights scanner
were aligned.
F3 (low) - the plan's section 8 decision 1, the B3 baseline row, Goal 5, the
risk table and P5.5 carried the superseded GMFlow premise; they now match the
P0.4 decision (the shipped checkpoint carries the flownet weights; the GMFlow
file is defensive and resolved only when the checkpoint has no flownet.* keys).
F4 (low) - the P0.1 fixture asserted rc == 0 before checking the probe's fatal
report, so the documented skip was unreachable. It now handles the probe's
exit code 2 + fatal report and skips with a reason.
F5 (low) - the baseline-import doc claimed all eight stub symbols fire in the
ffmpeg-blocked run; it is 7 of 8 (loader_face_restore_helper_is_dummy does not
fire - the real facelib imports fine).
F6 (low) - the scanner's exit-code contract (0 = no findings, 1 = findings) is
documented in the tool docstring and in the audit doc.
F7 (low) - the root conftest registered '__init__' and the invalid directory
basename; it now registers the plan's alias 'ComfyUI_KEEP' and provides
execute_package_init() so the V3 entrypoint is executed rather than bypassed.
P1.2 is specified to replace that bootstrap.
Gates: pytest tests/ -q -> 37 passed, 1 xfailed; scan_compat.py --quiet ->
closure 132, 10 findings, exit 1; baseline_probe.py -> import ok / model build
fails (unchanged baseline); scan_weights.py -> 0 of 8 present.
- P1.2: io and ui are sibling modules (from . import _io_public as io /
_ui_public as ui); ui is not an attribute of io. Mock also exposes
ComfyAPI/ComfyAPISync with a no-op execution.set_progress recorder.
- P4.2: progress API verified by probe - class-level ComfyAPI.execution /
ComfyAPISync.execution raise AttributeError; use ComfyAPISync().execution
.set_progress(value, max_value) (sync) or await ComfyAPI().execution
.set_progress(...) (async), with one cached instance and try/except fallback.
- Replace the stale 2026-10-22 status review with the 2026-10-23 one.
WildAi