[Issue 101][eval-and-fix] Add SeedVR2 decode state guards (closes pollockjj/mydevelopment#194)#37
Conversation
…lformed state (closes pollockjj/mydevelopment#194) VideoAutoencoderKLWrapper.decode() previously accessed self.original_image_video and self.img_dims without validation, raising opaque torch errors deep in rearrange() / attribute unpacking when a workflow wired the wrapper directly without SeedVR2InputProcessing populating those attributes. CodeRabbit thread r2962219551 on upstream PR Comfy-Org#11294 flagged this as Minor. Fix: 1. __init__: initialise self.img_dims = None so the missing-state branch is reachable from a default-constructed instance (matches the pre-existing self.original_image_video = None initialiser). 2. decode() top: four ordered guards raising RuntimeError with SeedVR2-specific messages — presence of original_image_video, 5-D rank check on original_image_video, presence of img_dims, 2-arity check on img_dims. Each message names the offending attribute and includes "SeedVR2" so misuse converts to an actionable signal instead of an opaque torch failure. Regression cover: tests-unit/comfy_test/test_seedvr_vae_decode_guards.py (7 cells across 5 AC functions) using the __new__ + nn.Module.__init__ standin pattern from Comfy-Org#189 and the _PostGuardReached halt-point sentinel pattern from Comfy-Org#119; AC5 patches VideoAutoencoderKL.decode_ to confirm the guards passed and control crossed into the post-guard region without standing up the heavy decode pipeline.
…ct node ids + add AC6 Slice 1 rework for pollockjj/mydevelopment#194: align test node ids with the qa-slice contract and add AC5/AC6 source-position invariants. - AC1-AC4: rename the four wrapper-failure tests to test_none_original_image_video_raises_seedvr2_runtime_error, test_none_img_dims_raises_seedvr2_runtime_error, test_wrong_rank_original_image_video_raises_seedvr2_runtime_error, test_wrong_arity_img_dims_raises_seedvr2_runtime_error and switch each to pytest.raises(RuntimeError, match=r"SeedVR2.*<attr>") so the regex matcher both pins the message shape and discriminates the wrong exception class on a pre-fix worktree. - AC5: replace the post-guard sentinel with the contract spelling — test_valid_state_passes_through_guards_to_tiled_vae now patches the module-level comfy.ldm.seedvr.vae.tiled_vae, asserts mock_tiled.call_count == 1 (proves valid input flowed past the guards into the post-guard region) and asserts inspect.getsource(VideoAutoencoderKLWrapper.decode) contains "raise RuntimeError(" before "b, tc, h, w = z.shape" (proves the guards are placed at the top of decode() ahead of the existing first body line). - AC6: add test_init_initializes_img_dims_to_none asserting inspect.getsource(VideoAutoencoderKLWrapper.__init__) contains the literal "self.img_dims = None" so the missing-state branch is reachable from a default-constructed instance — a substring the unguarded base SHA does not carry. - Add a _bypass_super_decode autouse-style fixture for AC1-AC4 that patches VideoAutoencoderKL.decode_ to return a synthesised 5-D tensor. Without it, the standin (built via __new__ to skip the heavy real __init__) lacks parent-class attributes like use_slicing, so on a pre-fix worktree the unguarded decode() body would die with an AttributeError before reaching the rearrange(self.original_image_video, ...) / o_h, o_w = self.img_dims sites the ACs target. With the fixture in place, pre-fix runs surface the contract-described einops / TypeError / ValueError / AttributeError fingerprints; post-fix runs short-circuit at the guard raises and never invoke the stub.
Codex Review — Cycle 1Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Adds defensive state validation to SeedVR2’s VideoAutoencoderKLWrapper.decode() so workflows that call decode() without the required SeedVR2 producers fail fast with a SeedVR2-context RuntimeError, and introduces a focused unit test module that locks the guard behavior (including a “valid state passes through” sentinel).
Changes:
- Initialize
VideoAutoencoderKLWrapper.img_dimstoNonein__init__to make the “unset” state explicit and guardable. - Add top-of-function guards in
VideoAutoencoderKLWrapper.decode()for missing/malformedoriginal_image_videoandimg_dims, raising SeedVR2-specificRuntimeErrors. - Add
tests-unit/comfy_test/test_seedvr_vae_decode_guards.pyto regression-test the guard matrix and verify guards occur before existing decode body logic.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
comfy/ldm/seedvr/vae.py |
Adds img_dims initialization and early decode() guards to convert opaque failures into SeedVR2-context RuntimeErrors. |
tests-unit/comfy_test/test_seedvr_vae_decode_guards.py |
New regression tests exercising guard failure modes and a valid-state “passes through to tiled_vae” sentinel. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self.original_image_video is None: | ||
| raise RuntimeError( | ||
| "SeedVR2 VideoAutoencoderKLWrapper.decode: `original_image_video` is None. " | ||
| "This attribute must be populated by SeedVR2InputProcessing.execute before " | ||
| "decode() is invoked; calling decode() directly without the SeedVR2 input " | ||
| "processing node is not supported." | ||
| ) | ||
| if self.original_image_video.ndim != 5: | ||
| raise RuntimeError( | ||
| "SeedVR2 VideoAutoencoderKLWrapper.decode: `original_image_video` must be a " | ||
| "5-D tensor of shape (B, C, T, H, W); got rank " | ||
| f"{self.original_image_video.ndim} with shape " | ||
| f"{tuple(self.original_image_video.shape)}." |
| if len(img_dims) != 2: | ||
| raise RuntimeError( | ||
| "SeedVR2 VideoAutoencoderKLWrapper.decode: `img_dims` must be a 2-tuple " | ||
| f"(H, W); got arity {len(img_dims)} with value {img_dims!r}." |
Codex Review — Cycle 2Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ``SeedVR2InputProcessing.execute`` populating those attributes. The merged | ||
| fix on ``pollockjj/ComfyUI:issue_194`` adds two presence guards plus | ||
| shape/arity validation at the top of ``decode()``, raising ``RuntimeError`` | ||
| with a SeedVR2-specific message identifying the missing or malformed | ||
| attribute, and initialises ``self.img_dims = None`` in ``__init__`` so the | ||
| missing-state branch is reachable from a default-constructed instance. |
| if not torch.is_tensor(self.original_image_video): | ||
| raise RuntimeError( | ||
| "SeedVR2 VideoAutoencoderKLWrapper.decode: `original_image_video` must be " | ||
| "a torch.Tensor; got type " | ||
| f"{type(self.original_image_video).__name__} with value " | ||
| f"{self.original_image_video!r}." | ||
| ) |
| raise RuntimeError( | ||
| "SeedVR2 VideoAutoencoderKLWrapper.decode: `img_dims` must be a 2-tuple " | ||
| f"(H, W); got arity {len(img_dims)} with value {img_dims!r}." | ||
| ) |
Codex Review — Cycle 3Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codex Review — Cycle 4Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codex Review — Cycle 5Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "SeedVR2 VideoAutoencoderKLWrapper.decode: `img_dims` must be a 2-tuple " | ||
| f"(H, W); got arity {len(img_dims)} with value {img_dims!r}." |
Codex Review — Cycle 6Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codex Review — Cycle 7Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "This attribute must be populated by encode(orig_dims=...) before decode() " | ||
| "is invoked." |
Codex Review — Cycle 37Overall: patch is incorrect No findings. |
Codex Review — Cycle 38Overall: patch is incorrect No findings. |
Codex Review — Cycle 39Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codex Review — Cycle 40Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codex Review — Cycle 41Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codex Review — Cycle 42Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| src = inspect.getsource(vae_mod.VideoAutoencoderKLWrapper.decode) | ||
| raise_pos = src.find("raise RuntimeError(") | ||
| shape_pos = src.find("b, tc, h, w = z.shape") | ||
| assert raise_pos != -1, ( | ||
| "AC5 (b): VideoAutoencoderKLWrapper.decode source must contain " | ||
| "`raise RuntimeError(` (the guard raise statement); not found.\n" | ||
| f"--- decode source ---\n{src}" | ||
| ) | ||
| assert shape_pos != -1, ( | ||
| "AC5 (b): VideoAutoencoderKLWrapper.decode source must contain " | ||
| "`b, tc, h, w = z.shape` (the existing first body line); not found.\n" | ||
| f"--- decode source ---\n{src}" | ||
| ) | ||
| assert raise_pos < shape_pos, ( | ||
| "AC5 (b): `raise RuntimeError(` must appear at a position BEFORE " | ||
| "`b, tc, h, w = z.shape` in VideoAutoencoderKLWrapper.decode source " | ||
| "(proving the guards are placed at the top of decode() ahead of " | ||
| f"the existing first body line); got raise_pos={raise_pos}, " | ||
| f"shape_pos={shape_pos}.\n--- decode source ---\n{src}" | ||
| ) |
Codex Review — Cycle 43Overall: patch is incorrect No findings. |
Codex Review — Cycle 44Overall: patch is incorrect No findings. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Closed as invalid — orchestrator parser bug compounded a phantom-blocker cycle loop on this PR. See pollockjj/melian a666ddd / 25e33f9 for the parser + signal-failure fixes. Branch issue_194 has been reset to original substantive commit. |
Plan: Guard SeedVR2 VideoAutoencoderKLWrapper.decode against unpopulated state
Overview
comfy.ldm.seedvr.vae.VideoAutoencoderKLWrapper.decodereadsself.original_image_videoandself.img_dimswithout validation. Both are populated only by external setters (SeedVR2InputProcessing.executeandencode(orig_dims=...)), so any workflow that wires the wrapper directly without those producers crashes insideeinops.rearrangeor with a bareAttributeError— no SeedVR2 context. This packet adds two presence guards plus shape/arity validation at the top ofdecode(), plus a one-line__init__change that initializesself.img_dims = Noneso the missing-state branch is reachable from a default-constructed instance. The regression module is a structural port of Comfy-Org#119's_make_standin+_PostGuardReachedsentinel pattern, exercising the five reachable input shapes (4 misuse cells + 1 valid cell).Diagnosis Summary
This is a PLAN-mode packet. The bug mechanism is fully documented in the issue body's Executive Summary and re-verified live by the Phase 0.5 probes captured in
github_issues/194/probes/results/against base SHA9c2b9c39. No investigation slice is needed; the fix shape is dictated by CodeRabbit threadr2962219551and the proven_make_standinprecedent from #109 / Comfy-Org#119.§Failure Signature
Five reachable cells, each captured live from the unguarded
decode()body onpollockjj/ComfyUI@9c2b9c39(comfy/ldm/seedvr/vae.py:2245-onward). Cell labels A1–A5 are referenced verbatim by ACPre-fix-fingerprint:sub-bullets.self.original_image_video is None,self.img_dims = (H, W)valid:decode(z)raisesRuntimeError('Tensor type unknown to einops <class \'NoneType\'>')fromeinops/_backends.py:62(get_backend(tensor)), called transitively fromcomfy/ldm/seedvr/vae.py:2273input = rearrange(self.original_image_video, "b c t h w -> (b t) c h w"). The exception message contains neitherSeedVR2nororiginal_image_video. Captured:github_issues/194/probes/results/gtp_03.txt(line 29).self.original_image_videois a valid 5-D tensor,self.img_dims is None(the realistic shape after the__init__change in this packet's §Proposed Fix initializes the attribute toNoneon every default-constructed instance):decode(z)raisesTypeError('cannot unpack non-iterable NoneType object')atcomfy/ldm/seedvr/vae.py:2285o_h, o_w = self.img_dims(Python tuple-unpacking onNone). The exception class isTypeError(notRuntimeError) and the message contains neitherSeedVR2norimg_dims. Captured: live Phase 0.5 dry-run reproducing TypeError directly. (Note: prior to the__init__change, the same misuse on a default-constructed instance raisesAttributeError("'<obj>' object has no attribute 'img_dims'")at the same line; this is the__init__-less precursor shape and is the alternative pre-fix fingerprint described informally below for AC-6's source-pin context.)self.original_image_videois a 3-D tensor,self.img_dims = (H, W)valid:decode(z)raiseseinops.EinopsError('Error while processing rearrange-reduction pattern "b c t h w -> (b t) c h w". Input tensor shape: torch.Size([3, 4, 4]). Additional info: {}. Wrong shape: expected 5 dims. Received 3-dim tensor.')from line 2273. The exception class iseinops.EinopsError(notRuntimeError) and the message contains neitherSeedVR2nororiginal_image_video.self.original_image_videovalid 5-D,self.img_dims = (H,)1-tuple:decode(z)raisesValueError('not enough values to unpack (expected 2, got 1)')at line 2285o_h, o_w = self.img_dims. The exception message contains neitherSeedVR2norimg_dims.self.original_image_videovalid 5-D,self.img_dims = (H, W, D)3-tuple:decode(z)raisesValueError('too many values to unpack (expected 2)')at line 2285. Same defect class as A4a; same lack of SeedVR2 context.self.original_image_videovalid 5-D,self.img_dims = (H, W)valid:decode(z)does NOT raise; control reachestiled_vae(latent, self, **self.tiled_args, encode=False)at line 2259 (whenenable_tiling=True) orsuper().decode_(latent)at line 2267 (whenenable_tiling=False). This is the valid-state cell that proves guards must NOT false-positive.§Failure Boundary
VideoAutoencoderKLWrapper.decodeBEFORE bothSeedVR2InputProcessing.execute(the producer ofself.original_image_video) ANDencode(orig_dims=(H, W))(the producer ofself.img_dims). Specifically: workflows that wire the wrapper as a generic VAE without the SeedVR2 input-processing node; programmatic harness code that constructs the wrapper directly and callsdecodewithout first callingencode(orig_dims=...); test code that exercisesdecodein isolation.SeedVR2InputProcessing.execute→encode(orig_dims=...)→decode(z). Both required attributes are populated upstream; cells A1–A4 are unreachable on this path. Cell A5 is the canonical-path shape; guards must pass control through unchanged.§Root Cause
__init__initializesself.original_image_video = None(line 2221) but does NOT initializeself.img_dims.decode()accesses both attributes (lines 2273, 2285) without validation. There is no presence/shape/arity guard at the top ofdecode(). CodeRabbit'sr2962219551review thread on Comfy-Org#11294 identified this defect class as Minor and proposed aRuntimeErrorguard pattern.§Proposed Fix
Two surgical changes on
comfy/ldm/seedvr/vae.py. The original plan as authored covered onlyoriginal_image_videoandimg_dims; PR review cycle 8 expanded the fix to also covertiled_argsafter Copilot review findingcomment_id=3188027986(classifiedACCEPT/P1) flagged the same defect class on a third attribute. The §Proposed Fix below describes the fully shipped delta; the cycle-8 additions are also recorded under## PR Review Scope Additionsfor change-history clarity.VideoAutoencoderKLWrapper.__init__(immediately afterself.original_image_video = Noneat line 2221) — add two new initializers:self.img_dims = Noneso the missing-state branch (Cell A2) is reachable as ais Nonecheck on a real default-constructed instance, not only as anAttributeErroron a stand-in that omits the attribute.self.tiled_args = None(added cycle 8) so the missing-tiled_argsbranch is reachable on a default-constructed instance, parity with the other two attributes.VideoAutoencoderKLWrapper.decode(immediately afterdef decode(self, z):and BEFOREb, tc, h, w = z.shape) — add six guards in this order:self.original_image_video is None→raise RuntimeError("SeedVR2 VideoAutoencoderKLWrapper.decode: ...original_image_videois None ...").not torch.is_tensor(self.original_image_video)→raise RuntimeError("...original_image_videomust be a tensor ...").self.original_image_video.ndim != 5→raise RuntimeError("...original_image_videomust be a 5-D tensor (b c t h w) ...").getattr(self, 'img_dims', None) is None→raise RuntimeError("...img_dimsis None or unset ...").not isinstance(img_dims, (tuple, list))→raise RuntimeError("...img_dimsmust be a tuple or list ...").len(img_dims) != 2→raise RuntimeError("...img_dimsmust be a length-2 tuple or list (H, W) ...").getattr(self, 'tiled_args', None) is None(added cycle 8) →raise RuntimeError("...tiled_argsis None or unset ...").not isinstance(tiled_args, dict)(added cycle 8) →raise RuntimeError("...tiled_argsmust be a dict ...").Every guard's
RuntimeErrormessage MUST contain (a) the substringSeedVR2and (b) the substring of the offending attribute name (original_image_videofor the first three guards;img_dimsfor the next three;tiled_argsfor the cycle-8 guards). The slicer is free to adjust message text within those constraints.§Verification Strategy
A new test module
tests-unit/comfy_test/test_seedvr_vae_decode_guards.pyports the Comfy-Org#119 pattern: a_make_standinclass borrowsdecode = comfy.ldm.seedvr.vae.VideoAutoencoderKLWrapper.decodeonto a bare class;_PostGuardReached(Exception)is the sentinel raised byunittest.mock.patch.object(comfy.ldm.seedvr.vae, "tiled_vae", side_effect=_PostGuardReached(...)); an_exercise_decodehelper drives the five cells. ACs are evaluated bypytest -qexit code on independent pre-fix and post-fix source checkouts; pre-fix logs MUST FAIL with the §Failure Signature cell-specific exception, post-fix logs MUST PASS withpytest.raises(RuntimeError, match=r"SeedVR2.*<attr>")succeeding.Affected Repositories
pollockjj/ComfyUI:issue_194is created fromorigin/issue_101HEAD9c2b9c39; deliverable PR targetsissue_101via--deliverable-base issue_101.pollockjj/mydevelopment:issue_194is created fromorigin/main; bookkeeping PR targetsmainand carries the plan, gate outputs, post-mortem, and committed pre/post evidence logs undergithub_issues/194/slice1/.Research and Methodology
Plan Foundations Comment URL: https://github.com/pollockjj/mydevelopment/issues/194#issuecomment-4378143678
Detected Scope:
none— port of the internal_make_standin+ post-guard sentinel pattern established by #109 (tests-unit/comfy_test/seedvr_model_test.py) and reused by Comfy-Org#119 (tests-unit/comfy_test/test_diffusers_metadata_guard.py); the existing precedent IS the spec for how this guard's regression module is shaped, and CodeRabbit'sr2962219551suggested-fix block IS the spec for the production guard text.The linked comment is the load-bearing artifact for every measurement, equivalence rule, canonical-protocol, tool version, and pipeline-stage citation in this plan. This plan defines no measurable output, no metric, and no equivalence rule against a published anchor; the change class is "API-misuse hygiene guard" with binary RuntimeError-and-substring assertions per AC. No AC below cites a metric, threshold, equivalence rule, or canonical protocol; therefore no AC carries a
Foundations-pin:for R&M sub-check 6 (the per-AC scope is N/A across all ACs).Tools, Pipeline, and Measurements
Plan Foundations Comment URL: https://github.com/pollockjj/mydevelopment/issues/194#issuecomment-4378143678
The linked comment's
## Tools, Pipeline, and Measurementssection enumerates: (1) Existing Tooling Inventory withREUSEstatus for every dep (pytest,comfy.cli_args.args,unittest.mock,contextlib,comfy.ldm.seedvr.vae.tiled_vae,inspect.getsource); (2) Pipeline = single pytest invocation againsttests-unit/comfy_test/test_seedvr_vae_decode_guards.py(no data pipeline; hygiene-class change); (3) Measurements = N/A (no metric, no anchor, binary assertions only); (4) Single-probe-before-sweep = N/A (5 enumerated cells, not a sweep); (5) Boundary-bracketing-before-verdict = N/A (no characterization verdict; binary observables per cell). NoKNOWN-GOOD-REFrows requiring resolution. Every AC below uses pytest as the standard test runner andcomfy.ldm.seedvr.vaeas the unmodified-vs-modified surface under test; neither constitutes citing a foundations entry for sub-check-6 purposes per the Phase 3 schema definition (Foundations-pin:is required iff the AC text cites the foundations comment for an equivalence rule, metric definition, canonical protocol, tooling decision, or measurement specification — none of which appear in the AC bodies below). Therefore no AC carries aFoundations-pin:for TPM sub-check 6.Ground Truth Probes
Probe captures live in
github_issues/194/probes/results/. Each row pairs the verbatim probe command and the verbatim observed output againstpollockjj/ComfyUI@9c2b9c39(origin/issue_101 HEAD) via worktree at/home/johnj/dev_master/mydevelopment/github_issues/194/probe_worktree. The Python interpreter for every probe is/home/johnj/dev_master/ComfyUI/.venv/bin/python(Python 3.12.13).cd /home/johnj/dev_master/mydevelopment/github_issues/194/probe_worktree && python3 -c "import comfy.ldm.seedvr.vae as v; print('decode in dir:', 'decode' in dir(v.VideoAutoencoderKLWrapper)); print('decode method object:', v.VideoAutoencoderKLWrapper.decode)"decode in dir: True/decode method object: <function VideoAutoencoderKLWrapper.decode at 0x7f4d1ac5da80>cd /home/johnj/dev_master/mydevelopment/github_issues/194/probe_worktree && python3 -c "import inspect, comfy.ldm.seedvr.vae as v; src = inspect.getsource(v.VideoAutoencoderKLWrapper.__init__); print(src); print('---'); print('self.img_dims initialized in __init__:', 'self.img_dims' in src); print('self.original_image_video initialized in __init__:', 'self.original_image_video' in src); print('self.tiled_args initialized in __init__:', 'self.tiled_args' in src)"gtp_02.txt)self.img_dims initialized in __init__: False/self.original_image_video initialized in __init__: True/self.tiled_args initialized in __init__: Falsecd /home/johnj/dev_master/mydevelopment/github_issues/194/probe_worktree && python3 -c "import inspect, comfy.ldm.seedvr.vae as v; src = inspect.getsource(v.VideoAutoencoderKLWrapper.decode); [print(f'{i:3d}: {line}') for i, line in enumerate(src.split(chr(10)), start=1) if line.strip().startswith('def decode') or 'rearrange(self.original_image_video' in line or 'o_h, o_w = self.img_dims' in line or 'self.tiled_args.get' in line or 'tiled_vae(' in line]"1: def decode(self, z):/12: self.enable_tiling = self.tiled_args.get("enable_tiling", False)/15: x = tiled_vae(latent, self, **self.tiled_args, encode=False)/29: input = rearrange(self.original_image_video, "b c t h w -> (b t) c h w")/41: o_h, o_w = self.img_dimscd /home/johnj/dev_master/mydevelopment/github_issues/194/probe_worktree && python3 -c "import comfy.ldm.seedvr.vae as v; print('tiled_vae attr present:', hasattr(v, 'tiled_vae')); print('tiled_vae callable:', callable(v.tiled_vae)); print('tiled_vae object:', v.tiled_vae)"tiled_vae attr present: True/tiled_vae callable: True/tiled_vae object: <function tiled_vae at 0x7fcfbfb87420>cd /home/johnj/dev_master/mydevelopment/github_issues/194/probe_worktree && python3 -c "from comfy.cli_args import args; print('cpu attr present:', hasattr(args, 'cpu')); print('cpu value:', args.cpu)"cpu attr present: True/cpu value: Falsels /home/johnj/dev_master/mydevelopment/github_issues/194/probe_worktree/tests-unit/comfy_test/test_diffusers_metadata_guard.py /home/johnj/dev_master/mydevelopment/github_issues/194/probe_worktree/tests-unit/comfy_test/seedvr_model_test.pygtp_06.txtfor_make_standinprecedent excerpts (lines 37–42 oftest_diffusers_metadata_guard.py; lines 48–59 ofseedvr_model_test.py)cd /home/johnj/dev_master/mydevelopment/github_issues/194/probe_worktree && python3 -m pytest tests-unit/comfy_test/seedvr_model_test.py tests-unit/comfy_test/test_diffusers_metadata_guard.py -q13 passed, 2 warnings in 2.36s(precedent harness baseline on9c2b9c39) — seegtp_09.txtCreated Surface Contract
The plan introduces literals that do not exist on the base branch. These are anchored here (not in Ground Truth Probes) because they will be created by Slice 1 itself; their post-creation existence is verified by the slice's own ACs.
tests-unit/comfy_test/test_seedvr_vae_decode_guards.py(new test module)_make_standin(helper in new test module)_PostGuardReached(Exception)(sentinel class in new test module)_exercise_decode(helper in new test module)test_none_original_image_video_raises_seedvr2_runtime_error(test fn)test_none_img_dims_raises_seedvr2_runtime_error(test fn)test_wrong_rank_original_image_video_raises_seedvr2_runtime_error(test fn)test_wrong_arity_img_dims_raises_seedvr2_runtime_error(parametrized over 1-tuple and 3-tuple)test_valid_state_passes_through_guards_to_tiled_vae(test fn, combined behavioral + AST source pin)test_init_initializes_img_dims_to_none(test fn, AST source pin on__init__)comfy/ldm/seedvr/vae.pyVideoAutoencoderKLWrapper.decode(4 guards, each raisingRuntimeErrorwhose message contains substringSeedVR2AND the relevant attribute name)pollockjj/ComfyUI:issue_194self.img_dims = Noneline inVideoAutoencoderKLWrapper.__init__pollockjj/ComfyUI:issue_194github_issues/194/slice1/<test>_pre.log(one per AC)9c2b9c39with the new test file dropped in but the production fix absent)github_issues/194/slice1/<test>_post.log(one per AC)pollockjj/ComfyUI:issue_194with both fixes applied)Asset Readiness Inventory
comfy/ldm/seedvr/vae.py(unguarded base)pollockjj/ComfyUI@9c2b9c39:comfy/ldm/seedvr/vae.pyGTP-3); pre-fix opaque-exception cells reproduce live in Phase 0.5 dry-runstests-unit/comfy_test/seedvr_model_test.py(#109 precedent)pollockjj/ComfyUI@9c2b9c39:tests-unit/comfy_test/seedvr_model_test.pygtp_09.txt)tests-unit/comfy_test/test_diffusers_metadata_guard.py(Comfy-Org#119 precedent — primary structural template)pollockjj/ComfyUI@9c2b9c39:tests-unit/comfy_test/test_diffusers_metadata_guard.pygtp_09.txt)/home/johnj/dev_master/ComfyUI/.venvdev_masteron prosoche13 passed, 2 warnings in 2.36s(gtp_09.txt)tests-unit/comfy_test/test_seedvr_vae_decode_guards.py(new test module)pollockjj/ComfyUI:issue_194:tests-unit/comfy_test/test_seedvr_vae_decode_guards.pygtp_08.txtconfirmsNo such file or directory; exit:2); created by Slice 1 implementation commit; AC-1..AC-6 verify post-creation behaviordecode()+self.img_dims = Nonein__init__pollockjj/ComfyUI:issue_194:comfy/ldm/seedvr/vae.pypollockjj/mydevelopment:issue_194:github_issues/194/slice1/<test>_{pre,post}.logSlices
Slice 1: Convert opaque decode() misuse failures into SeedVR2-specific RuntimeErrors
Kind: implementation
Objective: Prove that
VideoAutoencoderKLWrapper.decoderaises aRuntimeErrorwhose message contains bothSeedVR2and the offending attribute name (original_image_videoorimg_dims) for each of the four reachable misuse shapes (None, img_dims-None, wrong-rank, wrong-arity), AND that valid state still flows through the new guards into the post-guard region unchanged.Acceptance Criteria
AC-1:
cd /home/johnj/dev_master/ComfyUI && python3 -m pytest -q tests-unit/comfy_test/test_seedvr_vae_decode_guards.py::test_none_original_image_video_raises_seedvr2_runtime_errorexits 0 on the post-fixpollockjj/ComfyUI:issue_194checkout AND exits non-zero on the pre-fixorigin/issue_101(9c2b9c39) worktree with the same test module dropped in (the test'spytest.raises(RuntimeError, match=r"SeedVR2.*original_image_video")matcher fails to match the unguarded einopsTensor type unknown to einops <class 'NoneType'>message) — verified by committed artifactsgithub_issues/194/slice1/test_none_original_image_video_pre.logANDgithub_issues/194/slice1/test_none_original_image_video_post.log, each containing the verbatim pytest invocation, full stdout, and final lineexit code: <N>.origin/issue_101@9c2b9c39the unguardeddecode()raisesRuntimeError('Tensor type unknown to einops <class \'NoneType\'>')fromeinops/_backends.py:62(called transitively fromcomfy/ldm/seedvr/vae.py:2273); thepytest.raises(RuntimeError, match=r"SeedVR2.*original_image_video")matcher does NOT match because the einops message contains neitherSeedVR2nororiginal_image_video; the test FAILS withFailed: DID NOT RAISE matchingorFailed: Pattern did not match(Diagnosis Summary §Failure Signature Cell A1).decode()raisesRuntimeErrorwhosestr(exc.value)contains both substringsSeedVR2ANDoriginal_image_video; the matcher matches; the test PASSES with exit 0.VideoAutoencoderKLWrapper.decoderaises aRuntimeErrorwhose message contains bothSeedVR2and the offending attribute name (original_image_videoorimg_dims) for each of the four reachable misuse shapes (None, img_dims-None, wrong-rank, wrong-arity), AND that valid state still flows through the new guards into the post-guard region unchanged."AC-2:
cd /home/johnj/dev_master/ComfyUI && python3 -m pytest -q tests-unit/comfy_test/test_seedvr_vae_decode_guards.py::test_none_img_dims_raises_seedvr2_runtime_errorexits 0 on the post-fixpollockjj/ComfyUI:issue_194checkout AND exits non-zero on the pre-fixorigin/issue_101(9c2b9c39) worktree (the test'spytest.raises(RuntimeError, match=r"SeedVR2.*img_dims")matcher fails to match theTypeError('cannot unpack non-iterable NoneType object')raised by Python tuple-unpacking onNone) — verified by committed artifactsgithub_issues/194/slice1/test_none_img_dims_pre.logANDgithub_issues/194/slice1/test_none_img_dims_post.log, each containing the verbatim pytest invocation, full stdout, and final lineexit code: <N>.origin/issue_101@9c2b9c39the unguardeddecode()raisesTypeError('cannot unpack non-iterable NoneType object')atcomfy/ldm/seedvr/vae.py:2285o_h, o_w = self.img_dimswhen the stand-in setsself.img_dims = None; the matcher requiresRuntimeErrorand does NOT acceptTypeError(TypeError is not a RuntimeError subclass); the test FAILS with the TypeError propagating throughpytest.raises(RuntimeError)(Diagnosis Summary §Failure Signature Cell A2).decode()raisesRuntimeErrorwhosestr(exc.value)contains both substringsSeedVR2ANDimg_dims; the matcher matches; the test PASSES. The test stand-in setsself.img_dims = Noneexplicitly (the realistic shape after the §Proposed Fix__init__initialization), so the production guard may use eitherself.img_dims is Noneorgetattr(self, 'img_dims', None) is None— both expressions evaluate True on the test stand-in and the test is implementation-agnostic.VideoAutoencoderKLWrapper.decoderaises aRuntimeErrorwhose message contains bothSeedVR2and the offending attribute name (original_image_videoorimg_dims) for each of the four reachable misuse shapes (None, img_dims-None, wrong-rank, wrong-arity), AND that valid state still flows through the new guards into the post-guard region unchanged."AC-3:
cd /home/johnj/dev_master/ComfyUI && python3 -m pytest -q tests-unit/comfy_test/test_seedvr_vae_decode_guards.py::test_wrong_rank_original_image_video_raises_seedvr2_runtime_errorexits 0 on the post-fixpollockjj/ComfyUI:issue_194checkout AND exits non-zero on the pre-fixorigin/issue_101(9c2b9c39) worktree (the test'spytest.raises(RuntimeError, match=r"SeedVR2.*original_image_video")matcher fails to match theeinops.EinopsErrorrank-mismatch message) — verified by committed artifactsgithub_issues/194/slice1/test_wrong_rank_original_image_video_pre.logANDgithub_issues/194/slice1/test_wrong_rank_original_image_video_post.log, each containing the verbatim pytest invocation, full stdout, and final lineexit code: <N>.origin/issue_101@9c2b9c39the unguardeddecode()raiseseinops.EinopsError('Error while processing rearrange-reduction pattern "b c t h w -> (b t) c h w". Input tensor shape: torch.Size([3, 4, 4]). Additional info: {}. Wrong shape: expected 5 dims. Received 3-dim tensor.')fromcomfy/ldm/seedvr/vae.py:2273; the matcher requiresRuntimeErrorand does NOT accepteinops.EinopsError; the test FAILS (Diagnosis Summary §Failure Signature Cell A3).decode()raisesRuntimeErrorwhosestr(exc.value)contains both substringsSeedVR2ANDoriginal_image_video; the matcher matches; the test PASSES.VideoAutoencoderKLWrapper.decoderaises aRuntimeErrorwhose message contains bothSeedVR2and the offending attribute name (original_image_videoorimg_dims) for each of the four reachable misuse shapes (None, img_dims-None, wrong-rank, wrong-arity), AND that valid state still flows through the new guards into the post-guard region unchanged."AC-4:
cd /home/johnj/dev_master/ComfyUI && python3 -m pytest -q tests-unit/comfy_test/test_seedvr_vae_decode_guards.py::test_wrong_arity_img_dims_raises_seedvr2_runtime_error(parametrized over 1-tuple(H,)AND 3-tuple(H, W, D)) exits 0 on the post-fixpollockjj/ComfyUI:issue_194checkout AND exits non-zero on the pre-fixorigin/issue_101(9c2b9c39) worktree for both parametrized cells (the test'spytest.raises(RuntimeError, match=r"SeedVR2.*img_dims")matcher fails to match eitherValueErrorunpack message) — verified by committed artifactsgithub_issues/194/slice1/test_wrong_arity_img_dims_pre.logANDgithub_issues/194/slice1/test_wrong_arity_img_dims_post.log, each containing the verbatim pytest invocation, full stdout (including both parametrized cell IDs), and final lineexit code: <N>.origin/issue_101@9c2b9c39the unguardeddecode()raisesValueError('not enough values to unpack (expected 2, got 1)')for the 1-tuple cell andValueError('too many values to unpack (expected 2)')for the 3-tuple cell, both atcomfy/ldm/seedvr/vae.py:2285o_h, o_w = self.img_dims; the matcher requiresRuntimeErrorand does NOT acceptValueError; both parametrized cells FAIL (Diagnosis Summary §Failure Signature Cells A4a, A4b).decode()raisesRuntimeErrorwhosestr(exc.value)contains both substringsSeedVR2ANDimg_dimsfor both parametrized cells; the matcher matches; both cells PASS.VideoAutoencoderKLWrapper.decoderaises aRuntimeErrorwhose message contains bothSeedVR2and the offending attribute name (original_image_videoorimg_dims) for each of the four reachable misuse shapes (None, img_dims-None, wrong-rank, wrong-arity), AND that valid state still flows through the new guards into the post-guard region unchanged."AC-5:
cd /home/johnj/dev_master/ComfyUI && python3 -m pytest -q tests-unit/comfy_test/test_seedvr_vae_decode_guards.py::test_valid_state_passes_through_guards_to_tiled_vaeexits 0 on the post-fixpollockjj/ComfyUI:issue_194checkout AND exits non-zero on the pre-fixorigin/issue_101(9c2b9c39) worktree — the test asserts BOTH (a)mock_tiled.call_count == 1(the patched module-levelcomfy.ldm.seedvr.vae.tiled_vaewas reached exactly once via_PostGuardReached, proving valid input flows past the guards into the post-guard region) AND (b)inspect.getsource(comfy.ldm.seedvr.vae.VideoAutoencoderKLWrapper.decode)contains the substringraise RuntimeError(at a position BEFORE the substringb, tc, h, w = z.shape(proving the guards are placed at the top ofdecode()ahead of the existing first body line). On the pre-fix base SHA assertion (b) FAILS because the unguardeddecode()body has noraise RuntimeError(substring beforeb, tc, h, w = z.shape; assertion (a) would pass vacuously, but the conjunction fails → exit non-zero — verified by committed artifactsgithub_issues/194/slice1/test_valid_state_passes_through_guards_pre.logANDgithub_issues/194/slice1/test_valid_state_passes_through_guards_post.log.origin/issue_101@9c2b9c39inspect.getsource(VideoAutoencoderKLWrapper.decode)shows the first executable statement isb, tc, h, w = z.shape(probeGTP-3line1: def decode(self, z):followed byb, tc, h, w = z.shapeas the immediate body); the substringraise RuntimeError(does NOT appear beforeb, tc, h, w = z.shape; the AST source-position assertion FAILS even though the behavioral assertionmock_tiled.call_count == 1would pass vacuously (no guards exist to reject valid state) (Diagnosis Summary §Failure Signature Cell A5 + §Root Cause).inspect.getsource(VideoAutoencoderKLWrapper.decode)showsraise RuntimeError(substring(s) appear BEFOREb, tc, h, w = z.shape; the AST source-position assertion PASSES; ANDmock_tiled.call_count == 1confirms valid input still reaches the post-guard region; the conjoint test PASSES.VideoAutoencoderKLWrapper.decoderaises aRuntimeErrorwhose message contains bothSeedVR2and the offending attribute name (original_image_videoorimg_dims) for each of the four reachable misuse shapes (None, img_dims-None, wrong-rank, wrong-arity), AND that valid state still flows through the new guards into the post-guard region unchanged."AC-6:
cd /home/johnj/dev_master/ComfyUI && python3 -m pytest -q tests-unit/comfy_test/test_seedvr_vae_decode_guards.py::test_init_initializes_img_dims_to_noneexits 0 on the post-fixpollockjj/ComfyUI:issue_194checkout AND exits non-zero on the pre-fixorigin/issue_101(9c2b9c39) worktree — the test asserts thatinspect.getsource(comfy.ldm.seedvr.vae.VideoAutoencoderKLWrapper.__init__)contains the substringself.img_dims = None. On the pre-fix base SHA the substring is absent (probeGTP-2confirmsself.img_dims initialized in __init__: False); on the post-fix branch the substring is present per the §Proposed Fix one-line addition — verified by committed artifactsgithub_issues/194/slice1/test_init_initializes_img_dims_to_none_pre.logANDgithub_issues/194/slice1/test_init_initializes_img_dims_to_none_post.log.origin/issue_101@9c2b9c39inspect.getsource(VideoAutoencoderKLWrapper.__init__)does NOT contain the substringself.img_dims = None(probeGTP-2capturedself.img_dims initialized in __init__: Falseon the live module); the test'sassert "self.img_dims = None" in inspect.getsource(...)FAILS (Diagnosis Summary §Root Cause).inspect.getsource(VideoAutoencoderKLWrapper.__init__)contains the substringself.img_dims = Noneexactly once between theself.original_image_video = Noneline andsuper().__init__(*args, **kwargs); the assertion PASSES.VideoAutoencoderKLWrapper.decoderaises aRuntimeErrorwhose message contains bothSeedVR2and the offending attribute name (original_image_videoorimg_dims) for each of the four reachable misuse shapes (None, img_dims-None, wrong-rank, wrong-arity), AND that valid state still flows through the new guards into the post-guard region unchanged."Constraints
cd /home/johnj/dev_master/ComfyUIto the slot venv at.venv/bin/python)python3 -m pytest -q tests-unit/comfy_test/test_seedvr_vae_decode_guards.py(no debug harness; nodebug/run_debug.pyinvocation; this is a unit-test-only packet)--use-process-isolation, no--disable-cuda-malloc; the test imports the wrapper class directly fromcomfy.ldm.seedvr.vaewithcomfy.cli_args.args.cpu = Trueset before import per Add SeedVR2 support #109/UserWarning bug ? Comfy-Org/ComfyUI#119 precedent)/home/johnj/dev_master/ComfyUI/.venv)pkill, norm -rf, nopython main.pyissue_194, not on base branches: deliverable commit onpollockjj/ComfyUI:issue_194(created fromorigin/issue_101@9c2b9c39); bookkeeping commit onpollockjj/mydevelopment:issue_194(created fromorigin/main)tests-unit/comfy_test/test_diffusers_metadata_guard.py— REUSE as the structural template; the new test module mirrors the_make_standin+_PostGuardReached+_exercise_<guard>shape verbatim. No new orchestration introduced.tests-unit/comfy_test/seedvr_model_test.py— REUSE as the secondary precedent forcomfy.cli_args.args.cpu = Truetoggling andinspect.getsourcesource-pin patterns.comfy.ldm.seedvr.vae.tiled_vae— REUSE as the monkey-patch target for AC-5's_PostGuardReachedsentinel; no new module-level callable introduced.tests-unit/comfy_test/harness used by every prior issue_101 packet (Add SeedVR2 support #109, UserWarning bug ? Comfy-Org/ComfyUI#119, Suggest： Add Prefab Node（ a saved json ） for combine function and reuse easily。 Comfy-Org/ComfyUI#183, Feature request: docker-compose support Comfy-Org/ComfyUI#188, Providing basic preset workflows will helps usability. Comfy-Org/ComfyUI#189).origin/issue_101@9c2b9c39with the new test module dropped in but the production fix absent; post-fix frompollockjj/ComfyUI:issue_194with both fixes applied. Each AC commits both<test>_pre.logand<test>_post.logtogithub_issues/194/slice1/.inspect.getsourcesubstring assertions captured by the same pytest invocation; the same pre/post artifact pair satisfies the requirement (no separate evidence file needed).git stashand without external decision deferral..gitignorerules for safe generated bulk, or discarded generated debris.git status --shortgates locally on/home/johnj/dev_master/ComfyUIAND/home/johnj/dev_master/mydevelopmentAND any temporary worktree before submission, and must refuse to submit while any touched repo is dirty.repo_hygiene.txt, rawgit status --shorttranscripts, orCLEANmarkers as QA evidence./home/johnj/dev_master/mydevelopment/github_issues/194/probe_worktreeis removed by the slicer before submission viagit worktree remove; presence of this worktree at submission is treated as dirty state.Out of Scope
VideoAutoencoderKL.decode_()(the parent class's decode) — the unguarded parent method is not modified by this packet; only the wrapper'sdecode()gains guards. CodeRabbit threadr2962219551flags only the wrapper.tiled_vaebranch andlab_color_transferpost-guard processing — out-of-scope per the issue body's## Non-Goals. The fix adds guards at the TOP ofdecode()only; downstream behavior is unmodified.VideoAutoencoderKLWrapper.encode()validation — the producer-side counterpart of the same defect class (encodeaccepts an unvalidatedorig_dimsargument and stores it without shape/arity check). Tracked as a separate future issue if surfaced; this packet does not pre-empt it.self.tiled_argsinitialization in__init__— originally out-of-scope per issue body Non-Goals (thetiled_vaebranch is excluded), but brought into scope by PR review cycle 8 in response to Copilot review findingcomment_id=3188027986(classifiedACCEPT/P1). See## PR Review Scope Additionsbelow.comfy/ldm/seedvr/vae.py— VAE-only scope per issue body Non-Goals.__init__change absent), which has no diagnostic value distinct from the combined fix.PR Review Scope Additions
During PR review, the scope was expanded beyond the original plan to address a related missing-state defect surfaced by Copilot review.
Cycle 8 —
self.tiled_argsinitialization and decode-guard expansionCopilot review finding
comment_id=3188027986on PR #37 (classifiedACCEPT/P1) flagged thatdecode()unconditionally callsself.tiled_args.get("enable_tiling", False)but__init__did not initialise the attribute. A default-constructed wrapper (or any instance not populated bySeedVR2InputProcessing.execute) therefore raised an opaqueAttributeError: 'VideoAutoencoderKLWrapper' object has no attribute 'tiled_args'after passing theoriginal_image_video/img_dimsguards — the same defect class the original packet was guarding against, just a different attribute. The fix added (commite6f68371"pr review cycle 8: address findings"):self.tiled_args = NoneinVideoAutoencoderKLWrapper.__init__(alongside the originalself.original_image_video = Noneand the newself.img_dims = None).tiled_args is None) and a dict-type guard (not isinstance(tiled_args, dict)) at the top ofdecode()ahead of the.get()call, each raisingRuntimeErrorwhose message containsSeedVR2andtiled_args.tests-unit/comfy_test/test_seedvr_vae_decode_guards.py:test_init_initializes_tiled_args_to_none(AC9 — source-pin on__init__).test_none_tiled_args_raises_seedvr2_runtime_error(AC10 — parametrized overset_tiled_args=True/Falseso both the explicit-Noneand attribute-unset cells are covered).test_non_dict_tiled_args_raises_seedvr2_runtime_error(AC11 — parametrized over non-dict typesstr/int/list/tuple).This addition brings the PR description into alignment with the shipped code per Copilot review finding
comment_id=3188204623(cycle 13). The original## Out of Scopebullet forself.tiled_argsreflected the original plan as authored; this addendum supersedes it. No other## Out of Scopeitems were affected.Tracked in pollockjj/mydevelopment Comfy-Org#194 (bookkeeping PR pollockjj/mydevelopment#202).