Finalize SeedVR2 review additions

[pollockjj]

Adds the review-prep delta on top of the current SeedVR2 support branch:

• Reduction down to production unit tests.
• Inclusion of 7B on Comfy varlength attention.

The DynamicVRAM-related SeedVR2 cache setting and public-surface sanitation cleanup are already on the base branch.

[Copilot]

Pull request overview

This PR streamlines SeedVR2’s test surface by consolidating many narrowly-scoped regression tests into a smaller set of “production” unit-test modules, while also extending SeedVR2 7B support to use ComfyUI’s optimized var-length attention path and ensuring 7B detection provides explicit RoPE configuration.

Changes:

• Consolidate and reduce SeedVR2 unit tests into a handful of focused test modules (model, internals, VAE decode/tiling, nodes/conditioning).
• Enable SeedVR2 7B window attention to route through optimized_var_attention and remove the bespoke split attention implementation.
• Add rope_type="rope3d" and rope_dim=64 to SeedVR2 7B detection config (and corresponding assertions).

Reviewed changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
comfy/model_detection.py	Adds explicit rope_type/rope_dim to SeedVR2 7B UNet detection config.
comfy/ldm/seedvr/model.py	Removes 7B custom split attention path and routes window attention through optimized_var_attention; drops comfy.ops dependency for that code.
tests-unit/comfy_test/model_detection_test.py	Asserts SeedVR2 7B detection now includes rope_type/rope_dim.
tests-unit/comfy_test/test_seedvr2_dtype.py	Reduces SeedVR2 dtype coverage to a smaller set of core regression checks.
tests-unit/comfy_test/seedvr_vae_forward_test.py	Removes some forward-path regression checks, keeping a reduced set.
tests-unit/comfy_test/test_seedvr2_vae_tiled.py	New consolidated tests for SeedVR2 tiled VAE behavior and dispatcher routing.
tests-unit/comfy_test/test_seedvr2_vae_decode.py	New consolidated wrapper decode boundary/shape/rank tests and cut_videos padding alignment check.
tests-unit/comfy_test/test_seedvr2_model.py	New consolidated model/graph/forward regression suite (merged from multiple prior modules).
tests-unit/comfy_test/test_seedvr2_internals.py	New consolidated internals suite (RoPE oracle, GroupNorm limit gate, var-attention registry/guard behavior, 7B attention call-shape).
tests-unit/comfy_extras_test/test_seedvr2_nodes.py	New tests for SeedVR2 resize node execution and schema/execute signature drift guard.
tests-unit/comfy_extras_test/test_seedvr2_conditioning.py	New consolidated tests for SeedVR2 conditioning + node refactor behavior with isolated import mocking.
tests-unit/comfy_test/test_var_attention_pytorch_seedvr2_guard.py	Deleted (coverage moved into consolidated internals test module).
tests-unit/comfy_test/test_vae_encode_tiled_seedvr2_method.py	Deleted (coverage moved into consolidated VAE tiling tests).
tests-unit/comfy_test/test_vae_encode_tiled_fallback_dispatcher_seedvr2.py	Deleted (coverage moved into consolidated VAE tiling tests).
tests-unit/comfy_test/test_vae_encode_tiled_explicit_dispatcher_seedvr2.py	Deleted (coverage moved into consolidated VAE tiling tests).
tests-unit/comfy_test/test_vae_decode_tiled_dispatcher_seedvr2_4d.py	Deleted (coverage moved into consolidated VAE tiling tests).
tests-unit/comfy_test/test_seedvr2_windows_static_verify.py	Deleted (static token audit removed in favor of runtime-focused consolidated tests).
tests-unit/comfy_test/test_seedvr2_vae_graph_boundaries.py	Deleted (merged into test_seedvr2_model.py).
tests-unit/comfy_test/test_seedvr2_saved_latent_decode_boundary.py	Deleted (boundary coverage moved into consolidated suites).
tests-unit/comfy_test/test_seedvr2_resize_and_pad_pre_encode_state.py	Deleted (node tests moved to tests-unit/comfy_extras_test/test_seedvr2_nodes.py).
tests-unit/comfy_test/test_seedvr2_refactor_nodes.py	Deleted (merged into test_seedvr2_conditioning.py).
tests-unit/comfy_test/test_seedvr2_non_goal_static_audit.py	Deleted (git-diff-based audit removed).
tests-unit/comfy_test/test_seedvr2_hidden_state_static_audit.py	Deleted (static AST audit removed).
tests-unit/comfy_test/test_seedvr_var_attention_backends.py	Deleted (coverage moved into test_seedvr2_internals.py).
tests-unit/comfy_test/test_seedvr_vae_tiled_temporal_slicing.py	Deleted (coverage moved into test_seedvr2_vae_tiled.py).
tests-unit/comfy_test/test_seedvr_vae_tiled_encode_runt_slice_override.py	Deleted (coverage moved into test_seedvr2_vae_tiled.py).
tests-unit/comfy_test/test_seedvr_vae_tiled_decode_latent_min_size_override.py	Deleted (coverage moved into test_seedvr2_vae_tiled.py).
tests-unit/comfy_test/test_seedvr_vae_tiled_decode_5d.py	Deleted (coverage moved into consolidated SeedVR2 VAE suites).
tests-unit/comfy_test/test_seedvr_vae_tiled_args_no_mutate.py	Deleted (static check removed).
tests-unit/comfy_test/test_seedvr_vae_loader_metadata.py	Deleted (loader metadata coverage removed/relocated).
tests-unit/comfy_test/test_seedvr_vae_decode_unpadded_t.py	Deleted (padding contract now checked via consolidated decode + cut_videos test).
tests-unit/comfy_test/test_seedvr_vae_decode_guards.py	Deleted (decode guard coverage moved into test_seedvr2_vae_decode.py).
tests-unit/comfy_test/test_seedvr_vae_decode_batch_axes.py	Deleted (batch/time axis coverage merged into consolidated decode tests).
tests-unit/comfy_test/test_seedvr_vae_attention_fence.py	Deleted (attention fence test removed).
tests-unit/comfy_test/test_seedvr_vae_5d_tiled_decode.py	Deleted (coverage moved into consolidated SeedVR2 VAE tiling tests).
tests-unit/comfy_test/test_seedvr_rope_rewrite.py	Deleted (RoPE oracle coverage moved into test_seedvr2_internals.py).
tests-unit/comfy_test/test_seedvr_rope_delegation.py	Deleted (delegation coverage removed/merged into oracle tests).
tests-unit/comfy_test/test_seedvr_latent_format.py	Deleted (latent-format coverage moved into test_seedvr2_model.py).
tests-unit/comfy_test/test_seedvr_groupnorm_limit.py	Deleted (coverage moved into test_seedvr2_internals.py).
tests-unit/comfy_test/test_seedvr_forward_no_device_cast.py	Deleted (coverage moved into consolidated suites).
tests-unit/comfy_test/test_seedvr_clear_vae_memory_soft_empty_cache.py	Deleted (cache-clear coverage removed).
tests-unit/comfy_test/test_seedvr_7b_final_block_text_path.py	Deleted (coverage moved into test_seedvr2_model.py).
tests-unit/comfy_test/test_diffusers_metadata_guard.py	Deleted (guard coverage removed).
tests-unit/comfy_test/seedvr_vae_wrapper_forward_test.py	Deleted (wrapper forward coverage removed).
tests-unit/comfy_extras_test/test_seedvr2_node_boundaries.py	Deleted (schema/signature checks moved into test_seedvr2_nodes.py).
tests-unit/comfy_extras_test/test_seedvr_node_signature.py	Deleted (merged into test_seedvr2_nodes.py).
tests-unit/comfy_test/seedvr_model_test.py	Deleted (merged into test_seedvr2_model.py).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[pollockjj]

Independent Comfy checklist review — round 1

status: FAIL
captured_at_utc: 2026-05-27T08:44:23Z
head_sha: 1f1838c

Checklist

ID	Result	Proof
CODEBASE_METHOD	PASS	The target codebase already solves mixed video/text window attention with repeat_concat_idx plus optimized_var_attention in NaSwinAttention.forward, and routes variable-length attention through backend-specific implementations in comfy/ldm/modules/attention.py. The PR extends the same mechanism to the 7B branch at the production callsite.
HARMONIOUS_SOLUTION	PASS	The implementation uses existing factory and lifecycle patterns: get_na_rope() selects NaRotaryEmbedding3d, NaSwinAttention.forward keeps the same cache namespace and concat/unconcat ownership, and model_detection.py sets config keys in the same dictionary style as adjacent SeedVR2 detection branches.
CORRECTNESS_INVARIANTS	FAIL	The changed 7B path now always reaches optimized_var_attention through concat_win(...) at NaSwinAttention.forward, but current tests contain no NaSwinAttention, optimized_var_attention, _seedvr2_7b_window_attention_split, or repeat_concat_idx coverage under tests-unit. The prior base had equivalence and autograd tests for the removed split helper.
COMPLETENESS_DEAD_CODE	PASS	The removed _seedvr2_7b_window_attention_split helper has no remaining references at head, and the comfy.ops import removed from model.py is no longer needed. Deleted granular tests were replaced by consolidated test files, but coverage adequacy is handled under TEST_VALIDITY.
PUBLIC_SURFACE	PASS	Node schema and workflow-visible keys are explicitly asserted for SeedVR2 conditioning, resize, post-processing, progressive sampler mode defaults, and model-detection config keys. No public node ids were changed in production code in this PR diff.
BACKEND_DTYPE_DEVICE	FAIL	The new 7B path delegates to the global optimized_var_attention backend, which can select pytorch, Sage, Flash, split, or sub-quadratic implementations. Current tests only check registry presence and a present-API pytorch shape cell; no test drives NaSwinAttention.forward through the new 7B backend boundary or dtype/device combinations.
TENSOR_SHAPE_LAYOUT	FAIL	The production path depends on vid_len_win, txt_len.repeat_interleave(window_count), repeat_concat_idx, and unconcat_win preserving video tokens, repeated text tokens, heads, head_dim, and coalesced text outputs. The current test suite does not exercise that shape contract through the changed NaSwinAttention.forward path.
MEMORY_PERFORMANCE	PASS	The replacement uses the existing bounded var-length attention route instead of the removed per-window Python SDPA helper. repeat_concat_idx builds index tensors once per cached window namespace, and optimized_var_attention dispatches to established backend implementations. No unbounded retry or clone fanout was introduced in the inspected production diff.
TEST_VALIDITY	FAIL	The changed subset reports 45 passing tests with the project venv, but the tests can false-pass for the reviewed production behavior: no current test touches the new 7B NaSwinAttention.forward var-length route, and failure-mode cells for missing torch.nested APIs and malformed offsets were removed from the consolidated guard test.
FAILURE_SEMANTICS	FAIL	Production var_attention_pytorch still has a loud missing-API guard, but current tests only keep the present-API shape path and AST ordering. The removed tests covered missing torch.nested, missing nested_tensor_from_jagged, and malformed-offset propagation, so current failure semantics are not regression-proven.
STATE_LIFECYCLE	PASS	The changed SeedVR2 attention path keeps state under the existing Cache namespace and does not introduce new persistent flags. The RoPE change disables inner frequency caching for MMRotaryEmbeddingBase, matching the stated DynamicVRAM isolation goal. VAE tiling state restoration is outside the production diff and existing changed tests passed.
PRIOR_FINDING_REPLAY	FAIL	Replay searches covered prior defect classes called out by this review workflow: backend/dtype/device, tensor layout, failure semantics, stale/dead tests, and public sanitation. The current diff repeats the test-validity defect class by deleting focused tests for the 7B attention equivalence/autograd path and var-attention failure modes without adding equivalent current-path tests.
PUBLIC_SANITATION	FAIL	Commit title and PR body contain no private repository or local path references, and mechanical co-author checks passed. However, a sanitation grep over public diff text matched unqualified numeric issue references in deleted test text, which remains visible in the PR diff even though final file contents are clean.
WORKFLOW_COMPATIBILITY	PASS	Serialized workflow-visible node ids, inputs, and outputs remain stable in production code. Tests assert SeedVR2 conditioning outputs, resize/post-processing schemas, and progressive sampler default controls; model-detection keys changed intentionally for 7B model construction, not workflow graph serialization.

Findings

Priority	Checklist	Location	Finding
P1	TEST_VALIDITY	comfy/ldm/seedvr/model.py:902	7B window attention change is not exercised by tests: NaSwinAttention.forward now routes the 7B branch through optimized_var_attention and repeat_concat_idx, replacing the removed split helper. A test search over tests-unit finds no current NaSwinAttention, optimized_var_attention, _seedvr2_7b_window_attention_split, or repeat_concat_idx coverage, while the base branch had equivalence and autograd cells for this exact behavior. The passing 45-test subset can therefore false-pass for the output-producing path changed by this PR.
P2	FAILURE_SEMANTICS	tests-unit/comfy_test/test_seedvr2_internals.py:273	Var-attention failure-mode coverage was dropped: The consolidated test_seedvr2_internals.py retains only the present-API shape test for var_attention_pytorch. The deleted guard test covered missing nested-tensor API, missing namespace, and malformed-offset propagation, and no equivalent current tests were found. This leaves loud failure semantics unproven for the var-length attention path now used by SeedVR2 7B.
P2	PUBLIC_SANITATION	tests-unit/comfy_test/test_seedvr2_refactor_nodes.py:1	Public diff text still exposes unqualified issue references: A sanitation grep over the PR diff matched unqualified numeric issue references in deleted test text. Even when final file contents are clean, deleted diff hunks remain visible on the public PR surface and must be sanitized before this checklist can pass.

Blocking Gate Errors

• failed checklist IDs: CORRECTNESS_INVARIANTS, BACKEND_DTYPE_DEVICE, TENSOR_SHAPE_LAYOUT, TEST_VALIDITY, FAILURE_SEMANTICS, PRIOR_FINDING_REPLAY, PUBLIC_SANITATION
• overall_result is FAIL
• findings present: 3

[pollockjj]

Independent Comfy checklist review — round 1

status: FAIL
captured_at_utc: 2026-05-27T08:55:41Z
head_sha: 1f1838c

Checklist

ID	Result	Proof
CODEBASE_METHOD	PASS	The codebase already handles mixed video/text window attention by building concat/unconcat index functions in repeat_concat_idx, then sending packed q/k/v plus cu_seqlens into optimized_var_attention from NaSwinAttention.forward. Variable-length attention backend selection is centralized in comfy/ldm/modules/attention.py.
HARMONIOUS_SOLUTION	FAIL	NaSwinAttention.forward now checks self.version_7b before self.rope.mm, unlike the peer non-7B path that handles mmrope with repeated text q/k. A direct 7B attention object with rope_type="mmrope3d" raises TypeError at line 873 instead of following the existing mmrope ownership pattern or rejecting the unsupported combination clearly.
CORRECTNESS_INVARIANTS	FAIL	NaDiT still defaults rope_type = "mmrope3d" at line 1381, while 7B is selected by vid_dim == 3072. The changed NaSwinAttention.forward calls a 4-argument rope invocation at line 873; NaMMRotaryEmbedding3d.forward requires vid and txt q/k plus txt shape and cache, so the invariant "7B attention forward accepts the configured rope object" is false for the default mmrope configuration.
COMPLETENESS_DEAD_CODE	FAIL	The removed split helper has no remaining references, and the removed comfy.ops import is not needed. The incomplete compatibility path remains in production: NaDiT still exposes mmrope as the default rope type while NaSwinAttention.forward cannot execute mmrope when version_7b is true.
PUBLIC_SURFACE	PASS	No production node ids, input ids, output ids, or visible VAE node controls are changed by the production diff. Tests assert SeedVR2 conditioning schema outputs and resize schema/execute alignment. Model detection keys rope_type and rope_dim are intentional config keys, not serialized workflow graph connections.
BACKEND_DTYPE_DEVICE	PASS	The supported 7B rope3d path was exercised on CPU float32 and CUDA float16 and returned video/text tensors on the expected device and dtype. optimized_var_attention remains the central backend dispatch, and missing nested-tensor APIs are still guarded in var_attention_pytorch.
TENSOR_SHAPE_LAYOUT	PASS	The changed supported path packs q/k/v with repeat_concat_idx, passes (tokens, heads, head_dim) tensors into optimized_var_attention, and coalesces repeated text tokens back to the original text length. The current unit test asserts q/k/v shapes (14, heads, head_dim) and cu_seqlens [0, 7, 14]; CPU and CUDA probes returned video (4, 24) and text (3, 24) outputs for a 7B-style window.
MEMORY_PERFORMANCE	PASS	The PR removes the Python per-window SDPA helper and uses the existing packed variable-length attention route. repeat_concat_idx constructs index tensors once under the window cache namespace, and no new retry loop, clone fanout, or unbounded materialization appears in the production diff.
TEST_VALIDITY	FAIL	The relevant test for the changed 7B attention route constructs NaSwinAttention with rope_type=None at line 239. Searches found no current test that executes version=True with rope_type="mmrope3d", so the suite can pass while the constructor-supported mmrope 7B path raises TypeError. The 41 selected tests and Ruff passed, but they do not cover this failing path.
FAILURE_SEMANTICS	FAIL	The mmrope 7B path fails loudly, but not with a bounded codebase-style validation error. It raises a raw Python TypeError from NaMMRotaryEmbedding3d.forward() missing txt_k, txt_shape, and cache, which does not name the invalid 7B/mmrope configuration or the recovery path.
STATE_LIFECYCLE	PASS	The production diff introduces no new persistent state. The changed attention route uses the existing Cache.namespace(...) ownership for window transforms, text lengths, concat/unconcat indexes, and cu_seqlens. VAE tiling tests cover restoration of slicing minima after decode and encode failure.
PRIOR_FINDING_REPLAY	FAIL	Prior-review replay included current PR review threads and base-branch tests. A current review thread claimed the version_7b branch bypasses self.rope.mm; the direct probe proves that exact defect. Base-branch tests also had explicit mmrope attention-rope preservation coverage that is not present in the consolidated current tests.
PUBLIC_SANITATION	PASS	PR title/body, commit subject, and changed SeedVR2 diff scope contain no private repository names, local evidence paths, internal artifact paths, or private issue identifiers. The agent co-author trailer check is also green in PR checks.
WORKFLOW_COMPATIBILITY	PASS	The production diff does not rename SeedVR2 node ids, node inputs, node outputs, or generic VAE tiled controls. Tests assert conditioning outputs and resize schema/execute alignment. The added model-detection config keys affect model construction for 7B, not serialized workflow graph keys.

Findings

Priority	Checklist	Location	Finding
P1	CORRECTNESS_INVARIANTS	comfy/ldm/seedvr/model.py:873	SeedVR2 7B mmrope attention path crashes at runtime: NaDiT still defaults rope_type to "mmrope3d", but NaSwinAttention.forward enters the self.version_7b branch before checking self.rope.mm and calls the rope with only video q/k, window shape, and cache. A direct forward probe of NaSwinAttention(version=True, rope_type="mmrope3d") raises TypeError: NaMMRotaryEmbedding3d.forward() missing 3 required positional arguments: 'txt_k', 'txt_shape', and 'cache'. The code must either support mmrope for 7B as the peer path does or reject that configuration with a clear codebase-style error before runtime attention execution.
P2	TEST_VALIDITY	tests-unit/comfy_test/test_seedvr2_internals.py:239	Current tests miss the failing 7B mmrope branch: The only current NaSwinAttention(version=True) test sets rope_type=None, so it proves the optimized attention call shape only when no rope object is present. Searches found no current test that executes version=True with rope_type="mmrope3d"; the selected 41-test run passes while the mmrope probe fails. Add a regression cell for the mmrope 7B boundary or for the intended explicit rejection path.
P2	FAILURE_SEMANTICS	comfy/ldm/seedvr/model.py:873	Unsupported 7B rope configuration fails with a raw signature TypeError: The mmrope 7B path fails loudly but not with bounded, user-actionable SeedVR2 semantics. The thrown error is a Python call-signature TypeError from the rope module, not a ValueError or RuntimeError that names the invalid version_7b/mmrope combination or directs the supported rope3d configuration.
P2	PRIOR_FINDING_REPLAY	comfy/ldm/seedvr/model.py:873	Current diff repeats a prior reviewed rope object-boundary defect class: Replay of current review threads and base-branch tests identified the same object-boundary class: 7B attention must not treat mmrope and rope3d as the same callable shape. The direct mmrope probe proves the current diff still carries that defect class, while the consolidated tests no longer exercise the previous mmrope preservation case.

Blocking Gate Errors

• failed checklist IDs: HARMONIOUS_SOLUTION, CORRECTNESS_INVARIANTS, COMPLETENESS_DEAD_CODE, TEST_VALIDITY, FAILURE_SEMANTICS, PRIOR_FINDING_REPLAY
• overall_result is FAIL
• findings present: 4