diff --git a/.github/codeql/dw-security/DwPathSanitizers.qll b/.github/codeql/dw-security/DwPathSanitizers.qll index a2891aec..acd4648b 100644 --- a/.github/codeql/dw-security/DwPathSanitizers.qll +++ b/.github/codeql/dw-security/DwPathSanitizers.qll @@ -44,7 +44,8 @@ private predicate pathValidatorName(string name) { name = [ "validate_path", "validate_workflow_path", "validate_output_path", - "validate_prompt_path", "safe_join_path", "validate_media_path" + "validate_prompt_path", "validate_lora_path", "safe_join_path", + "validate_media_path" ] } @@ -56,7 +57,8 @@ private predicate pathValidatorName(string name) { private predicate nameValidatorName(string name) { name = [ - "validate_workspace_name", "validate_prompt_reference", "validate_asset_reference", + "validate_workspace_name", "validate_prompt_reference", "validate_lora_name", + "validate_asset_reference", "validate_output_reference", "validate_variable_name", "validate_commit_hash" ] } diff --git a/.github/codeql/extensions/dw-models/models/dw-security.model.yml b/.github/codeql/extensions/dw-models/models/dw-security.model.yml index 26e9a32f..d792a20e 100644 --- a/.github/codeql/extensions/dw-models/models/dw-security.model.yml +++ b/.github/codeql/extensions/dw-models/models/dw-security.model.yml @@ -10,6 +10,7 @@ extensions: - ["dw.security", "Member[validate_workflow_path].ReturnValue", "path-injection"] - ["dw.security", "Member[validate_output_path].ReturnValue", "path-injection"] - ["dw.security", "Member[validate_prompt_path].ReturnValue", "path-injection"] + - ["dw.security", "Member[validate_lora_path].ReturnValue", "path-injection"] # The name validators are regex whitelists that raise InvalidInputError: # a workspace name is one path segment (^[\w][\w.-]*\Z - no separator, @@ -20,4 +21,5 @@ extensions: - ["dw.security", "Member[validate_asset_reference].ReturnValue", "path-injection"] - ["dw.security", "Member[validate_output_reference].ReturnValue", "path-injection"] - ["dw.security", "Member[validate_prompt_reference].ReturnValue", "path-injection"] + - ["dw.security", "Member[validate_lora_name].ReturnValue", "path-injection"] - ["dw.security", "Member[validate_variable_name].ReturnValue", "path-injection"] diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a0ca65f2..01717496 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,7 +42,7 @@ jobs: # goes on afterwards so the resolver doesn't replace it with a # release run: | - pip install -r requirements.txt -r requirements-test.txt + pip install -c constraints-openapi.txt -r requirements.txt -r requirements-test.txt pip install git+https://github.com/huggingface/diffusers - name: Format check run: ruff format --check dw dw_mcp tests scripts @@ -68,10 +68,19 @@ jobs: cache: npm cache-dependency-path: ui/package-lock.json - run: npm ci + # The UI's response types are generated from the server's OpenAPI + # document (scripts/dump_openapi.py writes it; the backend job's + # tests/test_api_contract.py fails when it is stale). A stale + # api-schema.ts fails here; a field the UI reads that the server no + # longer sends fails the type check below + - name: Response contract + run: npm run gen:api && git diff --exit-code src/lib/generated - name: Format check - run: npx prettier --check src e2e *.ts *.js + run: npx prettier --check src e2e scripts *.ts *.js - name: Lint run: npm run lint + - name: Architecture ratchet + run: npm run metrics -- --check ../docs/stabilization/ui/baseline.json - name: Type check run: npm run check - name: Unit tests @@ -80,10 +89,17 @@ jobs: run: npm run build e2e: - # Playwright against a real dw.serve, on PRs into master only - the - # release PR is where a stale spec has to be caught, and the job carries - # the backend install as well as the browser - if: github.event_name == 'pull_request' && github.base_ref == 'master' + # Playwright against a real dw.serve before develop moves - on PRs into + # develop or master, and on pushes to develop, since the agent loop + # pushes develop directly with no PR. The release PR (this repo's + # develop into master) is skipped: its every commit is a develop push, + # already run. The job carries the backend install as well as the browser + if: >- + (github.event_name == 'pull_request' && + (github.base_ref == 'develop' || github.base_ref == 'master') && + !(github.head_ref == 'develop' && + github.event.pull_request.head.repo.full_name == github.repository)) || + (github.event_name == 'push' && github.ref == 'refs/heads/develop') runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 @@ -100,7 +116,7 @@ jobs: run: pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu - name: Install dependencies run: | - pip install -r requirements.txt -r requirements-test.txt + pip install -c constraints-openapi.txt -r requirements.txt -r requirements-test.txt pip install git+https://github.com/huggingface/diffusers - uses: actions/setup-node@v7 with: diff --git a/README.md b/README.md index 87af6fba..7134cc23 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ once; without it the run fails partway through with a 401/403 from the Hub. ## Drive it from an agent -Then just ask. The agent has 59 tools covering the whole surface — the +Then just ask. The agent has 62 tools covering the whole surface — the workflow catalog, the real diffusers pipeline signatures, the job queue, the gallery, the model cache: diff --git a/constraints-openapi.txt b/constraints-openapi.txt new file mode 100644 index 00000000..8c55ba88 --- /dev/null +++ b/constraints-openapi.txt @@ -0,0 +1,8 @@ +# The FastAPI and Pydantic that ui/src/lib/generated/openapi.json is +# generated under. Either one's release can change the document with no +# change here, so CI installs these exactly (pip -c) and +# scripts/dump_openapi.py refuses to write under anything else. To move +# them: bump both lines, install them, run the dump and `npm run gen:api`, +# and commit the four files together. +fastapi==0.142.2 +pydantic==2.13.5 diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ac9a34eb..07281392 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -49,6 +49,8 @@ one to open. | Reference name shape | `dw/reference_names.py`: `reference_name_errors` | The shape of every `asset:`, `prompt:` and `output:` name is checked before the queue, and existence is checked later. | `tests/test_reference_names.py::TestTheValidationPass::test_a_malformed_reference_is_refused_before_the_queue` | | Library reads | `dw/library.py`: `LibraryPath` | Reads go through the roots front to back, so an earlier name shadows a later one. | `tests/test_library_path.py::TestResolution::test_the_front_of_the_path_shadows_the_rest` | | Library writes | `dw/library.py`: `LibraryPath` | Writes go only to the front root, so saving something opened from a read-only root writes a copy. | `tests/test_library_path.py::TestConstruction::test_the_writable_root_comes_first`, `tests/test_library_sources.py::TestServer::test_saving_an_example_prompt_writes_a_copy` | +| LoRA catalog matching | `dw/lora_catalog.py`: `matches`, `workflow_bases` | An entry fits a base only by exact repo id (and H3 partition); no alias or family match. | `tests/test_lora_catalog.py::TestMatching::test_the_base_must_match_exactly` | +| Hub search | `dw/lora_hub.py`: `search_hub` | The Hub is searched only by `GET /api/loras/recommend`, never raises, and offers no pickle-only repo. | `tests/test_lora_hub.py::TestCandidates::test_a_pickle_only_repo_is_dropped`, `tests/test_lora_hub.py::TestFailure::test_a_hub_error_is_returned_not_raised` | | Packaged builtins | `dw/library.py`: `builtin_root`, `resolve_sub_workflow_reference` | `builtin:` names the packaged `dw/workflows/`, not the top-level `workflows/` examples. | `tests/test_sub_workflow_resolver.py::TestTheResolver::test_a_builtin_resolves_in_the_packaged_root` | | Sub-workflow paths | `dw/library.py`: `resolve_sub_workflow_reference`, `resolve_sub_workflow` | A sub-workflow path is confined to the root it resolves in. The search order is in the docstring of `resolve_sub_workflow`. | `tests/test_sub_workflow_resolver.py::TestTheResolver::test_a_path_escaping_its_root_is_refused` | | Workspace | `dw/workspace.py`: `resolve_workspace`, `set_workspace` | `--workspace` beats `DW_WORKSPACE`, which beats the setting, which beats a working directory that looks like a workspace, which beats `~/diffusers-workspace`. | `tests/test_workspace.py::TestResolution::test_a_flag_wins_over_everything`, `tests/test_workspace.py::TestResolution::test_a_bare_working_directory_falls_back_to_the_home_workspace` | @@ -99,6 +101,7 @@ one to open. | Worker protocol | `dw/worker_protocol.py`: `parse_reply` | Every command and reply is a frozen dataclass that travels as a wire dict. | `tests/test_worker_messages.py::test_from_wire_inverts_to_wire`, `tests/test_worker_messages.py::test_an_unknown_reply_type_is_kept_whole_rather_than_raised` | | Persistent worker | `dw/worker.py`, `dw/worker_manager.py`, `dw/serve.py` | Jobs run in one spawned worker that keeps models loaded, so a change to engine code needs a server restart. | — | | Failed-run reporting | `dw/worker.py`, `dw/worker_protocol.py`: `Failed`, `Cancelled` | A failed or cancelled run's reply still carries the manifest of the steps that ran. | `tests/test_worker_execute.py::test_failure_carries_the_manifest_of_the_steps_that_ran`, `tests/test_worker_execute.py::test_cancellation_carries_the_manifest_too` | +| Failure path redaction | `dw/path_redaction.py`: `redact_paths`, called by `dw/worker.py` | A failed run's message and traceback name a file under the job's asset roots or output directory by its `asset:`/`output:` reference, never its absolute server path. | `tests/test_worker_execute.py::test_a_failure_names_an_asset_by_reference_not_by_server_path`, `tests/test_path_redaction.py` | | Run-time warnings | `dw/events.py`: `emit_warning` | A warning found at run time is emitted as an event, so it reaches the caller and not just the log. | `tests/test_concat_videos.py::TestWarningsReachTheCaller::test_the_level_spread_warning_is_emitted_as_an_event` | ## Media and DSP @@ -140,7 +143,13 @@ one to open. | `dw_mcp` stays torch-free | `dw_mcp/` | `dw_mcp` reaches `dw.serve` over HTTP and imports no `dw` module, because `dw/__init__.py` pulls in torch. | `tests/test_mcp_server.py::TestStartupWeight::test_the_server_starts_without_importing_the_engine` | | Tool surface | `dw_mcp/server.py`, `dw_mcp/tools_*.py` | Only these modules import the MCP SDK, and each tool body is a one-line call into a handler. | `tests/test_mcp_server.py::test_the_wiring_table_covers_every_registered_tool`, `tests/test_mcp_server.py::test_the_stated_tool_count_is_the_registered_one` | | Surface text budget | the tool docstrings in `dw_mcp/tools_*.py`, the instructions in `dw_mcp/server.py` | The instructions and each tool description stay at or under 2,048 characters, and the whole surface stays within its token budget. | `tests/test_mcp_server.py`: `SURFACE_BUDGET`, `CLIENT_TEXT_LIMIT`; `tests/test_mcp_server.py::test_no_text_the_agent_reads_is_cut_off_by_the_client`, `tests/test_mcp_server.py::test_the_tool_surface_fits_the_budget` | -| Spending needs consent | `dw_mcp/diagnose.py` | `run_workflow` and `rerun_job` refuse until `acknowledged_cost` is set. | `tests/test_mcp_diagnose.py::test_run_refuses_without_an_acknowledged_cost`, `tests/test_mcp_diagnose.py::test_rerun_refuses_without_an_acknowledged_cost` | +| Spending needs consent | each handler behind a tool that takes `acknowledged_cost` (`dw_mcp/diagnose.py`, `models.py`, `prompts.py`, `workspaces.py`) | Every tool that takes `acknowledged_cost` refuses until it is set; `delete_workspace`'s refusal is the server's 409. | `tests/test_mcp_twins.py::test_every_tool_that_takes_acknowledged_cost_refuses_without_it` | +| Copies of engine rules | the constants and word lists in `dw_mcp/` and `dw/run.py` | `dw_mcp` cannot import its owners, so each copy equals its owner; a copy that needs no twin is deleted, not pinned. | `tests/test_mcp_twins.py` | +| The CLI is a client of the server | `dw/run.py` | `dw.run` queues on `dw.serve` through `dw_mcp.client`, so `dw` imports `dw_mcp` and never the reverse. | `tests/test_mcp_server.py::TestStartupWeight::test_the_server_starts_without_importing_the_engine`, `tests/test_mcp_twins.py` | +| Inline images | `dw/server/inline_media.py` | Fitting an image to a longest side and halving it under a base64 budget happens on the server, for `/image` and `/frames` alike; `dw_mcp` forwards the size and budget and imports no Pillow. | `tests/test_server_gallery_image.py`, `tests/test_mcp_server.py::TestStartupWeight::test_the_server_starts_without_importing_the_engine` | +| Workflow patch | `dw/library.py`: `merge_patch`; `dw/server/routes/library.py`: `patch_workflow` | A merge patch is applied on the server under the lock `PUT` takes; no client reads, merges and writes back. | `tests/test_server_library_path.py::TestPatchWorkflow::test_patch_merges_onto_the_stored_definition` | +| Deleting a job's run | `dw/server/jobs.py`: `JobManager.run_location`; `dw/server/outputs.py`: `delete_run_directory` | The server resolves a job's run directory against the root the job ran in, and refuses a job still queued or running. | `tests/test_server_jobs.py::TestDeleteAJobsRun::test_a_running_job_is_409` | +| Level findings | `dw/server/assess.py`: `level_findings` | Gallery metadata reports level problems from `dw/audio_qc.py`'s thresholds; no client restates a threshold. | `tests/test_server_assess.py::TestMetadataLevelFindings::test_level_findings_read_audio_qc_thresholds` | | API errors | `dw_mcp/client.py` | An API failure becomes a message a person can act on, here and nowhere else. | `tests/test_mcp_client.py::test_a_400_surfaces_the_servers_detail_verbatim` | ## UI @@ -148,3 +157,16 @@ one to open. | Concept | Owner | Rule | Enforced by | | --- | --- | --- | --- | | The UI reads engine fields | `ui/src/lib/plan.ts`: `describePlan`, `ui/src/lib/results.ts`: `sectionBySubfolder` | The UI reads `plan`, `version` and `subfolder` as fields the server sends and derives nothing of its own, and it never sends `acknowledged_cost`. | `ui/src/lib/plan.test.ts`, `ui/src/lib/results.test.ts` | +| Reference prefixes in the UI | `ui/src/lib/references.ts` | The only module in `ui/src` that spells a reference prefix; each equals `dw/references.py`'s. | `tests/test_ui_twins.py::test_the_ui_spells_every_reference_prefix_the_engine_does`; `prefix_literals` in `ui/scripts/arch-metrics.mjs` | +| Output kinds on the job page | `dw/server/outputs.py`: `MEDIA_KINDS`, `output_kinds` | The page renders an output by the `output_kinds` the job detail carries, never by its extension. | `tests/test_server.py::test_a_job_reports_each_output_files_media_kind`, `ui/src/lib/pages/JobPage.test.ts` | +| The editor's live reference check | `ui/src/lib/flow.ts`: `danglingReferenceDetails`, owned by `dw/previous_results.py` and `dw/for_each.py` | The editor warns as the author types, so it keeps a copy of the reference rules; one case file is run through both, the engine deciding each case. | `tests/test_ui_twins.py::test_the_engine_decides_each_shared_reference_case`, `ui/src/lib/flow.test.ts` | +| Name segments in the UI | `ui/src/lib/names.ts`: `isNameSegment` | Workspaces, folders and stored names follow `dw/security.py`'s `WORKSPACE_NAME_PATTERN` and length cap, counted in code points. | `tests/test_ui_twins.py::test_the_engine_decides_each_shared_workspace_name_case`, `ui/src/lib/names.test.ts` | +| UI copies of engine rules | the constants `tests/test_ui_twins.py` reads | A list the UI must hold equals its owner, read from the TS source; a rule tested on both sides reads one case file in `tests/fixtures/`. | `tests/test_ui_twins.py` | +| UI overlays and suggestions | `ui/src/lib/ui/`: `ConfirmDialog`, `Modal`, `Popover`, `Suggest` | Bits UI is imported only here; pages and components use the wrappers, which own focus, Escape, outside presses and scroll lock. Overlay styles and the stacking scale (`--layer-*`) live in `ui/src/app.css`. | `ui/eslint.config.js` (`no-restricted-imports`), `ui/src/lib/ui/*.test.ts`, `ui/e2e/chrome.spec.ts` | +| Escape on a page under an overlay | `ui/src/lib/ui/layers.svelte.ts`: `holdLayer`, `overlayOpen` | Every open overlay holds a layer; a page's own Escape handling acts only when none is open. | `ui/src/lib/pages/GalleryPage.test.ts` (an Escape that closes a confirm leaves the selection) | +| The editors' document | `ui/src/lib/editorShell.svelte.ts`: `DocumentEditor` | The workflow and prompt editors keep one rule each for the view (remembered per editor), the JSON draft (a failed parse pins it), dirty against the last load or save, the save path, and the tab-close guard. | `ui/src/lib/editorShell.test.ts`, `ui/src/lib/pages/PromptEditorPage.test.ts`, `ui/src/lib/pages/EditorPage.test.ts` | +| UI polling | `ui/src/lib/poll.ts`: `poll`, `sleep` | A page that refreshes on a timer starts it through `poll` and stops it with the function `poll` returns. | `ui/src/lib/poll.test.ts` | +| Workspace state in the UI | `ui/src/lib/workspaceState.svelte.ts`: `workspace` | The current workspace is one `$state`; `api.ts` reads it from here, so the request layer imports no page state. | `import_cycles` in `ui/scripts/arch-metrics.mjs` | +| The UI's response contract | `dw/server/api_models.py`: `ApiModel`, `send_rejected_responses`; `scripts/dump_openapi.py` | Every JSON route `api.ts` calls declares a response model, and `types.ts` re-exports the types generated from it (`ui/src/lib/generated/`). The payload does not change: an absent key stays absent (`response_model_exclude_unset`), a sometimes-sent key is `sometimes()`. Runtime is lenient - an undeclared key passes and a rejected response is logged and sent as built; tests, the e2e server and the dump run strict (`DW_STRICT_RESPONSES=1`). The document is generated under the FastAPI and Pydantic `constraints-openapi.txt` pins: CI installs them, and the dump refuses to write under others. | `tests/test_api_contract.py`, `tests/test_ui_twins.py::test_every_json_route_the_ui_calls_declares_its_response`, CI's "Response contract" step | +| Where `--live` appears | `ui/src/app.css` (its header) | The state colour marks machine state only, in the files `ui/scripts/design-rules.test.ts` lists. | `ui/scripts/design-rules.test.ts` | +| UI architecture ratchet | `ui/scripts/arch-metrics.mjs` | No metric rises above `docs/stabilization/ui/baseline.json`. | the `ui` job in `.github/workflows/ci.yml`; `npm run preflight` | diff --git a/docs/LORAS.md b/docs/LORAS.md index 79269366..c08a8c04 100644 --- a/docs/LORAS.md +++ b/docs/LORAS.md @@ -129,3 +129,69 @@ python -m dw.run workflow.json lora="other-user/other-lora" - [lora.json](../workflows/templates/lora.json) — Flux with realism LoRA and variables - [lora.json](../workflows/templates/lora.json) — SD 3.5 with yarn art style LoRA - [lora.json](../workflows/templates/lora.json) — A LoRA adapter on FLUX.1 dev, with the SD 3.5 variant in its description + +## LoRA catalog + +The catalog records LoRAs that were tried on a base model: `proven`, `trial`, +or `rejected` with the reason. It is a library like the prompt library - the +server's own `loras/` at the workspace root (writable, shared by every +workspace), then the `loras/` an examples tree brings (read-only). One JSON +file per LoRA, under a family folder: `loras/qwen-image/voxel-style.json`. + +```json +{ + "model_name": "fal/MiniMax-H3-Realism-People-LoRA", + "weight_name": "h3-realism-people-t2v-i2v-r2v.safetensors", + "revision": "", + "base_models": ["MiniMaxAI/MiniMax-H3"], + "workflow": "t2va", + "description": "Realistic people and faces", + "use_when": "People, crowds or faces should look photoreal", + "scale": { "default": 0.7, "range": [0.7, 0.7] }, + "status": "proven", + "evidence": [{ "issue": 585, "note": "Best arm of the 2026-10-03 eval" }] +} +``` + +`model_name`, `weight_name`, `revision` and `scale.default` drop straight into +a step's `loras` entry. `base_models` holds exact repo ids, and matching is +exact - an adapter on the wrong base usually loads and is quietly worse. +`workflow` constrains a MiniMax-H3 entry to a partition (`t2va`, `fl2va`, +`ref2va`) - a string, or a list of them for an adapter that covers several +(the turbo keyframe files are `["t2va", "fl2va"]`). `proven` and `rejected` entries need `evidence`; a rejected +entry's first note is why. The schema is `GET /api/lora-schema`. + +Promotion: a trial that worked is saved with `save_lora` (or +`PUT /api/loras/{name}`) as `proven`, its job id in `evidence`. + +## Finding LoRAs on the Hub + +`recommend_loras(model, query)` (`GET /api/loras/recommend`) is the only +place dw searches the Hub, and only when called. It returns the catalog's +entries for the model first, ranked against the query, then Hub adapters +whose card declares that exact base (`base_model:adapter:`), most +downloaded first. Nothing is downloaded; the weight file's header is read to +check its layout, for a single-weight repo only. Hub rows are candidates to trial, never recommendations, +and carry `warnings`: + +| Warning | Meaning | +| --- | --- | +| `will_not_load` | Full-weight `.diff` keys diffusers' converters refuse | +| `unknown_format` | The tensor names match no known LoRA layout | +| `header_unreadable` | The header range read failed; format unchecked | +| `multiple_weights` | Several `.safetensors`; pick one from `weights` | +| `gated` | Needs the server's HF token to have accepted the gate | +| `no_license` | The card declares none | +| `stale` | Last changed before its base was - trained on an older revision | + +A repo holding only pickle `.bin` weights is never offered. A repo the +catalog marks `rejected` comes back in `hub` as `rejected` with the reason +only when the Hub search turns it up; rejected entries are left out of +`catalog` (`list_loras` still shows them). When the +Hub is unreachable the catalog rows still come back, with `hub_error`. + +Limits: the search runs the typed query plus at most 4 of its words; +`query` is capped at 200 characters and `limit` is 1-25. One Hub search runs +at a time per server, and a concurrent call gets catalog rows plus +`hub_error` saying a search is already running. An empty `hub` with +`hub_error` set means the search failed, not that no adapters exist. diff --git a/docs/MCP.md b/docs/MCP.md index 2985e52f..3e8c4eeb 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -187,7 +187,7 @@ Nothing in this sequence costs GPU time. ## Tool reference -59 tools in six groups. Names and arguments below are transcribed from +62 tools in six groups. Names and arguments below are transcribed from `dw_mcp/tools_*.py` — nothing here is renamed or reshaped for the docs. ### Catalog (read-only) @@ -227,7 +227,7 @@ when no single workflow covers it. | `get_server_info()` | — | What this installation can do and where it keeps things: `device` (the accelerator a run will use), `version`, the `workspace` this session is working in and the workflow/asset/output/prompt `directories` of *that* workspace, the bind address and port, whether a token is required, and whether MCP is mounted. Check the device before authoring - a CUDA-only choice (bitsandbytes, `torch.compile`, flash attention) is not available on an `mps` or `cpu` server. `runtime` (#222) reports the Python version, torch version and the CUDA version torch was built against, the NVIDIA driver version (when `nvidia-smi` is reachable), and the installed versions of diffusers, transformers, accelerate, bitsandbytes, peft, safetensors and sentencepiece (`null` for one not installed) - for diagnosing an environment mismatch between boxes without shelling in | | `list_jobs(limit=20, status=None, workspace=None)` | optional `limit` (newest N), `status` (one state or a comma-separated set of `queued`, `running`, `succeeded`, `failed`, `cancelled`), `workspace` | List queued, running and recent jobs, **newest first**. Bounded by default: the unbounded listing was over a client's tool-result limit on a server with a few months of history, which made it a tool that could not be called at all. `total` says how many matched and `truncated`/`next` say so when the answer was cut - raise `limit` or narrow with `status`. Without `workspace`, a named workspace lists its own jobs and the default one lists every job the server holds | | `list_gallery(limit=50, subfolder=None, only_orphans=False, workspace=None, folder=None, version=None)` | `limit`, `subfolder`, `only_orphans`, `workspace`, `folder`, `version` | List generated output files, newest first. A name is `//`, where `` may sit in the subfolder the step chose (`final/episode.mp4`); each entry carries `folder` (the workflow) and `subfolder` (by convention `final` or `intermediate`, `''` when the step chose none, any path the workflow wrote otherwise), and `subfolder="final"` lists only deliverables. Each entry carries `run_id` and `version` - that run's ordinal among the workflow's runs, which is how one of several runs that wrote the same basename is named to a person: the web UI labels the same file `v5`. The number is assigned when the run opens and never renumbered, so deleting a run leaves a gap rather than sliding the rest down (a failed run, or a rerun that reused every step, leaves one too - it took a number and may have nothing to list), and it is `null` under the flat output layout, which has no runs. `folder` with `version` lists that one run's files, and `output:/v5/` names one in a workflow; every other tool takes `name`. Each entry also carries a ready-made `url`, already scoped to the workspace that made it - a hand-built `/outputs/` URL 404s for anything but the default workspace. `only_orphans=True` inverts the call: instead of files, it returns run directories with no media anywhere under them (`runs`, each `{name, mtime}`) - a run whose output was deleted before `delete_output` could remove it by name, or one that failed before writing anything; `subfolder` does not apply in this mode, and `name` is exactly what `delete_output` accepts (#170). `workspace` names the workspace for this one call without switching the session to it - the same pin `run_workflow` takes, so a job run into another workspace stays reachable from the session that queued it | -| `get_gallery_metadata(name, envelope=False, workspace=None)` | `name`, `workspace` | Get the metadata embedded in a generated file — or, when `name` is an `asset:` reference, what an *input* asset holds (`source` says which; `job` is null for an asset). Reading an input's duration, frame count, fps and sample rate before a run is how a caller learns the `total_frames`, `fps` and `sample_rate` a workflow expects it to supply: the exact workflow and arguments that produced it, and, for audio/video, a `media` block (duration, rate, channels, fps, size, peak/mean dBFS). Only an image (PNG/JPEG/WebP) embeds `metadata` this way — it is always null for audio and video, and `next` then points at `get_job_workflow(job_id)` when `job` is known, or says a kept asset carries no provenance at all when it isn't. `envelope=true` adds `media.envelope` — `rms_dbfs` and `peak_dbfs` one entry per second — which is what locates something in a track rather than measuring the whole of it. `media.shots` is set on an output joined from shots: one `{name, start_frame, num_frames, start_sample, num_samples}` per shot, as the join measured them (see the run manifest in docs/WORKFLOW_GUIDE.md), else null. `workspace` names the workspace for this one call without switching the session to it - the same pin `run_workflow` takes, so a job run into another workspace stays reachable from the session that queued it | +| `get_gallery_metadata(name, envelope=False, workspace=None)` | `name`, `workspace` | Get the metadata embedded in a generated file — or, when `name` is an `asset:` reference, what an *input* asset holds (`source` says which; `job` is null for an asset). Reading an input's duration, frame count, fps and sample rate before a run is how a caller learns the `total_frames`, `fps` and `sample_rate` a workflow expects it to supply: the exact workflow and arguments that produced it, and, for audio/video, a `media` block (duration, rate, channels, fps, size, peak/mean dBFS) and `findings` - each level problem the server measured against `dw/audio_qc.py`'s thresholds (`full_scale`, `near_silent`), with the fix. Only an image (PNG/JPEG/WebP) embeds `metadata` this way — it is always null for audio and video, and `next` then points at `get_job_workflow(job_id)` when `job` is known, or says a kept asset carries no provenance at all when it isn't. `envelope=true` adds `media.envelope` — `rms_dbfs` and `peak_dbfs` one entry per second — which is what locates something in a track rather than measuring the whole of it. `media.shots` is set on an output joined from shots: one `{name, start_frame, num_frames, start_sample, num_samples}` per shot, as the join measured them (see the run manifest in docs/WORKFLOW_GUIDE.md), else null. `workspace` names the workspace for this one call without switching the session to it - the same pin `run_workflow` takes, so a job run into another workspace stays reachable from the session that queued it | ### Media @@ -239,7 +239,7 @@ when no single workflow covers it. | `get_output_text(name, max_characters=20000, workspace=None)` | `name`, `max_characters`, `workspace` | Read a text output — a prompt enhancement, or any step whose result is `text/plain` or JSON. Reports the file's real length and whether it was truncated. `workspace` names the workspace for this one call without switching the session to it - the same pin `run_workflow` takes, so a job run into another workspace stays reachable from the session that queued it | | `assess_output(name, probe=None, detail=False, workspace=None)` | `name` (a gallery name or `asset:`), `probe`, `detail`, `workspace` | Measure a finished cut on the server, without queueing: the assessment probes (`analyze_shots`, `analyze_seams`, `analyze_sync_drift`) run in the server process over one decode of the file, beside whatever job holds the GPU. Shot boundaries are the ones its run recorded (the manifest for an output, the `keep_output` sidecar for an asset). The answer merges every applicable probe's `findings` (`{rule, severity, at, value, threshold, says}`) with `rules_applied`, `rules_skipped` and `not_applicable` (`{probe: why}` - a still, a mute file, a file with no recorded shots); `detail=true` adds each probe's full body under `probes`, and `probe=` returns that one probe's full body. `probe` is checked against the three names before anything else is read, and an unknown one is refused naming them. Findings are places to look, not verdicts: drill in with `get_output_frames(seams=[n])` / `get_output_audio(start, duration)`. See *Assessing a run's output* in docs/WORKFLOW_GUIDE.md | | `download_output(name, destination=None, overwrite=False, workspace=None)` | `name`, `destination`, `overwrite`, `workspace` | Save one output file to local disk, of any content type. `destination` may be a full path or a directory; `~` expands and missing parent directories are created. `overwrite=True` is required to replace a file already at the resolved path. Over the stdio `dw-mcp`, omitting `destination` saves under the output's own name in the current working directory. Over a `dw.serve --mcp` endpoint the file lands on the server confined to that workspace (a relative path is joined onto it), and `destination` is required there - an omitted one is refused rather than dropped loose in the workspace root, where nothing can find or delete it later (#353); use the `url` `list_gallery` reports, `get_output_image`/`get_output_audio`/`get_output_frames` for inline content, or `keep_output` to make it a named asset instead. Returns nothing to the conversation but where the file landed — unlike the other media tools, the point is a file on disk, not a payload in context. Writes on the machine running the MCP server - over `dw.serve --mcp` that is the GPU box. A write that fails there (a path that exists only on the client, for instance) comes back as an error naming the server-side write and the client-side alternatives, not as an anonymous tool failure. `workspace` names the workspace for this one call without switching the session to it - the same pin `run_workflow` takes, so a job run into another workspace stays reachable from the session that queued it | -| `delete_output(name=None, workspace=None, job_id=None)` | exactly one of `name` / `job_id`, `workspace` | Permanently remove one generated file from the output directory, or - with `name` a `/` run directory, or with `job_id` - a whole run. By `job_id` the run directory is read from the job record (`run_dir`) and the reply adds `job_id` and the resolved `run_dir` to the usual `name` / `deleted` / `run_swept`; a job that never wrote a run directory, or an unknown one, is an error. `workspace` names the workspace for this one call without switching the session to it - the same pin `run_workflow` takes, so a job run into another workspace stays reachable from the session that queued it; a `job_id` delete with no `workspace` goes to the workspace the job ran in | +| `delete_output(name=None, workspace=None, job_id=None)` | exactly one of `name` / `job_id`, `workspace` | Permanently remove one generated file from the output directory, or - with `name` a `/` run directory, or with `job_id` - a whole run. By `job_id` the server reads the run directory from the job record and the root the job ran in (`DELETE /api/jobs/{id}/run`), and the reply is `job_id`, `run_dir`, `deleted` and `run_swept`; a job that never wrote a run directory, an unknown one, or one still queued or running is an error. `workspace` names the workspace for a `name` delete without switching the session to it - the same pin `run_workflow` takes, so a job run into another workspace stays reachable from the session that queued it | ### Authoring, assets and workspaces @@ -272,7 +272,7 @@ tools) rather than relying on the session pin. | `validate_workflow(workflow=None, name=None, workspace=None, arguments=None)` | exactly one of `workflow` (inline definition) or `name` (a stored workflow, as `list_workflows` reports it), optional `workspace`, optional `arguments` | Check a workflow against the schema and against real pipeline signatures. Free and instant. Validating by name uses the workflow file's own directory as the base directory, so it sees what a run would. Returns every schema violation in `errors`, each with the JSON path it sits at, so a draft is fixed in one pass, and a `previous_result:` that names no earlier step is one of them. `warnings` covers what still runs but is probably wrong - a signature mismatch, and, for a list-driven variable, an entry key no step reads, at the entry's path. `workspace` names the workspace for this one call without switching the session to it - use it to pin a job whose `output:` or `asset:` references live in a workspace other than the session's. Pass the same `arguments` you will pass to `run_workflow` and they are checked too - an undeclared or renamed variable name, a value that will not coerce to the declared type, and an `asset:`, `prompt:` or `output:` reference that names nothing this workspace can reach, each reported at `arguments.`. `checked_arguments` lists what was covered, so a `valid: true` about the stored defaults cannot be mistaken for one about your values. A reference set the model would refuse - too many images, videos or audio clips, or, for MiniMax-H3, audio as the only reference - is an error here too, rather than a failure minutes into a run you acknowledged. `run_workflow` makes the same check and refuses a bad argument rather than queuing a job that fails on its first step. A valid answer carries `plan` - the fingerprint, step count, list lengths, `downloads_required`, `estimate` (with `basis`) and `elided_steps` for the arguments given; quote from it. `steps` counts what will run: a step nothing reads and which saves no file does not run, and is named in `elided_steps` instead | | `list_workspaces()` | — | The server's workspaces and which one this session is using. Each has its own workflows, assets and outputs; the prompt library is shared by all of them | | `use_workspace(name)` | `name` | Work in that workspace for the rest of the session - every later call reads and writes there. This is how to keep your work out of another agent's namespace rather than sharing the default one. Checked against the server, so a typo fails here rather than scoping every later call to nothing. On a `dw.serve --mcp` endpoint the pin is shared by every connected client (#298, see the note above the table) - a `warning` field appears when this call overwrote a pin already pointed away from `default` | -| `create_workspace(name, use=False)` | `name`, `use` | Create a workspace. Pass use=true to switch this session to it as well; otherwise the session stays where it was and the result says so. `use=true` carries the same mounted-server caveat and `warning` field as `use_workspace` | +| `create_workspace(name, use=False)` | `name`, `use` | Create a workspace. Pass use=true to switch this session to it as well; otherwise the session stays where it was and the result says so. The result names the workspace (`name`, `default`, `current`, `next`), not its folders - `list_workspaces(detail=true)` is the opt-in for those. `use=true` carries the same mounted-server caveat and `warning` field as `use_workspace` | | `delete_workspace(name, acknowledged_cost=False)` | `name`, `acknowledged_cost` | Permanently delete a workspace and everything in it. Refuses without the acknowledgement, reporting what it would remove | | `list_assets()` | — | The input media on the server, each with the `asset:` reference a workflow argument carries. Look here before asking for a file - what a workflow needs may already be there. `libraries` names the roots searched and which are writable; `shadowed` lists names a nearer library hides | | `keep_output(name, asset_name=None, overwrite=False, shared=False, workspace=None)` | `name`, optional `asset_name`, `overwrite`, `shared`, `workspace` | Keep a generated file as an input asset under a stable `asset:` name, so a later workflow can rely on it. The copy happens on the server: nothing is downloaded or re-uploaded. `asset_name` may name a folder and takes the kept file's extension when it has none; `shared=true` keeps it in the library every workspace shares, which is where a recurring cast belongs. `workspace` names the workspace for this one call without switching the session to it - the same pin `run_workflow` takes, so a job run into another workspace stays reachable from the session that queued it | @@ -297,6 +297,9 @@ references written in the same session. | `delete_prompt(name)` | `name` | Permanently delete a stored prompt. A workflow still referencing it will fail to load | | `list_enhancers()` | — | List the enhancer presets `enhance_prompt` accepts | | `enhance_prompt(idea, preset="h3", model_name=None, device=None, acknowledged_cost=False)` | `idea`, `preset`, optional `model_name` and `device`, `acknowledged_cost` | Expand a short idea into a full prompt with a language model. Queued as an ordinary job, so it passes the gate; the enhanced text is the text file in the finished manifest, readable with `get_output_text` | +| `list_loras(model=None, workflow=None, status=None, tag=None)` | optional `model`, `workflow`, `status`, `tag` | List the LoRAs tried on a base model - proven, trial or rejected - with `use_when`, `trigger` and `scale`; exact match on the base. Guide: `loras` | +| `save_lora(name, entry)` | `name`, `entry` | Save a catalog entry, overwriting any entry of that name; promote a trial to `proven` with its job in `evidence` | +| `recommend_loras(model, query, limit=8)` | `model`, `query`, optional `limit` | Opt-in: catalog LoRAs ranked against a style request, then Hugging Face Hub adapters of that exact base. Queries the Hub (the one tool with `open_world_hint`); no download, no GPU. Hub rows are candidates to trial. A failed or busy Hub (another search already running) returns the catalog rows plus `hub_error`; an empty `hub` with `hub_error` means the search failed | ### Diagnose diff --git a/docs/RELEASING.md b/docs/RELEASING.md index e1d0a29d..29a25a3a 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -7,6 +7,139 @@ notes from commits at tag time (see below). This section is a scratch pad for items a branch's author wants the next release note to name; clear it when a release ships. +### 0.8.0 + + + +Most of this range landed on `develop` without PRs, so the auto-generated +notes are close to empty. Most of it is the UI stabilization (gates 0-4) +and the second `dw_mcp` pass; the per-phase detail is in +docs/stabilization/ui/. The rest is the LoRA catalog, the H3 latent +upscaler and two security fixes. + +**Upgrade order** + +- The MCP tools now call new server routes, so a stdio `dw-mcp` needs a + `dw.serve` at least as new as itself: upgrade the server first. A mounted + MCP (`dw.serve --mcp`) is always the same version. +- `loras` is now a reserved workspace name. A workspace already called + `loras` is no longer listed, and the server logs a warning at start; + rename its directory. + +**Security** + +- GHSA-fwg5-jfjg-fxpf: a job error no longer carries absolute server paths. + A file under the job's asset search path is reported as `asset:`, + one under its output directory as `output:`, and asset not-found + messages name the libraries searched by origin (workspace, common, + examples) instead of by directory. The server log keeps the paths. Still + open: a traceback's frame lines name the server's source and + site-packages paths, and `get_job` returns the traceback. +- GHSA-crqf-hw9p-r739: `create_workspace`'s MCP result is `name`, + `default`, `current` and `next` only; `list_workspaces(detail=true)` is + the opt-in for folder paths. `POST /api/workspaces` is unchanged. +- UI lockfile bumps for open Dependabot alerts (devalue, dompurify, + brace-expansion, undici). + +**LoRA catalog** (docs/LORAS.md) + +- A library of the LoRAs tried on a base model, one JSON file each, marked + `proven`, `trial` or `rejected` with the evidence. The writable `loras/` + at the server root is shared by every workspace and is read ahead of the + shipped read-only one. An entry's `model_name`, `weight_name`, `revision` + and `scale.default` drop straight into a step's `loras` entry. Base models + match exactly. +- Shipped entries: 7 for MiniMax-H3 (Realism People and the turbo keyframe + and reference adapters as proven; Acc-PDD, HyperFlow and FastH3 as + rejected, because they need loaders dw doesn't have), 4 LTX-2.5 IC-LoRAs + and 2 for FLUX. +- MCP: `list_loras`, `save_lora`, and the opt-in `recommend_loras`. It is + the only call that searches the Hugging Face Hub. It returns the catalog's + entries ranked against the request first, then Hub adapters whose card + declares that exact base. Nothing is downloaded; a single-weight repo's + header is read to check its layout. Hub rows are candidates to trial and + carry warnings (`will_not_load`, `unknown_format`, `gated`, `stale`, ...). +- HTTP: `GET /api/loras`, `GET/PUT/DELETE /api/loras/{name}`, + `GET /api/loras/recommend`, `GET /api/lora-schema`. One Hub search runs + at a time per server. A concurrent call gets the catalog rows plus + `hub_error`, and the query is capped at 200 characters and 4 search + terms. + +**New** + +- `upscale_h3_latents` and `decode_h3_latents` take a 960x544 MiniMax-H3 + take to 768p in latent space (#499). This is the build withdrawn before + 0.7.0, relanded with the ComfyUI node's normalization wrapper. Its + `weight_name` must be a bare file name, and `model_name` must be a Hub + repo id. docs/WORKFLOW_GUIDE.md has the recipe; the #500 A/B found it + softer than a native 768p render. +- Server routes the MCP tools now use: + - `GET /api/gallery/{name}/image` returns an image output or `asset:` + cropped, fitted to `max_dimension` and halved until it fits `max_bytes`. + - `max_total_bytes` on `/frames` shrinks every tile to one shared size. + - `PATCH /api/workflows/{name}` applies a JSON merge patch under the save + lock. `save_workflow`'s patch mode calls it. + - `DELETE /api/jobs/{id}/run` deletes a finished job's run directory. + `delete_output(job_id=)` calls it, and a job still queued or running is + a 409. + - `findings` on gallery metadata reports measured level problems + (`full_scale`, `near_silent`) with the fix. + - `acknowledge` in a cost 409 is the `{fingerprint, minutes, downloads}` + to resend. + - `output_kinds` on a job, and on each `step_end` event, maps each file to + `image`, `video`, `audio`, `text` or null. +- Every JSON route declares its response model, and the UI's types are + generated from the server's OpenAPI document. +- `dw-mcp` no longer imports Pillow; images and frame tiles are fitted on + the server. +- Web UI: dialogs and popovers run on Bits UI, with keyboard help on the + modal. Every datalist is now a suggestion combobox that keeps typed text. + The job page renders outputs by the server's media kinds, audio and text + included. +- Plugin: the `minimax-h3` skill names the 4-step draft and the stacked + Realism People LoRA, and rules out loader-only LoRAs (#585). `ltx-2.5` + names the wait reply's `timeout_applied_seconds` and `timeout_capped` + (#546). + +**Fixes** + +- Validating a workflow that needs an uncached gated repo (e.g. flux-dev on + a fresh box) no longer answers 500. A response that fails its model is + logged and sent rather than 500ing, so a retried `POST /api/jobs` can no + longer queue a job twice. +- A scheduler parameter defaulting to `-inf` reads `"-inf"`, not null. +- The phase watchdog measures silence from the last event, so it no longer + reports false stalls. +- The unquantized FLUX templates use sequential offload. Model offload left + the 22 GiB transformer no room on a 24 GB card (#580). +- `mix_audio`'s rate-mismatch warning no longer advises `sample_rate`, + which relabels the rate rather than resampling (#586). `find_loop_bed`'s + `no_loop_bed` names the in-shot rule that actually ruled windows out + (#587). +- Deleting a job's run twice says the run is already gone. +- MCP: `upload_asset` confines a source to the workspace the call names. + `get_output_frames`' audio excerpts share the response byte budget. A bad + `DW_MCP_MAX_WAIT_SECONDS` keeps the default with a warning instead of + failing the import. The startup probe reads a 401 from the status code. +- UI: workspace, folder and prompt names follow the engine's one + name-segment rule. The dtype select keeps a dtype it doesn't list. Text + outputs render as text, and audio and text stay out of workflow card + proofs (#573). + +**For developers** + +- UI architecture ratchet over `ui/src`, in CI alongside the engine's. + Reference prefixes are spelled only in `references.ts`, and the UI's + copies of engine vocabularies are pinned to their owners. +- The response contract is generated under the FastAPI and Pydantic pinned + in `constraints-openapi.txt`. Every route `api.ts` calls must declare its + response model. +- `dw_mcp`'s copied constants are pinned to their engine owners + (`tests/test_mcp_twins.py`), and every tool that takes + `acknowledged_cost` is checked to refuse without it. +- CI runs the e2e suite before `develop` moves, once per push while the + release PR is open. + ### 0.7.0 - A sub-workflow path is resolved by one function (`library.resolve_sub_workflow_reference`) at every site, so a path a run can open is one validation, the realized workflow's digest and the observed-cost lookup can open too. @@ -443,8 +576,10 @@ Releases are cut by pushing a `v` tag. CI does the rest. Before merging `develop` into `master`, run `scripts/preflight.sh` and get it passing. It covers more than CI: ruff over the whole repo rather than -`dw dw_mcp tests`, the real-model integration tests (`pytest -m -integration`), and the UI's Playwright e2e tests, none of which CI runs. +`dw dw_mcp tests` and the real-model integration tests (`pytest -m +integration`), neither of which CI runs. (CI runs the UI's Playwright e2e +tests on every develop push and on PRs into develop; the release PR from +develop relies on the push runs.) ```bash scripts/release.sh 0.38.0 diff --git a/docs/SECURITY.md b/docs/SECURITY.md index b09cb0a9..fd1ee783 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -10,6 +10,7 @@ diffusers-workflow validates all file paths, user inputs, and URLs to protect ag - `validate_path()` — Blocks `../` (anywhere in the path), `~/` (or `~\`), and paths rooted at `/dev/`, `/proc/`, `/sys/`. Rejects null bytes and overlong paths (> 4096 chars). Resolves to an absolute, realpath'd path. If `base_dir` is given, raises `PathTraversalError` when the resolved path falls outside it. - `validate_workflow_path()` — `validate_path()` plus a required `.json` extension (via `validate_file_extension()`) +- `validate_lora_path()` / `validate_lora_name()` — Confine a LoRA catalog entry to the `loras/` root: a plain name or one family folder deep, `.json` only, no traversal - `validate_output_path()` — `validate_path()` with `allow_create=True`, for directories/files that don't need to exist yet - `validate_file_extension()` — Checks a path's extension against an allowed set (used internally by `validate_workflow_path()` and by `arguments.py` for media files) diff --git a/docs/SERVER.md b/docs/SERVER.md index 97a66b58..da763f29 100644 --- a/docs/SERVER.md +++ b/docs/SERVER.md @@ -164,9 +164,10 @@ agent from another machine, plus the queue across every workspace: | Route | What it does | | --- | --- | -| `POST /api/jobs` | Queue a run: `{"workflow_path": ...}` or an inline `{"workflow": {...}, "base_dir": ...}`, plus `arguments` for variable overrides. `workflow_path` accepts a stored workflow name as listed by `/api/workflows` (with or without `.json`, nested names included), or a relative/absolute path that still resolves under `--workflow-dir` - confined the same way the `/api/workflows` CRUD routes are; a path that names a real file outside that directory is rejected with 400, not opened. Answers with argument warnings from signature checking. Takes an optional `acknowledged_cost`: `true` is recorded as `acknowledged: boolean`; the object `{fingerprint, minutes, downloads}` from a validate answer's `plan` is `bound` - the server re-plans the run for the arguments given and answers **409** when the fingerprint differs or a repo in `downloads_required` is not in `downloads` (a download that has since vanished is not a refusal); the body is `{"detail": {message, reason: "fingerprint" \| "downloads" \| "unplannable", acknowledged, plan}}` with the current plan, so the caller re-quotes from it. `minutes` is recorded, never compared. Nothing is required: the web UI and every caller that sends nothing are `acknowledged: none`, and every job answer and history row carries `acknowledged` (and `acknowledged_cost` when bound). `POST /api/jobs/{id}/rerun` takes the same field and checks against the stored spec; a fresh seed does not change a fingerprint. | +| `POST /api/jobs` | Queue a run: `{"workflow_path": ...}` or an inline `{"workflow": {...}, "base_dir": ...}`, plus `arguments` for variable overrides. `workflow_path` accepts a stored workflow name as listed by `/api/workflows` (with or without `.json`, nested names included), or a relative/absolute path that still resolves under `--workflow-dir` - confined the same way the `/api/workflows` CRUD routes are; a path that names a real file outside that directory is rejected with 400, not opened. Answers with argument warnings from signature checking. Takes an optional `acknowledged_cost`: `true` is recorded as `acknowledged: boolean`; the object `{fingerprint, minutes, downloads}` from a validate answer's `plan` is `bound` - the server re-plans the run for the arguments given and answers **409** when the fingerprint differs or a repo in `downloads_required` is not in `downloads` (a download that has since vanished is not a refusal); the body is `{"detail": {message, reason: "fingerprint" \| "downloads" \| "unplannable", acknowledged, plan, acknowledge}}` with the current plan, so the caller re-quotes from it, and `acknowledge` - the `{fingerprint, minutes, downloads}` to resend - whenever there is a plan. A `null` in `downloads` (a URL with no repo) is ignored. `minutes` is recorded, never compared. Nothing is required: the web UI and every caller that sends nothing are `acknowledged: none`, and every job answer and history row carries `acknowledged` (and `acknowledged_cost` when bound). `POST /api/jobs/{id}/rerun` takes the same field and checks against the stored spec; a fresh seed does not change a fingerprint. | | `GET /api/jobs?workspace=&status=&limit=` | Queue + history summaries, oldest first, with `total` beside them. `status` narrows to one state or a comma-separated set (`queued`, `running`, `succeeded`, `failed`, `cancelled`; anything else is a 400); `limit` keeps the newest N, and `total` still reports how many matched, so a bounded answer cannot be mistaken for a complete one. No parameters means every job, which is what the web UI polls | -| `GET /api/jobs/{id}` | Full detail: spec, events, manifest, error. A manifest entry for a step served from the step cache carries `reused: true`. Every entry carries `subfolder` - the in-run subfolder the step's `result.subfolder` chose, `''` for none. A `for_each` step appears in the manifest as its members (`shot@wide_open`, `shot@closeup`), because the manifest records what ran; the run's `workflow.json` keeps the `for_each` form, because it records what was asked | +| `GET /api/jobs/{id}` | Full detail: spec, events, manifest, error. A manifest entry for a step served from the step cache carries `reused: true`. Every entry carries `subfolder` - the in-run subfolder the step's `result.subfolder` chose, `''` for none. A `for_each` step appears in the manifest as its members (`shot@wide_open`, `shot@closeup`), because the manifest records what ran; the run's `workflow.json` keeps the `for_each` form, because it records what was asked Carries `output_kinds`: each manifest file mapped to its media kind (`image`, `video`, `audio`, `text`, or null), and each file-carrying run event (`step_end`) carries the same map for its own files. | +| `DELETE /api/jobs/{id}/run` | Delete the run directory a finished job wrote, whole, from the output root the job ran against (no workspace selector applies): `{job_id, run_dir, deleted, run_swept}`. 404 for an unknown job or one with no run directory, 409 for one still queued or running | | `GET /api/jobs/{id}/workflow` | The workflow the job ran: `{id, definition, realized, seed_variable}`. `seed_variable` names the variable a `new_seed` rerun would draw into (null when the workflow has none), read from the workflow as written rather than the realized copy, whose seed is pinned. `realized: true` is the copy the run itself wrote (`workflow.json` in its run directory), with arguments, seed, prompts and `output:latest` pinned; `false` falls back to the submitted definition, which is what a job from before run tracking has. 404 means neither is readable - the job itself still is. The equivalent MCP tool is `get_job_workflow` (see [MCP.md](MCP.md#diagnose)) | | `POST /api/jobs/{id}/export?workspace=&overwrite=` | Gather one finished job into `/exports//`: `workflow.json`, `manifest.json`, `job.json`, `README.md`, `assets/`, `inputs/`, `outputs/`. 201 with the file list, total bytes, anything it could not find, a `zip_url`, and the three JSON files inline. 404 unknown job, 409 for a job still running or an existing export without `overwrite` | | `GET /exports/{id}.zip?workspace=` | The same tree as one archive, built on request rather than kept as a second copy. Entries are named `/`. Ungated exactly as `/outputs` is | @@ -421,7 +422,9 @@ The editor's forms come from these; they are just as usable from scripts: otherwise. With no params the response is what it always was, plus the new fields - `GET/PUT/DELETE /api/workflows/{name}` — read, save, delete workflow files - (confined to `--workflow-dir`) + (confined to `--workflow-dir`); `PATCH` applies a JSON merge patch (RFC 7396, + `application/merge-patch+json` or JSON) to the stored version and saves the + result as `PUT` would, under the same lock, so a save in between is not lost - `GET /api/workflows/{name:path}/download` — download a workflow file as JSON - `GET /api/workflows/{name:path}/variables` — a workflow's variables and what they default to, without the definition around them. Long string defaults are @@ -436,6 +439,18 @@ The editor's forms come from these; they are just as usable from scripts: 403; saves are validated against the prompt schema, served at `GET /api/prompt-schema` - `GET /api/prompts/{name:path}/download` — download a prompt file as text +- `GET /api/loras?model=&workflow=&status=&tag=`, `GET/PUT/DELETE /api/loras/{name}` — + the LoRA catalog (see [LORAS.md](LORAS.md#lora-catalog)): one JSON file per + tried LoRA, in the root's writable `loras/` ahead of the shipped read-only + ones. The listing carries `libraries` and an `origin`/`writable` per entry; + `model` is a Hub repo id or a workflow name and matches exactly. `GET` of an + entry that cannot be parsed is a 404 naming it unreadable; deleting a + read-only entry is a 403; saves are validated against the entry schema, + served at `GET /api/lora-schema` +- `GET /api/loras/recommend?model=&query=&limit=` — catalog entries for the + model ranked against the query, then Hub candidates for the same exact base + (the one place dw searches the Hub, only when called); `hub_error` when the + Hub is unreachable, and a concurrent search gets the catalog rows only - `GET /api/enhancers`, `POST /api/enhance` — prompt-enhancement presets, and `{"idea": ..., "preset": ..., "model_name": ..., "device": ...}` to queue an enhancement as an ordinary job whose saved text file is the @@ -450,6 +465,16 @@ The editor's forms come from these; they are just as usable from scripts: `subfolders` list every distinct value over the whole tree, `''` always a member of each so root-level files stay selectable - `GET /api/gallery/{name:path}/download` — download an output file +- `GET /api/gallery/{name}/metadata` also carries `findings`: each level + problem the probed soundtrack shows (`full_scale`, `near_silent`), measured + against `dw/audio_qc.py`'s thresholds, in the assess route's finding shape +- `GET /api/gallery/{name}/image?max_dimension=768&crop=x,y,w,h&max_bytes=&format=auto` + — an image output or `asset:` sized for an inline answer: cropped (clamped), + fitted to `max_dimension`, and with `max_bytes` (a base64 budget) halved + until it fits. Headers `X-DW-Original-Size`, `X-DW-Returned-Size`, and when + they apply `X-DW-Crop` and `X-DW-Downscaled-To`. Takes `?token=` +- `GET /api/gallery/{name}/frames` takes `max_total_bytes`: over it, every + tile shrinks to one shared size and `downscaled_to` says which - `POST /api/gallery/archive` — `{"names": [...]}` (1-1000) bundles a multi-file selection into one zip, named by each file's gallery-relative path so output subfolders survive. A browser cannot zip on its own and @@ -528,7 +553,7 @@ The editor's forms come from these; they are just as usable from scripts: under a generated name, or under `asset_name` when one is given (`cast/priya-voice.wav`, folders allowed, the uploaded file's extension assumed, confined to the library the way `keep_output`'s name is). - Answers 201 with `path` - `asset:uploads/`, the reference a saved + Answers 201 with `reference` - `asset:uploads/`, the reference a saved workflow can carry and still resolve on a later run - and `url`, the same file under the `/inputs` mount, for the editor's preview. A server started without an asset library falls back to the output directory's `uploads/` diff --git a/docs/WORKFLOW_GUIDE.md b/docs/WORKFLOW_GUIDE.md index 3e19ca4d..613ddd4f 100644 --- a/docs/WORKFLOW_GUIDE.md +++ b/docs/WORKFLOW_GUIDE.md @@ -297,7 +297,7 @@ for existence. curl -H "Authorization: Bearer $DW_API_TOKEN" --data-binary @portrait.jpg \ "http://:8765/api/uploads?filename=portrait.jpg&asset_name=cast/portrait.jpg&workspace=" ``` - It answers 201 with `path` - the `asset:` reference to use in a workflow + It answers 201 with `reference` - the `asset:` reference to use in a workflow argument (the bearer token only when the server requires one). The same route is also how a file assembled entirely on the client - a finished cut stitched locally rather than by a workflow step - gets onto the server at @@ -1587,6 +1587,246 @@ it — see [workflows/templates/minimax/last-frame-only.json](../workflows/templ See [workflows/templates/minimax/music.json](../workflows/templates/minimax/music.json) and [workflows/templates/minimax/video-with-audio.json](../workflows/templates/minimax/video-with-audio.json) for full examples. +### Promoting an H3 take to 768p in latent space: upscale_h3_latents and decode_h3_latents + +Once a 960x544 MiniMax-H3 take reads the way it should, `upscale_h3_latents` and +`decode_h3_latents` promote it to 1344x768 without denoising it again - cheaper than a +native 768p render, since only a small 3D-convolution network and a VAE decode run, +not the transformer. There is no refine pass over the upscaled latents, so the result +is sharper than the 544p take but cannot show detail the base pass never generated. +Measured on a 124-frame crowd scene, it took 7.8 min against a native 768p render's +12.7 min, and the faces came out soft and waxy where the native render's were distinct. +So this is a measurement path, not a catalog template: the catalog's native 768p render +is `templates/minimax/video-with-audio-768p`, and it is the one to use when faces matter. + +Target `width`/`height` must be multiples of 16, each between 1x and 4x the base +latents' own size, and within H3's 1344x768 (or portrait 768x1344) canvas. The latents +pass from `base` to `up` to `decode` entirely in memory. A `previous_result:` property +on a modular step names a key of the dict its `output` list returns, spelled as the +pipeline spells it: `base.latents`, `base.audio` and `base.sampling_rate` - not +`sample_rate`, which is what the saved AudioVideo calls it, and which the dict does +not carry. A property no result carries is an error rather than an empty list, so a +misspelt one fails the step instead of skipping the mux that reads it. The base +step carries no `result` at all, so nothing beyond its return value is written - +upscaling only makes sense for a take chosen from something already reviewed, so the +544p pass that produced it is not itself a deliverable here. + +The upscaler weights (~691 MB, `LBH-123-AI/Minimax_h3_latent_Upscaler`, Apache-2.0, +read at a pinned revision) and the H3 VAE download on first use, the same as any other +model. Neither is counted in `plan.downloads_required`, since that walk collects +`from_pretrained_arguments` sources on pipeline steps and does not see a task +argument naming a Hugging Face repo - a box that has run the base pass before but never +this task can still stall mid-run pulling the upscaler. + +```json +{ + "id": "H3LatentUpscalePreview", + "description": "Promote a MiniMax-H3 take from 960x544 to 1344x768 in latent space, no refine pass.", + "variables": { + "prompt": "prompt:minimax/fox_dawn_context_ir", + "num_frames": 124, + "num_inference_steps": 9, + "video_shift": 12.0, + "audio_shift": 3.0, + "weights_dtype": "{int4}", + "lora_scale": 1.0, + "lora_alpha": null, + "lora_model_name": "lightx2v/Minimax-h3-Turbo", + "lora_weight_name": "minimax_h3_fl2v_turbo_8step_v1.0_bf16.safetensors", + "lora_adapter_name": "turbo", + "seed": 42 + }, + "variable_constraints": { + "num_frames": { + "modulus": 17, + "remainder": 5, + "min_frames": 124, + "max_frames": 345, + "snap": "up", + "reason": "the video VAE encodes 17 * n + 5 frames, and MiniMax-H3 generates between 5 and 15 seconds at 24 fps" + } + }, + "seed": "variable:seed", + "steps": [ + { + "name": "base", + "pipeline": { + "configuration": { + "component_type": "ModularPipeline", + "pre_load_modules": [ + "sdnq" + ], + "load_components": { + "dtype": "torch.bfloat16", + "quantization_config": { + "transformer": { + "configuration": { + "config_type": "sdnq.SDNQConfig" + }, + "arguments": { + "weights_dtype": "variable:weights_dtype", + "quantization_device": "cuda", + "return_device": "cpu", + "use_quantized_matmul": true, + "dequantize_fp32": false, + "modules_to_not_convert": [ + "proj_in", + "audio_proj_in", + "context_embedder", + "time_embedder", + "time_proj", + "token_refiner", + "norm_out", + "proj_out", + "audio_proj_out" + ] + } + }, + "text_encoder": { + "configuration": { + "config_type": "sdnq.SDNQConfig" + }, + "arguments": { + "weights_dtype": "variable:weights_dtype", + "quantization_device": "cuda", + "return_device": "cpu", + "dequantize_fp32": false, + "modules_to_not_convert": [ + ".model.visual", + "lm_head" + ] + } + }, + "vae": { + "configuration": { + "config_type": "sdnq.SDNQConfig" + }, + "arguments": { + "weights_dtype": "{int8}", + "quant_conv": true, + "use_quantized_matmul_conv": true, + "quantization_device": "cuda", + "return_device": "cpu", + "dequantize_fp32": false + } + } + } + }, + "components": { + "transformer": { + "group_offload": { + "offload_type": "block_level", + "num_blocks_per_group": 2, + "use_stream": true, + "record_stream": true, + "low_cpu_mem_usage": true + } + }, + "text_encoder": { + "remove_modules": [ + "lm_head" + ] + }, + "text_encoder.model": { + "truncate_layers": { + "language_model.layers": 51 + }, + "group_offload": { + "offload_type": "leaf_level" + } + }, + "vae": { + "device": "cuda", + "residency": "on_demand" + }, + "audio_vae": { + "device": "cuda", + "residency": "on_demand" + } + } + }, + "from_pretrained_arguments": { + "model_name": "MiniMaxAI/MiniMax-H3", + "workflow": "t2va" + }, + "loras": [ + { + "model_name": "variable:lora_model_name", + "weight_name": "variable:lora_weight_name", + "adapter_name": "variable:lora_adapter_name", + "scale": "variable:lora_scale", + "alpha": "variable:lora_alpha" + } + ], + "scheduler": { + "shift": "variable:video_shift" + }, + "audio_scheduler": { + "shift": "variable:audio_shift" + }, + "arguments": { + "prompt": "variable:prompt", + "num_frames": "variable:num_frames", + "width": 960, + "height": 544, + "num_inference_steps": "variable:num_inference_steps", + "output": [ + "videos", + "audio", + "sampling_rate", + "latents" + ] + } + } + }, + { + "name": "up", + "task": { + "command": "upscale_h3_latents", + "arguments": { + "latents": "previous_result:base.latents", + "width": 1344, + "height": 768 + } + } + }, + { + "name": "decode", + "task": { + "command": "decode_h3_latents", + "arguments": { + "latents": "previous_result:up" + } + } + }, + { + "name": "mux", + "task": { + "command": "pair_audio", + "arguments": { + "video": "previous_result:decode", + "audio": "previous_result:base.audio", + "sample_rate": "previous_result:base.sampling_rate", + "fit": "video" + } + }, + "result": { + "content_type": "video/mp4", + "subfolder": "final" + } + } + ] +} +``` + +To keep the 544p take beside the promotion for comparison, add one more `pair_audio` +step with `"video": "previous_result:base.videos"` and the same `audio`/`sample_rate`, +saved with `"result": {"content_type": "video/mp4", "fps": 24, "subfolder": +"intermediate"}`. `base.videos` is the pipeline's batch - a list holding the one video - +and `pair_audio` unwraps a batch of one (a batch of several is refused, since one track +goes under one video). The dict's frames carry no frame rate, so the result's `fps` +says it; without it the file is written at the 8 fps fallback. + ### Chained Video Generation Video pipelines generate short clips - a `chain` block on a pipeline step runs the diff --git a/docs/WORKSPACES.md b/docs/WORKSPACES.md index 4de8307a..849ab1ae 100644 --- a/docs/WORKSPACES.md +++ b/docs/WORKSPACES.md @@ -245,6 +245,7 @@ it is working in - `dw.run --workspace NAME` among them. / workflows/ assets/ outputs/ <- the 'default' workspace prompts/ <- shared by all of them + loras/ <- the LoRA catalog, shared by all of them common/assets/ <- shared by all of them studio/ workflows/ assets/ outputs/ <- the 'studio' workspace @@ -260,8 +261,11 @@ reference, and a prompt duplicated per workspace would resolve to different text depending on where a workflow happened to be saved. `workflows`, `prompts`, `assets` and `outputs` are reserved names for that reason. -Two more names are reserved beside `workflows`, `prompts`, `assets` and -`outputs`: `exports` and `common`. `POST /api/jobs/{id}/export` gathers one +Three more names are reserved beside `workflows`, `prompts`, `assets` and +`outputs`: `exports`, `common` and `loras`. `loras/` is the LoRA catalog, +shared by every workspace like `prompts/`; a workspace already named `loras` +stops being listed on upgrade, and the server logs a warning at start naming +its directory. `POST /api/jobs/{id}/export` gathers one finished job into `/exports//`, and that folder is never mistaken for a workspace. diff --git a/docs/proposals/audits/2026-10-02-ltx-2.5-audit.md b/docs/proposals/audits/2026-10-02-ltx-2.5-audit.md new file mode 100644 index 00000000..e5ddf3f7 --- /dev/null +++ b/docs/proposals/audits/2026-10-02-ltx-2.5-audit.md @@ -0,0 +1,354 @@ +# LTX-2.5 knowledge re-audit — `diffusers-workflow` + +Research date: 2026-10-02. Repo branch `develop` (03d477fe), ltx2 templates last touched for #549 +(refine-clip). Prior audit: `docs/proposals/audits/2026-09-07-ltx-2.5-audit.md`. +Installed library: `diffusers 0.41.0.dev0` from git commit `578c9b2` (2026-10-01), which is +current `main`; `pipelines/ltx2/utils.py` is byte-identical to upstream main as of today. +Nothing in the repo was edited. + +Two facts the task statement had wrong, settled first: + +- `plugins/dw/skills/ltx-2.5/SKILL.md` is **12,288 bytes — exactly the 12 KiB cap** + (`tests/test_plugin_skills.py::SKILL_SIZE_LIMIT = 12 * 1024`, asserted `<=`), not ~8 KiB. + The quoted caption spec is 3,825 bytes of that and is pinned verbatim (whitespace-normalised) + to `LTX2_5_T2V_DEFAULT_SYSTEM_PROMPT` by `test_the_quoted_caption_spec_is_the_library_constant`. + There is zero headroom; anything added must displace something. +- The "describe only changes from the image" rule the skill attributes to + `LTX2_5_I2V_DEFAULT_SYSTEM_PROMPT` is not in it. It is in the Gemma-3 era + `I2V_DEFAULT_SYSTEM_PROMPT` (LTX-2.0/2.3, `utils.py:197`). The LTX-2.5 I2V prompt says the + opposite (details under Claim-by-claim). + +--- + +## Sources + +| URL | Type | Date | Contributes | +| --- | --- | --- | --- | +| `venv/.../diffusers/pipelines/ltx2/utils.py` (commit 578c9b2) | primary (library) | 2026-10-01; identical to upstream main 2026-10-02 | `DISTILLED_SIGMA_VALUES` (8), `STAGE_2_DISTILLED_SIGMA_VALUES` `[0.909375, 0.725, 0.421875]`, `LTX2_5_IMAGE_CRF = 18`, `MAX_CONDITIONING_FPS = 60`, `SNAP_CONDITIONING_FPS_ABOVE = 30`, `ANCHOR_KEYFRAME_STRENGTH = 0.95`, `TEMPORAL_ANCESTRAL_ETA = 0.5`, `GEMMA4_PROMPT_ENHANCEMENT_CONFIG` (600 tokens, greedy, no-repeat-5), both LTX-2.5 system prompts, `DEFAULT_NEGATIVE_PROMPT` pinned to LTX-2 commit `ae855f8` | +| `venv/.../ltx2/pipeline_ltx2.py`, `_ic_lora.py`, `_dfr.py`, `_image2video.py`, `_latent_upsample.py`, `scheduling_ltx_euler_ancestral_rf.py` | primary (library) | read 2026-10-02 | `check_inputs` w/h % 32 (`ValueError`), `min_seconds`/`max_seconds` 1.0/20.0, `noise_scale` renoise formula `noise_scale*noise + (1-noise_scale)*latents`, `reference_downscale_factor` / `conditioning_attention_strength` defaults, `LTXEulerAncestralRFScheduler` used only by DFR temporal refine | +| https://github.com/Lightricks/LTX-2 `packages/ltx-core/.../gemma4_t2v_system_prompt.txt` and `gemma4_i2v_system_prompt.txt` (via `gh api`) | primary | fetched 2026-10-02 | **Byte-identical** to the installed `LTX2_5_T2V_DEFAULT_SYSTEM_PROMPT` (3,733 chars, 539 words) and `LTX2_5_I2V_DEFAULT_SYSTEM_PROMPT` (4,662 chars, 680 words) | +| https://github.com/Lightricks/LTX-2 `CHANGELOG.md` | primary | 1.4.0 **2026-09-29**, 1.4.1 2026-09-30, 1.4.2 2026-10-01 | Euler-ancestral sampling on LTX-2.5+ (output changed), chunked long video (97-frame windows / 25-frame carry), two-stage `ICLoraPipeline` with `--stage-2-ic-lora` and transformer tiling, `AudioConditionByLatentIndex`, audio-length fix for off-grid frame counts, `alpha_gen` pipeline | +| https://github.com/Lightricks/LTX-2 `README.md` | primary | fetched 2026-10-02 | "Prompting for LTX-2": single flowing paragraph, start with the action, "Keep within 200 words", 7-item structure, links the ltx.io blog; DFR "production-quality path … same distilled transformer … not a different prompting style" | +| `.../LTX-2/packages/ltx-pipelines/docs/pipelines.md` | primary | fetched 2026-10-02 | DistilledPipeline "8 steps in stage 1, 4 steps in stage 2 … Euler ancestral on an LTX-2.5+ checkpoint"; ICLoraPipeline CLI "always two stages"; DFR section: w/h % 64, 4K = 3840x2176, detailing LoRA strength "fixed at 0.5", temporal rounds 121/241/481 at 24/48/96 fps, "snaps conditioning fps to 60 whenever playback fps is above 30", 96 fps player warning | +| `.../LTX-2/packages/ltx-pipelines/docs/pipeline-selection.md` | primary | fetched 2026-10-02 | Decision tree unchanged in substance: DistilledPipeline "starting point", DFR "production quality", RetakePipeline for editing, ICLoraPipeline for video conditioning, KeyframeInterpolationPipeline | +| `.../LTX-2/packages/ltx-pipelines/docs/conditioning.md` | primary | fetched 2026-10-02 | Replacing vs guiding latents ("better for smooth interpolation between keyframes"); video conditioning "ICLoraPipeline only"; generated keyframe slots cost (+16 % tokens for 5 at 512x768x241) | +| `.../LTX-2/packages/ltx-pipelines/src/ltx_pipelines/utils/constants.py` | primary | fetched 2026-10-02 | Upstream sigmas with trailing 0.0; `LTX_2_4_IMAGE_CRF = 18`; stage-1 default 512x768, 121 frames, 24 fps; guided defaults CFG 3.0 / STG 1.0 / rescale 0.7 / modality 3.0, STG block 29 (2.0) / 28 (2.3+) | +| https://huggingface.co/Lightricks/LTX-2.5-Diffusers (HTML card) | primary | repo modified 2026-08-24; fetched 2026-10-02 | Distilled recipe (sigmas, guidance 1.0, STG 0, modality 1.0); two-stage "half resolution, 2x latent upsample, then 3-sigma tail at full resolution"; `transformer_full/`; "long, single-paragraph audio-visual captions … short prompts degrade quality significantly"; still says **19B** (stale; weights card and filenames say 22B) | +| https://huggingface.co/Lightricks/LTX-2.5 (HTML card + file API) | primary | created 2026-07-23, **modified 2026-10-02** (README diff not readable: raw is gated) | File list (dev/distilled 22B bf16 42.0 GB each, NVFP4 18.7 GB, Gemma-4 26.3 GB, spatial + temporal x2 upscalers, duration head, distilled-lora-450); `num_frames % 8 == 1`; w/h % 32; "native multishot" headline; I2V strength "0.8–1.0" | +| https://huggingface.co/Lightricks/LTX-2.5-22b-IC-LoRA-Pixel-Spatial-Upscaler | primary | created 2026-08-11, modified 2026-09-14 | Weight `…-x2-1.0.safetensors`, strength 1.0 pre-scaled, `reference_downscale_factor` 2, ~280p draft base, "not a blind denoiser or a compression-artefact remover", not for live-action fidelity | +| https://huggingface.co/Lightricks/LTX-2.5-22b-IC-LoRA-Ingredients | primary | created/modified 2026-09-10 | `…-ingredients-0.9.safetensors`, 768x448x121 @ 24, `Reference sheet:` / `Generated video:` form, reference ≥121 frames and "match the output resolution", strength 1.0 | +| https://huggingface.co/Lightricks/LTX-2.5-22b-IC-LoRA-Deblur | primary | created 2026-09-09, modified 2026-09-10 | `…-deblur-0.9.safetensors`, 960x544x121 @ 24, DEBLUR dual-panel prompt, 1.0 → 0.8 for haloing, spatial defocus only | +| https://huggingface.co/Lightricks/LTX-2.5-22b-IC-LoRA-Decompression | primary | created/modified 2026-09-10 | `…-decompression-0.9.safetensors`, 960x544x121 @ 24, ENHANCE QUALITY dual-panel prompt, strength 1.0, compression artefacts only | +| https://huggingface.co/Lightricks/LTX-2.5-22b-IC-LoRA-Refine-Details | primary | **created 2026-09-27** | `…-refine-details-1.0.safetensors`; rebuilds detail on soft/compressed/upscaled clips; tile 1024x576x97 @ 24; `reference_downscale_factor` 1; strength 1.0; "keep prompts generic … rendering qualities … not specific subjects"; 8 distilled sigmas with tiled latent fusion (overlap 0.5) | +| https://huggingface.co/Lightricks/LTX-2.5-22b-IC-LoRA-Restore | primary | **created 2026-09-27** | `…-restore-1.0.safetensors`; archive-footage restoration incl. colourisation; tile 960x544, 49/97 frames; strength 1.0 "behaves as a switch"; positive prompt = period/place/lighting/materials, negative = modern objects/logos | +| Hugging Face org listing (`/api/models?author=Lightricks&search=LTX-2.5`) | primary | 2026-10-02 | 18 LTX-2.5 repos incl. new Alpha-Gen 0.9 (2026-09-28), Clean-Plate, Colorization, Day-To-Night, Layout-To-Render, SDR-To-HDR, Water-Simulation, LoRA-Cinemagraph, LoRA-Slow-Motion-Control (dates not fetched) | +| https://docs.ltx.io/api-documentation/implementation-guides/prompting-guide (+ `.md`) | primary (vendor docs) | undated; no API-changelog entry for it; a secondary page citing it is dated 2026-09-22 | Full text captured below: 6 key elements, single-shot 4–8 sentences, screenplay style for dialogue scenes, pacing rule for the duration predictor, **Multi-Shot Prompts (LTX-2.5)** section, enhancer guidance, on-screen-text and physics limits, Dub-It template, sample prompts, term lists | +| https://docs.ltx.io/open-source-model/usage-guides/prompting-guide.md | primary | same content, same headings (the open-source overview links this path) | Confirms one guide served at two paths | +| https://docs.ltx.io/llms.txt | primary (index) | 2026-10-02 | Full page list (implementation-guides holds only prompting-guide + authentication); feature guides for refine-and-restore, native-resolution, dub-it, ic-lora, two-stage, t2v, i2v, extend/retake API | +| https://docs.ltx.io/models/ltx-2-5.md | primary (hosted API) | 2026-10-02 | `ltx-2-5-fast`/`-pro`: 24/25/48/50 fps; 6–20 s at 720p/1080p 24/25 fps, 6–10 s otherwise; 16:9 / 9:16; `"duration": null` auto; retake/extend not on 2.5 endpoints | +| https://docs.ltx.io/api-documentation/api-reference/async-video-generation/submit-extend.md | primary (hosted API) | 2026-10-02 | Extend: 2–20 s, input ≥73 frames, `context + duration ≤ 505 frames`, mode `end`/`start`, prompt "what should happen in the extended portion" | +| https://docs.ltx.io/open-source-model/usage-guides/text-to-video.md, image-to-video.md | primary (ComfyUI templates) | 2026-10-02 | Defaults 768x512 base, 97 frames (`1 + 8n`), 24/25/30 fps, output 2x = 1536x1024; I2V: "describe what happens, not what already exists in the image"; Prompt Enhance on by default | +| https://docs.ltx.io/open-source-model/vfx-post-production/refine-and-restore.md, native-resolution.md, usage-guides/ic-lo-ra.md | primary | 2026-10-02 | Refine Details / Restore workflows and tiles; multiples of 32 and 8k+1 "model-legal canvases"; trigger-phrase convention (DEBLUR etc.); match reference resolution/fps to the generation | +| https://docs.ltx.io/open-source-model/feature-guides/audio/dub-it-beta.md | primary | 2026-10-02 | `ltx-2.3-22b-ic-lora-dub-it-0.9.safetensors` (a 2.3 weight), template, native script, single speaker, 8+3 step two-stage | +| https://docs.ltx.io/api-changelog/* (2026-08-11, 09-07, 09-23, 09-24) | primary | as dated | 2026-08-11 is the hosted LTX-2.5 launch (camera motion controls, `last_frame_uri`, `duration: null`); September entries are v1-endpoint retirement (2026-10-26) and reframe — nothing on prompting | +| https://huggingface.co/docs/diffusers/main/en/api/pipelines/ltx2 | primary (library docs) | 2026-10-02 | Distilled two-stage code (`noise_scale=STAGE_2_DISTILLED_SIGMA_VALUES[0]`), FLF2V with strength 1.0 both ends, LTX-2.5 full/SFT recipe (`transformer_full`, dynamic shifting, 30 steps, CFG 3/7, STG block 28), DFR recipe with detailing adapter weight 0.5, diffusion decoder recipe, resolution rule "32 single pass, 64 for stage 1 of two-stage" | +| https://ltx.io/model/model-blog/prompting-guide-for-ltx-2 (and the two blog URLs tried in the prior audit) | secondary | — | **Fetch failed again** (HTTP header overflow). The docs.ltx.io guide supersedes it for this audit. | +| https://docs.sogni.ai/models/ltx-2-5/ | secondary | page dated 2026-09-22 | Restates the vendor guide; **adds** "state a numeric timestamp for the cut" and "one sentence per shot", which the vendor guide does not say (it says name the transition in prose). Treat as a host's house style, not LTX guidance. Also quotes a 64-px grid and 25–505 frames — that host's own limits. | +| WebSearch (soloa.ai, jxp.com, sogni blog, nyu.edu) | secondary | Aug–Sep 2026 | All restate "prose not tags, present tense, chronological"; none contradicts a primary source; none adds evidence | + +--- + +## The vendor prompting guide, exact rules (docs.ltx.io, fetched 2026-10-02) + +Captured here because it is new to the audit trail. + +**Key elements** (six): establish the shot ("cinematography terms … shot scale"); set the scene +("lighting conditions, color palette, surface textures, and atmosphere"); describe the action +("Give each sentence a verb that does something — walks, turns, exhales, reaches … Appearance alone +gives the model little to animate"); define the characters ("age, hairstyle, clothing, and +distinguishing features. Express emotion through physical cues, not abstract labels"); identify +camera movements ("Specify how and when the camera moves. Describing how subjects appear after the +movement helps the model complete the motion accurately"); describe the audio ("Place spoken +dialogue in quotation marks. Specify language and accent if needed"). + +**Structure**: "LTX responds best to cinematic, single-subject scenes with clear camera language, +consistent lighting, and well-described audio." Principles: "Keep the scene focused", "Keep lighting +consistent — use one coherent light logic per shot", "Start simple and layer". "Coming from another +model? Don't paste a prompt written for another video model (e.g. Kling or Seedance) into LTX +unchanged … tag syntax and shot-list formatting don't [carry over] … Rewrite it into LTX's +flowing-paragraph structure, or run the original through the prompt enhancer." + +**Single-shot**: "single flowing paragraph", "present tense verbs", "Match the level of detail to the +shot scale (close-ups need more detail than wide shots)", "Describe camera movement relative to the +subject", "Aim for roughly 4–8 descriptive sentences". + +**Longer / screenplay-style**: "When a scene involves dialogue, multiple beats, or precise timing, +write it in a screenplay style, with scene headers, character cues, and quoted dialogue … Keep the +same fundamentals: present tense, physical emotion cues, and dialogue in quotation marks." + +**Length**: "Match length to complexity. A simple single shot is often 4–8 sentences; a longer +screenplay-style scene can run longer, provided every sentence adds concrete visual or audio detail." +**Pacing**: "LTX-2.5's optional duration predictor sizes the clip to the action you describe and +times it as written. It won't stretch a moment or add a pause you didn't prompt for. Write the beats +you want into the prompt ('she pauses', 'a beat of silence') … or set an explicit duration." + +**Multi-shot (LTX-2.5)**: "Write the full scene as one chronological paragraph (or a short sequence +of sentences). Do not use a shot list, numbered beats, or screenplay sluglines unless you also +describe the cut in prose." At every cut: (1) "Name the transition in natural language — e.g. 'A +hard cut transitions to…', 'The view cuts to a close-up of…', 'A match cut connects…', 'The image +dissolves into…'"; (2) "Re-establish the new shot — shot scale, camera angle, who or what is in +frame, and lighting if it changed"; (3) "Keep identity consistent — reuse the same visual +identifiers for recurring people or objects"; (4) "State audio continuity — e.g. 'the piano score +continues across the cut' or 'the dialogue drops; only wind remains.'" Tips: "Prefer 2–4 shots in +one generation; more cuts usually need clearer, shorter beats per shot"; "Give each shot a clear job +(establish → detail → reaction, or wide → medium → close-up)"; "Keep action chronological e.g. +'Initially…', 'A moment later…', 'Simultaneously…'"; "Avoid conflicting geography or unexplained +costume changes between cuts unless the cut is meant to jump time or place and you say so." +**When to stay single-shot**: "unbroken camera motion, intimate performance, or dialogue that must +stay lip-synced in one framing. For image-to-video from a first frame, prefer a single continuous +take unless you intentionally describe a cut away from that opening image." + +**Enhancer**: Gemma 4 E2B rewrite pass; "We recommend using it when your prompt is short, rough, or +was originally written for a different model. It helps least when your prompt already follows the +structure detailed in this post." + +**Limits**: on-screen text ("exact spelling and consistency across frames are not guaranteed … add +critical titles, labels, or logos in post"); "Complex physics — highly chaotic motion can introduce +artifacts". + +**Dub-It**: `[Speaker] is speaking [Language/Accent], saying: "[Dialogue]"`; validated languages +English, French, Spanish, German, Russian; native script; single speaker; match syllable length. + +Note the vendor's own sample prompts open with "The shot opens on…" and use `EXT. TOWN STREET – +MORNING` sluglines, both of which the training-caption spec forbids. See Assessment. + +--- + +## Changes since 2026-09-07 + +1. **LTX-2 tooling 1.4.0 (2026-09-29), 1.4.1, 1.4.2 (2026-10-01).** Distilled and DFR now sample + with **Euler ancestral (`eta=1.0, s_noise=1.0`) on LTX-2.5+ checkpoints**, "so generated output + for those checkpoints differs from the previous release". Diffusers (`main` today) still runs + `LTX2Pipeline` on deterministic `FlowMatchEulerDiscreteScheduler`; its `LTXEulerAncestralRFScheduler` + is wired only into the DFR temporal-refine pipeline (eta 0.5). The repo therefore matches the + library, not the vendor's current reference sampler. Not a repo error; a divergence to know about. +2. **Chunked long-video generation** (1.4.0): 97-frame windows, 25-frame carry, crossfade over the + carried overlap by default, `--chunked` on Distilled/IC-LoRA/Dub-It. This is now the vendor's + documented answer to "longer than one pass", where on 2026-09-07 there was none. +3. **ICLoraPipeline is now always two-stage** on the CLI (half-res with the IC-LoRA and reference, + 2x upsample, full-res refine without the LoRA unless `--stage-2-ic-lora`), with opt-in transformer + tiling (window 1024x1536). The repo's IC-LoRA templates are single-stage at target size with + `reference_downscale_factor` 2 (upscaler) or 1 (restorers) — the diffusers pattern, still valid + per the cards. +4. **1.4.0 fix**: "Generated audio could outlast the decoded video when `num_frames` was not on the + video VAE temporal grid". The repo refuses off-grid counts, so unaffected. +5. **New IC-LoRAs on the Hub**: Refine-Details 1.0 and Restore 1.0 (2026-09-27), Alpha-Gen 0.9 + (2026-09-28, with a dedicated `alpha_gen` pipeline in 1.4.2), plus Clean-Plate, Colorization, + Day-To-Night, Layout-To-Render, SDR-To-HDR, Water-Simulation and two plain LoRAs. The four the + repo uses are unchanged in filename and version (Deblur/Decompression/Ingredients still 0.9, + Pixel-Spatial-Upscaler still `x2-1.0`; its repo was modified 2026-09-14, filename unchanged). +6. **`Lightricks/LTX-2.5` README modified 2026-10-02** — raw README is gated; the HTML render I got + lists the same files as before. Content of the change unverified. +7. **The vendor docs site** (`docs.ltx.io`) now carries a full prompting guide with a multi-shot + section, undated; a secondary page citing "the 2.5 prompt guide" is dated 2026-09-22. The prior + audit could not reach any vendor prompting page beyond the GitHub README. +8. **diffusers**: DFR pipeline merged 2026-08-31 (#14567, already present on 09-07); since then only + doc/typo and return-type commits touched `pipelines/ltx2`. No behavioural change to anything the + repo uses. + +### Prior findings — repaired vs still open + +| Prior finding | Status | +| --- | --- | +| Two-stage missing the 3-sigma refine, decoding to pixels between stages | **Repaired.** `two-stage.json` `refine`: `latents: previous_result:upscale.frames`, `audio_latents: previous_result:base.audio`, `noise_scale: 0.909375`, `sigmas: STAGE_2_DISTILLED_SIGMA_VALUES`; base step `output_type: latent`. | +| Prompt library wrong genre (35–55 words, tags) | **Repaired.** All seven captions are now 165–215 words, open on the action, carry shot type / camera motion / viewpoint in prose, interleave audio chronologically, use "Initially / A moment later / Simultaneously"; `intended_model: "ltx-2.5"`. Dual-panel and reference-sheet prompts follow their cards' forms. | +| README links to eight non-existent files; "LTX-2.5" linked to LTX-Video | **Repaired.** | +| Two-stage billed as the recommended quality path | **Repaired.** README: "Lightricks' newer DFR pipeline is the follow-up"; RECIPES: "Since 2026-08 Lightricks route production quality through their DFR pipeline". | +| CRF-18 image re-compression undocumented | **Repaired** (skill hard rule). | +| fps trap undocumented | **Repaired** (skill hard rule, `MAX_CONDITIONING_FPS`). | +| `"intended_model": "ltx-2"` / summaries saying LTX-2 | **Repaired.** | +| IC-LoRA "clean low-res render" and creative-not-faithful caveat | **Repaired** (upscale-clip description and README). | +| "~38GB in bf16" for the dev transformer | **Now CONFIRMED as approximately right**: `ltx-2.5-22b-dev-transformer-bf16.safetensors` is 42.0 GB decimal = 39.1 GiB. The prior "likely off" was wrong. | +| Keyframe strength below 1.0 for interpolation | **Still open**, downgraded: the diffusers docs' own FLF2V example also pins both ends at 1.0; only `conditioning.md`'s guiding-latent note and the weights card's "0.8–1.0" argue for lower. Optional. | +| DFR, generated keyframe slots, temporal upscaler, NVFP4/fp8, HDR, Retake | **Still open** (README and RECIPES now acknowledge DFR exists; none shipped). | +| Multishot | **Still open and now the largest gap** — the repo has zero multi-shot content (grep for multi-shot/multishot/hard cut/dissolve/match cut/screenplay/docs.ltx.io returns nothing in the skill, templates, prompts or RECIPES). | +| Enhancer on CPU at uint4 | Unchanged, unsourced. | + +--- + +## Claim-by-claim verdict + +### `plugins/dw/skills/ltx-2.5/SKILL.md` + +| Claim | Verdict | +| --- | --- | +| "generates video and a soundtrack together, 24 fps, on a distilled schedule that is not a knob" | **CONFIRMED** — Diffusers card; `constants.py` 24 fps default; distilled sigmas. | +| "Every template here fits a 24 GB card" | **UNSOURCED** (repo-measured; vendor publishes no 24 GB recipe). | +| text-to-video "960x544, 121 frames, 5s" | **CONFIRMED** legal (960/544 % 32 = 0; 121 = 8·15+1; 5.04 s). Vendor ComfyUI default is 768x512x97 base → 1536x1024; both legal. | +| image-to-video "481 frames is 20s in one pass, so extend or chain only past that" | **CONFIRMED** — `max_seconds` default 20.0; hosted API caps 2.5 at 20 s; 481 = 8·60+1. | +| keyframes "first and last frames pinned" | **CONFIRMED** — diffusers docs FLF2V recipe, `index 0` / `index -1`. | +| enhance-prompt "the model's enhancer writes the caption" | **CONFIRMED** — Diffusers card: dedicated `google/gemma-4-E2B-it`; vendor guide: "Gemma 4 E2B model". | +| two-stage "eight sigmas at 768x448, a 2x latent upsample, then renoise and three stage-two sigmas at 1536x896 carrying audio latents through. The upsample alone is soft; the refine pass supplies the detail." | **CONFIRMED** — Diffusers card "3-sigma tail", `pipelines.md` "8 steps in stage 1, 4 steps in stage 2" (upstream's 4th is the trailing 0.0), diffusers docs code. "Soft without refine" is repo-measured, consistent with the sources. | +| diffusion-decode "without a `shi-labs/natten` build its fallback OOMs on 24GB at any size" | **UNSOURCED** upstream (repo measurement #153); the diffusers docs call the NATTEN processor "optional" and the vendor says "Install the `natten` extra for production DiffVAE decode". Not contradicted. | +| generative-upscale: IC-LoRA re-render at 2x, `base_*` is the draft size | **CONFIRMED** — Pixel-Spatial-Upscaler card (~280p draft, factor 2, strength 1.0). | +| reference-sheet "the family's only identity route"; "reference_frames must stay at or above 121" | **CONFIRMED** — Ingredients card (≥121 frames, 768x448x121, two-part prompt). "Only route" is true of the repo, and of the vendor's adapter table (Ingredients is the one identity adapter). | +| restore-deblur / restore-decompression "each inverts one defect and no other … neither upscales or removes motion blur or grain" | **CONFIRMED** — both cards list those exclusions verbatim. | +| upscale-clip / refine-clip rules (2x, `num_frames` ≤ source, silent source refused) | **UNSOURCED** (repo engineering). The vendor's own clip-refine route is now the **Refine-Details IC-LoRA 1.0** (2026-09-27), which the repo lacks. | +| extend-clip / chained-segments "Neither is a Lightricks recipe; a single 481-frame pass reaches 20 seconds before either is needed" | **CONFIRMED** as stated, with a dated caveat: since 2026-09-29 Lightricks *does* have a long-video recipe (chunked windows, 97/25). The hosted Extend API: input ≥73 frames, `context + duration ≤ 505`. | +| `num_frames` is `8k+1`; w/h multiples of 32 | **CONFIRMED** — `check_inputs` (ValueError), weights card, `native-resolution.md`. | +| "RoPE time is frame / fps … trained around 24, 25, 30 and 60 … condition at 60 at most, never 120 … 48 and 96 fps are the DFR pipelines'" | **CONFIRMED** — `utils.py` comment block; `pipelines.md` DFR table and "snaps conditioning fps to 60 whenever playback fps is above 30". Hosted 2.5 also offers 25/48/50 fps, consistent. | +| eight sigmas, guidance 1.0, STG and modality off, no `num_inference_steps`; "those knobs are the dev transformer's" | **CONFIRMED** — Diffusers card; diffusers docs full/SFT recipe (`transformer_full`, 30 steps, CFG 3/7, STG block 28). | +| "renoise at 0.909375 (the first `STAGE_2_DISTILLED_SIGMA_VALUES` entry)" | **CONFIRMED** — `utils.py`; diffusers docs `noise_scale=STAGE_2_DISTILLED_SIGMA_VALUES[0]`. | +| "An image condition is re-compressed at CRF 18 … needs a PIL image; a multi-frame video condition is not" | **CONFIRMED** — `utils.py` `LTX2_5_IMAGE_CRF = 18`, `apply_image_conditioning_crf` requirements; `constants.py` `LTX_2_4_IMAGE_CRF = 18`. | +| "Audio is generated in the first pass and nothing refines it" | **CONFIRMED** — `pipelines.md` DFR: "Audio comes from stage 1 … nothing refines audio after stage 1." | +| Prompts: "one paragraph of roughly 150 to 220 words in the present progressive, opening on the action, stating for every shot a shot type, a camera motion (say static when none) and a viewpoint, soundscape interleaved" | **CONFIRMED** — byte-identical system prompt on GitHub and in diffusers. | +| "For an image-conditioned clip describe only what changes from the image; restating it invites a scene cut" and "(the image-to-video variant, `LTX2_5_I2V_DEFAULT_SYSTEM_PROMPT`, adds the describe-only-changes rule)" | **CONTRADICTED.** That rule is in `I2V_DEFAULT_SYSTEM_PROMPT` (Gemma 3; LTX-2.0/2.3; `utils.py:197`). `LTX2_5_I2V_DEFAULT_SYSTEM_PROMPT` says: "the opening of your caption must match the reference image exactly — same subject(s), identity, appearance, clothing, setting, lighting, and composition as shown … describe it faithfully, then narrate chronologically … Never contradict, replace, or invent things not consistent with the image. Single continuous take — no hard cuts." The vendor ComfyUI I2V page says "describe what happens, not what already exists in the image", so the human-facing advice and the enhancer spec differ; what both agree on is *never contradict the image* and *single continuous take*. The "scene cut" consequence is 2.0/2.3-era text. | +| "The `ltx2/` stored prompts follow it" | **CONFIRMED** (see prompt table). | +| Quoted spec block | **CONFIRMED** byte-identical to GitHub `gemma4_t2v_system_prompt.txt` and the installed constant. | +| Run-and-judge section (validate, cost, `get_output_frames`, subfolders) | Repo MCP mechanics, out of scope; **UNSOURCED** by design. | +| Sources footer "Read 2026-09-07" | Accurate but stale; omits docs.ltx.io. | + +### `workflows/templates/ltx2/README.md` + +| Claim | Verdict | +| --- | --- | +| Links to `Lightricks/LTX-2.5-Diffusers`; "fitted onto a single 24GB consumer GPU" | **CONFIRMED** / repo-measured. | +| "trained on one-paragraph captions of roughly 150-220 words that carry a shot type, a camera motion and a viewpoint in prose, with the soundscape interleaved … ships inside diffusers as `LTX2_5_T2V_DEFAULT_SYSTEM_PROMPT` and `LTX2_5_I2V_DEFAULT_SYSTEM_PROMPT` … which is what the enhancer runs" | **CONFIRMED** — identical to upstream; `pipeline_ltx2_image2video.py` defaults to the 2.5 I2V prompt when a dedicated enhancer is present. | +| "Audited against those sources on 2026-09-07" | Accurate; now superseded by this re-audit. | +| text-to-video "fixed eight-step distilled schedule, quantized per component" | **CONFIRMED** / **UNSOURCED** (SDNQ). | +| image-to-video "VAE tiling for the longer clip" | **CONFIRMED** — diffusers docs: `enable_tiling` "essential for ≥960p". | +| keyframes "each with the latent index it lands on" | **CONFIRMED**. | +| enhance-prompt "rewrites a one-line idea into a trained-format prompt, conditioned on the reference frame" | **CONFIRMED**. | +| two-stage "three moves … Lightricks' newer DFR pipeline is the follow-up" | **CONFIRMED**. | +| generative-upscale "in-context LoRA re-renders a clip at twice the size, inventing detail" | **CONFIRMED** (card). | +| upscale-clip "any other ratio is center-cropped … Generative, not a faithful resize; restore a compressed source first" | Crop: repo behaviour. "Not faithful" and "not an artefact remover": **CONFIRMED** (card). | +| refine-clip "the source's own latents, not a reference re-render, and no LoRA … stretched" | Repo design. **Note**: the vendor's documented clip-refine is Refine-Details IC-LoRA (reference-driven, factor 1, 8 sigmas, tiled); the repo's 3-sigma latent refine of a user clip has no vendor analogue. **UNSOURCED**, not contradicted. | +| diffusion-decode "needs a `shi-labs/natten` build … FlexAttention fallback needs ~25.5GiB" | **UNSOURCED** upstream (measured #153). Vendor: "without it the decoder falls back to Triton" (ltx-pipelines, not diffusers). | +| reference-sheet "0.9 preview weight, trained at 768x448x121 @ 24fps"; sheet authoring rules; two-part prompt | **CONFIRMED** — Ingredients card. | +| restore-* "0.9 preview weights trained at 960x544x121 @ 24fps; generating far above that bucket weakens the effect"; "Lower `lora_scale` toward 0.8 if it over-sharpens into haloing" | **CONFIRMED** — Deblur and Decompression cards. | +| extend-clip "conditioning on it in full" | **CONFIRMED mechanically** (`LTX2VideoCondition` sequences); still **UNSOURCED as a Lightricks recipe** (`conditioning.md` scopes whole-video conditioning to ICLoraPipeline; 1.4.0's chunked path is the vendor's long-clip route). | +| chained-segments | **UNSOURCED** (repo feature); 1.4.0 chunking is the dated vendor analogue (25-frame carry, full-overlap crossfade vs the repo's `trim_frames: 2`, `crossfade_ms: 80`). | + +### Templates — shared settings + +| Claim | Verdict | +| --- | --- | +| `sigmas: constant:…DISTILLED_SIGMA_VALUES`, no `num_inference_steps`; guidance/STG/modality 1.0/0.0/1.0 for video and audio | **CONFIRMED** — Diffusers card, diffusers docs. | +| `negative_prompt: DEFAULT_NEGATIVE_PROMPT` | **CONFIRMED** (library constant). With `guidance_scale` 1.0 the negative prompt is ignored (diffusers docs: "ignored if `guidance_scale=1.0`"), so it is inert in every distilled template. Harmless; worth one line somewhere. | +| `frame_rate: 24.0` | **CONFIRMED**. | +| `torch_dtype: torch.bfloat16`; Gemma4Unified text encoder from the `text_encoder` subfolder; `duration_head` present | **CONFIRMED**. | +| `variable_constraints` reasons: "pipelines refuse a width not divisible by 32 outright (ValueError)"; "an off-grid count is … floored to the grid below" | **CONFIRMED** — `check_inputs`; 1.4.0 fix text ("87 frames at 24 fps decoded 81 picture frames") confirms the floor. | +| `vram_estimate` calibration text (text-to-video) | Repo-measured; **UNSOURCED**. | +| SDNQ uint4 / int8, group offload | **UNSOURCED**; vendor documents fp8-cast, NVFP4 (18.7 GB file now on the Hub), `--offload cpu|disk`. | + +### Per-template specifics + +- **text-to-video**: "First load adds about 10 minutes" — repo-measured. Default prompt `fox_dawn_choir` — now **CONFIRMED** to the spec (~190 words, wide shot → medium shot from a low angle, tracking camera, choir interleaved). +- **image-to-video**: 768x768x481 — **CONFIRMED** legal. `marmot_robot_overlords` (~190 words) opens on the marmot's existing appearance (hat, monocle) then narrates — this matches the **2.5** I2V spec ("describe it faithfully, then narrate") and is what the skill currently calls a mistake. Single take, no cuts: **CONFIRMED**. +- **keyframes**: strength 1.0 both ends — **CONFIRMED** by the diffusers docs example; `conditioning.md` / weights card "0.8–1.0" leave room below 1.0. `polaroid_lighthouse` describes a push past the border into live footage "in a matched close-up" — a framing change inside one take, legal. +- **enhance-prompt**: `google/gemma-4-E2B-it` + `AutoProcessor`, no `system_prompt` → 2.5 I2V default; `prompt_max_new_tokens` 600 (redundant, library picks it) — **CONFIRMED**. `min_seconds` 2.0 / `max_seconds` 8.0 — legal (defaults 1.0/20.0). Vendor guide adds: the predictor "times it as written … Write the beats you want into the prompt" — **missing** from the template description. Enhancer on `cpu` at uint4 — **UNSOURCED**. +- **two-stage**: description's three moves, audio latents carried, latents not decoded, `noise_scale` 0.909375 held by a test, output exactly 2x, "delivered size always a multiple of 64" — all **CONFIRMED** (diffusers docs: "64 for stage 1 of two-stage"). `adain_factor` / `tone_map_compression_ratio` 0.0 — library defaults, no source recommends otherwise. +- **generative-upscale / upscale-clip**: LoRA repo, filename, `scale` 1.0, `reference_downscale_factor` 2, 480x288 → 960x576 — **CONFIRMED** (card). Note the vendor's DFR uses the same adapter at **0.5** as a detailing pass; the standalone card says 1.0. Both right for their role. +- **reference-sheet**: `reference_downscale_factor` 1, `loop_frames` to 121, 768x448x121 — **CONFIRMED** (card: reference ≥121 frames, "match the output resolution"). +- **restore-deblur / restore-decompression**: filenames `…-0.9.safetensors`, 960x544x121, factor 1, dual-panel prompts — **CONFIRMED** (cards). The vendor's `ic-lo-ra.md` adds the general rule "Adapters with trigger phrases (e.g. `DEBLUR`…) require prepending the trigger" — the stored prompts put `DEBLUR` / `ENHANCE QUALITY` mid-paragraph after the "Reference shows … Edited shows …" preamble, which is the card's own form; fine. +- **extend-clip**: 768x448, 121 → 241, whole opening at `index 0`, strength 1.0 — mechanically **CONFIRMED**; `lighthouse_keeper_gallery` re-identifies "The same lighthouse keeper … still in the yellow oilskin" — which is exactly the vendor's multi-shot identity rule applied across passes. Good. +- **chained-segments**: `trim_frames: 2`, `crossfade_ms: 80` — repo; each segment pays CRF-18 re-compression (confirmed by `utils.py`). +- **refine-clip**: see README row. 512x288 default, `normalize_audio` first — repo. +- **diffusion-decode**: `denormalize: false` — **CONFIRMED** (diffusers docs "latents pre-normalized by pipeline"); "upstream treats it as the production path" — **CONFIRMED** (`pipelines.md`: "Install the `natten` extra for production DiffVAE decode"). + +### `prompts/ltx2/*.json` + +| Prompt | Words | Verdict | +| --- | --- | --- | +| `fox_dawn_choir` | ~190 | **CONFIRMED** — framing triple twice (wide/side/tracking → medium/low-angle), audio interleaved, "Initially / a moment later / Simultaneously", quality descriptors woven ("warm cinematic light"). | +| `hummingbird_garden` | ~185 | **CONFIRMED**. | +| `lighthouse_keeper` / `_gallery` | ~190 each | **CONFIRMED**; the gallery caption's re-identification matches the vendor's cut rule. | +| `marmot_robot_overlords` | ~190 | **CONFIRMED** to the 2.5 I2V spec (grounded opening, single take). | +| `night-tram` | ~215 | **CONFIRMED** — at the top of the band; "a wide shot from a low angle in side view as the camera remains static" is the spec's exact "say static" form. | +| `polaroid_lighthouse` | ~165 | **CONFIRMED**. | +| `best-of-n-video-motion` | ~190 | **CONFIRMED** (I2V-shaped, grounded opening). | +| `deblur_dual_panel`, `decompression_dual_panel`, `reference_sheet_workshop` | — | **CONFIRMED** to their cards' forms. `reference_sheet_workshop`'s "Generated video:" half is plain prose, not the caption spec ("her expression settles … into a small, satisfied smile" infers emotion); the Ingredients card prescribes no caption style for that half. Unsourced either way. | + +### `docs/RECIPES_24GB.md` (LTX-2.5 section) + +| Claim | Verdict | +| --- | --- | +| Per-component placement table | **UNSOURCED** (repo). | +| "`transformer` is the distilled model … fixed 8-step … not a knob … referenced from diffusers" | **CONFIRMED**. | +| "`subfolder: "transformer_full"`, the dev model" | **CONFIRMED** — Diffusers card lists `transformer_full/`; diffusers docs recipe uses it. | +| "the same ~38GB in bf16" | **CONFIRMED (approx.)** — 42.0 GB decimal / 39.1 GiB on the Hub. | +| "guidance … costs three transformer passes per step" | Consistent with the diffusers docs' three guidance types; **UNSOURCED** as a count. | +| Diffusion decoder paragraph (FlexAttention fallback, NATTEN, measured GiB) | **UNSOURCED** upstream; internally consistent. | +| "Spend headroom on the two-stage flow … the flow the model card, Lightricks' pipeline notes and the diffusers docs all describe … Since 2026-08 Lightricks route production quality through their DFR pipeline" | **CONFIRMED**. | +| "Measured on an RTX 3090 the refined clip is sharper than the 2x upsample alone" | Repo measurement; consistent with the sources. | +| Example links | **CONFIRMED** (all eight exist). | + +### `docs/ACCELERATION.md` / `docs/QUANTIZATION.md` + +Unchanged from the prior audit: diffusers-internals observations (two hidden-state streams), **UNSOURCED** upstream, not contradicted. Still no mention of fp8-cast / NVFP4 (the `…-distilled-transformer-nvfp4.safetensors` is 18.7 GB — the vendor's own 24 GB-class route), CUDA-graph capture, or the 1.4.0 `ltx_kernels_inductor` backend. CUDA/Blackwell-only, so low priority for the catalog. + +--- + +## Missing knowledge + +Vendor recommendations the repo does not carry, newest first. + +1. **Multi-shot prompting grammar (docs.ltx.io, undated, ≤2026-09-22).** The LTX-2.5 headline capability; the repo has nothing. The rules are short (four things at every cut, 2–4 shots, when to stay single-shot, I2V stays single-take). +2. **Chunked long-video generation (LTX-2 1.4.0, 2026-09-29).** 97-frame windows, 25-frame carry, crossfade across the carried overlap. The repo's extend/chain templates predate a vendor recipe and now have one to compare against; the skill's "Neither is a Lightricks recipe" needs a date on it. +3. **Refine-Details IC-LoRA 1.0 (2026-09-27)** — the vendor's route for "sharpen the user's clip": factor 1, 1024x576x97 tile, generic rendering-only prompt, 8 distilled sigmas, tiled. The repo's `refine-clip` and `upscale-clip` are the nearest and neither is this. +4. **Restore IC-LoRA 1.0 (2026-09-27)** — archive footage incl. colourisation, 960x544 tile, strength as a switch, positive/negative prompt split. Sits beside Deblur/Decompression as a third restoration route. +5. **Euler-ancestral sampling on 2.5 checkpoints (1.4.0).** The vendor's reference output now differs from diffusers' deterministic Euler. Nothing for the repo to do until diffusers follows; a note prevents a future "why does ComfyUI look different" ticket. +6. **The pacing rule for the duration head** (vendor guide): "times it as written … write the beats … or set an explicit duration". Belongs in `enhance-prompt`'s description and the skill. +7. **"Coming from another model? rewrite it"** (vendor guide) — directly relevant to the `script-to-video` skill, which routes shots between H3 and LTX-2.5. +8. **Vendor limits to warn about** (guide): on-screen text not reliable ("add … in post"); chaotic physics; "one coherent light logic per shot"; "describing how subjects appear after the movement helps the model complete the motion". +9. **Hosted-API numbers an agent may be asked about** (docs.ltx.io/models/ltx-2-5, 2026-08-11): 24/25/48/50 fps, 6–20 s, 16:9 / 9:16, 4K on both tiers; Extend 2–20 s with `context + duration ≤ 505` frames and input ≥73 frames. +10. **Negative prompt is inert at `guidance_scale` 1.0** (diffusers docs) — every distilled template passes one. +11. Carried over, still open: DFR (w/h % 64, 4K = 3840x2176, detailing adapter at 0.5, temporal rounds 48/96 fps), generated keyframe slots (fast motion), NVFP4/fp8, HDR/EXR, Retake, keyframe strength < 1.0, IC-LoRA steps/guidance trade-off (fixed on the distilled schedule anyway). +12. The Diffusers card's "19B" is stale (weights are 22B); the repo says 22B, so nothing to fix, but an agent reading the card will see a conflict. + +--- + +## Assessment + +**What a skill can trust as-is.** Every mechanical rule in the skill survives re-audit: the +distilled sigmas and guidance-off settings, `8k+1` and multiples of 32, 24 fps with the 60-fps +conditioning cap, the CRF-18 image rule, the three-move two-stage with `noise_scale` 0.909375 and +audio latents carried, the IC-LoRA filenames/versions/strengths/buckets for all four adapters the +repo uses, the 20-second single-pass ceiling, and the quoted caption spec, which is byte-identical +to Lightricks' file on GitHub today. The prompt library is now a correct exemplar set. The prior +audit's two blocking findings (missing stage 2, wrong-genre prompts) are fully repaired. + +**What needs correction.** One claim is wrong: the skill says the LTX-2.5 I2V system prompt adds +"describe only changes from the image; restating invites a scene cut". That is the Gemma-3 +(2.0/2.3) prompt. The 2.5 I2V prompt says ground the opening *in* the image, then narrate, single +continuous take, no hard cuts. The repo's own I2V captions already do the 2.5 thing; the skill's +sentence should say *never contradict the image; keep one continuous take; the image supplies the +look, the caption supplies the motion* — which reconciles the enhancer spec with the vendor's +"describe what happens" ComfyUI advice. Also stale: "Neither is a Lightricks recipe" (true until +2026-09-29) and the sources footer. + +**Spec vs vendor guide.** They are complementary, not in conflict, once the audience is named. The +system prompt is the *enhancer's* instruction: turn a short request into one training-style caption +for one continuous shot — 150–220 words, exactly one shot type *per shot*, no headers. The docs +guide is for *humans typing prompts*: 4–8 sentences (≈ the same word band), and it adds the two +things the spec never addresses — the multi-shot grammar (name the cut, re-establish the framing, +re-identify, state audio continuity, 2–4 shots) and screenplay form for dialogue-heavy scenes. The +spec's "for every shot you MUST include…" already presumes shots can be plural. The only real +tension is cosmetic: the vendor's samples open with "The shot opens on…" and use sluglines, which +the spec forbids; the spec reflects training captions, so prefer it, and do not run the enhancer on +a multi-shot or screenplay prompt (the vendor says it "helps least" on structured input, and the +2.5 I2V enhancer prompt forces a single take). + +**Proposal under the cap.** The skill is at 12,288 of 12,288 bytes. Add roughly 900 bytes by +removing roughly 900: (a) replace the I2V sentence (~190 B) with the corrected one (~170 B); +(b) add a "Several shots" paragraph (~550 B): one chronological paragraph, at each cut name the +transition in prose, re-establish shot scale/angle/lighting, reuse the subject's identifiers, say +what the audio does; 2–4 shots; stay single-take for I2V, lip-synced dialogue or unbroken camera +moves; don't enhance a multi-shot prompt; (c) add the pacing rule (~150 B) and "on-screen text and +chaotic physics are unreliable" (~90 B); (d) pay for it by cutting the `get_memory` paragraph in +"Before anything" (~700 B, generic MCP procedure duplicated in the `minimax-h3` skill and the +guides) and the `export_job` sentence in "Run and judge" (~250 B); (e) update the sources footer to +name docs.ltx.io and 2026-10-02. The 3.8 KB spec block stays — it is test-pinned and is the one +vendor text the plugin carries. Separately, in the catalog rather than the skill: a Refine-Details +template is the vendor's "sharpen my clip" route and would retire the unsourced parts of +`refine-clip`'s description. diff --git a/docs/proposals/complete/h3-latent-upscale-complete.md b/docs/proposals/complete/h3-latent-upscale-complete.md new file mode 100644 index 00000000..5a01c1ad --- /dev/null +++ b/docs/proposals/complete/h3-latent-upscale-complete.md @@ -0,0 +1,176 @@ +# H3 latent upscaler: promote a 544p take to 1344x768 in latent space (#471) + +Written by model `claude-opus-5-5` via provider `anthropic` (close-out, +2026-10-03). Plan v1 was approved by Don on 2026-09-27 without answers to +Q1-Q4, so each question's stated default stood. Stage 1, #499, shipped in +its final form 2026-10-03 and was verified the same day. Stage 2, #500, +was not built: Don's gate after stage 1 came back "close but soft". + +## The idea + +[LBH-123-AI/Minimax_h3_latent_Upscaler](https://huggingface.co/LBH-123-AI/Minimax_h3_latent_Upscaler) +is a 345M-parameter 3D-conv network that resizes H3's 24-channel video +latents in H×W (1x-4x, T kept). The idea was an opt-in path: generate an H3 +clip at 960x544, upscale its latents to 1344x768 (the grid goes 34×60 → +48×84), and decode. That skips a native 768p render. The existing +`templates/minimax/*` templates and the `minimax-h3` skill's defaults were +to stay exactly as they were. + +## Verdict + +**Build smaller.** The demand was Don's own: #484's native 768p Ref2VA +shots turned smeared crowd faces crisp, but took ~31 min each. Nobody had +shown that the upscaler's output was any good without a refine pass, so the +plan built the measurement first (stage 1) and gated the catalog template +(stage 2) on Don comparing it against a native 768p render. The refine pass +(a new denoise-schedule block, ~$8-12) and persisted latents were deferred. + +## What the plan found against the issue + +1. **dw cannot drop H3's decode block.** The base step decodes at 544p and + also hands its `latents` on through `output` (`previous_result:base.latents`). +2. **Promotion without regenerating isn't free.** Latents live only in the + in-process step cache, so promoting an earlier take means re-running its + 544p base with the same seed inside the promoting workflow. +3. **There was no H3 decode step to reuse.** A decode task has to run + diffusers' `MiniMaxH3VideoDecodeStep` (denormalize, fp16 autocast decode, + pixel revert), or the colours come out wrong. +4. **The target is given in pixels**, multiples of 16, and divided by 16 in + the task. The non-uniform 34×60 → 48×84 is allowed. +5. **Normalization: the plan was wrong here, see below.** The plan (and + the issue) said that pipeline latents are already normalized, so the + ComfyUI node's `(x-mean)/std → net → x*std+mean` wrapper would normalize + twice and should be dropped. +6. **`plan.downloads_required` doesn't count a task's weights** (a gap that + predates this feature; `upscale` and RIFE miss it too). Q3's default kept + that fix out of this feature. + +## What was built + +### Stage 1 (#499): `upscale_h3_latents` and `decode_h3_latents` + +Final form: `develop` @ `f01ae1fa` (`924a973d`, a revert of the +withdrawal plus the fix), then `8a5136da` (`f24b4406`, the docstring fix +from the architecture review). `c1e8455b` (#584) later gave the guide's +workflow a `num_frames` constraint. + +- **`upscale_h3_latents(latents, width, height, model_name?, weight_name?)`** + in `dw/tasks/h3_latent_upscale.py`. The network is vendored under MIT in + `dw/tasks/h3_latent_upscaler_model.py`, which asserts the v1 architecture + instead of inferring it freely. + - The default weights are the bf16 safetensors (Q4), read from the + `minimax_h3_latent_upscaler_3d_conv_v1/` subfolder at the pinned + revision `3f941d5d182014dd5c0a5e16330420ee2d4aa0c6`. + - It applies the node's normalization wrapper in float32. The node's + mean/std are checked to equal the H3 VAE's `latents_mean`/`latents_std`. + - Refusals, each naming the argument: not a 5-D, 24-channel tensor; a + target that isn't a multiple of 16; a per-axis scale outside 1x-4x; a + target over the 1344x768 (or 768x1344) canvas. `task_domains` refuses + `width`/`height` ≤ 0 at validate. + - `model_name` must be a Hub repo id (paths, URLs, `a/b/c` and `..` are + refused at validate). `weight_name` must be a bare `.safetensors` file + name. `/`, `\`, `..`, absolute paths and `.pth`/`.bin`/`.pt`/`.ckpt` are + refused at validate, and checked again at run time before any download. + So the trust gate was not widened. +- **`decode_h3_latents(latents, model_name?)`** runs diffusers' own + `MiniMaxH3VideoDecodeStep` on the H3 VAE. +- **Audio policy:** the base pass's track, re-paired with `pair_audio`. + Nothing re-denoises. +- **Docs:** `docs/WORKFLOW_GUIDE.md` *Promoting an H3 take to 768p in latent + space* holds the inline `H3LatentUpscalePreview` workflow (base → up → + decode → mux). `get_guide` serves it, and `list_tasks`/`get_task` + describe both tasks. +- **General fixes that landed under this stage:** + - `3ec965e6`: a `previous_result:` property that no result carries is an + error, not a step that "succeeds" after zero iterations; + - `465a61ed`: `pair_audio` unwraps a pipeline's batch of one video; + - `a6f816e8`: a location refusal no longer echoes a server directory. + +### Normalization: what went wrong, and the fix + +The first build dropped the node's wrapper, as the plan said. Its output was +garbage: checkerboard, ghosting and magenta blobs, while +`decode_h3_latents` on the *un-upscaled* latents decoded clean. The stage +was withdrawn for release 0.5.0 (#526, `777f2eb2`). Don then asked for a +tensor diff against the ComfyUI reference node before any rebuild: + +- The vendored network against the node's own class, with the real + weights: max |diff| = 0.0. The network was never the bug. +- ComfyUI's H3 VAE `encode` already returns normalized latents, the same + space a diffusers H3 pipeline returns. The node then normalizes a + **second** time. So the network was trained on doubly normalized latents, + and the wrapper has to be applied. +- On a unit-variance input with no wrapper, the output std was 5.28 and the + mean −0.83. With the wrapper, they were 0.92 and 0.00. + +The lesson for a revival or a v2 re-pin: check a vendored model's input +space against the reference implementation's numbers, not against reasoning +about which side normalizes. + +### Stage 2 (#500): not built + +`templates/minimax/upscale-preview` and its `minimax-h3` skill line were +gated on Don's comparison and closed as not planned on 2026-10-03. + +## Measured + +**Verification** (lem, RTX 3090, `develop` @ `8a5136da`, 124 frames, job +`baaeda980afa`): +- 1344×768, 124 frames at 24 fps, base audio (32 kHz stereo, 5.17 s). No + colour cast, no wash-out. The identity decode of `base.latents` matches + the 544p reference. +- Latency 629.99 s in all: the upscale took 4.3 s, the decode 131.3 s, and + decode VRAM was 10.93 GB reserved. That is a point reading at step + boundaries, because the events carry no true peak. +- With `base` served from the step cache, a 1x run took 76 s and a 960×768 + run took 96 s. + +**Don's gate** (same T2VA crowd-faces prompt, seed 42, 124 frames): +- upscale preview (job `f90ea84a796e`): 7.8 min; +- native `video-with-audio-768p` (job `074e65929a9d`): 12.7 min. + +The native render held distinct faces. The preview's faces were soft and +waxy, with eyes smeared in several. Candle highlights and cobblestones +sharpened well: it is a clean upscale of a soft 544p base, and as the plan +predicted, it can't show detail the base never generated. Saving ~5 min +(~39%) didn't offset that. Caveats: one seed and one prompt, and the two +paths use different LoRAs and shifts. The #484 Ref2VA crowd case itself was +not tested. + +## Bounces per stage + +- **Stage 1 (#499): 4 tester bounces, then a park, a withdrawal and a + rebuild, then 1 architecture-review bounce.** + 1. M-F039: the guide section was a `####` heading, which `get_guide` + can't reach. + 2. M-F041: the mux read `base.sample_rate`, a key the modular result + doesn't carry, and silently wrote nothing (hence `3ec965e6`). + 3. M-F041: `pair_audio` crashed on the unwrapped video batch (hence + `465a61ed`). + 4. M-F041: the upscaled output was garbage (normalization, above). The + loop driver parked the stage after this one, at four bounces. + - Architecture review of the rebuild: the vendored module's docstring + still gave the withdrawn reasoning about normalization. Fixed in + `f24b4406`. + - The rebuild then verified on its first pass. +- **Stage 2 (#500):** not built. + +No `usage:` figures were recorded on the stages, so cost is left out. + +## Deferred + +- **The refine pass**: a dw-owned (or upstream) replacement for H3's + set-timesteps block that takes `sigmas`, renoises the upscaled latent to + σ_start and runs the tail of the schedule with the 768p LoRA at shift 6, + with audio held or re-paired. The gate's "close but soft" is the result + plan v1 named as bringing it back. Building it needs a new plan version, + which is Don's call. #585's H3 LoRA eval is relevant: the fal Realism + People LoRA on the 768p turbo path gave the best faces at ~zero added + cost, which raises the bar a refine pass has to clear. +- **Persisted latents** (a `.safetensors` result type reachable by + `output:`): only worth it if a promotion template lands. +- **`downloads_required` counting task weights** (Q3): a separate issue + for every task, not done here. +- **Q1**: the tasks stay, since the gate wasn't a "no". They are surface + for a future refine, and the vendored v1 module stays pinned until a + deliberate re-pin to upstream's v2 or 2D variant. diff --git a/docs/proposals/complete/ltx2-upscale-clip-complete.md b/docs/proposals/complete/ltx2-upscale-clip-complete.md new file mode 100644 index 00000000..ce5de5e8 --- /dev/null +++ b/docs/proposals/complete/ltx2-upscale-clip-complete.md @@ -0,0 +1,142 @@ +# `ltx2/upscale-clip`: LTX-2.5 generative 2x upscale of an existing mp4 (#542) + +Written by model `claude-opus-5-5` via provider `anthropic` (close-out, +2026-10-02). Plan v2 was approved by Don on 2026-09-27. He answered Q1-Q4: +build, the default prompt, `pair_audio` for the source's track, and +`upscaled` kept in `intermediate/`. It was built as one stage, #548, which +shipped 2026-09-28 and was verified 2026-10-03. + +## The report + +`templates/ltx2/generative-upscale` already did a generative 2x upscale in +its `upscaled` step: `LTX2InContextPipeline` with +`reference_downscale_factor: 2`. Its reference, though, was always a clip +the template had just generated (`previous_result:low_resolution.frames`). +#542 asked for the same upscale on an mp4 from the asset library. #512 (a +general video resize task) was declined, so nothing else gave a way to add +resolution to footage dw did not make. + +## Verdict + +**Build smaller.** One template, one stage, and no validation that probes +the source. The value was stated as thin: no field report asked for it, and +none of Don's recent jobs ran the nearest siblings. It was worth building +because it is the only route of its kind and fully reversible: a catalog +entry with no engine, MCP, REST or syntax surface. + +**Cut:** checking `width`/`height` and `num_frames` against the probed +asset. No pipeline reference is probed at validate today, so it would have +been an engine change. It comes back if a field report shows a run wasted +on a mismatched source. + +## What the plan found against the issue + +1. **"width/height must be 2x the source" is neither true nor enforceable.** + The pipeline scales the reference to width/2 × height/2 and center-crops + it (`pipeline_ltx2_ic_lora.py` `:1403`, `:1424`, + `resize_mode="crop"`). Any source runs. A source with a different aspect + ratio is silently cropped, and a larger one is silently downscaled. +2. **"num_frames must match the source" doesn't hold either.** The + reference is trimmed to `num_frames`. A shorter source isn't padded, so + the output's tail is generated with no reference. +3. **It isn't `generative-upscale` minus a step.** `upscaled` borrowed its + transformer and text encoder through `reused_components`. A standalone + step needs `restore-deblur`'s full SDNQ component blocks. +4. **The upscaler's numbers were pinned nowhere**, including for + `generative-upscale`. + +## What was built + +### Stage A (#548): the template, its pins, and its docs + +Shipped `develop` @ `aa04263` (`4a826ae` template + tests, `53baa0b` +docs). The bounce fix is `77bb905` (`d8d4970`). + +- **`workflows/templates/ltx2/upscale-clip.json`**: + - `upscaled`: `LTX2InContextPipeline` + `LTX2ReferenceCondition` over + `variable:source_video` (default `asset:clip.mp4`), with + `reference_downscale_factor: 2`. The lora is + `Lightricks/LTX-2.5-22b-IC-LoRA-Pixel-Spatial-Upscaler` / + `ltx-2.5-22b-ic-lora-pixel-spatial-upscaler-x2-1.0.safetensors` at + `lora_scale` 1.0, on the restore templates' SDNQ components. Saved to + `intermediate/`. + - `with_source_audio`: `pair_audio(video=previous_result:upscaled, + audio=variable:source_video, fit="video")`, saved to `final/`. A silent + source fails here with "carries no audio track", after the upscale is + saved. + - Defaults are 960×544, 121 frames, 24 fps: a 480×272 source doubled. + `variable_constraints` are 32n width/height and 8n+1 `num_frames` + (min 9, no snap). `cost_drivers` are `num_frames`/`width`/`height`, with + no curated `cost`. + - The description states the crop/downscale rule, `num_frames` ≤ the + source's length, reading the source first with `get_gallery_metadata`, + that it is generative and not a faithful resize, and to run + `restore-decompression` first on a compressed source. +- **The default prompt (Q2): a literal**, not a stored `prompt:`. The vendor + card states no caption convention, so there was no trained form to store. + The literal is "A high-resolution rendering of the scene in the reference + video, with the same subject, framing and motion, and sharp, fine + detail.", and the description tells the caller to replace it. +- **Tests:** + - `tests/test_ltx2_ic_loras.py`: a `CARDS` entry for `upscale-clip`, its + place in the foreign-clip parametrize, and a new pin for + `generative-upscale` against the same card. + - `tests/test_pair_audio.py`: an mp4's track read through `pair_audio`, + and a silent mp4 named as the fault. + - `COMPACT_BUDGET` went from 8_850 to 9_000 (measured at 8_910). +- **Docs:** + - the `ltx-2.5` skill: the description, the 2x line and the "Repairing + the user's footage" line; + - the `ltx2` README: a "Quality and scale" row and the restore section's + wording; + - `CLAUDE.md` "LTX-2.5 IC-LoRAs". + +**Vendor card against `generative-upscale`:** no contradiction. It names the +same weight, factor 2 and strength 1.0, so `generative-upscale` was left +unchanged and is now pinned. The card states no trained bucket, so the +defaults are the family's. The card also scopes the LoRA as a creative +step, not for live-action footage where fidelity matters, and the +description says so. + +## Measured in verification + +From verify pass 3 on lem, `develop` @ `f01ae1fa`: +- **Happy path**, 480×272/121f/24 fps source with a soundtrack: 220.4 s. + The final is 960×544, 121 frames, 24 fps. Its audio is the source's + (every level within 0.03 dB, and 18 ms longer). `assess_output` found no + sync issue, and the framing is unchanged. +- **16:9 source** (640×360): center-cropped to 960×544 with only a thin + side trim, as 1.778 → 1.765 predicts. 49.5 s, warm. +- **`num_frames` 161 on a 121-frame source**: 161 frames, with an invented + tail. `pair_audio` warns that it padded the track with silence. 140.3 s. +- **Silent source**: fails at `with_source_audio` with the upscale kept in + `intermediate/`. +- **Larger source** (960×544): downscaled first, then runs (200.7 s, pass 2). + +## Bounces per stage + +- **Stage A (#548): one bounce.** M-F058 failed because the skill's + "Repairing the user's footage" section didn't name `upscale-clip`. The + fix had to fit the skill's size cap (`SKILL_SIZE_LIMIT` 12288, now + exactly 12288), so "restore a compressed source first" was dropped from + the skill; it stays in the template's description. +- One park, which wasn't a bounce: the `upscale/*` fixtures (480×272 with + and without audio, 640×360) had to be cut off-box with ffmpeg. Don placed + them in `regression-model-specific`'s asset library on 2026-10-02. The + suite recipe's `upload_asset` path was corrected in dkackman/harnest + `17d65f4`. + +No `usage:` figures were recorded on the stage, so cost is left out. + +## Deferred + +- **Probing the source at validate** for size and length. It comes back on + a field report of a wasted run. +- **Giving the restore templates the same source-audio treatment** (Q3). + This needs its own issue. +- **The vendor's Refine-Details IC-LoRA 1.0** (2026-09-27), the vendor's + own route for sharpening a user's clip, is not in the repo + (`audits/2026-10-02-ltx-2.5-audit.md`). Neither `upscale-clip` nor + `refine-clip` is that route. +- #543's `refine-clip` (latent two-stage refine) is the companion route, + recorded in `complete/ltx2-refine-clip-complete.md`. diff --git a/docs/proposals/complete/mcp-long-wait-complete.md b/docs/proposals/complete/mcp-long-wait-complete.md new file mode 100644 index 00000000..d7aa9b09 --- /dev/null +++ b/docs/proposals/complete/mcp-long-wait-complete.md @@ -0,0 +1,129 @@ +# One `wait_for_job` call that covers a long render (#377) + +Written by model `claude-opus-5-5` via provider `anthropic` (close-out, +2026-10-03). Plan v1 was approved by Don on 2026-09-27 with no answers to +Q1-Q4, so the defaults stood: a cap of 1800 on `lem`, stage 2 conditional +on a measured cut, no background-shell wait in the skills, and the old +proposal moved to `declined/`. Stage 1 (#546) shipped 2026-09-28 and was +verified 2026-10-03. Stage 2 (#547) was not built. + +## The report + +`wait_for_job` capped each call at 55 s. On 2026-09-25 a 41-minute H3 +render (5 ref2va shots, job `51441504a8b6`) needed about 45 calls, or the +workaround Don used: two blind background `sleep`s with polls between them. +At ~$0.08 a poll at ~130k context (#248), that wait cost about $3.60 in +polling alone. The proposal on file, `mcp-job-notifications.md`, asked for +cursor-based gapless signaling plus a `failure_kind`. + +## Verdict + +**Build smaller.** Build only a single call that lasts as long as the +render. The 55 s cap was a deployment default, not a client limit, and the +means to raise it (`DW_MCP_MAX_WAIT_SECONDS`) had shipped in #248 but was +never turned on or measured. + +**Cut:** both halves of the proposal. The reasons and reopen triggers are +at the top of `../declined/mcp-job-notifications.md`. In short, the cursor +fixes a gap that does not exist (terminal status is sticky), and the +proposed `failure_kind` would have labelled most GPU OOMs `error`. + +## What the plan found against the proposal + +1. **No seam gap.** Terminal status is sticky, so any later snapshot sees + it. The one real miss is a server restart dropping the job, which is + #300's and is outside a cursor's reach. +2. **OOM detection was in the wrong place.** CUDA OOMs arrive as ordinary + errors from a live worker. Only the worker can classify them. +3. **The real risk was the transport, which the proposal never + considered.** The HTTP mount answers with `json_response=True`, so a long + wait is one silent request. Any idle timeout on the path would cut it. + Stage 1 measured this, and stage 2 (a progress heartbeat over SSE) was + held back until a cut was seen. + +## What was built + +### Stage 1 (#546): the cap on lem, the measurement, the docs + +Shipped `develop` @ `e5bdbed` (`305305a`). The bounce fix is `5d48399c` +(`2666c6fc`). + +- **Deploy.** `scripts/dw-serve.service` carries + `Environment=DW_MCP_MAX_WAIT_SECONDS=1800`. `scripts/deploy.sh` passes it + (default 1800) on the screen-path launch for a box with no unit, which + covers mini-ai. #248's "no unit manages the process" was out of date: + lem runs the systemd user unit `dw-serve`, whose *installed* copy didn't + carry the variable. Don replaced it with the repo's unit on 2026-10-01 + (the old one is kept as `dw-serve.service.bak-2026-10-01`). The code + default stays 55, so an unknown deployment is unchanged. +- **Docs.** WORKFLOW_GUIDE "The loop" step 5 states the rule once: ask for + `plan.estimate` plus a margin as `timeout_seconds`; the reply's + `timeout_applied_seconds`/`timeout_capped` say what you got; call again + while `still_running`; the default is 20 s. It quotes no cap. + `docs/MCP.md` (the `wait_for_job` and `run_workflow` rows, loop step 3) + names the default of 55, the env var and the client-timeout caveat, + because it is written for operators. `dw_mcp/CLAUDE.md` names the env var. +- **Skills.** `ltx-2.5`, `minimax-h3`, `minimax-music3` and + `script-to-video` state the same rule. `series-episodes` and the guide's + transcribe example now say `wait_seconds=60`: a request sized to a short + job, not a cap. The three family skills sat at the 12,288-byte + `SKILL_SIZE_LIMIT`, so each was tightened elsewhere (about 150 bytes, + no fact removed). +- **No code change.** No engine, REST or MCP surface change, and no + description text: the tool description already interpolates the live + cap ("at most 1800.0 seconds"). +- **Client side.** Claude Code's `MCP_TOOL_TIMEOUT` defaults to about 28 h, + but its idle limit for an HTTP MCP server, + `CLAUDE_CODE_MCP_TOOL_IDLE_TIMEOUT`, defaults to 5 min and would cut a + silent long wait first. Harness sessions export both at 1900000 ms + (dkackman/harnest `b7e7e47`). An interactive session needs the same. + +### Stage 2 (#547): progress heartbeat. Not built + +It was conditional on stage 1 recording a long call cut by the transport. +Neither measurement was cut, so it closed `not planned` on 2026-10-03. +`mcp_mount` keeps `json_response=True`, and `wait_for_job` stays silent +while it blocks. + +## Measured in verification + +On lem, `templates/minimax/shots-batch`, seed 377, shots 1-3: +- **First verify** (`develop` @ `f01ae1fa`, job `865715efb4d8`, estimate + 12.4 min): one `wait_for_job(timeout_seconds=1116)` held 902 s of wall + clock and returned `succeeded`, `waited_seconds: 900.0`, not capped. +- **Second verify** (`develop` @ `3926d430`, job `eeae7f9f25d9`, estimate + 13.6 min): one `wait_for_job(timeout_seconds=1224)` returned `succeeded`, + `waited_seconds: 706.4`, not capped. +- **The clamp** on a finished job: 5000 and 1801 apply 1800 capped, 1800 + applies 1800 uncapped, and no timeout applies 20. +- **An unknown id** with `timeout_seconds=1800` fails at once with + "Unknown job" (an empty id gets "Not Found"). It never blocks for the + budget, which was the edge owed to #300. + +## Bounces per stage + +- **Stage 1 (#546): one bounce.** C-F173 failed because the `ltx-2.5` skill + never named `timeout_applied_seconds`/`timeout_capped`, though the build + comment said it did. The one-line fix had to fit 3 bytes under the size + cap, so three other phrases in the skill were trimmed. +- One park that wasn't a bounce: the cap wasn't live on lem until Don + replaced the installed systemd unit. +- **Stage 2 (#547):** not built. + +No `usage:` figures were recorded on the stages, so cost is left out. + +## Deferred + +- **The heartbeat (stage 2).** Reopen it if a long wait is cut by a + transport or idle timeout: Don's own ≥10-minute interactive wait (the + plan's last acceptance line, his to check), or a client that cannot raise + its idle limit. The design is #547's body. +- **Don's interactive line.** His own Claude Code session holding a + ≥10-minute `wait_for_job` was his check, not a suite case. It depends on + his session exporting `CLAUDE_CODE_MCP_TOOL_IDLE_TIMEOUT` above the wait. +- **C-F171's "names the id" clause.** The error says "Unknown job" without + the id. The plan didn't promise it, and the case amendment is + dkackman/harnest#48. +- **The cursor and `failure_kind`:** cut, see + `../declined/mcp-job-notifications.md`. +- **A job lost on restart** stays #300's. diff --git a/docs/proposals/mcp-job-notifications.md b/docs/proposals/declined/mcp-job-notifications.md similarity index 80% rename from docs/proposals/mcp-job-notifications.md rename to docs/proposals/declined/mcp-job-notifications.md index 46ba94f0..c33788a8 100644 --- a/docs/proposals/mcp-job-notifications.md +++ b/docs/proposals/declined/mcp-job-notifications.md @@ -1,6 +1,38 @@ # Proposal: gapless job-completion signaling across the MCP bridge -Status: **not started** - design only, written after tracing the current +Status: **declined 2026-10-03** (issue #377, superseded by its plan v1). +Plan v1 built something smaller instead: one `wait_for_job` call long enough +to cover a long render. That record is +`../complete/mcp-long-wait-complete.md`. Both halves of this proposal were +cut. Read #377's plan before reviving either. + +- **The cursor (`since_seq`/`last_seq`).** Its premise is wrong. A snapshot + poll cannot "skip past finished" between two calls, because terminal + status is sticky. `cancel` returns early on a terminal job, a rerun is a + new id, and a history row replays the terminal `job_status` event + (`dw/server/jobs.py`). The one real way to miss a terminal state is a + server restart dropping the job ("Unknown job"). That is #300's, and a + cursor over an in-memory log doesn't fix it either. The cursor would also + have renamed `get_job_events`' `after` (pinned in tests and the UI's + `streamJobEvents`) and needed a gap signal for logs trimmed at + `MAX_PERSISTED_EVENTS`. **Reopen on:** a field report of an agent that + stopped polling on a non-terminal status and missed a job that really + finished. +- **`failure_kind`.** Basing `"oom"` on `crash_details()`'s SIGKILL + heuristic mislabels the common case. A CUDA `torch.OutOfMemoryError` + reaches the server as an ordinary `"error"` from a live worker + (`dw/worker.py`), and SIGKILL is itself only a guess at the kernel OOM + killer. Doing it right means classifying inside the worker, plus a + `jobs.sqlite` column, which shifts the positional indexing in + `_to_detail`/`get`/`recent_summaries`. **Reopen on:** a report of an agent + failing to act on an OOM's error text, or a caller that must branch + automatically (retry smaller) without reading prose. + +The original text follows unchanged. + +--- + +Original status: **not started** - design only, written after tracing the current `wait_for_job` / event-log path end to end. No code changes yet. ## The problem, as reported diff --git a/docs/proposals/todo.md b/docs/proposals/todo.md index 25855ef8..5002f9ae 100644 --- a/docs/proposals/todo.md +++ b/docs/proposals/todo.md @@ -10,7 +10,7 @@ original Tier 1 items and both fully-finished proposals (`score-and-select`, `script-to-video-agent-skill`) were removed. Since 2026-09-23 every open item below is also a GitHub issue labeled -`feature` (#374, #377, #379, #380, and #244 for `resume.md`; #375 was declined, #376 and #378 shipped), parked with Don. Its +`feature` (#374, #379, #380, and #244 for `resume.md`; #375 was declined, #376, #377 and #378 shipped), parked with Don. Its `priority:N` label mirrors the tier here. Work starts from the issue. ## Tier 1 — do these first (small, scoped, clear payoff) @@ -24,9 +24,6 @@ remaining deferred fix is recorded in 2. **orphaned-run-directories.md** — real, recurring disk-usage annoyance (leftover manifests invisible to gallery/asset listings); the proposal already recommends the simple option (A). Moderate but bounded work. -5. **mcp-job-notifications.md** — improves reliability of the wait/poll loop - (cursor-based, `failure_kind`), but there's no reported live pain forcing - this yet; medium complexity touching the event/job-record schema. ## Tier 3 — high benefit, but big lifts (stage carefully, don't take all at once) @@ -73,6 +70,23 @@ remaining deferred fix is recorded in task. Record, including what was deferred (a 1x refine and the VAE-encode task it needs, encoding the source soundtrack into audio latents): `complete/ltx2-refine-clip-complete.md`. +- **`ltx2/upscale-clip`, LTX-2.5's generative 2x upscale of an existing + mp4** (#542, stage #548), shipped 2026-09-28 as one template that keeps + the source's soundtrack. Record, including what was deferred (probing + the source at validate, source audio for the restore templates, the + vendor's Refine-Details IC-LoRA): `complete/ltx2-upscale-clip-complete.md`. +- **One `wait_for_job` call that covers a long render** (#377, stage + #546), shipped 2026-09-28: `DW_MCP_MAX_WAIT_SECONDS=1800` on lem's unit + and one wait rule in the guide and skills, with no code change. Record, + including what was deferred (the progress heartbeat, stage #547, not + needed since no long wait was cut): `complete/mcp-long-wait-complete.md`. +- **Promoting an H3 take to 1344x768 in latent space** (#471, stage #499; + #500 not built), shipped 2026-10-03 as two tasks, `upscale_h3_latents` and + `decode_h3_latents`, with an inline workflow in the guide and no template. + Don's gate found the preview ~39% faster than a native 768p render but + soft in the faces. Record, including what was deferred (the refine pass, + persisted latents, task weights in `downloads_required`): + `complete/h3-latent-upscale-complete.md`. ## Declined @@ -82,6 +96,11 @@ the analysis rather than repeating it. - **declined/workspace-folders.md** — grouped workspace names (`QA/EP1`). Declined 2026-09-23 on #375: thin demonstrated value against a loosened security boundary and a change that can't be taken back. +- **declined/mcp-job-notifications.md** — a gapless cursor and a + `failure_kind` on `wait_for_job`. Superseded 2026-10-03 by #377's smaller + plan: terminal status is sticky, so the cursor closes no gap, and the + proposed OOM label would have missed CUDA OOMs. Reopen triggers are at the + top of the doc. ## Backlog ideas with no doc on file diff --git a/docs/stabilization/ROADMAP.md b/docs/stabilization/ROADMAP.md index 268357e8..3ee57f75 100644 --- a/docs/stabilization/ROADMAP.md +++ b/docs/stabilization/ROADMAP.md @@ -821,6 +821,16 @@ Regenerated by `scripts/arch_report.py` at Phase 1 Task 2. The complexity figure | 3096 | 172 | 18 | dw/server/app.py | | 3024 | 36 | 84 | dw/worker.py | +## After gate 4: the API's client and the UI + +The gates measured `dw_mcp` and `ui/src` but never read them. Two follow-up +surveys do: [mcp-assessment.md](mcp-assessment.md) and +[ui/ASSESSMENT.md](ui/ASSESSMENT.md). Every gate's UI SLOC row above counts only `.ts`: +pygount has no lexer for `.svelte` and reports those files as 0 lines, so +`ui/src` is about 15,500 raw lines, not the ~2,270 code lines shown. With `.svelte` counted (`scripts/arch_report.py`'s +`svelte_code_lines`), the UI is 12,969 code lines at `stabilization-gate-4`, +against 20,362 for the engine and 6,956 for the API. + ## Working rules for the duration These held from gate 0 to gate 4. `FREEZE` was deleted at gate 4 (`5623dd03`); the hot zone, the ratchet and the new-module rule outlive it in the harness's stage C. diff --git a/docs/stabilization/baseline.json b/docs/stabilization/baseline.json index 8408bfa6..91c0cba3 100644 --- a/docs/stabilization/baseline.json +++ b/docs/stabilization/baseline.json @@ -1,11 +1,11 @@ { - "modules": 165, + "modules": 175, "modules_over_size_ceiling": 0, "functions_over_150_lines": 0, "prefix_literals": 0, "prefix_handling": 0, "test_dw_patch_targets": 284, - "claude_md_lines": 129, + "claude_md_lines": 128, "duplicate_blocks": 5, "complex_functions": 7, "import_cycles": 0, diff --git a/docs/stabilization/mcp-assessment.md b/docs/stabilization/mcp-assessment.md new file mode 100644 index 00000000..81824b80 --- /dev/null +++ b/docs/stabilization/mcp-assessment.md @@ -0,0 +1,91 @@ +# dw_mcp assessment (2026-10-01, develop 2b2b82ce) + +The engine stabilization (gates 0-4) measured `dw_mcp` but never read it +module by module; the 2026-09-28 assessment's whole verdict was "correctly a +thin HTTP client; only the shared-session defect (B6)". This is that read: +all 18 modules (4,597 lines), against the owners the stabilization put in +`dw/`. + +## Verdict + +Mostly true. Nothing in `dw_mcp` parses reference prefixes, walks a +definition, does cost or VRAM math, computes run versions or handles sample +rates: those all come from the server. But `dw_mcp` cannot import `dw` +(it stays torch-free), so every rule it does know is a second copy, and it +knows about twenty. All of them agree with their owners today and only one +is pinned by a test; `tests/test_mcp_*.py` run against `httpx.MockTransport`, +so they test `dw_mcp`'s idea of the server, not the server. The one real +bug is the stabilization's pattern in miniature: a fix landed in one of two +twin functions. + +## Findings + +| # | Severity | Finding | Evidence | Fix | +| --- | --- | --- | --- | --- | +| M1 | bug, medium | #389's per-call `workspace` reached `media._remote_root` but not its twin `assets._remote_roots`. A mounted session pinned to `ep4` calling `upload_asset(file_path="/assets/x.wav", workspace="default")` is confined to ep4's roots and refused. Fails closed. | `dw_mcp/assets.py:52`, `:353`; `dw_mcp/media.py:494` | Pass `workspace` through; merge the two root/confine pairs (M8) | +| M2 | structure, medium | `dw` imports `dw_mcp`: `dw/run.py` is a client of `dw.serve` through `dw_mcp.client`, and keeps a third `TERMINAL_STATUSES`. `dw/media.py:37` says the packages do not import each other. No seam-map row. | `dw/run.py:17`, `:36` | Either a map row naming the edge, or move the HTTP client to a torch-free module both import | +| M3 | duplication, medium | Image and frame handling copied from the server: `_crop_box` clones `dw/media_frames.resolve_crop_box`; the one-selector check and `_fit` repeat `dw/server/routes/media.py`; `MIN_DIMENSION`, `MAX_RETURNED_BYTES` twin server constants. `get_output_image` downloads and decodes the whole file client-side; `_fit_tiles_within_budget` re-encodes tiles the server already made. | `dw_mcp/media.py:96`, `:136`, `:274`, `:364` | A server image route with `max_dimension`/`crop`/`max_bytes` and a byte budget on `/frames`; `media.py` becomes call-and-reshape, and the UI can use the same routes | +| M4 | duplication, medium | `get_gallery_metadata` writes audio-QC thresholds (-0.5 / 0.0 / -40 dBFS) owned by `dw/audio_qc.py`, and the Music 3 "within 0.2 s of the ceiling" rule owned by `plugins/dw/skills/minimax-music3/SKILL.md` - model knowledge outside plugins and the catalog. | `dw_mcp/catalog.py:304-361` | The metadata route emits its findings and next step, as assess already does | +| M5 | duplication, low | Twin constants, none pinned except `MAX_DECODE_PIXELS`: upload extensions and size cap (`routes/assets.py`), `LOOPBACK_HOSTS` (`server/netinfo.py`), `TERMINAL_STATUSES` (`job_record.py`), `DEFAULT_WORKSPACE` (`workspace.py`), `ASSESSMENT_PROBES` (`server/assess.py`; redundant, the server returns the same 400). Three owner comments name `dw/server/app.py` or `dw/security.py`, which no longer own them. | `dw_mcp/assets.py:18-31`, `client.py:17`, `:130`, `diagnose.py:15-18`, `media.py:426` | One test (tests may import `dw`) asserting each twin equals its owner; delete `ASSESSMENT_PROBES`; fix the comments | +| M6 | duplication, low | Engine word lists in tool text: shapes and traits, reserved prompt-text prefixes (`references.RESERVED_TEXT`), reserved workspace names, estimate bases, `move_job` directions, job states. A test checks some words appear, not that the lists match. | `dw_mcp/server.py:65`, `tools_catalog.py:26`, `:251`, `tools_authoring.py:36`, `:76`, `:192`, `tools_jobs.py:201` | Pin the rendered descriptions against the owners' tuples | +| M7 | misplaced logic, low-medium | Work the server should own: `save_workflow` patch mode is GET + merge + PUT, not atomic, so a UI save in between is lost; `delete_output(job_id=)` derives the run dir; the acknowledgement body strips null repos because admission types `downloads: List[str]`; the 409 re-acknowledge body is rebuilt client-side; `workspaces.server_info` re-derives directories `/api/server` already scopes. | `dw_mcp/authoring.py:96`, `media.py:477`, `diagnose.py:57`, `client.py:426`, `workspaces.py:195` | A `PATCH` workflow route; `DELETE /api/jobs/{id}/run`; the server tolerates nulls and sends the body in its 409; delete `server_info`'s re-derivation | +| M8 | structure, low | Duplicates inside `dw_mcp`: the root/confine pairs in `media.py` and `assets.py` (source of M1); three field-projection helpers; the base64-size formula three times; two loopback sets that differ; two copies of the name/path/inline alias parsing; the acknowledgement gate in six places while the map's *Spending needs consent* row names only `diagnose.py`. `diagnose.py` is mostly queue operations. | `dw_mcp/media.py:523`, `assets.py:118`, `:164`, `authoring.py:39`, `diagnose.py:104` | Consolidate each pair; widen the map row to every gated tool | +| M9 | defects, low | `get_output_frames` with `hear` budgets frames and audio separately, so a reply can reach about 2x the cap; `_upload_inline` decodes before checking the size cap; `_probe` matches "401" in text, not `status_code`; a bad `DW_MCP_MAX_WAIT_SECONDS` fails the import; `get_gallery_metadata` reads `job["id"]` unguarded. | `dw_mcp/media.py:317`, `:341`, `assets.py:414`, `__main__.py:112`, `diagnose.py:27`, `catalog.py:312` | Each a one-line fix with a test | + +Complexity over 10 (ruff C901): `media.get_output_frames` 16, +`client._format_detail` 13, `assets._remote_roots` 11, +`tools_media.get_output_frames` 11. None is over the ratchet's limit. + +## What is already sound + +- `dw_mcp` stays torch-free and only `server.py` and `tools_*.py` import the + MCP SDK, both pinned (`docs/ARCHITECTURE.md`, *MCP*). +- B6, the shared session pin on the mounted surface, is documented as + deliberate and warns on change; a per-call `workspace` reaches every tool + but M1's. +- Drift that is caught today: `MAX_DECODE_PIXELS` + (`tests/test_security_decoder_bombs.py`), the tool listing over the real + mount (`tests/test_server_mcp.py`), symlink confinement through the real + app (`tests/test_security_symlinks.py`). + +## Proposed pass + +Three stages, each small. The ratchet already covers `dw_mcp`, so none needs +new tooling. + +1. **Fixes and pins.** M1, M9; the twin-constant test and word-list pin + (M5, M6); stale comments; the M2 map row; widen the consent row (M8). + No server change. + Done 2026-10-01. M1 fixed. M9 fixed except `_upload_inline`'s decode + order, left as it is: the base64 text is already in memory as the tool + call's argument, so checking first saves nothing. `ASSESSMENT_PROBES` + deleted; the server's 400 reaches the caller. `tests/test_mcp_twins.py` + pins every M5 constant (and `dw.run`'s copies) and every M6 word list + to its owner, and checks every tool that takes `acknowledged_cost` + refuses without it. The map has rows for the copies, for `dw.run` as a + client, and the consent rule over all seven gated tools. +2. **Move logic to the server.** M3, M4, M7: new or widened routes, then + `dw_mcp` calls them. The UI is the second consumer, so this stage is + shared with the UI pass. + Done 2026-10-02, as UI Phase 1 stage 1b (`docs/stabilization/ui/phase-1.md`). + M3: `GET /api/gallery/{name}/image` and `/frames?max_total_bytes` fit + and budget on the server (`dw/server/inline_media.py`); `dw_mcp` no + longer imports Pillow. The `hear` excerpt budget stays client-side, since + the server never sees `hear`. M4: gallery metadata carries `findings` from + `dw/audio_qc.py`'s thresholds; the client restates none, and the Music 3 + sentence is left to its skill. M7: `PATCH /api/workflows/{name}` under the + save lock; `DELETE /api/jobs/{id}/run` (which also refuses a running job, + a check the client never made); null downloads tolerated and a ready + `acknowledge` in the 409; `get_server_info` reads `/api/server` alone. + The UI uses none of the new routes - it shares the server helper code, + not the routes. +3. **Consolidate inside `dw_mcp`.** M8's pairs, once stage 2 has removed the + code some of them guard. + Done 2026-10-02, as stage 1c. `dw_mcp/confine.py` is the one + confinement rule for reads and writes (the pair M1 came from); + `client.py` holds `base64_size` (pinned to `dw.media`'s), `project`, + `workflow_source` and `UNSHAREABLE_HOSTS`; `DwClient.get_bytes` is gone. + Not done, deliberately: a `consent.py` for the seven acknowledgement + gates (each is two lines with its own refusal, and + `tests/test_mcp_twins.py` checks all seven) and a split of `diagnose.py` + (it would churn test imports for no rule gained). diff --git a/docs/stabilization/phase-0.md b/docs/stabilization/phase-0.md index d419fcb5..373d5865 100644 --- a/docs/stabilization/phase-0.md +++ b/docs/stabilization/phase-0.md @@ -84,7 +84,10 @@ def _long_function(lines): def test_a_long_function_and_a_long_module_are_counted(tmp_path): metrics = _load().measure( - _tree(tmp_path, {"dw/a.py": _long_function(151) + "\n" * 900, "dw/b.py": "x = 1\n"}) + _tree( + tmp_path, + {"dw/a.py": _long_function(151) + "\n" * 900, "dw/b.py": "x = 1\n"}, + ) ) assert metrics["functions_over_150_lines"] == 1 assert metrics["modules_over_1000_lines"] == 1 @@ -167,8 +170,16 @@ import re import sys REFERENCE_PREFIXES = frozenset( - ("asset:", "output:", "prompt:", "variable:", "previous_result:", - "constant:", "item:", "gather:") + ( + "asset:", + "output:", + "prompt:", + "variable:", + "previous_result:", + "constant:", + "item:", + "gather:", + ) ) # Modules allowed to spell a reference prefix. Empty until Phase 2 gives the # prefixes one owner (dw/references.py). @@ -189,8 +200,13 @@ def _duplicate_blocks(paths): from pylint.checkers.symilar import Symilar except ImportError: return None - similar = Symilar(min_lines=8, ignore_comments=True, ignore_docstrings=True, - ignore_imports=True, ignore_signatures=True) + similar = Symilar( + min_lines=8, + ignore_comments=True, + ignore_docstrings=True, + ignore_imports=True, + ignore_signatures=True, + ) for path in paths: with open(path, encoding="utf-8") as stream: similar.append_stream(str(path), stream) @@ -247,10 +263,14 @@ def regressions(current, baseline): def main(argv=None): parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--root", default=pathlib.Path(__file__).resolve().parent.parent) + parser.add_argument( + "--root", default=pathlib.Path(__file__).resolve().parent.parent + ) group = parser.add_mutually_exclusive_group() group.add_argument("--write", help="write the metrics to this JSON file") - group.add_argument("--check", help="fail if any metric is worse than this JSON file") + group.add_argument( + "--check", help="fail if any metric is worse than this JSON file" + ) args = parser.parse_args(argv) current = measure(args.root) print(json.dumps(current, indent=2)) @@ -402,7 +422,11 @@ def snap_constraints(definition, variables): if notice is not None: entry[name] = snapped(entry[name], constraints[name]) changes.append( - (f"{variable}[{index}]: {notice}", f"{variable}[{index}].{name}", entry[name]) + ( + f"{variable}[{index}]: {notice}", + f"{variable}[{index}].{name}", + entry[name], + ) ) return changes ``` @@ -489,12 +513,11 @@ Change the signature to `def sub_workflow_warnings(self, arguments=None):`. Repl Inside the loop, compute `source = source_indices[index] if index < len(source_indices) else index`, and append a string: ```python - warnings.append( - f"steps[{source}].workflow.arguments.{name}: " - f"'{reference['path']}' declares no variable '{name}' - the " - "value is dropped. Declared: " - + (", ".join(sorted(declared)) or "") - ) +warnings.append( + f"steps[{source}].workflow.arguments.{name}: " + f"'{reference['path']}' declares no variable '{name}' - the " + "value is dropped. Declared: " + (", ".join(sorted(declared)) or "") +) ``` Grep for other callers (`grep -rn "sub_workflow_warnings" dw dw_mcp tests`). In `dw/server/app.py`, change `candidate.sub_workflow_warnings()` to `candidate.sub_workflow_warnings(request.arguments)`. @@ -541,7 +564,9 @@ class TestDetailCachePruning: cache[f"/new/{len(cache)}.json"] = 0 return False - with patch.object(app_module.os.path, "exists", exists_while_another_thread_inserts): + with patch.object( + app_module.os.path, "exists", exists_while_another_thread_inserts + ): app_module._prune_missing(cache) assert "/gone/a.json" not in cache and "/gone/b.json" not in cache @@ -556,7 +581,9 @@ class TestDetailCachePruning: cache.pop("/gone/b.json", None) return False - with patch.object(app_module.os.path, "exists", exists_while_another_thread_prunes): + with patch.object( + app_module.os.path, "exists", exists_while_another_thread_prunes + ): app_module._prune_missing(cache) assert cache == {} ``` @@ -611,39 +638,45 @@ git commit -m "fix(server): detail-cache pruning tolerates concurrent requests" Add to `TestRunVersions` in `tests/test_runs.py`: ```python - def test_two_runs_with_the_same_id_get_distinct_directories_and_versions(self, tmp_path): - from dw.runs import open_run +def test_two_runs_with_the_same_id_get_distinct_directories_and_versions( + self, tmp_path +): + from dw.runs import open_run + + first = open_run(str(tmp_path), None, "wf", "20260928T120000Z-aaaaaaaa") + second = open_run(str(tmp_path), None, "wf", "20260928T120000Z-aaaaaaaa") + assert first[0] != second[0] + assert (first[1], second[1]) == (1, 2) + assert os.path.isdir(first[0]) and os.path.isdir(second[0]) - first = open_run(str(tmp_path), None, "wf", "20260928T120000Z-aaaaaaaa") - second = open_run(str(tmp_path), None, "wf", "20260928T120000Z-aaaaaaaa") - assert first[0] != second[0] - assert (first[1], second[1]) == (1, 2) - assert os.path.isdir(first[0]) and os.path.isdir(second[0]) - def test_concurrent_opens_never_share_a_version(self, tmp_path): - import threading +def test_concurrent_opens_never_share_a_version(self, tmp_path): + import threading - from dw.runs import open_run + from dw.runs import open_run - barrier = threading.Barrier(8) - results = [] + barrier = threading.Barrier(8) + results = [] - def opener(i): - barrier.wait() - results.append(open_run(str(tmp_path), None, "wf", f"20260928T120000Z-{i:08x}")) + def opener(i): + barrier.wait() + results.append(open_run(str(tmp_path), None, "wf", f"20260928T120000Z-{i:08x}")) - threads = [threading.Thread(target=opener, args=(i,)) for i in range(8)] - for thread in threads: - thread.start() - for thread in threads: - thread.join() - assert sorted(version for _, version in results) == list(range(1, 9)) + threads = [threading.Thread(target=opener, args=(i,)) for i in range(8)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + assert sorted(version for _, version in results) == list(range(1, 9)) - def test_the_first_run_is_version_one_even_though_its_own_directory_exists(self, tmp_path): - from dw.runs import open_run - _, version = open_run(str(tmp_path), None, "wf", "20260928T120000Z-aaaaaaaa") - assert version == 1 +def test_the_first_run_is_version_one_even_though_its_own_directory_exists( + self, tmp_path +): + from dw.runs import open_run + + _, version = open_run(str(tmp_path), None, "wf", "20260928T120000Z-aaaaaaaa") + assert version == 1 ``` The last test matters: the run's own freshly claimed, manifest-less directory must not be ranked as an older sibling. @@ -681,7 +714,9 @@ def open_run(output_dir, file_spec, workflow_id, run_id): name = os.path.basename(candidate) versions = record_run_versions(identity_dir, exclude=name) version = max(versions.values(), default=0) + 1 - write_manifest(candidate, {"run_id": name, "version": version, "status": "running"}) + write_manifest( + candidate, {"run_id": name, "version": version, "status": "running"} + ) return candidate, version ``` @@ -822,17 +857,25 @@ Then add: def test_a_step_borrowing_a_pipeline_misses_when_the_source_model_changes(tmp_path): definition = _pipeline_reference_workflow_def() definition["variables"] = {"model_a": "m1"} - definition["steps"][0]["pipeline"]["from_pretrained_arguments"]["model_name"] = "variable:model_a" - order = _run_twice_recording_order(tmp_path, definition, [{"model_a": "m1"}, {"model_a": "m2"}]) + definition["steps"][0]["pipeline"]["from_pretrained_arguments"]["model_name"] = ( + "variable:model_a" + ) + order = _run_twice_recording_order( + tmp_path, definition, [{"model_a": "m1"}, {"model_a": "m2"}] + ) assert order == ["A", "B", "A", "B"] def test_a_step_reusing_components_misses_when_the_sharing_model_changes(tmp_path): definition = _shared_components_workflow_def() definition["variables"] = {"model_a": "m1"} - definition["steps"][0]["pipeline"]["from_pretrained_arguments"]["model_name"] = "variable:model_a" - order = _run_twice_recording_order(tmp_path, definition, [{"model_a": "m1"}, {"model_a": "m2"}]) - assert order[len(order) // 2:] == order[: len(order) // 2] + definition["steps"][0]["pipeline"]["from_pretrained_arguments"]["model_name"] = ( + "variable:model_a" + ) + order = _run_twice_recording_order( + tmp_path, definition, [{"model_a": "m1"}, {"model_a": "m2"}] + ) + assert order[len(order) // 2 :] == order[: len(order) // 2] ``` Write `_run_twice_recording_order` from the pattern the file already uses: `_mock_pipeline_load`, a `FakeResult`, and a spy on step execution. Adapt the step names and the `model_name` path to what the helper definitions actually contain. Read them, then write the assertions against their real step names. diff --git a/docs/stabilization/phase-1.md b/docs/stabilization/phase-1.md index 5d16be28..558ef870 100644 --- a/docs/stabilization/phase-1.md +++ b/docs/stabilization/phase-1.md @@ -1132,7 +1132,10 @@ DEFINITION = { }, "variable_constraints": { "num_frames": { - "modulus": 17, "remainder": 5, "min_frames": 124, "max_frames": 345, + "modulus": 17, + "remainder": 5, + "min_frames": 124, + "max_frames": 345, "snap": "up", } }, @@ -1256,21 +1259,32 @@ def test_expansion_is_computed_once_per_workflow_and_arguments(tmp_path): ```python @dataclass class Admission: - workflow: Workflow # the one instance this request builds - arguments: dict # as the caller sent them ({} when omitted) - supplied: bool # the caller sent `arguments` at all - errors: list # [{path, message}]: schema, argument, constraint, reference - warnings: list # [str]: every warning /api/validate reports - plan: dict | None = None # build_plan's answer, when plan_for was given + workflow: Workflow # the one instance this request builds + arguments: dict # as the caller sent them ({} when omitted) + supplied: bool # the caller sent `arguments` at all + errors: list # [{path, message}]: schema, argument, constraint, reference + warnings: list # [str]: every warning /api/validate reports + plan: dict | None = None # build_plan's answer, when plan_for was given @property def ok(self): return not self.errors -def admit(*, workflow_path, workflow, arguments, base_dir, workspace, - sources, ceiling_index, output_dir, workflow_dir, - supplied=True, plan_for=None) -> Admission: +def admit( + *, + workflow_path, + workflow, + arguments, + base_dir, + workspace, + sources, + ceiling_index, + output_dir, + workflow_dir, + supplied=True, + plan_for=None, +) -> Admission: """Load the request's workflow once and check it once, with the workspace's asset library active for every check (the validate route used to leave it off for its warnings). `plan_for`, a callable taking @@ -1357,13 +1371,20 @@ def _bridge(app): def handler(request): headers = {k: v for k, v in request.headers.items() if k.lower() != "host"} response = local.request( - request.method, request.url.path, params=request.url.params, - content=request.content, headers=headers, + request.method, + request.url.path, + params=request.url.params, + content=request.content, + headers=headers, + ) + return httpx.Response( + response.status_code, headers=response.headers, content=response.content ) - return httpx.Response(response.status_code, headers=response.headers, content=response.content) # TestClient's own host, so host-checking middleware sees what it expects - return DwClient(base_url="http://testserver", transport=httpx.MockTransport(handler)) + return DwClient( + base_url="http://testserver", transport=httpx.MockTransport(handler) + ) ``` - Tests, one behavior each: diff --git a/docs/stabilization/phase-2.md b/docs/stabilization/phase-2.md index 588b6904..571527dd 100644 --- a/docs/stabilization/phase-2.md +++ b/docs/stabilization/phase-2.md @@ -333,13 +333,16 @@ from dw import ( @pytest.mark.parametrize( - "module", [adapter_compatibility, reference_names, reference_limits, video_extensions] + "module", + [adapter_compatibility, reference_names, reference_limits, video_extensions], ) def test_the_unresolved_checkers_share_one_set(module): assert module._UNRESOLVED_PREFIXES is references.UNRESOLVED -@pytest.mark.parametrize("module", [content_types, kernel_availability, result_fps, subfolders]) +@pytest.mark.parametrize( + "module", [content_types, kernel_availability, result_fps, subfolders] +) def test_the_substituted_checkers_still_check_step_results(module): assert module._UNRESOLVED_PREFIXES is references.SUBSTITUTED diff --git a/docs/stabilization/ui/ASSESSMENT.md b/docs/stabilization/ui/ASSESSMENT.md new file mode 100644 index 00000000..5d74b85a --- /dev/null +++ b/docs/stabilization/ui/ASSESSMENT.md @@ -0,0 +1,98 @@ +# UI assessment (2026-10-01, develop 2b2b82ce) + +The engine stabilization (gates 0-4) never read `ui/` and has no ratchet over +it. This is the survey that scopes a pass: what is there, where it already +disagrees with the engine, and what tooling a ratchet needs. + +## The gate reports undercounted the UI + +Every gate's *SLOC by layer* row says the UI is about 2,270 code lines. +`scripts/arch_report.py` lists `.svelte` in `SLOC_SUFFIXES`, but pygount has +no lexer for it and returns `SourceState.unknown` with 0 lines, so only the +`.ts` files were counted. `ui/src` without tests is **15,508 raw lines**: +11,623 in `.svelte`, the rest `.ts` and `app.css`, over three times +`dw_mcp`'s 4,597. The measurement needs fixing before any UI baseline is taken. + +## What is there + +- **Tooling is good and green.** ESLint (typescript-eslint, eslint-plugin-svelte, + recommended presets only), svelte-check (0 errors, 0 warnings), vitest (36 + files, 340 tests), prettier, Playwright (7 specs). CI's `ui` job runs + prettier, lint, check, test and build; `e2e` runs only on PRs into master. + `tsconfig.app.json` is `strict`. +- **One API client.** Every `fetch` and `EventSource` is in `ui/src/lib/api.ts`. + Response types are hand-written in `types.ts` (379 lines), and nothing checks + them against the server: no route declares a `response_model`, so the + OpenAPI document has no response schemas to generate from. +- **Layering holds.** `App` imports pages, pages import components, everything + imports `lib/*.ts`; no `.ts` imports a `.svelte` but `main.ts`. One import + cycle: `lib/api.ts` <-> `lib/workspace.svelte.ts`. +- **Size.** 14 non-test files over 400 lines: `EditorPage.svelte` 1,075, + `PromptEditorPage.svelte` 954, `JobPage.svelte` 855, `editor/StepEditor.svelte` + 802, `api.ts` 671, then `AssetsPage`, `GalleryPage`, `ModelsPage`, + `WorkflowsPage`, `App`, `app.css`, `FlowView`, `OverviewPage`, `ServerPage`. + In the big pages about 30% is `\n" + ) + counts = _load().sloc(tmp_path) + assert counts["UI (ui/src, tests excluded)"] == 7 +``` + +The seven: ``, `

...`, ``. + +- [ ] **Step 2: Run it to see it fail** + +Run: `venv/bin/python -m pytest tests/test_arch_report.py -q -k svelte` +Expected: FAIL, `0 == 7`. + +- [ ] **Step 3: Count `.svelte` by hand** + +In `scripts/arch_report.py`, add above `sloc`: + +```python +# A line that is only a comment. A multi-line /* */ comment's inner lines +# count as code: matching them by a leading '*' would also match a CSS '*' +# selector, and an undercount hides growth where an overcount does not. +SVELTE_COMMENT = re.compile(r"^(|//.*|/\*.*\*/)$") + + +def svelte_code_lines(path): + """Code lines in a .svelte file, which pygount reads as 0: every line + that is neither blank nor a comment on its own. Script, markup and style + all count, since all three are what a reader holds.""" + return sum( + 1 + for line in path.read_text(encoding="utf-8").splitlines() + if line.strip() and not SVELTE_COMMENT.match(line.strip()) + ) +``` + +and in `sloc`, replace the `counts[layer] += ...` line with: + +```python +if path.suffix == ".svelte": + counts[layer] += svelte_code_lines(path) +else: + counts[layer] += SourceAnalysis.from_file(str(path), layer).code_count +``` + +Add `import re` if the module does not already import it. + +- [ ] **Step 4: Run the test and the file** + +Run: `venv/bin/python -m pytest tests/test_arch_report.py -q` +Expected: PASS. + +- [ ] **Step 5: Record the corrected number** + +Run: `venv/bin/python scripts/arch_report.py 2>/dev/null | grep -A4 "SLOC by layer"` (or the script's documented invocation for the current tree). In `docs/stabilization/ROADMAP.md`'s *After gate 4* section, add one sentence giving the corrected UI code-line count from this run, measured the same way the API row is. + +- [ ] **Step 6: Commit** + +```bash +git add scripts/arch_report.py tests/test_arch_report.py docs/stabilization/ROADMAP.md +git commit -m "fix(stabilization): the gate report counts .svelte code lines" +``` + +--- + +### Task 3: One speller of reference prefixes; `isReference` knows them all (U2) + +`isReference` knows 4 of the engine's prefixes. A value like `item:flag` on +a parameter annotated `bool` gets a checkbox, and toggling it replaces the +reference with `true`/`false`. + +**Files:** +- Create: `ui/src/lib/references.ts` +- Modify: `ui/src/lib/editor.ts` (`isReference` moves out; re-exported) +- Modify: `ui/src/lib/runstate.ts` (imports `MEMBER_SEPARATOR`) +- Test: `ui/src/lib/editor.test.ts` +- Create: `tests/test_ui_twins.py` + +**Interfaces:** +- Produces: from `ui/src/lib/references.ts`: `ASSET`, `OUTPUT`, `PROMPT`, `VARIABLE`, `PREVIOUS_RESULT`, `CONSTANT`, `ITEM`, `GATHER`, `BUILTIN`, `CONSTRAINT` (each `':'`), `PREFIXES` (all ten), `MEMBER_SEPARATOR` (`'@'`), `FROM_PREVIOUS_RESULT_KEY` (`'from_previous_result'`), `isReference(value: unknown): value is string`. `editor.ts` keeps exporting `isReference` (re-export) so existing imports hold. +- Produces: `tests/test_ui_twins.py` helpers `ts_constants(path) -> dict[str, str]` and `ts_string_array(path, name) -> list[str]`, used by Tasks 5, 7 and 8. + +- [ ] **Step 1: Write the failing UI tests** + +Append to `ui/src/lib/editor.test.ts`: + +```ts +describe('a reference is always edited as text', () => { + const boolParam = param({ annotation: 'bool' }) + const intParam = param({ annotation: 'int' }) + it.each([ + 'item:flag', + 'gather:shots', + 'asset:cast/priya.png', + 'output:run/v2/final.mp4', + ])('%s', (value) => { + expect(isReference(value)).toBe(true) + expect(widgetFor(boolParam, value)).toBe('text') + expect(widgetFor(intParam, value)).toBe('text') + }) +}) +``` + +- [ ] **Step 2: Write the failing twin test** + +Create `tests/test_ui_twins.py`: + +```python +"""The UI's copies of rules the engine owns, pinned to their owners. + +The UI cannot import Python, so a rule it must know is a copy. These tests +read the TypeScript source and compare each copy with its owner: a change +to one side fails until the other follows. A copy that can be read from the +server instead is deleted, not pinned. +""" + +import pathlib +import re + +from dw import references + +REPO = pathlib.Path(__file__).resolve().parent.parent +UI_LIB = REPO / "ui" / "src" / "lib" + + +def ts_constants(path): + """`export const NAME = 'value'` string constants in a TS file.""" + return dict( + re.findall(r"^export const ([A-Z_]+) = '([^']*)'", path.read_text(), re.M) + ) + + +def ts_string_array(path, name): + """The quoted strings of `export const NAME = [ ... ]` in a TS file. + Exported only: a module-private copy is not the one other modules use.""" + found = re.search( + rf"^export const {name}\b[^=]*= \[(.*?)\]", path.read_text(), re.M | re.S + ) + assert found, f"no array {name} in {path}" + return re.findall(r"'([^']*)'", found.group(1)) + + +def test_the_ui_spells_every_reference_prefix_the_engine_does(): + engine = { + name: value + for name, value in vars(references).items() + if name.isupper() + and isinstance(value, str) + and re.fullmatch(r"[a-z_]+:", value) + } + ui = { + name: value + for name, value in ts_constants(UI_LIB / "references.ts").items() + if value.endswith(":") + } + assert ui == engine + + +def test_the_member_separator_and_reference_key_are_the_engines(): + ui = ts_constants(UI_LIB / "references.ts") + assert ui["MEMBER_SEPARATOR"] == references.MEMBER_SEPARATOR + assert ui["FROM_PREVIOUS_RESULT_KEY"] == references.FROM_PREVIOUS_RESULT_KEY +``` + +- [ ] **Step 3: Run both to see them fail** + +Run: `cd ui && npx vitest run src/lib/editor.test.ts` then `venv/bin/python -m pytest tests/test_ui_twins.py -q` +Expected: the four `item:`/`gather:`/`asset:`/`output:` cases FAIL; the twin tests FAIL (no `references.ts`). + +- [ ] **Step 4: Create the owner** + +`ui/src/lib/references.ts`: + +```ts +/** The engine's reference prefixes and the names that go with them. + * dw/references.py owns them; tests/test_ui_twins.py pins this copy. The + * one module in ui/src that spells a prefix: the UI ratchet's + * prefix_literals counts any other. */ + +export const ASSET = 'asset:' +export const OUTPUT = 'output:' +export const PROMPT = 'prompt:' +export const VARIABLE = 'variable:' +export const PREVIOUS_RESULT = 'previous_result:' +export const CONSTANT = 'constant:' +export const ITEM = 'item:' +export const GATHER = 'gather:' +export const BUILTIN = 'builtin:' +export const CONSTRAINT = 'constraint:' + +export const PREFIXES = [ + ASSET, + OUTPUT, + PROMPT, + VARIABLE, + PREVIOUS_RESULT, + CONSTANT, + ITEM, + GATHER, + BUILTIN, + CONSTRAINT, +] as const + +/** Joins a for_each step's name to an entry's: `@`. */ +export const MEMBER_SEPARATOR = '@' + +/** The key a reference object names an earlier step under, unprefixed. */ +export const FROM_PREVIOUS_RESULT_KEY = 'from_previous_result' + +/** A value the engine resolves later, so it is always edited as text: + * a widget that coerces it (a checkbox, a number) would replace it. */ +export function isReference(value: unknown): value is string { + return ( + typeof value === 'string' && PREFIXES.some((p) => value.startsWith(p)) + ) +} +``` + +- [ ] **Step 5: Point the old homes at it** + +In `ui/src/lib/editor.ts`, delete the `isReference` function and its doc comment, and add near the top imports: + +```ts +import { isReference } from './references' +export { isReference } +``` + +In `ui/src/lib/runstate.ts`, replace `const MEMBER_SEPARATOR = '@'` with `import { MEMBER_SEPARATOR } from './references'` (keep the doc comment above it, which explains the separator's use there). + +- [ ] **Step 6: Run the tests, the ratchet and the checks** + +Run: `cd ui && npx vitest run && npm run check && npm run lint && npm run metrics -- --check ../docs/stabilization/ui/baseline.json` then `venv/bin/python -m pytest tests/test_ui_twins.py -q` +Expected: all PASS; `prefix_literals` falls by 4. + +- [ ] **Step 7: Lower the baseline and commit** + +Run: `cd ui && npm run metrics -- --write ../docs/stabilization/ui/baseline.json` + +```bash +git add ui/src/lib/references.ts ui/src/lib/editor.ts ui/src/lib/runstate.ts ui/src/lib/editor.test.ts tests/test_ui_twins.py docs/stabilization/ui/baseline.json +git commit -m "fix(ui): every reference prefix is edited as text; references.ts owns them" +``` + +--- + +### Task 4: The live reference check agrees with the engine (U1) + +`danglingReferenceDetails` (`ui/src/lib/flow.ts`) is a second copy of +`dw/previous_results.py`'s `previous_result_reference_errors`. It flags a +`for_each` member reference the engine accepts, splits a name on its first +`.` where the engine matches the whole name or the name plus a property, and +never looks at a `from_previous_result` key the engine checks. The engine +checks the expanded definition, where a `for_each` step `g` becomes members +`g@`; the editor holds `g` unexpanded, so it accepts any +`g@...` reference to a `for_each` step. Whether this check should become the +server's validate instead is Phase 1's call; here it only stops disagreeing. + +**Files:** +- Modify: `ui/src/lib/flow.ts` (`danglingReferenceDetails`, a new `resolvesTo`) +- Test: `ui/src/lib/flow.test.ts` + +**Interfaces:** +- Consumes: `PREVIOUS_RESULT`, `VARIABLE`, `GATHER`, `PROMPT`, `MEMBER_SEPARATOR`, `FROM_PREVIOUS_RESULT_KEY` from `./references` (Task 3). +- Produces: `danglingReferenceDetails(workflow, promptNames?)` keeps its signature and `DanglingReference` shape. + +- [ ] **Step 1: Write the failing tests** + +Append inside `describe('danglingReferenceDetails', ...)` in `ui/src/lib/flow.test.ts`: + +```ts + const forEachStep = (name: string) => ({ + ...step(name, { x: 'item:prompt' }), + for_each: [{ name: 'a', prompt: 'p' }], + }) + + it('accepts a member reference to a for_each step, with or without a property', () => { + const wf = { + variables: {}, + steps: [ + forEachStep('g'), + step('b', { one: 'previous_result:g@a', two: 'previous_result:g@a.mask' }), + ], + } + expect(danglingReferenceDetails(wf)).toEqual([]) + }) + + it('flags a member reference to a step that is not for_each', () => { + const wf = { + variables: {}, + steps: [step('g', {}), step('b', { y: 'previous_result:g@a' })], + } + expect(danglingReferenceDetails(wf)).toHaveLength(1) + }) + + it('matches a dotted step name whole, and a property of a plain one', () => { + const wf = { + variables: {}, + steps: [ + step('x.y', {}), + step('seg', {}), + step('b', { v: 'previous_result:x.y', m: 'previous_result:seg.mask' }), + ], + } + expect(danglingReferenceDetails(wf)).toEqual([]) + }) + + it('flags a from_previous_result that names no earlier step', () => { + const wf = { + variables: {}, + steps: [step('b', { image: { from_previous_result: 'nope' } })], + } + const details = danglingReferenceDetails(wf) + expect(details).toHaveLength(1) + expect(details[0].message).toContain('nope') + }) +``` + +- [ ] **Step 2: Run them to see three fail** + +Run: `cd ui && npx vitest run src/lib/flow.test.ts` +Expected: the member, dotted-name and `from_previous_result` cases FAIL; "flags a member reference to a step that is not for_each" passes already. + +- [ ] **Step 3: Rewrite the check** + +In `ui/src/lib/flow.ts`, import from `./references` and replace `danglingReferenceDetails` with: + +```ts +import { + FROM_PREVIOUS_RESULT_KEY, + GATHER, + MEMBER_SEPARATOR, + PREVIOUS_RESULT, + PROMPT, + VARIABLE, +} from './references' + +interface EarlierStep { + name: string + forEach: boolean +} + +/** Whether a previous_result reference resolves to an earlier step: the + * name itself or the name plus a property (`seg.mask`), as the engine's + * reference_resolves_to decides - or, for a for_each step, one of the + * `@` members the engine expands it into, which the editor + * cannot list because it holds the step unexpanded. */ +function resolvesTo(reference: string, step: EarlierStep): boolean { + if (reference === step.name || reference.startsWith(step.name + '.')) + return true + return step.forEach && reference.startsWith(step.name + MEMBER_SEPARATOR) +} + +export function danglingReferenceDetails( + workflow: Record, + promptNames?: string[], +): DanglingReference[] { + const problems: DanglingReference[] = [] + const variables = new Set(Object.keys(workflow.variables ?? {})) + const prompts = promptNames === undefined ? null : new Set(promptNames) + const steps: Array> = workflow.steps ?? [] + + steps.forEach((step, index) => { + const earlier: EarlierStep[] = steps + .slice(0, index) + .filter((s) => typeof s.name === 'string' && s.name) + .map((s) => ({ name: s.name, forEach: s.for_each !== undefined })) + const report = (message: string) => + problems.push({ stepIndex: index, message: `Step '${step.name}': ${message}` }) + + scanStringsWithPath(step, [], (value, path) => { + if (path[path.length - 1] === FROM_PREVIOUS_RESULT_KEY) { + if (!earlier.some((s) => resolvesTo(value, s))) + report(`${FROM_PREVIOUS_RESULT_KEY} '${value}' - no earlier step has that name`) + } else if (value.startsWith(VARIABLE)) { + const name = value.slice(VARIABLE.length) + if (!variables.has(name)) + report(`${VARIABLE}${name} - no such variable is declared`) + } else if (value.startsWith(PREVIOUS_RESULT)) { + const reference = value.slice(PREVIOUS_RESULT.length) + if (!earlier.some((s) => resolvesTo(reference, s))) + report(`${PREVIOUS_RESULT}${reference} - no earlier step has that name`) + } else if (value.startsWith(GATHER)) { + const name = value.slice(GATHER.length) + if (!earlier.some((s) => s.name === name)) + report(`${GATHER}${name} - no earlier step has that name`) + } else if (value.startsWith(PROMPT)) { + // Without a listing the server resolves these at run time - only + // a supplied library can say a name is missing + const name = value.slice(PROMPT.length) + if (prompts !== null && !prompts.has(name)) + report(`${PROMPT}${name} - the prompt library has no such prompt`) + } + }) + }) + return problems +} +``` + +If `scanStrings` is now unused, delete it; `npm run lint` says so. + +- [ ] **Step 4: Run the tests, checks and ratchet** + +Run: `cd ui && npx vitest run && npm run check && npm run lint && npm run metrics -- --check ../docs/stabilization/ui/baseline.json` +Expected: all PASS. The existing messages keep their wording, so the existing flow and EditorPage tests still pass; if one asserted the exact old message for a `previous_result:` with a property, update it to the full reference, which is what the engine names. + +- [ ] **Step 5: Lower the baseline if it fell, and commit** + +```bash +cd ui && npm run metrics -- --write ../docs/stabilization/ui/baseline.json && cd .. +git add ui/src/lib/flow.ts ui/src/lib/flow.test.ts docs/stabilization/ui/baseline.json +git commit -m "fix(ui): the live reference check resolves names as the engine does" +``` + +--- + +### Task 5: Workspace names follow the engine's rule (U3) + +The UI allows `[A-Za-z0-9_][A-Za-z0-9_-]*`; the engine's +`WORKSPACE_NAME_PATTERN` is `^[\w][\w.-]*\Z` with Python's Unicode `\w` +(letters, digits, underscore) and a 100-character cap. One case file is read +by a pytest and a vitest, so the two rules cannot drift without a test +failing. + +**Files:** +- Create: `tests/fixtures/workspace_names.json` +- Modify: `ui/src/lib/workspaceActions.ts` (`NAME`, a length cap) +- Test: `ui/src/lib/workspaceActions.test.ts` +- Test: `tests/test_ui_twins.py` + +**Interfaces:** +- Produces: `workspaceNameError(name: string): string | null` keeps its signature; `MAX_WORKSPACE_NAME_LENGTH` (100) exported from `workspaceActions.ts`. + +- [ ] **Step 1: Write the shared cases** + +`tests/fixtures/workspace_names.json` (the 100- and 101-character names written out in full): + +```json +[ + { "name": "ep4", "valid": true }, + { "name": "ep4.v2", "valid": true }, + { "name": "under_score-dash", "valid": true }, + { "name": "café", "valid": true }, + { "name": "_lead", "valid": true }, + { "name": ".hidden", "valid": false }, + { "name": "-lead", "valid": false }, + { "name": "a/b", "valid": false }, + { "name": "a b", "valid": false }, + { "name": "outputs", "valid": false }, + { "name": "", "valid": false }, + { "name": "<100 x characters>", "valid": true }, + { "name": "<101 x characters>", "valid": false } +] +``` + +- [ ] **Step 2: Write both tests** + +Append to `tests/test_ui_twins.py`: + +```python +import json + +import pytest + +from dw.security import InvalidInputError, SecurityError, validate_workspace_name +from dw.workspace import RESERVED_WORKSPACE_NAMES + +WORKSPACE_NAMES = json.loads( + (REPO / "tests" / "fixtures" / "workspace_names.json").read_text() +) + + +@pytest.mark.parametrize( + "case", WORKSPACE_NAMES, ids=lambda c: c["name"][:12] or "empty" +) +def test_the_engine_decides_each_shared_workspace_name_case(case): + if case["valid"]: + validate_workspace_name(case["name"], reserved=RESERVED_WORKSPACE_NAMES) + else: + with pytest.raises((InvalidInputError, SecurityError)): + validate_workspace_name(case["name"], reserved=RESERVED_WORKSPACE_NAMES) + + +def test_the_ui_reserves_the_engines_workspace_names(): + assert ts_string_array( + UI_LIB / "workspaceActions.ts", "RESERVED_WORKSPACE_NAMES" + ) == list(RESERVED_WORKSPACE_NAMES) +``` + +Append to `ui/src/lib/workspaceActions.test.ts`: + +```ts +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' + +const cases: { name: string; valid: boolean }[] = JSON.parse( + readFileSync( + fileURLToPath( + new URL('../../../tests/fixtures/workspace_names.json', import.meta.url), + ), + 'utf8', + ), +) + +it.each(cases)('decides $name as the engine does', ({ name, valid }) => { + expect(workspaceNameError(name) === null).toBe(valid) +}) +``` + +(`workspaceNameError` is imported at the top of the file with the module's other exports; add it if it is not.) + +- [ ] **Step 3: Run both to see them fail** + +Run: `venv/bin/python -m pytest tests/test_ui_twins.py -q` then `cd ui && npx vitest run src/lib/workspaceActions.test.ts` +Expected: pytest PASSES (the engine is the owner; if a case fails here, the case is wrong). vitest FAILS on `ep4.v2`, `café` and the 101-character name. + +- [ ] **Step 4: Match the rule** + +In `ui/src/lib/workspaceActions.ts`, replace `const NAME = ...` and the pattern check: + +```ts +/** dw/security.py's WORKSPACE_NAME_PATTERN, `^[\w][\w.-]*\Z`: Python's \w + * is Unicode letters and digits plus underscore, so \p{L}\p{N}_ here. + * tests/fixtures/workspace_names.json is read by both sides' tests. */ +const NAME = /^[\p{L}\p{N}_][\p{L}\p{N}_.-]*$/u +export const MAX_WORKSPACE_NAME_LENGTH = 100 +``` + +and in `workspaceNameError`, before the pattern test: + +```ts + if (name.length > MAX_WORKSPACE_NAME_LENGTH) + return `Use at most ${MAX_WORKSPACE_NAME_LENGTH} characters` + if (!NAME.test(name)) + return 'Start with a letter, digit or _; then letters, digits, _, - and .' +``` + +- [ ] **Step 5: Run the tests and checks** + +Run: `cd ui && npx vitest run && npm run check && npm run lint && npm run metrics -- --check ../docs/stabilization/ui/baseline.json` then `venv/bin/python -m pytest tests/test_ui_twins.py -q` +Expected: all PASS. If an existing test asserted the old message text, update it to the new one. + +- [ ] **Step 6: Commit** + +```bash +git add tests/fixtures/workspace_names.json tests/test_ui_twins.py ui/src/lib/workspaceActions.ts ui/src/lib/workspaceActions.test.ts +git commit -m "fix(ui): workspace names follow the engine's pattern and length cap" +``` + +--- + +### Task 6: The job page renders by the server's media kinds (U4) + +`JobPage.svelte` decides image or video from its own extension regexes, which +miss `.bmp`, `.mov`, `.mkv`, `.avi` and every audio file; an audio output is +shown as a bare link. `dw/server/outputs.py`'s `MEDIA_KINDS` is the owner. +The job detail gains one top-level map, `output_kinds`, so no manifest entry +changes shape and no existing manifest assertion moves. + +**Files:** +- Modify: `dw/server/outputs.py` (`output_kinds`) +- Modify: `dw/server/routes/jobs.py` (`get_job`) +- Test: `tests/test_server.py` +- Modify: `ui/src/lib/types.ts` (`JobDetail.output_kinds`) +- Modify: `ui/src/lib/pages/JobPage.svelte` +- Test: `ui/src/lib/pages/JobPage.test.ts` + +**Interfaces:** +- Produces: `output_kinds(manifest: list | None) -> dict[str, str | None]` in `dw/server/outputs.py`: every file a manifest entry lists, mapped to its `MEDIA_KINDS` value, or `None` for a kind the gallery does not show. `GET /api/jobs/{id}` carries it as `output_kinds`. +- Produces: `JobDetail.output_kinds?: Record` in `types.ts`. + +- [ ] **Step 1: Write the failing server test** + +Append to `tests/test_server.py`, after `test_job_files_are_reported_relative_to_the_output_dir`: + +```python +def test_a_job_reports_each_output_files_media_kind(server, tmp_path): + """The job page renders by kind, so the kind comes from MEDIA_KINDS + rather than an extension list in the browser.""" + outputs = tmp_path / "outputs" + files = [str(outputs / name) for name in ("a.bmp", "b.mov", "c.flac", "d.bin")] + + def script(command): + yield { + "type": "success", + "message": "ok", + "run_count": 1, + "manifest": [{"step": "gen", "files": files}], + } + + with server(script) as client: + job = client.post("/api/jobs", json={"workflow": valid_workflow()}).json() + detail = wait_for_status(client, job["id"], TERMINAL_STATES) + assert detail["output_kinds"] == { + "a.bmp": "image", + "b.mov": "video", + "c.flac": "audio", + "d.bin": None, + } + + +def test_output_kinds_tolerates_a_job_with_no_manifest(): + from dw.server.outputs import output_kinds + + assert output_kinds(None) == {} + assert output_kinds([{"step": "s"}, "not an entry"]) == {} +``` + +- [ ] **Step 2: Run them to see them fail** + +Run: `venv/bin/python -m pytest tests/test_server.py -q -k "media_kind or output_kinds"` +Expected: FAIL (`KeyError: 'output_kinds'`, `ImportError`). + +- [ ] **Step 3: Classify on the server** + +In `dw/server/outputs.py`, after `MEDIA_KINDS`: + +```python +def output_kinds(manifest): + """Each file a job manifest lists, mapped to its MEDIA_KINDS entry, or + None for a kind the gallery does not show. A client renders an output + by this rather than keeping its own extension list.""" + kinds = {} + for entry in manifest or []: + if not isinstance(entry, dict): + continue + for name in entry.get("files") or []: + kinds[name] = MEDIA_KINDS.get(os.path.splitext(name)[1].lower()) + return kinds +``` + +(`os` is already imported there; add it if not.) In `dw/server/routes/jobs.py`'s `get_job`, replace the return: + +```python + # a historical job is already a detail dict; a live one renders itself + detail = job if isinstance(job, dict) else manager.describe(job) + return {**detail, "output_kinds": output_kinds(detail.get("manifest"))} +``` + +with `from ..outputs import output_kinds` among the module's imports. + +- [ ] **Step 4: Run the server tests** + +Run: `venv/bin/python -m pytest tests/test_server.py tests/test_server_jobs.py tests/test_mcp_*.py -q` +Expected: PASS. + +- [ ] **Step 5: Write the failing UI test** + +In `ui/src/lib/pages/JobPage.test.ts`, add: + +```ts +it('renders each output by the kind the server reports', async () => { + detail.job = { + ...job([{ step: 'generate', files: ['a.bmp', 'b.mov', 'c.flac'] }]), + output_kinds: { 'a.bmp': 'image', 'b.mov': 'video', 'c.flac': 'audio' }, + } + const { container } = render(JobPage, { jobId: 'j1' }) + await waitFor(() => expect(container.querySelector('audio')).toBeTruthy()) + expect(container.querySelector('img')?.getAttribute('src')).toContain('a.bmp') + expect(container.querySelector('video')?.getAttribute('src')).toContain('b.mov') + expect(container.querySelector('audio')?.getAttribute('src')).toContain('c.flac') +}) +``` + +In the existing tests that render media and read metadata (the one around the `'a.png', 'clip.mp4'` manifest, and any other that expects an `` or a `galleryMetadata` call), add `output_kinds` to the job they build, for example `{ ...job([...]), output_kinds: { 'a.png': 'image', 'clip.mp4': 'video' } }`, because the page no longer guesses from the extension. + +- [ ] **Step 6: Run it to see it fail** + +Run: `cd ui && npx vitest run src/lib/pages/JobPage.test.ts` +Expected: the new test FAILS (no `