Five review findings of the P2 phase, each fixed at its root:
1. (medium) The P2.6 guard could not see an undeclared third-party import:
the closure's imports were filtered through a hard-coded 10-name map, so
every module outside it was dropped silently (a module-level `import av`
in a copy of the repo still gave "1 passed"). The map is now a *rename*
map (cv2 -> opencv-python, PIL -> Pillow, yaml -> pyyaml); every other
module is mapped to its own name, so an unknown module shows up as an
undeclared dependency. The guard is split by import time: every
third-party module imported at import time must be a declared
distribution, every lazy (function-level) one must be allowlisted with a
reason (dlib, gdown, mc, wandb - all unreachable from the node).
2. (low) The P2.1 (c) progression test was vacuous: it returned before its
assertions and probed a second, top-level copy of the vendored tree
(`sys.path.insert(modules/deps)`). It now imports the packaged
architecture, asserts that FeedForward / AdaLayerNorm come from the
vendored _diffusers_compat and that no top-level `basicsr` shadows an
installed one.
3. (low) Two swallow-and-stub fallbacks remained in the vendored tree
(facelib/detection, facelib/parsing - their stubs called an unimported
`urlparse` and would have raised NameError). Both now import the vendored
symbol directly; a guard forbids a fallback stub in the vendored tree.
4. (low) `basicsr/utils/file_client.py` still appended to `sys.path` (the
optional `sys_path` of MemcachedBackend), which the plan's §3 rule
forbids anywhere in the package. The mutation and the parameter are gone
and guard (d) now covers the whole package (the vendored tree included).
5. (low) The unreachable body of `KEEPFaceProcessor.process_video` still
called the deleted VideoReader / VideoWriter. The 141 dead lines are
removed; an AST guard keeps the references out.
Found while fixing the findings: the CI job did not install `requests`,
which the vendored `basicsr/utils/download_util.py` imports at module level
(P2.6 added it to requirements.txt but not to the workflow) - a clean CI
runner died with "No module named 'requests'". The install list is now
completed and a test derives it from requirements.txt.
Gate: pytest -m "unit or mock_integration" -> 109 passed, 1 skipped,
4 xfailed; plan's local gate (+ headless) -> 109 passed, 3 skipped,
4 xfailed; tools/scan_compat.py -> finding_count 0 (exit 0); ruff check . ->
All checks passed. A mutation check re-introduces six defects into a copy of
the repository: the matching guard fails in 6/6 cases.
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.
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.
WildAi