test: kill the 22 chronic failures — they were stub collisions, not redundant tests

Every one of them was a real test of a live feature. 18 failed only in a full-suite run and
passed alone, 4 asserted behaviour that was deliberately changed and never got updated.

Ordering (18): each test module built its own partial `comfy` stub and registered it with
`sys.modules.setdefault(...)`, so the first module to import won and every later module's
richer stub was silently discarded — the scene chain sampler then died on `comfy` having no
`.sample`, and the mask-safe attention install bailed at its first guard because
`import comfy.ldm.modules.attention` resolves through the PARENT's attribute, which one
module set and another did not. tests/_comfy_stubs.py now builds that package tree once,
wiring every parent attribute, and test modules attach their fakes as attributes on the
shared modules (per-test, via fixtures, where the fake is only theirs — the idiom
test_detailing.py already used). Same file also stubs `server.PromptServer`, whose absence
made every module that imports templates.py (i.e. anything importing conditioning) fail on
`No module named 'server'`.

Stale assertions (4):
* the "every learning rating" loop excluded non-learning ratings by hardcoded LABEL, so the
  later-added `Wrong appearance|loved` and the two control sentinels were asserted to train
  phrase memory. Now filtered by profile key, with the reason per key.
* three split_by_transitions tests still used "scene ten" / "cut to" as cut markers. The
  unified splitter cuts on a real transition trigger or the generic `scene <N>` label, on
  purpose — an incidental phrase must never split a prompt. Rewritten on the current
  contract, and they now also pin that the delimiter is dropped rather than encoded.

Verified: 584 pass, and every file passes alone, in reverse order, and with movie_editor's
suite standalone. Mutation-checked the rewritten split tests (disabling the scene-label
pattern fails 4 of 5) so they are not passing vacuously.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

tests/_comfy_stubs.py

@@ -0,0 +1,148 @@
+"""Stand-ins for the ComfyUI runtime, installed once for the whole suite.
+
+FunPack's modules are ComfyUI custom nodes: importing one pulls in `folder_paths`, `comfy.*`
+and (through `templates.py`, which registers its HTTP routes at module scope) `server`. None
+of those exist outside a running ComfyUI, so they have to be in `sys.modules` BEFORE the
+first test module imports — a conftest is the only hook early enough.
+
+**Why this file exists at all.** Every test module used to build its own partial `comfy`
+stub and register it with `sys.modules.setdefault(...)`. First file wins, so in a full-suite
+run a later file's richer stub was silently discarded and its tests failed on a missing
+attribute — while passing when run alone. Two shapes of the same bug:
+
+* `import comfy.sample` resolves through the PARENT's attribute, so a stub that sets
+  `sys.modules["comfy.sample"]` without also setting `comfy.sample` leaves the import
+  reading whatever the first file wired (or nothing).
+* the same for `comfy.ldm.modules.attention`, which several files replace wholesale.
+
+So the package tree is built ONCE here, with every parent attribute wired, and test modules
+attach their own fakes as ATTRIBUTES on these shared module objects instead of replacing
+them. Anything a single file needs only for its own tests belongs in a fixture that restores
+the previous value (see `test_detailing.py` / `test_minimax_h3_sampler.py`), because these
+objects are shared for the whole session.
+"""
+import sys
+import types
+
+
+def install_module(name, **attrs):
+    """Register `name` in sys.modules AND on its parent, then set `attrs` on it.
+
+    Returns the module (existing one if already registered, so callers merge rather than
+    replace). Wiring the parent attribute is the part that is easy to forget and the whole
+    reason imports used to resolve to a stale stub.
+    """
+    mod = sys.modules.get(name)
+    if mod is None:
+        mod = types.ModuleType(name)
+        sys.modules[name] = mod
+    if "." in name:
+        parent_name, _, leaf = name.rpartition(".")
+        parent = install_module(parent_name)
+        setattr(parent, leaf, mod)
+    for key, value in attrs.items():
+        setattr(mod, key, value)
+    return mod
+
+
+def _install_folder_paths():
+    install_module(
+        "folder_paths",
+        models_dir="",
+        get_input_directory=lambda: "",
+        get_output_directory=lambda: "",
+        get_temp_directory=lambda: "",
+        get_folder_paths=lambda _name: [],
+        get_filename_list=lambda _name: [],
+    )
+
+
+def _install_server():
+    """`server.PromptServer` — route decorators that register nothing and return the fn.
+
+    templates.py decorates ~20 handlers with `@PromptServer.instance.routes.get(...)` at
+    import time. The handlers are exercised through the Movie Editor's own aiohttp tests, so
+    the decorator only has to be a no-op that leaves the function callable.
+    """
+    if "server" in sys.modules:
+        return
+
+    class _Routes:
+        """aiohttp's RouteTableDef surface, for any verb.
+
+        Generated rather than listed: movie_editor/server.py alone uses get/post/put/patch/
+        delete, and a missing verb fails at IMPORT time (the decorators run at module scope),
+        which reads as a mysterious collection error rather than a missing stub.
+        """
+
+        def __init__(self):
+            self.registered = []
+
+        def _register(self, method, path):
+            def decorator(fn):
+                self.registered.append((method, path, fn))
+                return fn
+            return decorator
+
+        def __getattr__(self, method):
+            if method.startswith("_"):
+                raise AttributeError(method)
+            if method in ("static", "view"):
+                return lambda *_a, **_k: None
+            return lambda path, **_kw: self._register(method.upper(), path)
+
+    class _PromptServer:
+        instance = None
+
+        def __init__(self):
+            self.routes = _Routes()
+            self.app = types.SimpleNamespace(router=self.routes)
+
+        def send_sync(self, *_a, **_k):
+            return None
+
+    _PromptServer.instance = _PromptServer()
+    install_module("server", PromptServer=_PromptServer)
+
+
+def _install_comfy():
+    """The comfy package tree the suite imports, wired parent-to-child.
+
+    Defaults are deliberately inert — a test that needs real behaviour installs it (see the
+    module docstring). What matters here is that the tree EXISTS and is consistent, so no
+    file can lose a race to register it.
+    """
+    # NOT installed here: comfy.context_windows and comfy.patcher_extension. The sampler
+    # feature-detects both, and their tests swap whole modules in per test — pre-creating
+    # them would leave a parent attribute that shadows the swap (the same resolution rule
+    # this file exists to fix, pointed the other way).
+    install_module("comfy")
+    install_module("comfy.k_diffusion")
+    install_module("comfy.k_diffusion.sampling")
+    install_module("comfy.model_sampling")
+    install_module("comfy.samplers")
+    install_module("comfy.ldm")
+    install_module("comfy.ldm.modules")
+    install_module("comfy.ldm.modules.attention")
+    install_module("comfy.nested_tensor", NestedTensor=object)
+    install_module(
+        "comfy.sample",
+        prepare_noise=lambda samples, seed, noise_inds=None: None,
+        sample_custom=lambda *a, **k: None,
+    )
+    install_module(
+        "comfy.utils",
+        ProgressBar=lambda total: types.SimpleNamespace(update_absolute=lambda *a, **k: None),
+        common_upscale=lambda samples, width, height, method, crop: samples,
+    )
+    install_module(
+        "comfy.model_management",
+        get_torch_device=lambda: "cpu",
+        intermediate_device=lambda: "cpu",
+    )
+
+
+def install_all():
+    _install_folder_paths()
+    _install_server()
+    _install_comfy()

tests/conftest.py

@@ -1,4 +1,10 @@
-import sys
-import types
+"""Install the ComfyUI stand-ins before any test module imports.
 
-sys.modules.setdefault("folder_paths", types.SimpleNamespace(models_dir=""))
+The stubs themselves live in `_comfy_stubs.py` rather than here: test modules need
+`install_module` too, and `import conftest` is ambiguous the moment a second conftest.py
+exists in the run (movie_editor/tests has one), so they import a uniquely-named module
+instead. See `_comfy_stubs` for what is stubbed and why.
+"""
+import _comfy_stubs
+
+_comfy_stubs.install_all()

tests/test_mask_safe_attention.py

@@ -13,37 +13,34 @@ import sys
 import types
 from pathlib import Path
 
-sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
+import _comfy_stubs
 
-# Minimal comfy stubs so `import samplers` works without a full ComfyUI env.
-for _name in (
-    "comfy", "comfy.k_diffusion", "comfy.k_diffusion.sampling",
-    "comfy.model_sampling", "comfy.nested_tensor", "comfy.sample",
-    "comfy.samplers", "comfy.utils",
-):
-    sys.modules.setdefault(_name, types.ModuleType(_name))
-sys.modules["comfy.nested_tensor"].NestedTensor = object
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
 
 import samplers  # noqa: E402
 
 
 def _install_attn_module(*, backend="xformers", with_pytorch_sentinel=True):
-    """Register a fake comfy.ldm.modules.attention with selectable active backend."""
-    am = types.ModuleType("comfy.ldm.modules.attention")
+    """Point comfy.ldm.modules.attention at a fake with a selectable active backend.
 
+    Installed through _comfy_stubs.install_module so the PARENT attribute moves too. Registering
+    only the sys.modules entry is not enough: `import comfy.ldm.modules.attention as _am`
+    (what samplers.py does) resolves through `comfy.ldm.modules.attention` the attribute, so
+    a stub left behind by another test module kept winning here and the install under test
+    bailed out at its first guard.
+    """
     def attention_xformers(*a, **kw):
         return "xformers"
 
     def attention_pytorch(*a, **kw):
         return "pytorch"
 
-    am.attention_xformers = attention_xformers
-    am.attention_pytorch = attention_pytorch
-    am.optimized_attention = attention_xformers if backend == "xformers" else attention_pytorch
-    for parent in ("comfy", "comfy.ldm", "comfy.ldm.modules", "comfy.ldm.modules.attention"):
-        sys.modules.setdefault(parent, types.ModuleType(parent))
-    sys.modules["comfy.ldm.modules.attention"] = am
-    return am
+    return _comfy_stubs.install_module(
+        "comfy.ldm.modules.attention",
+        attention_xformers=attention_xformers,
+        attention_pytorch=attention_pytorch,
+        optimized_attention=attention_xformers if backend == "xformers" else attention_pytorch,
+    )
 
 
 def _fake_model():

tests/test_scene_chain_sampler.py

@@ -1,10 +1,11 @@
 import sys
-import types
 from pathlib import Path
 
 import pytest
 import torch
 
+import _comfy_stubs
+
 sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
 
 
@@ -86,16 +87,6 @@ def _sample_snapshot(value):
     return value.detach().clone()
 
 
-comfy_mod = types.ModuleType("comfy")
-comfy_kd_mod = types.ModuleType("comfy.k_diffusion")
-comfy_kd_sampling_mod = types.ModuleType("comfy.k_diffusion.sampling")
-comfy_model_sampling_mod = types.ModuleType("comfy.model_sampling")
-comfy_nested_mod = types.ModuleType("comfy.nested_tensor")
-comfy_sample_mod = types.ModuleType("comfy.sample")
-comfy_samplers_mod = types.ModuleType("comfy.samplers")
-comfy_utils_mod = types.ModuleType("comfy.utils")
-
-
 class FakeProgressBar:
     """Records what the sampler declared as the run's total step count."""
     last = None
@@ -109,27 +100,36 @@ class FakeProgressBar:
         self.value = int(value)
 
 
-comfy_utils_mod.ProgressBar = FakeProgressBar
-comfy_nested_mod.NestedTensor = FakeNestedTensor
-comfy_sample_mod.prepare_noise = fake_prepare_noise
-comfy_sample_mod.sample_custom = fake_sample_custom
+# These fakes ARE the test: a full sample() run is driven through them. They attach to the
+# shared comfy stubs conftest built rather than registering modules of their own — the old
+# `sys.modules.setdefault(...)` lost the race whenever another test module imported first,
+# and every test here then died on a missing attribute in a full-suite run while passing
+# alone. Only this file and test_minimax_h3_sampler.py touch the sampling entry points, and
+# that one installs inert lambdas, so owning them for the session is safe.
+_comfy_stubs.install_module("comfy.utils", ProgressBar=FakeProgressBar)
+_comfy_stubs.install_module("comfy.nested_tensor", NestedTensor=FakeNestedTensor)
+_comfy_stubs.install_module("comfy.sample",
+                        prepare_noise=fake_prepare_noise, sample_custom=fake_sample_custom)
 
-comfy_mod.k_diffusion = comfy_kd_mod
-comfy_kd_mod.sampling = comfy_kd_sampling_mod
-comfy_mod.model_sampling = comfy_model_sampling_mod
-comfy_mod.nested_tensor = comfy_nested_mod
-comfy_mod.sample = comfy_sample_mod
-comfy_mod.samplers = comfy_samplers_mod
-comfy_mod.utils = comfy_utils_mod
 
-sys.modules.setdefault("comfy", comfy_mod)
-sys.modules.setdefault("comfy.k_diffusion", comfy_kd_mod)
-sys.modules.setdefault("comfy.k_diffusion.sampling", comfy_kd_sampling_mod)
-sys.modules.setdefault("comfy.model_sampling", comfy_model_sampling_mod)
-sys.modules.setdefault("comfy.nested_tensor", comfy_nested_mod)
-sys.modules.setdefault("comfy.sample", comfy_sample_mod)
-sys.modules.setdefault("comfy.samplers", comfy_samplers_mod)
-sys.modules.setdefault("comfy.utils", comfy_utils_mod)
+@pytest.fixture(autouse=True)
+def _own_comfy_stubs(monkeypatch):
+    """Re-pin this file's fakes around every test.
+
+    Module-level installation is not enough: several other test modules assign
+    `comfy.nested_tensor.NestedTensor = object` on the same shared module at COLLECTION
+    time, so whichever is collected last wins and a run() here builds `object()` latents.
+    Re-pinning per test makes these assertions about the sampler instead of about
+    collection order."""
+    monkeypatch.setattr(sys.modules["comfy.nested_tensor"], "NestedTensor",
+                        FakeNestedTensor, raising=False)
+    monkeypatch.setattr(sys.modules["comfy.utils"], "ProgressBar",
+                        FakeProgressBar, raising=False)
+    monkeypatch.setattr(sys.modules["comfy.sample"], "prepare_noise",
+                        fake_prepare_noise, raising=False)
+    monkeypatch.setattr(sys.modules["comfy.sample"], "sample_custom",
+                        fake_sample_custom, raising=False)
+
 
 import run_phase  # noqa: E402
 from samplers import FunPackLTXAVSceneChainSampler  # noqa: E402

tests/test_v2_category_alignment.py

@@ -236,12 +236,22 @@ def test_awful_lora_feedback_reduces_before_missing_axis_boosts():
     assert suggestion["model_weight"] < 1.0
 
 
+# Ratings that deliberately do NOT train phrase memory, by profile key rather than by
+# label text — the label list grows (the "|loved" variants, the control sentinels) and a
+# hardcoded exclusion list silently starts asserting the wrong thing about the new ones.
+#   forget / continue / fresh_prompt — control actions, not ratings at all
+#   wrong_appearance                 — a character-consistency signal, handled by the
+#                                      appearance anchor, never by phrase categories
+NON_LEARNING_RATING_KEYS = {"forget", "wrong_appearance", "continue", "fresh_prompt"}
+
+
 def test_category_weights_are_recorded_for_every_learning_rating():
     refiner = FunPackVideoRefinerV2()
     learning_labels = [
         label for label in V2_RATING_LABELS
-        if label not in {"-Just forget it-", "Wrong appearance"}
+        if normalize_refiner_v2_rating(label).get("key") not in NON_LEARNING_RATING_KEYS
     ]
+    assert len(learning_labels) > 10   # the filter must not quietly empty the loop
 
     for label in learning_labels:
         entry, _, _ = train_phrase(refiner, f"running test {label}", label)
@@ -1781,8 +1791,11 @@ def test_split_by_transitions_shows_scenes_in_encoded_prompts(tmp_path):
     state_path = tmp_path / "state.json"
     refiner._v2_state_path = lambda refinement_key: str(state_path)
 
+    # `scene <N>` is the canonical split delimiter (split_scenes): a real transition trigger
+    # from the user's DB, or this generic label. Word-numbers ("scene ten") and bare prose
+    # ("cut to") deliberately do NOT cut — an incidental phrase must never split a prompt.
     cond, status, _, _, encoded_prompts, _, _ = refiner.refine_v2(
-        "a woman in a red dress, scene ten she runs, cut to she stops",
+        "a woman in a red dress, scene 1 she runs, scene 2 she stops",
         FakeClip(),
         "Perfect",
         "transition-test",
@@ -1792,9 +1805,15 @@ def test_split_by_transitions_shows_scenes_in_encoded_prompts(tmp_path):
     assert len(cond) == 2
     assert cond[0][1]["funpack_scene_index"] == 0
     assert cond[0][1]["funpack_scene_count"] == 2
+    # the leading run is the anchor and folds into every scene; each scene keeps its own text
     assert "a woman in a red dress" in cond[0][1]["funpack_scene_text"]
-    assert "scene ten she runs" in cond[0][1]["funpack_scene_text"]
+    assert "a woman in a red dress" in cond[1][1]["funpack_scene_text"]
+    assert "she runs" in cond[0][1]["funpack_scene_text"]
     assert "she stops" in cond[1][1]["funpack_scene_text"]
+    assert "she stops" not in cond[0][1]["funpack_scene_text"]
+    # the delimiter is a boundary, never content — encoding it would pollute the prompt
+    assert "scene 1" not in cond[0][1]["funpack_scene_text"]
+    assert "scene 2" not in cond[1][1]["funpack_scene_text"]
     assert "Scene chain mode" in status
     assert "Transition split" in status
     assert "Detected scenes" in encoded_prompts
@@ -2049,7 +2068,7 @@ def test_split_by_transitions_attaches_scene_seeds(tmp_path):
     refiner._v2_state_path = lambda refinement_key: str(state_path)
 
     cond, _, _, _, _, _, _ = refiner.refine_v2(
-        "anchor, scene one she walks, cut to she turns",
+        "anchor, scene 1 she walks, scene 2 she turns",
         FakeClip(),
         "Perfect",
         "split-seed-test",
@@ -2068,7 +2087,7 @@ def test_split_by_transitions_uses_provided_scene_seeds(tmp_path):
     refiner._v2_state_path = lambda refinement_key: str(state_path)
 
     cond, _, _, _, _, _, _ = refiner.refine_v2(
-        "anchor, scene one she walks, cut to she turns",
+        "anchor, scene 1 she walks, scene 2 she turns",
         FakeClip(),
         "Perfect",
         "split-seed-test",