LTX-2.3 video-gen lane: NVFP4 model + LoRA studio + video bridge#20
Conversation
Quantize LTX-2.3-22B-distilled to NVFP4 mixed-precision for Blackwell/sm120. Lightricks ships fp4 only for the 19B; 2.3-22B is bf16-only — this fills the gap. Converter (quantize.py) streams bf16 through comfy_kitchen's own NVFP4 layout, replicating Lightricks' 19B layer policy (fp4 transformer_blocks 1-42; keep block 0 + last 5 + gates + VAE/vocoder bf16) and stamping the _quantization_metadata header ComfyUI's loader needs. Measured (960x544, reproduced 2x): 46.1→22.9 GB, 2.85→1.82 s/it (1.57x), ~60→~37 GB peak VRAM. Ship distilled-decode (full-decode shows mild fp4 artifacting). Card/BLOG/LICENSE + honest numbers. Live: https://huggingface.co/protoLabsAI/LTX-2.3-22B-distilled-NVFP4 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…8 piece 2)
Standalone video bridge co-located with ComfyUI on protolabs — the shape the team
settled on (litellm's native /v1/videos router shadows passthrough routes, so the
job layer can't live in the gateway). Implements the AGREED 3-URL contract:
POST /v1/videos, GET /v1/videos/{id}, GET /v1/videos/{id}/content.
Reuses protoBanana's ComfyUIClient (inherits the #39 cache-nonce fix); protoBanana
stays image-only. Restart-survivable JSON-backed job store (job_id→prompt_id,
status re-derived from ComfyUI /history); mp4 served off local disk; LTX knobs in
extra_body. Piece 1 = parameterized distilled-decode T2V workflow (workflows/
ltx2-t2v.json). Verified end-to-end: POST→poll→206KB mp4 in ~14s.
Stubbed/next: I2V (input_reference accepted, needs I2V template variant), fine
progress via ws, edge route in homelab-iac.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…quests
The multipart branch built body from only {model, prompt} Form params, so any
input_reference (I2V) request silently dropped seconds/size/seed/negative_prompt/
extra_body. Parse the full form instead (coerce seconds/seed numeric, JSON-decode
extra_body). Verified: multipart POST now propagates all contract fields.
Reported by protoDirector review on protoBanana#38.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
make_video_lora.py runs the full chain one-command + resumable: caption → preprocess → fix-conditions (the embeds-land-next-to-clips gotcha) → config → train → wire (symlink LoRA into ComfyUI loras/ + emit a T2V workflow with LoraLoaderModelOnly on the fp4 base). Each stage skips if its output exists. Proves the "make our own LoRAs" path is now point-a-folder → get-a-LoRA. Config/wire stages validated against the toy data; full caption/train run is per-dataset. Runbook (README) documents the one command + the manual stages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
app.py — two-tab UI on the orchestrator: Train (clips→LoRA, streaming log) + Generate & Compare (pick a trained LoRA, generate a video inline, or A/B base-vs-LoRA at the same seed side-by-side via ComfyUI). Verified end-to-end: compare() produced base + LoRA videos at matched seed. Runs in the LTX-2 venv (gradio 6.10), UI on :7862. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Generate & Compare tab now takes an optional reference image (first frame). When provided, _build uploads it to ComfyUI, points LoadImage at it, and flips the PrimitiveBoolean bypass (4977) off → LTXVImgToVideoConditionOnly drives I2V first-frame conditioning; a strength slider controls the anchor. Empty = plain T2V. Verified: I2V graph wired (bypass=False, image set) + generated a clip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
![protoreview[bot]](https://avatars.githubusercontent.com/in/4233969?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA panel review — PASS
code-review-structural · head cc823065d245 · formal
[review-synthesizer completed: workflow code-review-structural:report]
Overall risk is low given the clean verification pass and absence of flagged defects. There are no items to fix first, and the panel reported no disagreements. Verification confirmed the empty input array, leaving the result unchanged. A structural analysis pass was skipped for this review cycle, so minor integration edge cases may remain unexamined.
[]
WalkthroughAdds LTX-2.3 LoRA training and generation tooling, NVFP4 quantization assets and licensing, plus an asynchronous OpenAI-compatible ComfyUI video bridge with workflow injection, job persistence, content retrieval, documentation, and smoke testing. ChangesLTX-2.3 LoRA tooling
LTX-2.3 NVFP4 experiment
ComfyUI video bridge
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant VideoBridge
participant WorkflowInjector
participant ComfyUI
Client->>VideoBridge: POST /v1/videos
VideoBridge->>WorkflowInjector: build_workflow(...)
WorkflowInjector-->>VideoBridge: injected workflow
VideoBridge->>ComfyUI: submit workflow
ComfyUI-->>VideoBridge: prompt_id
VideoBridge-->>Client: queued job
Client->>VideoBridge: GET /v1/videos/{job_id}
VideoBridge->>ComfyUI: query history and queue
ComfyUI-->>VideoBridge: status and output metadata
VideoBridge-->>Client: job status
Client->>VideoBridge: GET /v1/videos/{job_id}/content
VideoBridge-->>Client: MP4 bytes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![protoquinn[bot]](https://avatars.githubusercontent.com/in/3282126?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA Audit — PR #20 | LTX-2.3 video-gen lane: NVFP4 model + LoRA studio + video bridge
VERDICT: PASS
CI Status
- No CI checks configured for this repo (Gap — not blocking)
Diff Review
- 15 new files, +2216/-0 — all additions, zero changes to existing code
- Three self-contained additions: NVFP4 quantizer, LoRA studio, video bridge
- Code is experiment/infra tooling; hardcoded paths (
/mnt/data/…,~/dev/LTX-2/…) are appropriate for local research scripts
Observations
- LOW:
app.py:107—open(path, "rb")in_upload_imagehas no try/except; a missing ref image will raise uncaught. Acceptable for a local Gradio studio. - LOW:
app.py:63—os.killpginstop()could raiseProcessLookupErrorif the process already exited betweenpoll()andkillpg. Harmless in practice. - LOW:
infra/video-bridge/— job store persistence claim ("restart-survivable") is self-asserted; no unit test included. Acceptable for infra prototype. - GAP: Clawpatch unavailable — repo not in project registry; structural review skipped.
No HIGH or CRITICAL findings. All new code, no regressions.
— Quinn, QA Engineer
|
Submitted APPROVE review on #20. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@experiments/ltx2-lora/app.py`:
- Around line 22-61: Replace the module-level _proc process registry with
per-session Gradio state. Update the train and stop functions and their UI
bindings to accept and return the session-specific Popen handle, ensuring each
session’s Stop action targets only its own training process and no shared
process state remains.
In `@experiments/ltx2-lora/make_video_lora.py`:
- Around line 122-137: In make_video_lora.py lines 122-137 and app.py lines
95-103, update the workflow-building logic in _build and its corresponding
generator to select an unused node ID before inserting the LoraLoaderModelOnly
node, and use that ID when rewiring guider inputs instead of hardcoded "9001";
also validate the CheckpointLoaderSimple lookup in both sites and raise a clear
error when no checkpoint node is found.
- Around line 63-80: Update stage_fix_conditions to locate each embed using the
latent’s mirrored relative path under clips_dir rather than basename-only rglob
matching. Remove the ineffective parent filter, verify the resolved candidate
corresponds to the expected relative path, and warn or fail without copying when
the mirrored embed is missing or ambiguous so latents cannot be paired with
another clip’s embedding.
In `@experiments/ltx2-nvfp4/LICENSE`:
- Around line 85-86: Add the upstream licensing URL or another stable contact
immediately after the commercial-use requirement in the LICENSE text, preserving
the existing license wording and making the required Licensor contact
actionable.
- Around line 14-17: Restore the `Control` definition in the license text to
require ownership of more than fifty percent (50%), rather than fifty percent or
more. Preserve the remaining ownership and management-control language
unchanged.
In `@experiments/ltx2-nvfp4/measurements-multires.json`:
- Around line 1-57: Align the documented performance claims with verified
measurement data: in experiments/ltx2-nvfp4/measurements-multires.json lines
1-57, add the missing 960×544 records or revise the documentation to use
existing resolutions, populate best_s_per_it with actual timings, add the
claimed duplicate runs, and verify or correct fp4_dog’s anomalous VRAM value.
Update experiments/ltx2-nvfp4/BLOG.md lines 10-18 and
experiments/ltx2-nvfp4/README.md lines 33-38 to match the resulting records,
including correcting the unsupported “fp4 1280×704 ≈ 3.85 s/it” claim.
In `@experiments/ltx2-nvfp4/quantize.py`:
- Line 50: Replace the assert in the CUDA availability check with an explicit
RuntimeError when torch.cuda.is_available() is false, preserving the existing
error message so execution stops clearly before NVFP4.quantize runs.
- Line 83: Update the quant_source assignment to use os.path.basename on
args.src instead of manually splitting on "/"; ensure the os module is imported
and preserve the resulting filename value.
In `@infra/video-bridge/bridge.py`:
- Around line 50-51: Update _load_jobs so its broad exception handler logs the
caught error before returning {}, using the module’s existing logging mechanism
and preserving the current fallback behavior; do not silently discard job-store
load failures.
- Around line 200-208: Validate the ComfyUI metadata before reading the local
file in the `/content` handling flow: resolve the path built from `OUTPUT_DIR`,
`out["subfolder"]`, and `out["filename"]`, then ensure it remains within the
resolved `OUTPUT_DIR`; reject or bypass unsafe traversal paths rather than
serving them. Keep the existing ComfyUI HTTP fallback only for valid paths.
In `@infra/video-bridge/inject.py`:
- Line 19: Align the video bridge’s FPS handling with the workflow template:
change DEFAULT_FPS from 30 to 24, add the N_FPS identifier for node 4978, and
set that node’s inputs.value from the resolved fps in the injection flow so
extra_body overrides reach rendering. Update the README’s documented default
from 30 to 24.
- Around line 34-38: Update parse_size to validate the size format before
unpacking or converting values: require exactly one “x”, ensure both dimensions
are valid positive integers, and raise a clear input-validation error for
malformed or zero-sized values while preserving the existing default for missing
size.
In `@infra/video-bridge/README.md`:
- Around line 46-47: Update the fps default in the README’s seconds-to-frames
documentation from 30 to 24 after correcting DEFAULT_FPS in inject.py, keeping
the surrounding LTX 8n+1 mapping guidance unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9982ce7e-dc98-4e93-becc-e216fdfb3fa6
📒 Files selected for processing (15)
experiments/ltx2-lora/README.mdexperiments/ltx2-lora/app.pyexperiments/ltx2-lora/make_video_lora.pyexperiments/ltx2-lora/toy_smoke.yamlexperiments/ltx2-nvfp4/BLOG.mdexperiments/ltx2-nvfp4/LICENSEexperiments/ltx2-nvfp4/README.mdexperiments/ltx2-nvfp4/measurements-multires.jsonexperiments/ltx2-nvfp4/quantize.pyinfra/video-bridge/README.mdinfra/video-bridge/bridge.pyinfra/video-bridge/inject.pyinfra/video-bridge/requirements.txtinfra/video-bridge/smoke.pyinfra/video-bridge/workflows/ltx2-t2v.json
| _proc = {"p": None} | ||
|
|
||
| # ------------------------------- TRAIN ------------------------------- | ||
| def train(mode, uploads, folder, style, rank, steps, bucket, with_audio, auto_caption, gpu): | ||
| if not style or not style.strip(): | ||
| yield "❌ provide a style / LoRA name"; return | ||
| style = style.strip().replace(" ", "_") | ||
| work = Path(f"/mnt/data/video-lora/{style}") | ||
| if mode == "Upload clips": | ||
| if not uploads: | ||
| yield "❌ upload at least one clip"; return | ||
| clips = work / "raw"; clips.mkdir(parents=True, exist_ok=True) | ||
| for f in uploads: | ||
| shutil.copy2(f, clips / Path(f).name) | ||
| clips_dir = str(clips) | ||
| else: | ||
| if not folder or not os.path.isdir(folder): | ||
| yield f"❌ not a folder on the box: {folder}"; return | ||
| clips_dir = folder | ||
| cmd = [PY, str(ORCH), clips_dir, style, "--rank", str(int(rank)), "--steps", str(int(steps)), | ||
| "--bucket", bucket, "--gpu", str(int(gpu))] | ||
| if with_audio: cmd.append("--with-audio") | ||
| if not auto_caption: cmd.append("--no-caption") | ||
| log = "$ " + " ".join(cmd) + "\n\n"; yield log | ||
| p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, | ||
| text=True, bufsize=1, preexec_fn=os.setsid) | ||
| _proc["p"] = p | ||
| for line in p.stdout: | ||
| log += line; yield log | ||
| p.wait(); _proc["p"] = None | ||
| log += (f"\n\n✅ DONE — '{style}' LoRA trained + wired. Switch to **Generate & Compare**, hit " | ||
| f"↻ Refresh, pick `{style}.safetensors`." if p.returncode == 0 | ||
| else f"\n\n❌ exited {p.returncode} — see log above.") | ||
| yield log | ||
|
|
||
| def stop(): | ||
| p = _proc.get("p") | ||
| if p and p.poll() is None: | ||
| os.killpg(os.getpgid(p.pid), signal.SIGTERM); return "🛑 stop sent." | ||
| return "nothing running." |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Global _proc shared across all sessions races when multiple users/tabs use Train concurrently.
_proc = {"p": None} is module-level state shared by every Gradio session. If two users (or two browser tabs) start a training run, the second train() call overwrites _proc["p"], and stop() will only ever be able to target the most-recently-started process — a user's "Stop" click can silently do nothing or (worse) terminate someone else's job. Given server_name="0.0.0.0" this isn't a purely local, single-operator surface.
Use per-session state (e.g. gr.State holding the Popen handle, threaded through train/stop) instead of a module-level dict.
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 45-46: Command coming from incoming request
Context: subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
text=True, bufsize=1, preexec_fn=os.setsid)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[error] 45-46: Use of unsanitized data to create processes
Context: subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
text=True, bufsize=1, preexec_fn=os.setsid)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@experiments/ltx2-lora/app.py` around lines 22 - 61, Replace the module-level
_proc process registry with per-session Gradio state. Update the train and stop
functions and their UI bindings to accept and return the session-specific Popen
handle, ensuring each session’s Stop action targets only its own training
process and no shared process state remains.
| for lat in latents: | ||
| rel = lat.relative_to(pre / "latents") # e.g. clips/foo.pt | ||
| dst = cond / rel | ||
| if dst.exists(): | ||
| continue | ||
| # find the embed: <clips_dir>/**/<stem>.pt (saved next to source video) | ||
| matches = list(Path(clips_dir).rglob(lat.name)) | ||
| embed = next((m for m in matches if m.parent != (pre / "latents" / rel.parent)), None) | ||
| if embed is None: | ||
| print(f"[fix] WARN no embed found for {rel} (looked for {lat.name} under {clips_dir})") | ||
| continue | ||
| dst.parent.mkdir(parents=True, exist_ok=True) | ||
| shutil.copy2(embed, dst); made += 1 | ||
| print(f"[fix] conditions/: {made} embeds relocated, {len(latents)} latents total") | ||
| n_cond = len(list(cond.rglob('*.pt'))) if cond.exists() else 0 | ||
| if n_cond < len(latents): | ||
| sys.exit(f"[fix] conditions ({n_cond}) < latents ({len(latents)}) — training will fail to pair. " | ||
| "Check that captions produced embeds for every clip.") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Basename-only embed matching can silently pair the wrong caption/latents.
stage_fix_conditions locates the text-embed for each latent via Path(clips_dir).rglob(lat.name) (basename only) and the m.parent != (pre / "latents" / rel.parent) filter is effectively always true (matches come from clips_dir, never from pre), so it doesn't disambiguate anything. If two clips in different subfolders share the same filename, next(...) silently picks an arbitrary match — pairing the wrong caption/embedding with a clip's latents, with no warning, no error, and a training-data integrity issue that later surfaces only as poor model quality.
🐛 Prefer matching by mirrored relative path over basename search
- # find the embed: <clips_dir>/**/<stem>.pt (saved next to source video)
- matches = list(Path(clips_dir).rglob(lat.name))
- embed = next((m for m in matches if m.parent != (pre / "latents" / rel.parent)), None)
+ # prefer the embed at the mirrored relative path; fall back to a basename
+ # search only if the direct path doesn't exist, and fail loudly on ambiguity
+ direct = Path(clips_dir) / rel
+ if direct.exists():
+ embed = direct
+ else:
+ matches = list(Path(clips_dir).rglob(lat.name))
+ if len(matches) > 1:
+ print(f"[fix] WARN ambiguous matches for {lat.name}: {matches} — skipping")
+ embed = None
+ else:
+ embed = matches[0] if matches else None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for lat in latents: | |
| rel = lat.relative_to(pre / "latents") # e.g. clips/foo.pt | |
| dst = cond / rel | |
| if dst.exists(): | |
| continue | |
| # find the embed: <clips_dir>/**/<stem>.pt (saved next to source video) | |
| matches = list(Path(clips_dir).rglob(lat.name)) | |
| embed = next((m for m in matches if m.parent != (pre / "latents" / rel.parent)), None) | |
| if embed is None: | |
| print(f"[fix] WARN no embed found for {rel} (looked for {lat.name} under {clips_dir})") | |
| continue | |
| dst.parent.mkdir(parents=True, exist_ok=True) | |
| shutil.copy2(embed, dst); made += 1 | |
| print(f"[fix] conditions/: {made} embeds relocated, {len(latents)} latents total") | |
| n_cond = len(list(cond.rglob('*.pt'))) if cond.exists() else 0 | |
| if n_cond < len(latents): | |
| sys.exit(f"[fix] conditions ({n_cond}) < latents ({len(latents)}) — training will fail to pair. " | |
| "Check that captions produced embeds for every clip.") | |
| for lat in latents: | |
| rel = lat.relative_to(pre / "latents") # e.g. clips/foo.pt | |
| dst = cond / rel | |
| if dst.exists(): | |
| continue | |
| # prefer the embed at the mirrored relative path; fall back to a basename | |
| # search only if the direct path doesn't exist, and fail loudly on ambiguity | |
| direct = Path(clips_dir) / rel | |
| if direct.exists(): | |
| embed = direct | |
| else: | |
| matches = list(Path(clips_dir).rglob(lat.name)) | |
| if len(matches) > 1: | |
| print(f"[fix] WARN ambiguous matches for {lat.name}: {matches} — skipping") | |
| embed = None | |
| else: | |
| embed = matches[0] if matches else None | |
| if embed is None: | |
| print(f"[fix] WARN no embed found for {rel} (looked for {lat.name} under {clips_dir})") | |
| continue | |
| dst.parent.mkdir(parents=True, exist_ok=True) | |
| shutil.copy2(embed, dst); made += 1 | |
| print(f"[fix] conditions/: {made} embeds relocated, {len(latents)} latents total") | |
| n_cond = len(list(cond.rglob('*.pt'))) if cond.exists() else 0 | |
| if n_cond < len(latents): | |
| sys.exit(f"[fix] conditions ({n_cond}) < latents ({len(latents)}) — training will fail to pair. " | |
| "Check that captions produced embeds for every clip.") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@experiments/ltx2-lora/make_video_lora.py` around lines 63 - 80, Update
stage_fix_conditions to locate each embed using the latent’s mirrored relative
path under clips_dir rather than basename-only rglob matching. Remove the
ineffective parent filter, verify the resolved candidate corresponds to the
expected relative path, and warn or fail without copying when the mirrored embed
is missing or ambiguous so latents cannot be paired with another clip’s
embedding.
| if not os.path.exists(BASE_API): | ||
| print(f"[wire] {BASE_API} not found — skip workflow gen (run one fp4 T2V in the UI first to seed it). " | ||
| f"Manually: add LoraLoaderModelOnly(lora_name={style}.safetensors) between the checkpoint MODEL and the guider.") | ||
| return | ||
| api = json.load(open(BASE_API)); api.pop("4823", None) # distilled-only | ||
| # find the guider that consumes the checkpoint MODEL (slot 0) on the distilled path | ||
| ckpt_id = next((n for n, v in api.items() if v["class_type"] == "CheckpointLoaderSimple"), None) | ||
| guiders = [(n, inp) for n, v in api.items() for inp, val in v["inputs"].items() | ||
| if isinstance(val, list) and val[0] == ckpt_id and val[1] == 0 and "Guider" in v["class_type"]] | ||
| api["9001"] = {"class_type": "LoraLoaderModelOnly", | ||
| "inputs": {"model": [ckpt_id, 0], "lora_name": f"{style}.safetensors", "strength_model": 1.0}} | ||
| for gid, inp in guiders: | ||
| api[gid]["inputs"][inp] = ["9001", 0] | ||
| out = COMFY / "user/default/workflows" / f"LTX-2.3_T2V_{style}_LoRA.json" | ||
| json.dump(api, open(out, "w"), indent=1) | ||
| print(f"[wire] wrote workflow (API) {out} — load via 'Open' or POST to /prompt") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Hardcoded ComfyUI node id "9001" can silently overwrite an existing node in base_api.json. Both sites inject a new LoraLoaderModelOnly node under the literal key "9001" without checking whether that id is already used in the loaded graph — if it is, the assignment silently clobbers the existing node and corrupts the generated/executed workflow with no error surfaced.
experiments/ltx2-lora/make_video_lora.py#L122-L137: beforeapi["9001"] = {...}, compute an id guaranteed unused, e.g.new_id = next(str(i) for i in range(9001, 99999) if str(i) not in api), and usenew_idin place of the literal"9001".experiments/ltx2-lora/app.py#L95-L103: apply the same unused-id computation beforeapi["9001"] = {...}in_build.
Both sites also assume ckpt_id/ckpt (the CheckpointLoaderSimple lookup) is found; make_video_lora.py defaults to None on miss (silently wiring "model": [None, 0]), while app.py's next(...) has no default and raises StopIteration if missing (caught by the caller's except Exception, but with an unhelpful message). Consider raising a clear error in both places when the checkpoint node isn't found.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 125-125: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(BASE_API)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 135-135: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(out, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
📍 Affects 2 files
experiments/ltx2-lora/make_video_lora.py#L122-L137(this comment)experiments/ltx2-lora/app.py#L95-L103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@experiments/ltx2-lora/make_video_lora.py` around lines 122 - 137, In
make_video_lora.py lines 122-137 and app.py lines 95-103, update the
workflow-building logic in _build and its corresponding generator to select an
unused node ID before inserting the LoraLoaderModelOnly node, and use that ID
when rewiring guider inputs instead of hardcoded "9001"; also validate the
CheckpointLoaderSimple lookup in both sites and raise a clear error when no
checkpoint node is found.
| "Control" means the direct or indirect ownership of fifty percent | ||
| (50%) or more of the voting securities or other ownership | ||
| interests, or the power to direct the management and policies of | ||
| such Entity through voting rights, contract, or otherwise. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the local LICENSE text around the cited lines
sed -n '1,40p' experiments/ltx2-nvfp4/LICENSE
# Look for the exact Control definition in the repository
rg -n 'Control|fifty percent|more than fifty percent|50%' experiments/ltx2-nvfp4/LICENSE
# Fetch the upstream license text for comparison if network access is available
python3 - <<'PY'
import urllib.request, ssl
url = "https://raw.githubusercontent.com/Lightricks/LTX-2/main/LICENSE"
ctx = ssl.create_default_context()
with urllib.request.urlopen(url, context=ctx, timeout=20) as r:
text = r.read().decode("utf-8", errors="replace")
for i, line in enumerate(text.splitlines(), 1):
if "Control" in line or "fifty percent" in line or "50%" in line:
print(f"{i}: {line}")
PYRepository: protoLabsAI/protoLab
Length of output: 1963
🌐 Web query:
Lightricks LTX-2 LICENSE Control definition "fifty percent" "more than fifty percent"
💡 Result:
In the Lightricks LTX-2 Community License Agreement, the term Control is defined as the direct or indirect ownership of more than fifty percent (50%) of the voting securities or other ownership interests, or the power to direct the management and policies of such entity through voting rights, contract, or otherwise [1][2]. While some secondary sources or specific document variants may inconsistently reference fifty percent or more [3][4], the primary license definition utilized in the core LTX-2 documentation explicitly specifies more than fifty percent (50%) [5][1][2][6].
Citations:
- 1: https://huggingface.co/Lightricks/LTX-2/blob/7bdc1de834d7a9eaa469f7f1ac93c2b3713fa866/LICENSE
- 2: https://huggingface.co/api/resolve-cache/models/Lightricks/LTX-2/1bbf5277f05848df49f89e192328576c61d0268a/LICENSE?download=true&etag=%2204f2b7901d4ff8ef487433034675638409bc15d7%22
- 3: https://huggingface.co/Lightricks/LTX-2/blob/main/LICENSE
- 4: https://github.com/Lightricks/LTX-Desktop/blob/main/NOTICES.md
- 5: https://github.com/Lightricks/LTX-2/blob/main/LICENSE
- 6: https://scancode-licensedb.aboutcode.org/ltx-2-cla-2026.html
Restore the upstream Control threshold.
Control should stay “more than fifty percent (50%)”; changing it to “50% or more” expands affiliate aggregation and Commercial Entity scope to exactly 50%-owned entities.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@experiments/ltx2-nvfp4/LICENSE` around lines 14 - 17, Restore the `Control`
definition in the license text to require ownership of more than fifty percent
(50%), rather than fifty percent or more. Preserve the remaining ownership and
management-control language unchanged.
| Commercial Entities interested in such a commercial license are | ||
| required to contact Licensor. Any commercial use of LTX-2 or |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Provide an actionable commercial-licensing contact.
Commercial Entities are required to contact Licensor, but this standalone license provides no address or URL. Add a stable licensing contact so the stated commercial-use path is actionable. The upstream license includes a licensing link. (github.com)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@experiments/ltx2-nvfp4/LICENSE` around lines 85 - 86, Add the upstream
licensing URL or another stable contact immediately after the commercial-use
requirement in the LICENSE text, preserving the existing license wording and
making the required Licensor contact actionable.
| except Exception: | ||
| return {} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
_load_jobs silently swallows all exceptions, risking undetected job-store corruption.
If JOB_STORE contains invalid JSON, the except Exception returns {} and the next _put overwrites the file with only the new job — all prior job mappings are lost silently. At minimum, log the error so an operator can investigate.
🛡️ Proposed fix
def _load_jobs() -> dict[str, Any]:
if JOB_STORE.exists():
try:
return json.loads(JOB_STORE.read_text())
- except Exception:
+ except Exception as e:
+ print(f"WARNING: job store corrupted, starting fresh: {e}", file=sys.stderr)
return {}
return {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except Exception: | |
| return {} | |
| except Exception as e: | |
| print(f"WARNING: job store corrupted, starting fresh: {e}", file=sys.stderr) | |
| return {} |
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 50-50: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/video-bridge/bridge.py` around lines 50 - 51, Update _load_jobs so its
broad exception handler logs the caught error before returning {}, using the
module’s existing logging mechanism and preserving the current fallback
behavior; do not silently discard job-store load failures.
| path = OUTPUT_DIR / out["subfolder"] / out["filename"] | ||
| if path.exists(): | ||
| data = path.read_bytes() | ||
| else: | ||
| r = await _client.http.get(f"{COMFY_URL}/view", | ||
| params={"filename": out["filename"], | ||
| "subfolder": out["subfolder"], "type": out["type"]}) | ||
| r.raise_for_status() | ||
| data = r.content |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Path traversal in /content endpoint via unsanitized ComfyUI output metadata.
out["subfolder"] and out["filename"] come from ComfyUI's /history response and are used directly in path construction. A compromised or misconfigured ComfyUI could return ../../etc/passwd as a filename, causing the bridge to serve arbitrary files. Resolve the path and verify it stays within OUTPUT_DIR.
🔒 Proposed fix
# read off local disk (co-located); fall back to ComfyUI /view
path = (OUTPUT_DIR / out["subfolder"] / out["filename"]).resolve()
- if path.exists():
+ if not str(path).startswith(str(OUTPUT_DIR.resolve())):
+ raise HTTPException(403, "output path outside allowed directory")
+ if path.exists():
data = path.read_bytes()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| path = OUTPUT_DIR / out["subfolder"] / out["filename"] | |
| if path.exists(): | |
| data = path.read_bytes() | |
| else: | |
| r = await _client.http.get(f"{COMFY_URL}/view", | |
| params={"filename": out["filename"], | |
| "subfolder": out["subfolder"], "type": out["type"]}) | |
| r.raise_for_status() | |
| data = r.content | |
| path = (OUTPUT_DIR / out["subfolder"] / out["filename"]).resolve() | |
| if not str(path).startswith(str(OUTPUT_DIR.resolve())): | |
| raise HTTPException(403, "output path outside allowed directory") | |
| if path.exists(): | |
| data = path.read_bytes() | |
| else: | |
| r = await _client.http.get(f"{COMFY_URL}/view", | |
| params={"filename": out["filename"], | |
| "subfolder": out["subfolder"], "type": out["type"]}) | |
| r.raise_for_status() | |
| data = r.content |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/video-bridge/bridge.py` around lines 200 - 208, Validate the ComfyUI
metadata before reading the local file in the `/content` handling flow: resolve
the path built from `OUTPUT_DIR`, `out["subfolder"]`, and `out["filename"]`,
then ensure it remains within the resolved `OUTPUT_DIR`; reject or bypass unsafe
traversal paths rather than serving them. Keep the existing ComfyUI HTTP
fallback only for valid paths.
|
|
||
| # node ids in ltx2-t2v.json (ComfyUI API format) | ||
| N_POS, N_NEG, N_LATENT, N_FRAMES, N_NOISE = "2483", "2612", "3059", "4979", "4832" | ||
| DEFAULT_FPS = 30 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
FPS default mismatch: DEFAULT_FPS=30 doesn't match the workflow template's 24.0 fps, and fps is never injected into the workflow. Frame counts are calculated at 30 fps but the video renders at 24 fps, causing all default-duration requests to produce clips ~19–28% longer than requested. The README documents "default 30", perpetuating the mismatch.
infra/video-bridge/inject.py#L19: ChangeDEFAULT_FPSfrom 30 to 24 to match the template's node 4978 value.infra/video-bridge/inject.py#L52: AddN_FPS = "4978"and injectwf[N_FPS]["inputs"]["value"] = float(fps)so custom fps values fromextra_bodypropagate to the actual render.infra/video-bridge/README.md#L46-47: Update "default 30" to "default 24" after the code fix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/video-bridge/inject.py` at line 19, Align the video bridge’s FPS
handling with the workflow template: change DEFAULT_FPS from 30 to 24, add the
N_FPS identifier for node 4978, and set that node’s inputs.value from the
resolved fps in the injection flow so extra_body overrides reach rendering.
Update the README’s documented default from 30 to 24.
| def parse_size(size: str | None) -> tuple[int, int]: | ||
| if not size: | ||
| return 1280, 704 | ||
| w, h = size.lower().split("x") | ||
| return int(w), int(h) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
parse_size lacks input validation.
Malformed size strings ("abc", "1024x768x480", "0x0") produce unhelpful ValueError or unpack tracebacks that surface as 500s in the bridge. Add basic validation.
🛡️ Proposed fix
def parse_size(size: str | None) -> tuple[int, int]:
if not size:
return 1280, 704
- w, h = size.lower().split("x")
- return int(w), int(h)
+ try:
+ w, h = size.lower().split("x")
+ w, h = int(w), int(h)
+ except (ValueError, AttributeError):
+ raise ValueError(f"size must be WxH, got {size!r}")
+ if w <= 0 or h <= 0:
+ raise ValueError(f"size dimensions must be positive, got {w}x{h}")
+ return w, h📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def parse_size(size: str | None) -> tuple[int, int]: | |
| if not size: | |
| return 1280, 704 | |
| w, h = size.lower().split("x") | |
| return int(w), int(h) | |
| def parse_size(size: str | None) -> tuple[int, int]: | |
| if not size: | |
| return 1280, 704 | |
| try: | |
| w, h = size.lower().split("x") | |
| w, h = int(w), int(h) | |
| except (ValueError, AttributeError): | |
| raise ValueError(f"size must be WxH, got {size!r}") | |
| if w <= 0 or h <= 0: | |
| raise ValueError(f"size dimensions must be positive, got {w}x{h}") | |
| return w, h |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/video-bridge/inject.py` around lines 34 - 38, Update parse_size to
validate the size format before unpacking or converting values: require exactly
one “x”, ensure both dimensions are valid positive integers, and raise a clear
input-validation error for malformed or zero-sized values while preserving the
existing default for missing size.
| - **`seconds` → frames** snaps to LTX's `8n+1` at `fps` (default 30). Verify the mapping matches | ||
| intended clip length on real requests. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update fps default documentation after fixing the inject.py mismatch.
Line 46 documents "default 30" which matches the current DEFAULT_FPS but not the template's actual 24.0 fps. Once DEFAULT_FPS is corrected to 24 (see inject.py review), update this line accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/video-bridge/README.md` around lines 46 - 47, Update the fps default in
the README’s seconds-to-frames documentation from 30 to 24 after correcting
DEFAULT_FPS in inject.py, keeping the surrounding LTX 8n+1 mapping guidance
unchanged.
1 participant
Ships the LTX-2.3 video-generation lane stood up this session on the Blackwell node — a published NVFP4 model, a video-LoRA studio, and the OpenAI-compatible video bridge for protoDirector. All new files (3 dirs), no changes to existing code.
1.
experiments/ltx2-nvfp4/— LTX-2.3-22B → NVFP4 (LIVE on HF)Quantizer for the LTX-2.3-22B video DiT to NVFP4 on sm120. Lightricks ships fp4 only for the 19B; the 2.3-22B is bf16-only — this fills the gap. Replicates their exact mixed-precision layer policy (fp4 transformer_blocks 1-42; keep block 0 + last 5 + gates + VAE/vocoder bf16) and stamps the
_quantization_metadataheader ComfyUI's loader needs.60→37 GB peak VRAM. Distilled-decode = visually unchanged; full-decode shows mild fp4 artifacting → ship distilled.2.
experiments/ltx2-lora/— video-LoRA studioMake + test LoRAs on the NVFP4 base end-to-end.
make_video_lora.py— one command, resumable: caption → preprocess → fix-conditions (the embeds-land-next-to-clips gotcha) → config → train → wire (symlink LoRA into ComfyUI + emit a T2V workflow withLoraLoaderModelOnlyon the fp4 base).app.py— Gradio studio (LTX-2 venv, :7862): Train tab (clips → LoRA, streaming log) + Generate & Compare tab (pick a LoRA, generate a video inline, A/B base-vs-LoRA at matched seed, reference-image → I2V first-frame conditioning with a strength anchor).3.
infra/video-bridge/— OpenAI/v1/videosover ComfyUI+LTX (protoBanana#38 piece 2)Standalone async video-jobs bridge co-located with ComfyUI — the shape the team agreed on (litellm's native
/v1/videosrouter shadows passthrough routes, so the job layer sits in front of the gateway). Implements the AGREED 3-URL contract (POST → job id, GET status/progress, GET /content mp4), restart-survivable job store, bytes off local disk, reuses protoBanana'sComfyUIClient(inherits the #39 nonce fix). Verified: POST→poll→mp4. Consumer = protoDirector; deploy tracked in homelab-iac#193/#194.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests