diff --git a/.gitignore b/.gitignore index 0c9a77e2..253a8407 100644 --- a/.gitignore +++ b/.gitignore @@ -165,3 +165,6 @@ workflows/*/assets/ /output/ /*.mp4 /*.wav + +# Claude Code worktrees +.claude/worktrees/ diff --git a/docs/MCP.md b/docs/MCP.md index fd4a4670..a54b4c64 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -222,21 +222,21 @@ when no single workflow covers it. | `list_tasks()` | — | List every task command a workflow's task step can name | | `get_task(command)` | `command` | Get a task command's argument schema | | `list_models()` | — | List what the Hugging Face model cache holds, largest first | -| `get_memory()` | — | Get the worker's VRAM and RAM statistics. `gpu_*` is the card; `host_memory_rss_mb` / `host_memory_peak_rss_mb` are the worker process's resident and high-water host memory, beside the machine's `host_memory_total_mb` / `host_memory_available_mb` - read both, since an offloading workflow keeps its weights in host RAM and the card says little about what it holds (a host field is absent, not null, where the platform cannot measure it). `live: true` means `info` was measured now and is the worker's own memory; only live readings are comparable with each other. `live: false` with `info: null` (and `stale: false`) means nothing has been measured because nothing is resident; `live: false` with a populated `info` is a cached earlier reading, with `reason` (`job_running`, `worker_stopped`, `worker_busy`, `worker_unreachable`) and `age_seconds` - one cached while a job loads a model understates what is resident, so ask again when the server is idle rather than comparing it with a live figure | +| `get_memory()` | — | Get the worker's VRAM and RAM statistics. `gpu_*` is the card; `host_memory_rss_mb` / `host_memory_peak_rss_mb` are the worker process's resident and high-water host memory, beside the machine's `host_memory_total_mb` / `host_memory_available_mb` - read both, since an offloading workflow keeps its weights in host RAM and the card says little about what it holds (a host field is absent, not null, where the platform cannot measure it). `host_pinned_reserved_mb` / `host_pinned_allocated_mb`, when present, are torch's pinned-host cache - the staging buffers group offloading moves weights through, part of `host_memory_rss_mb` and invisible in every `gpu_*` figure, which is what a worker holding GB after releasing every model is usually holding (#98). `live: true` means `info` was measured now and is the worker's own memory; only live readings are comparable with each other. `live: false` with `info: null` (and `stale: false`) means nothing has been measured because nothing is resident; `live: false` with a populated `info` is a cached earlier reading, with `reason` (`job_running`, `worker_stopped`, `worker_busy`, `worker_unreachable`) and `age_seconds` - one cached while a job loads a model understates what is resident, so ask again when the server is idle rather than comparing it with a live figure | | `get_health()` | — | Check that the server is alive, and which machine answered: `version`, `device`, whether the worker process is up, the job running now and the queue depth | | `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 | | `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)` | `limit`, `subfolder` | 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 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 | -| `get_gallery_metadata(name, envelope=False)` | `name` | Get the metadata embedded in a generated file: the exact workflow and arguments that produced it, and, for audio/video, a `media` block (duration, rate, channels, fps, size, peak/mean dBFS). `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 | +| `list_gallery(limit=50, subfolder=None, workspace=None)` | `limit`, `subfolder`, `workspace` | 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 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. `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: the exact workflow and arguments that produced it, and, for audio/video, a `media` block (duration, rate, channels, fps, size, peak/mean dBFS). `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. `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 | Tool | Arguments | Purpose | | --- | --- | --- | -| `get_output_image(name, max_dimension=768)` | `name`, `max_dimension` | Look at a generated image, downscaled to `max_dimension` on its longest side. Returns the image plus a text part reporting `original_size`, `returned_size` and `bytes`, so a downscale is never silent | -| `get_output_text(name, max_characters=20000)` | `name`, `max_characters` | 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 | -| `download_output(name, destination=None, overwrite=False)` | `name`, `destination`, `overwrite` | Save one output file to local disk, of any content type. `destination` may be a full path, a directory, or omitted to save under the output's own name in the current working directory; `~` expands and missing parent directories are created. `overwrite=True` is required to replace a file already at the resolved path. 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 | -| `delete_output(name)` | `name` | Permanently remove one generated file from the output directory | +| `get_output_image(name, max_dimension=768, workspace=None)` | `name`, `max_dimension`, `workspace` | Look at a generated image, downscaled to `max_dimension` on its longest side. Returns the image plus a text part reporting `original_size`, `returned_size` and `bytes`, so a downscale is never silent. `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_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 | +| `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, a directory, or omitted to save under the output's own name in the current working directory; `~` expands and missing parent directories are created. `overwrite=True` is required to replace a file already at the resolved path. 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, workspace=None)` | `name`, `workspace` | Permanently remove one generated file from the output directory. `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 | ### Authoring, assets and workspaces @@ -255,7 +255,7 @@ The session starts in `default` and stays there unless it is told otherwise. | `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 | | `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 | -| `keep_output(name, asset_name=None, overwrite=False, shared=False)` | `name`, optional `asset_name`, `overwrite`, `shared` | 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 | +| `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 | | `upload_asset(file_path, asset_name=None, shared=False)` | `file_path` | Push a local image, video or audio file into the server's asset library and get back its `asset:` reference. The file is read from the machine the MCP server runs on, so this is how an input reaches a dw.serve running somewhere else. `asset_name` stores it under a readable name (`cast/priya-voice.wav`) instead of a random one; `shared=true` puts it in the library every workspace shares | | `delete_asset(name)` | `name` | Permanently remove one file from the asset library, by the name `list_assets` reports. Deletes from whichever library holds it - this workspace's own before the shared one; one from a read-only examples library is refused. Any workflow still carrying that `asset:` reference stops loading | | `save_workflow(name, workflow)` | `name`, `workflow` | Save a workflow into the server's writable workflow directory, overwriting any existing workflow of that name there. A name that currently resolves to a read-only source (an examples directory) is not overwritten - the copy lands in the writable directory and shadows it | @@ -370,11 +370,27 @@ one phase, so two polls otherwise come back identical: read `denoise_step` moving (slow but healthy) against a `denoise_step` that is a number and stays put while `seconds_since_event` climbs (nothing is happening). A null `denoise_step` under `generating` is neither - it is the lead-in the -pipeline runs before the loop, encoding the prompt and any reference image -or audio, ~90 s on MiniMax H3 with nothing emitted, so silence there is -expected. `cancel_job` stops at the next denoise or +pipeline runs before the loop, encoding the prompt and every reference, with +nothing emitted, so silence there is expected. Its length follows what it +encodes: ~90 s on MiniMax H3 for a prompt with an image or audio reference, +~10 min once a *video* reference is among them (measured: 629 s for one 5 s +960x544 clip on an RTX 3090). `get_job_events` names the block it is in +while that runs - a `log` line per top-level block of a modular pipeline +(`MiniMaxAI/MiniMax-H3: vae_encoder`), which is the difference between +silence and knowing it is encoding the reference. And once the counter is a number the gaps +between steps are uneven wherever a transformer block cache is configured - +cheap cached steps, then a full one - so a 140 s gap on H3 is a healthy run; +read liveness as the counter moving between polls minutes apart rather than +as silence under a threshold. `cancel_job` stops at the next denoise or step boundary, which `denoise_step` is also the measure of. +The other frozen-counter stretch is at the end: under `saving`, `denoise_step` +sits at a completed-looking `8/8` and cannot move again, because the step is +writing files. `get_job_events` carries a `log` per file there too - named as +the write starts (`writing shot.mp4 (121 frames)`) and costed as it finishes +(`wrote shot.mp4 in 1.3s (1.4 MB)`) - so that stretch is attributable rather +than silent (#97). + ## Security The MCP server adds no authentication of its own — it inherits the REST diff --git a/docs/SERVER.md b/docs/SERVER.md index 5a0ef570..80de09dc 100644 --- a/docs/SERVER.md +++ b/docs/SERVER.md @@ -164,7 +164,7 @@ Every event in the stream carries a `seq` and an `event` name: | event | when | payload | | --- | --- | --- | | `job_status` | queued/running/terminal transitions | `status` | -| `log` | worker output lines | `message` | +| `log` | worker output lines; each top-level block of a `ModularPipeline` as it starts (`MiniMaxAI/MiniMax-H3: vae_encoder`) - the lead-in before the denoise loop is where a reference encode's minutes go, and the block name is what says which one it is in; and each file the `saving` phase writes, named as it starts (`writing shot.mp4 (121 frames)`) and costed as it finishes (`wrote shot.mp4 in 1.3s (1.4 MB)`), which is the other stretch a step spends with its denoise counter frozen | `message`, and for a file `file` plus `seconds` on the closing one | | `memory` | device memory after a run | `info` | | `run_start` | the run directory is chosen, before the first step | `run_id`, `identity`, `run_dir` | | `workflow_start` | the run begins | `workflow`, `total_steps`, `steps`, `seed` | @@ -173,6 +173,7 @@ Every event in the stream carries a `seq` and an `event` name: | `pipeline_step` | each denoise step | `step`, `total_steps`. Emitted for a pipeline that takes a `callback_on_step_end`, and for a `ModularPipeline` (H3, LTX-2, Qwen-Image), which takes none - there the denoise block's own progress bar is what reports | | `phase` | the step changes what it is doing | `phase`, `detail` | | `pipeline_released` | a step with `release_pipeline` drops its pipeline | `step`, `index`, `gpu_memory_allocated_mb` and `gpu_memory_allocated_before_mb` (both `null` where the backend cannot say). Emitted between the step's generation and its files being written, which is where the release happens - so the ordering is readable off the event stream rather than by trying to poll memory through a sub-second write | +| `warning` | a step finds something wrong with what it is about to write | `message`, plus a `kind` and the figures behind it (`level_spread`: `spread_db`, `measure`, `command`; `fps_mismatch`: `declared_fps`, `source_fps`). Also appended to the job's `warnings`, prefixed with the step it fired in - the event keeps the moment, `warnings` keeps it where a caller polling the finished job will look, since a warning about the artifact outlives the run that noticed it | | `workflow_end` | the run finishes | `manifest` | A step spends most of its wall clock outside the denoise loop, and @@ -181,7 +182,9 @@ A step spends most of its wall clock outside the denoise loop, and pipeline as a previous run - milliseconds, not minutes), `generating` (the denoise loop, or a chain's `segment N/M` - which is why the counter restarts), `decoding` (latents, after the last denoise step), `saving` -(writing files, including video encode) and `task` (a task step, named in +(writing files, including video encode - it names each file on the `log` +stream rather than running silent, since the denoise counter is frozen at its +last step throughout) and `task` (a task step, named in `detail`). Emits are a handful per step, not per denoise tick. ### Progress on a running job @@ -201,12 +204,31 @@ to learn where a long render is: | `denoise_step`, `denoise_total_steps` | the denoise loop's counter, `null` until it starts | A null `denoise_step` under `generating` is the pipeline's lead-in - encoding -the prompt and any reference image or audio - which emits nothing and runs -well over a minute on a large video model (~90 s on MiniMax H3). The keys are -always present so that lead-in can be told from a loop that has stopped +the prompt and every reference - which emits nothing and runs well over a +minute on a large video model. How long it runs follows what it has to +encode: on MiniMax H3, ~90 s for a prompt with an image or audio reference, +but ~10 min once a *video* reference is among them - a measured run encoding +one 5 s 960x544 clip on an RTX 3090 sat silent from 94 s to 723 s. The keys +are always present so that lead-in can be told from a loop that has stopped advancing: `seconds_since_event` is a stall signal once `denoise_step` is a number, or in any phase other than `generating`. +The lead-in is no longer silent, though: each of a modular pipeline's +top-level blocks emits a `log` naming it as it starts (`before_encode`, +`text_encoder`, `vae_encoder`, `denoise`, `decode` on H3), so the last event +says which one the run is inside. Only a `SequentialPipelineBlocks` is +narrated this way - a conditional container picks one branch rather than +running them all, and walking its sub-blocks would be a wrong answer bought +with a progress message. + +It is a coarse one even then. A step's cost is not uniform when the pipeline +configures a transformer block cache (`"cache": {"type": "first_block"}`): +most steps are served from it in seconds and every few steps one is computed +in full, so the same healthy run emits four `pipeline_step` events in 20 s +and then nothing for 133 s. Liveness is `denoise_step` having moved between +polls minutes apart, not silence measured against a fixed threshold - on H3 +that threshold would have to exceed ~140 s to mean anything. + ## Introspection API The editor's forms come from these; they are just as usable from scripts: @@ -264,7 +286,11 @@ The editor's forms come from these; they are just as usable from scripts: the maintainer-measured `{device, name, vram_gb, minutes}` runs, or `null` when nobody has measured it - a list-driven workflow's `cost` entry may also carry a measured `per_entry` (`{variable, minutes, entries}`), the - cost of one entry of the list it was measured against. A `models/` entry + cost of one entry of the list it was measured against. The response's + `cost_basis` says what that is - `curated`: figures a maintainer measured + once and wrote into the workflow, never derived from this server's own job + history, so `null` means nobody wrote one down rather than "this box has + never run it". A `models/` entry takes its `shape` and `traits` from the template it configures and keeps its own `cost`. A list-driven workflow (one with a `for_each` step) also carries `lists`: per list variable, the fields an entry takes, the steps diff --git a/docs/TASKS.md b/docs/TASKS.md index a1339842..cef619f2 100644 --- a/docs/TASKS.md +++ b/docs/TASKS.md @@ -204,7 +204,7 @@ video generation" in the workflow guide): | `crossfade_ms` | No | Equal-power crossfade at each audio seam, drawn from the trimmed material - no effect when `trim_frames` is 0, and validation warns when one is written there (default: 75) | | `audio_bleed_ms` | No | How long the outgoing video's tail rings on over the head of the next one, at seams with nothing trimmed to crossfade (default: 0, off) | | `seam_fade_ms` | No | Fade on each side of a seam that gets neither a crossfade nor a bleed - for tonal material, not for a continuous bed (default: 3, just enough not to click) | -| `fps` | No | Frame rate of the videos - required to join audio when trimming | +| `fps` | No | Frame rate of the videos - required to join audio when trimming, and the rate the joined file is written at unless `result.fps` overrides it | | `match_levels` | No | Even the shots' loudness out before joining - `"rms"` for perceived level (the measurement `get_gallery_metadata` reports as `mean_dbfs`), `"peak"` for the loudest sample. Off by default | | `match_levels_dbfs` | No | The level `match_levels` moves every shot to (default: -1 dBFS for `peak`, -20 dBFS for `rms`) | @@ -294,8 +294,10 @@ same" means, and `"peak"` matches the loudest sample, which is the safer choice on material with big transients. A shot whose gain would clip at the target is held just below full scale and the log says so. Left off - the default, so nothing existing changes - a spread of 6 dB or more across the -tracks being joined is logged as a warning rather than passing in silence. -`dissolve_videos` takes the same pair. +tracks being joined is reported as a warning rather than passing in silence: +on the job's `warnings` and as a `warning` event in its stream, not only in +the server's log, since the caller who can act on it is the one who asked for +the run. `dissolve_videos` takes the same pair. ### dissolve_videos @@ -327,7 +329,7 @@ montage cut to a score wants: | `fade_in_frames` | No | Frames over which the first video rises out of `fade_color` (default: 0) | | `fade_out_frames` | No | Frames over which the last video sinks into it (default: 0) | | `fade_color` | No | The RGB colour the fades come from and go to (default: black) | -| `fps` | No | Frame rate of the videos - required to crossfade audio at a dissolve | +| `fps` | No | Frame rate of the videos - required to crossfade audio at a dissolve, and the rate the dissolved file is written at unless `result.fps` overrides it | | `match_levels` | No | Even the shots' loudness out before joining - `"rms"` or `"peak"`, as with [`concat_videos`](#concat_videos). Off by default | | `match_levels_dbfs` | No | The level `match_levels` moves every shot to (default: -1 dBFS for `peak`, -20 dBFS for `rms`) | diff --git a/docs/WORKFLOW_GUIDE.md b/docs/WORKFLOW_GUIDE.md index f26d58b3..7a11624a 100644 --- a/docs/WORKFLOW_GUIDE.md +++ b/docs/WORKFLOW_GUIDE.md @@ -151,7 +151,22 @@ Invoke another workflow file: } ``` -Paths can be relative to the current file or use `builtin:` to reference built-in workflows in `dw/workflows/`. +`path` is read the way `run_workflow`'s `workflow_path` is: a catalog name as +`list_workflows` reports it (`templates/minimax/reference-to-video`), with or +without `.json`; a path relative to the file that names it (`../models/x.json`); +or `builtin:name.json` for the packaged fragments in `dw/workflows/`. A name +resolves beside the referencing file first, then against the run's own +`workflows/` directory, then against each read-only source the server lists - +so a stored template can be composed without copying it into the workspace. A +path that lands outside every source is refused, and one that resolves nowhere +is a validation error rather than a run that fails on its first step. + +When the composing step declares a `result`, that is where the composed output +is written, once: the child's own last step does not save it a second time +under its own name. A composing step that declares no `result` (or one with no +`content_type`) leaves the saving to the child, as before. The child's other +steps write into the same run directory, with the composing step's name +leading their file names. ## Cross-Step Data Flow @@ -543,6 +558,28 @@ subfolder written is one a later workflow can name: validation error at its JSON path. `file_base_name` is a name, not a path: a separator there is refused, and `subfolder` is the way to place a file. +### Composing a stored workflow + +A step with a `workflow` block runs another workflow as one step of this one, +with `arguments` handed down as that workflow's variables. Its `path` is read +the way `run_workflow`'s `workflow_path` is - a catalog name from +`list_workflows`, with or without `.json`, a path relative to the file that +names it, or `builtin:name.json` - and resolves beside the referencing file +first, then in this workspace's `workflows/`, then in each read-only source +the server lists. A stored template is composed by its catalog name; copying +it into the workspace to reach it is no longer necessary, and a copy silently +stops tracking the original. + +Declare a `result` on the composing step and the composed output is saved +there, once, under that step's name and subfolder - the composed workflow's +own last step does not write a second copy. Its other steps write into the +same run directory, prefixed with the composing step's name. + +`validate_workflow` resolves the path, so a name that reaches nothing is an +error at `steps[N].workflow.path` before anything is queued; it also validates +the workflow named, refuses a composition cycle, and warns about an argument +the composed workflow declares no variable for. + ### Being found next time The catalog derives each entry's `shape` — one of `image`, `image-set`, diff --git a/docs/proposals/acknowledged-cost-binding.md b/docs/proposals/acknowledged-cost-binding.md new file mode 100644 index 00000000..ff0a41af --- /dev/null +++ b/docs/proposals/acknowledged-cost-binding.md @@ -0,0 +1,177 @@ +# Proposal: bind `acknowledged_cost` to an estimate, not just to a boolean + +Status: **design only** - written in answer to issue #85 (forum feedback), +after reading the gate and every path by which a run's size is decided. +No code changes yet. Written by the implementer agent (model `opus`, +provider `anthropic`). + +## The question asked + +> The acknowledgement should bind to an estimated budget or operation +> fingerprint, not just a boolean, otherwise the plan can change after +> consent while the flag remains true. + +## What the gate is today + +It is a boolean, checked in the MCP layer only: + +- `dw_mcp/diagnose.py:45` - `run_workflow` raises `COST_REFUSAL` unless + `acknowledged_cost` is truthy; `rerun_job` the same at `:248`. +- `dw_mcp/models.py` / `dw_mcp/workspaces.py` - `download_model`, + `delete_model`, `update_diffusers`, `delete_workspace` the same. +- `POST /api/jobs` does **not** require it. The gate is an agent-behaviour + gate - "say the number out loud to a human before you spend it" - not a + resource guard, and the HTTP API and the web UI queue work without it. + +Nothing is bound. The flag records that *something* was consented to, not +what. The figure the agent quoted comes from the catalog's `cost` block +(`list_workflows`), which is measured against the workflow's *stored +defaults*; the run is queued with the caller's `arguments`, which is a +different document. + +## Where the plan can grow after consent + +Each of these is reachable with `acknowledged_cost=true` and a quote that +was honest when it was made: + +1. **List fan-out.** A `for_each` step is expanded per entry + (`dw/for_each.py`, 32 entries max). `cost.minutes` is the measured cost + of the *default* list; a caller passing a 12-entry `shots` list through + `arguments` runs 12 shots. `cost.per_entry` exists precisely to price + this - and is advisory, unenforced, and set on no template yet. +2. **Cartesian product.** Several `previous_result:` references in one step + multiply (4 images x 3 masks = 12 iterations), and the multiplicands can + come from arguments. +3. **Plain numeric arguments.** `num_images_per_prompt`, `num_frames`, + `num_inference_steps` scale the run roughly linearly and are ordinary + variables. +4. **A model download mid-run.** Weights not on disk are pulled by + `from_pretrained` when the step reaches it - tens of GB and many minutes, + appearing in no `cost` block. This is the forum comment's sharpest case: + the same workflow costs 4 minutes on a warm box and 50 on a cold one. +5. **`inline_workflow`.** No catalog entry, so no `cost` at all - the quote + is whatever the agent believed. +6. **`rerun_job(new_seed=true)`.** Draws a fresh seed, which defeats the + step cache; the rerun of a "finished instantly" job is a full generation. +7. **Sub-workflows.** A `composes-workflows` step's real cost is the child's. + +So the premise in #85 holds: the flag stays `true` while the work grows, and +nothing at queue time compares what will run against what was acknowledged. + +## What the engine already has + +Almost all of the raw material: + +- `POST /api/validate` is free, takes the caller's `arguments`, and already + folds them exactly as the run will (`argument_errors`), expands `for_each` + and reports per-entry problems. +- `expand_for_each` yields the exact member list a run will execute. +- `realize_workflow` (`dw/realize.py`) already produces the canonical + realized document - arguments folded, seed pinned, prompts inlined, + `output:latest` resolved - which is the natural thing to fingerprint. +- `hub_cache.scan_models` knows what is on disk, so "which repos this run + will have to download first" is answerable before queuing. +- The manifest and `workflow.json` make the run inspectable afterwards, + which is what makes an estimate checkable rather than decorative. + +The missing pieces are an estimate in the pre-flight answer, and a check at +queue time. + +## Proposed change + +### 1. `POST /api/validate` returns a plan + +Beside `valid`/`errors`/`warnings`, on a valid answer: + +```json +"plan": { + "fingerprint": "sha256:9f13…", + "steps": 14, + "iterations": 14, + "list_entries": {"shots": 12}, + "downloads_required": [{"repo": "MiniMaxAI/MiniMax-H3", "gb": 41.2}], + "estimate": {"minutes": 38.0, "basis": "per_entry", "confidence": "measured"} +} +``` + +- `fingerprint` is a SHA-256 over the plan-shaping inputs: the realized + workflow (`realize.py`'s document, minus the seed), the expanded step + names, and the iteration count per step. It changes when the *work* + changes and not when something cosmetic does. +- `estimate.basis` is one of `per_entry` (fixed + per-entry x N), + `catalog` (the stored `cost.minutes`, defaults only), or `unknown` + (an inline workflow with no cost block) - the honesty is in the field, + not in a fabricated number. +- `downloads_required` closes case 4 on its own, and is useful with or + without the rest of this proposal. + +### 2. `acknowledged_cost` accepts what was acknowledged + +`run_workflow` / `rerun_job` keep taking `true`, and additionally take an +object: + +```json +"acknowledged_cost": {"fingerprint": "sha256:9f13…", "minutes": 38.0} +``` + +`POST /api/jobs` recomputes the plan for the arguments it was actually +given and refuses with **409** when the fingerprint differs or the recomputed +estimate exceeds the acknowledged minutes by more than a tolerance (25%, +settable), naming both figures and what changed. The agent's recovery is to +re-quote to the user - which is the behaviour the gate was for. + +### 3. Bare `true` stays legal, and says so + +An unbound `true` keeps working - the web UI, the CLI-equivalent callers and +every existing script depend on it, and a hard requirement would be a +breaking change to every MCP consumer. But the job records which kind of +acknowledgement it got (`acknowledged: "boolean" | "bound"`), so "was this +run consented to at its actual size?" is answerable after the fact, and the +skills can teach the bound form as the normal one. + +## Alternatives considered + +- **A hard budget ceiling** (`max_minutes`, run aborted when exceeded). + Needs runtime metering the engine does not have, and killing a 90%-done + video render to honour an estimate wastes exactly the resource the gate + protects. The fingerprint check is pre-flight, which is where a refusal is + free. +- **Requiring the bound form.** Breaking, and buys little over recording + which form was used. +- **Estimating cost server-side with no acknowledgement change.** Half the + value (the agent can quote better) with none of the binding - the plan can + still change between the quote and the call. +- **Doing nothing.** Defensible: the gate is a prompt-discipline device, and + the human is in the loop by construction. But the forum comment's case - + "validates cheaply, expands at runtime" - is real on this engine today + (cases 1, 4 and 6 above), and the plan-at-validate half is cheap and + useful even alone. + +## Scope + +- `dw/plan.py` (new): fingerprint + estimate from a definition and + arguments, reusing `realize_workflow`, `expand_for_each` and `hub_cache`. +- `dw/server/app.py`: `plan` on the validate answer; the 409 check on + `POST /api/jobs`. +- `dw/server/jobs.py`: record the acknowledgement form on the job and in + `jobs.sqlite`. +- `dw_mcp/diagnose.py` + `dw_mcp/server.py`: accept the object form, surface + `plan` from `validate_workflow`. +- Docs: SERVER.md, MCP.md, WORKFLOW_GUIDE.md's authoring section, and the + three plugin skills' cost step. +- Tests: fingerprint stability against cosmetic edits, change under a longer + list, the 409, the tolerance, and the boolean path unchanged. + +Roughly a two-stage piece of work: stage 1 the plan on validate (useful on +its own), stage 2 the binding and the 409. + +## Open questions for approval + +1. Is the bound form worth it at all, given that a human is already in the + loop on every acknowledgement? (Doing nothing is a legitimate answer; + stage 1 alone is another.) +2. Tolerance: 25% of the acknowledged minutes, or a fingerprint-only check + with no numeric comparison at all? A numeric one needs `per_entry` on the + templates to be meaningful, and today no template carries it. +3. Should `POST /api/jobs` grow the gate for HTTP callers too, or stay an + MCP-layer concept? (The web UI would have to send something.) diff --git a/docs/proposals/measured-cost-from-job-history.md b/docs/proposals/measured-cost-from-job-history.md new file mode 100644 index 00000000..3a4874e0 --- /dev/null +++ b/docs/proposals/measured-cost-from-job-history.md @@ -0,0 +1,158 @@ +# Proposal: derive a workflow's cost from this server's own job history + +Status: **design only** - written in answer to issue #91 (tester feedback), +which asks for three things and gets the cheapest of them shipped without a +proposal. This document covers the expensive one. Written by the implementer +agent (model `opus`, provider `anthropic`). + +## The report + +`list_workflows(shape="shot", traits="identity-referenced")` answers +`cost: null` for seven of eight entries, including +`templates/minimax/reference-to-video`, on a box that has run that template +five times at the same size. The consumer is instructed to quote a price +before spending GPU minutes (`run_workflow`'s own description, the MCP +instructions), and for the template its series actually uses, the catalog +says "unknown". The tester has carried a hand-kept cost table across six +cycles, re-deriving it from job history each time it is lost - from +`started_at`/`finished_at` on jobs this server stored. + +Five runs of that template, one RTX 3090, 124 frames at 960x544 / 20 steps: +511, 464.9, 478, ~460, ~467 seconds. ~7.8 minutes, spread under 10% - a +tighter figure than the 10.1 the one populated entry claims by hand. + +## What shipped without this proposal + +`GET /api/workflows` now answers `cost_basis: "curated"`, and +`list_workflows`' description says what that means: a `cost` block is a +figure a maintainer measured once on the devices it names and wrote into the +workflow; nothing derives one from job history; `null` means nobody wrote one +down, not that the run is cheap or that this box has never run it. That is +the tester's option 3, and it stops `null` reading as a data gap. + +It does not give anyone a number. + +## Why the rest is a proposal and not a fix + +`cost` is documented in `dw/workflow_schema.json` as *"Measured runs, one per +device the maintainer measured on. **Never derived**; absent means unknown."* +That sentence is load-bearing: a curated figure is a claim a person stands +behind, on a named card, at a named size. Deriving one changes what the field +*is*, for every consumer that reads it - which is the "new concept consumers +would have to learn" bar. It also decides questions with no obvious answer +(below). So: design, then Don, then code. + +## The shape + +A second field rather than a second meaning for the first: + +```json +"cost": [{"device": "cuda", "name": "RTX 3090", "vram_gb": 24, "minutes": 10.1}], +"observed": { + "device": "cuda", + "name": "NVIDIA GeForce RTX 3090", + "runs": 5, + "median_minutes": 7.8, + "p10_minutes": 7.6, + "p90_minutes": 8.5, + "since": "2026-09-08T14:02:11Z", + "comparable": "same-arguments" +} +``` + +`cost` keeps meaning exactly what it means today. `observed` is this +server's own history, always about *this* box's accelerator, and absent when +there is nothing to report. A consumer quoting a price prefers `observed` +when it is there (it is this machine, measured), falls back to `cost`, and +says "unknown" only when neither exists. `cost_basis` stays, and the listing +gains nothing else. + +### Where the numbers come from + +`jobs.sqlite` already holds, per job: the workflow name, `started_at`, +`finished_at`, `status`, `run_id`/`run_dir`, and the workspace. The manifest +in the run directory holds the realized workflow - the arguments folded in +and the seed pinned (`dw/realize.py`). The device is the server's, from +`get_server_info`. + +So the query is: finished jobs, `status = completed`, for one workflow +identity, on this device, most recent N. Median of +`finished_at - started_at`. + +### The four questions that make it a design + +1. **What counts as comparable?** A 141-frame run does not inform a + 124-frame estimate, and a different `weights_dtype` is a different model. + Three options, cheapest first: + - *Ignore it.* Report the median of every run of that workflow name, with + `runs` and a spread. Honest if the spread is published, useless for a + workflow whose variables move cost by 3x. + - *Bucket by the arguments that move cost.* Requires naming them, which + is per-workflow knowledge the catalog does not have. A `cost_drivers` + key in the workflow (`["num_frames", "num_inference_steps"]`) would + declare them, and the bucket key is those values. This is the honest + one and it is more schema. + - *Report the default-arguments runs only.* A run whose arguments equal + the workflow's variable defaults is comparable to the curated figure by + construction. Simple, exact, and thin - most real runs pass arguments. + + Recommendation: **bucket by declared drivers**, falling back to + default-arguments-only when a workflow declares none. It degrades to the + third option rather than to a wrong number. + +2. **Cold vs warm.** The tester's own `text-to-image` figures are 13.6 s and + 6.3 s - the same run, model on disk vs model resident. These are two + numbers and averaging them produces one that describes neither. The job + events already distinguish them (a `loading` phase that takes minutes vs + one that takes none), so the split is available: `median_minutes` warm, + `cold_minutes` when the run had to load. A consumer quoting a first run + of the session wants the cold one. + +3. **A cached run is not a run.** A seeded workflow whose every step hit the + step cache finishes in seconds and wrote nothing. Those jobs are already + flagged (`reused` on every manifest entry) and must be excluded, or the + median collapses toward zero for exactly the templates that get re-run + most. + +4. **Pruning.** Job history is prunable and a workspace is deletable. The + figures move when it happens, which is fine (`runs` says how much is + behind it), but a listing that quietly loses a number people relied on + should not be surprising - `since` and `runs` are what make it legible. + +### Cost of the feature + +A per-workflow aggregate over `jobs.sqlite`, cached like `workflow_details` +is (by the jobs table's own high-water mark rather than by mtime), computed +on listing. Reading the realized workflow of each candidate run to bucket by +drivers is the expensive part - one small JSON read per run, bounded by the +N most recent, and only for workflows the listing actually returns. Nothing +in the run path changes. No new storage; `finished_at - started_at` is +already recorded. + +Rough size: a new `dw/server/observed_cost.py` (aggregate + cache), a +`cost_drivers` key in the schema, the `observed` field in the compact and +full listings, the MCP description, docs, and tests. Half a day, most of it +the comparability rule. + +## What I would not do + +Overwrite `cost` with a derived figure, or let `observed` inherit the +`cost` shape closely enough to be mistaken for it. The distinction between +"a maintainer measured this on a 4090" and "this box averaged that last +week" is the whole value of reporting both. + +## Recommendation + +Ship it as `observed`, bucketed by declared `cost_drivers`, cold/warm split, +cached runs excluded. If that is more than the problem is worth, the +fallback that costs almost nothing is the third comparability option - +default-arguments runs only, with `runs` and `since` published - which would +have answered the tester's question today, because their five runs were all +at one size. + +## Open question for Don + +Is `cost` allowed to gain a sibling that is derived, or does "never derived" +apply to the whole of what the catalog says about price? If the latter, this +belongs in a separate tool (`get_workflow_history(name)`) rather than in the +listing, and the consumer pays a call for it. diff --git a/docs/proposals/preflight-argument-bounds.md b/docs/proposals/preflight-argument-bounds.md new file mode 100644 index 00000000..e3b6d839 --- /dev/null +++ b/docs/proposals/preflight-argument-bounds.md @@ -0,0 +1,131 @@ +# Pre-flight argument bounds + +Status: proposed, 2026-09-14. Raised by #96. Written by the implementer +agent, model `opus` via provider `anthropic`. + +## The problem + +`validate_workflow` passed `num_frames: 61` on `templates/minimax/video-with-audio` +and reported `valid: true`, naming `num_frames` in `checked_arguments` - so the +answer claimed to cover the caller's value. The run then spent 138.7 s loading +the H3 weights and the turbo LoRA, entered the text encoder, and failed on: + +``` +MiniMax-H3 generates between 5.0 and 15.0 seconds at 24 fps, so `num_frames`, +rounded up to the next `17 * n + 5` the video VAE can encode, must be between +120 and 360, got 61 (rounded up to 73). +``` + +Every term in that message is a property of the model and its VAE. None of it +needed a loaded pipeline: the check is `align_num_frames(...)` against +`min_duration` / `max_duration` / `fps`, all of which are constants on +`MiniMaxH3ModularPipeline` until a VAE overrides them with the same values. + +Two things follow, and the second is the one that makes this worth a proposal +rather than a patch: + +1. A bound the engine can state exactly is only enforced after two minutes of + GPU work. +2. **Nothing on the MCP surface states it at all.** `get_workflow(variables_only=true)` + gives `num_frames: 124` with no range. The tester picked 61 as `4 * 15 + 1` + because `4n + 1` is the common convention; H3's is `17n + 5` with a floor of + 120. A consumer cannot guess this and has nowhere to read it. + +There is also a silent rounding: a legal value that is not of the form +`17n + 5` is rounded up, and the caller gets a frame count they did not ask +for with a `logger.warning` that reaches no consumer (the #82 rule - a +diagnostic that only reaches the log does not exist out there). + +## Why this is a decision and not an edit + +`CLAUDE.md` says it plainly: model knowledge lives in the composition skills +and the catalog, **never in engine code**, and every number a skill states is +pinned to a diffusers symbol by `tests/test_plugin_skills.py`. Any fix here +puts a model's frame-count rule *somewhere*, and the three candidate somewheres +have different consequences for that rule. Picking one is yours. + +## Option A - declare the constraint in the workflow (recommended) + +Precedent exists. A chain step already declares the same rule, because the +chain has to snap its final segment to a legal length: + +```json +"frame_snap": {"modulus": 17, "remainder": 5, "min_frames": 124, "max_frames": 345} +``` + +That block is written in the workflow, by the author who knows the model - the +engine consumes it and holds no model knowledge of its own. Extend the same +shape to a declared variable, so it can be checked before anything is queued: + +```json +"variables": {"num_frames": 124}, +"variable_constraints": { + "num_frames": { + "minimum": 124, + "maximum": 345, + "modulus": 17, + "remainder": 5, + "snap": "up", + "reason": "the video VAE encodes 17 * n + 5 frames, 5 to 15 seconds at 24 fps" + } +} +``` + +- `validation_errors` gains a pass over it, after substitution and `for_each` + expansion like the others, reporting at `arguments.` where the + caller's value sits and at `variables.` for a stored default. So + `POST /api/validate`, `validate_workflow` and the pre-queue check in + `POST /api/jobs` all get it for free, in that order of usefulness. +- `snap: "up"` makes the rounding explicit and gives validation a *warning* to + report when the value it would snap differs from the value passed - which is + the adjacent finding in #96, and which the run-time path should emit as an + `emit_warning` too so it reaches the job's `warnings`. +- The catalog reports it: `list_workflows` and `get_workflow(variables_only=true)` + carry the constraint beside the default, so a consumer reads the rule instead + of guessing it. This is the half that stops the next person picking 61. + +Cost: every template that has a bound has to declare it, and a template that +declares none validates exactly as it does now. The numbers get the same +discipline the skills have - a test that pins each one to the diffusers symbol +it came from, so a library change fails in CI rather than in a cold session. + +Risk: a declared constraint can go stale against the library. The pinning test +is what keeps it honest, and a *wrong* bound refuses a legal run - which is +why the check should refuse only on the declared rule and never invent one. + +## Option B - a pre-flight registry in the engine, keyed by pipeline class + +`dw/preflight.py` holds a per-pipeline check that imports the diffusers symbols +(cheap - no weights) and computes the bound: `align_num_frames`, +`MINIMAX_H3_FPS`, `min_duration.fget` / `max_duration.fget`. No template +markup, and it covers an inline workflow an agent wrote from scratch, which +Option A does not. + +It also puts model knowledge in engine code, which is the thing `CLAUDE.md` +forbids - softened, but not removed, by every number being read from diffusers +rather than written here. It needs a new entry per family, forever, and the +"which pipeline is this" dispatch is a second place model identity is spelled. + +## Option C - curated bounds in the catalog, beside `cost` + +`cost` is already a maintainer's measured figure carried per workflow and +reported by `list_workflows` (#91). Bounds could ride there. Same authoring +cost as A with none of A's enforcement: the catalog is descriptive, and a +consumer-facing number nothing checks drifts. Useful as the *reporting* half +of A, not as a substitute. + +## Recommendation + +A, with the reporting from C folded into it: declare the constraint where the +`frame_snap` precedent already puts it, check it at validation time, warn on a +value that will be snapped, and report it beside the variable's default so it +can be read before it is violated. Mark the H3 templates first (the bound in +#96), then LTX-2.5's own frame rule, then whatever the next family brings in +through `model-family-onboarding`. + +## Not in scope + +Checking a value against a *loaded* pipeline's signature - that is what the +existing signature warnings do. This is only about bounds that are constants +of the model, which is the class of error that costs two minutes of GPU before +it is reported. diff --git a/dw/events.py b/dw/events.py index d6f8db50..e761fa26 100644 --- a/dw/events.py +++ b/dw/events.py @@ -102,6 +102,23 @@ def deactivate_context(token): NON_INTERRUPTIBLE_PHASES = ("loading", "task") +def emit_warning(message, **data): + """Report something the run's result carries but its status will not. + + A warning a step discovers at run time - shots being cut together 10 dB + apart, a video about to be written at a frame rate nothing chose - is + only useful where whoever asked for the run can read it. The server's + own log is not that place: a consumer over the API or MCP sees the event + stream and the job's `warnings` list and nothing else, so a diagnostic + that only reaches the log does not exist out there (#82). + + Logged as well as emitted, because the CLI and the REPL have no event + sink and the log is the whole of their surface. + """ + logger.warning(message) + get_context().emit("warning", message=message, **data) + + def emit_phase(phase, detail=None): """Report a coarse phase change on the active run. @@ -114,3 +131,18 @@ def emit_phase(phase, detail=None): context = get_context() context.note_phase(phase) context.emit("phase", phase=phase, detail=detail) + + +def emit_log(message, **data): + """Narrate one step of a long, otherwise silent stretch of a run. + + A `log` event rather than a phase: `PHASES` is a closed set a consumer + switches on, and "which file is being written" is a detail inside one of + them, not a new state. The modular block lead-in (#95) is the same shape + at the other end of a step. + + Logged as well as emitted, because the CLI and the REPL have no event + sink and the log is the whole of their surface. + """ + logger.info(message) + get_context().emit("log", message=message, **data) diff --git a/dw/host_memory.py b/dw/host_memory.py index 89b48340..4a09371b 100644 --- a/dw/host_memory.py +++ b/dw/host_memory.py @@ -18,7 +18,13 @@ logger = logging.getLogger("dw") -__all__ = ["host_memory_stats", "host_memory_fields"] +__all__ = [ + "host_memory_stats", + "host_memory_fields", + "trim_host_memory", + "release_host_caches", + "pinned_host_memory_fields", +] _MB = 1024.0 * 1024.0 @@ -58,6 +64,23 @@ def host_memory_stats(): if all(stats[key] is not None for key in ("rss_mb", "total_mb")): break + return _hold_the_high_water_mark(stats) + + +def _hold_the_high_water_mark(stats): + """Keep peak_rss_mb >= rss_mb, which is what a high-water mark means. + + The two readings come from different places - getrusage's ru_maxrss, + quantized to whole pages and taken first, against psutil's rss taken a + moment later - so a process that has never peaked meaningfully above its + current size reports them within a megabyte of each other in either + order. `peak - rss` is the whole point of the pair (what a run took and + did not give back), and a small negative there reads as "these fields + are not comparable" rather than "nothing leaked" (#83). + """ + peak, rss = stats["peak_rss_mb"], stats["rss_mb"] + if peak is not None and rss is not None and peak < rss: + stats["peak_rss_mb"] = rss return stats @@ -132,3 +155,104 @@ def host_memory_fields(): return { FIELD_NAMES[key]: value for key, value in stats.items() if value is not None } + + +def trim_host_memory(): + """Hand memory the process has already freed back to the operating + system, and report how much that was in MB (0.0 when the platform has no + way to ask). + + Dropping the last reference to a model frees it inside the process, not + back to the kernel: glibc keeps the arenas the weights were read into and + hands them out again to *this* process. That is normally invisible and + correct - until a template needs 96% of host RAM to run at all, at which + point the several GB the previous job's arenas are sitting on is the + difference between a run and a SIGKILL five minutes in (#98). The + templates here load tens of GB of weights through host memory, so the + arenas in question are large ones and the fragmentation that keeps + `malloc_trim` from returning them is the exception rather than the rule. + + Linux/glibc only: `malloc_trim` is a GNU extension. Everywhere else - + macOS included - this is a no-op that reports 0.0, because there is + nothing to ask and a fabricated number would be worse than none. + """ + import ctypes + import ctypes.util + import sys + + if not sys.platform.startswith("linux"): + return 0.0 + before = host_memory_stats()["rss_mb"] + try: + libc = ctypes.CDLL(ctypes.util.find_library("c") or "libc.so.6", use_errno=True) + libc.malloc_trim(ctypes.c_size_t(0)) + except (OSError, AttributeError) as e: + # musl and friends have no malloc_trim; not having one is not an error + logger.debug(f"malloc_trim unavailable: {e}") + return 0.0 + after = host_memory_stats()["rss_mb"] + if before is None or after is None: + return 0.0 + return max(0.0, before - after) + + +def pinned_host_memory_fields(): + """What CUDA's pinned-host allocator is holding, in MB, or {} where + there is none to report. + + Group offloading with `use_stream` stages a component's weights through + *pinned* host memory, which torch caches per process exactly as it + caches device memory: freeing the tensors returns the blocks to that + cache, not to the OS, so they stay in this process's RSS and count + against the next job's host budget. It is invisible in every figure the + memory payload carried before - `rss_mb` includes it without saying so + and `gpu_memory_*` does not see it at all (#98). + """ + try: + import torch + + stats = torch.cuda.host_memory_stats() + except Exception: + return {} + fields = {} + for key, name in ( + ("allocated_bytes.all.current", "host_pinned_allocated_mb"), + ("reserved_bytes.all.current", "host_pinned_reserved_mb"), + ): + value = stats.get(key) + if value is not None: + fields[name] = value / _MB + return fields + + +def release_host_caches(): + """Give back host memory this process is holding but no longer using, + and report what came back in MB. + + Two caches, neither of which `gc.collect()` touches: + + - torch's pinned-host allocator, where group offloading's staging + buffers live. `_host_emptyCache` frees the blocks nothing is using; + blocks a still-loaded pipeline is staging through are in use and are + not touched, so this is safe to call with models resident. + - glibc's heap arenas, via `malloc_trim`. Freeing a large block inside + the process does not hand its pages back to the kernel. + + Together they are why a worker that has released every model still sat + on 14.5 GB, which is the difference between the next job running and + being OOM-killed five minutes in on a template that needs 96% of host + RAM (#98). + """ + before = host_memory_stats()["rss_mb"] + try: + import torch + + if hasattr(torch._C, "_host_emptyCache"): + torch._C._host_emptyCache() + except Exception as e: # a cleanup is never worth failing the run for + logger.debug(f"Could not empty the pinned host cache: {e}") + trim_host_memory() + after = host_memory_stats()["rss_mb"] + if before is None or after is None: + return 0.0 + return max(0.0, before - after) diff --git a/dw/pipeline_processors/chain.py b/dw/pipeline_processors/chain.py index 690c6616..6e8811a2 100644 --- a/dw/pipeline_processors/chain.py +++ b/dw/pipeline_processors/chain.py @@ -35,7 +35,12 @@ from diffusers.utils import encode_video, is_av_available from .. import empty_device_cache -from ..result import AudioVideo, get_artifact_list, output_file_path +from ..result import ( + AudioVideo, + frames_for_encoding, + get_artifact_list, + output_file_path, +) from ..tasks.audio_utils import ( as_channels_samples, equal_power_crossfade_join, @@ -260,7 +265,7 @@ def write(self, frames, audio, sample_rate): audio_track = torch.from_numpy(numpy.ascontiguousarray(audio)) encode_video( - frames, + frames_for_encoding(frames), fps=self.fps, output_path=path, audio=audio_track, @@ -381,9 +386,11 @@ def run_chain(pipeline, chain_definition, arguments): # is muxed in so the soundtrack has no seams if spill is None: frames = frames[: config.total_frames] - return AudioVideo(frames, config.source_audio, config.source_rate) + return AudioVideo( + frames, config.source_audio, config.source_rate, fps=config.fps + ) - return AudioVideo(frames, audio, audio_rate) + return AudioVideo(frames, audio, audio_rate, fps=config.fps) class ChainConfig: diff --git a/dw/pipeline_processors/pipeline.py b/dw/pipeline_processors/pipeline.py index c84be296..aa03f430 100644 --- a/dw/pipeline_processors/pipeline.py +++ b/dw/pipeline_processors/pipeline.py @@ -545,6 +545,11 @@ def _call_pipeline(self, arguments, attn_backend): # drive a tqdm bar, and a bar that reports each advance is # the difference between a slow run and a hung one stack.enter_context(reported_progress_bars(self.pipeline)) + # The bar only covers the denoise loop; the blocks around it + # are where a reference encode's minutes go (#95) + stack.enter_context( + reported_blocks(self.pipeline, self.segment_label or self.name) + ) return self.pipeline(**arguments) @@ -1933,6 +1938,98 @@ def reporting(iterable=None, total=None, _original=original): holder.progress_bar = original +def _runs_its_blocks_in_sequence(blocks): + """Whether a modular block container runs every sub-block in order. + + diffusers' own `SequentialPipelineBlocks` is the answer; the import is + local and forgiving because a pipeline that is not modular at all never + reaches here, and a diffusers without the class is one with no modular + pipelines to narrate. + """ + try: + from diffusers.modular_pipelines.modular_pipeline import ( + SequentialPipelineBlocks, + ) + except ImportError: # pragma: no cover - a diffusers without modular + return False + return isinstance(blocks, SequentialPipelineBlocks) + + +@contextlib.contextmanager +def reported_blocks(pipeline, label): + """Name each of a modular pipeline's top-level blocks as it starts. + + The denoise loop is only one of them, and on a reference-conditioned + model it is not the long one: encoding a video reference runs for + minutes inside `vae_encoder` before a single bar advances, so the whole + lead-in went by with nothing emitted and a consumer could not tell it + from a hang (#95). The blocks are named - `before_encode`, + `text_encoder`, `vae_encoder`, `denoise`, `decode` on H3 - and naming + each one as it begins turns that silence into "it is encoding the + reference", plus a `seconds_since_event` that resets at every boundary. + + Reported as `log` events rather than phases: `PHASES` is a closed set a + consumer switches on, and a block name is a detail, not a new state. + + The patch is on the class because `block(pipeline, state)` resolves + `__call__` on the type, not the instance - so it is guarded by identity + (only the pipeline's own top-level blocks report) and undone on the way + out. + """ + blocks = getattr(pipeline, "_blocks", None) + sub_blocks = getattr(blocks, "sub_blocks", None) + if blocks is None or not hasattr(sub_blocks, "items"): + yield + return + # Only a sequence runs all of its sub-blocks. A conditional container + # (AutoPipelineBlocks, which is what several of H3's own steps are) + # *picks* one on its inputs, so narrating it by walking the mapping + # would run every branch - hence the check for the one dispatch this + # reproduces rather than a duck-typed `sub_blocks` + if not _runs_its_blocks_in_sequence(blocks): + yield + return + + holder = type(blocks) + original = holder.__call__ + was_own = "__call__" in vars(holder) + run_context = get_context() + + @torch.no_grad() + def reporting(self, pipe, state): + # A nested SequentialPipelineBlocks shares the class; only the + # pipeline's own top-level sequence is the one worth narrating + if self is not blocks: + return original(self, pipe, state) + for name, block in self.sub_blocks.items(): + run_context.emit("log", message=f"{label}: {name}") + # A block boundary is a cancellation checkpoint the lead-in + # otherwise has none of + run_context.check_cancelled() + try: + pipe, state = block(pipe, state) + except WorkflowCancelled: + raise + except Exception: + # What diffusers' own dispatch logs, kept because this + # replaces that loop + logger.error(f"Error in block: ({name}, {block.__class__.__name__})") + raise + return pipe, state + + holder.__call__ = reporting + try: + yield + finally: + if was_own: + holder.__call__ = original + else: + try: + del holder.__call__ + except AttributeError: + holder.__call__ = original + + @contextlib.contextmanager def stateful_cache_context(pipeline): """Provide the context a stateful cache hook reads its state through. diff --git a/dw/realize.py b/dw/realize.py index 5368f218..eb980019 100644 --- a/dw/realize.py +++ b/dw/realize.py @@ -31,6 +31,7 @@ resolve_output_reference, ) from .security import SecurityError, validate_workflow_path +from .workflow_sources import resolve_sub_workflow, SubWorkflowNotFound from .variables import set_variables logger = logging.getLogger("dw") @@ -210,20 +211,18 @@ def scan(value): def _digest(path, base_dir, workflow_dir): """The SHA-256 of a sub-workflow file, or None when it cannot be read. - Resolved the way `Workflow.create_step_action` resolves it - relative to - the referencing file's directory, then through `validate_workflow_path` - confined to `workflow_dir` - so a path this run could not have loaded is - not one realization reads either. + Resolved the way `Workflow.create_step_action` resolves it - beside the + referencing file, then across the workflow search path, then through + `validate_workflow_path` confined to the root it came from - so a path + this run could not have loaded is not one realization reads either, and + a catalog name the run composed is digested rather than recorded as + unreadable (#90). """ try: - candidate = ( - path - if os.path.isabs(path) - else os.path.normpath(os.path.join(base_dir or ".", path)) - ) - validated = validate_workflow_path(candidate, workflow_dir) + candidate, root = resolve_sub_workflow(path, base_dir or ".", workflow_dir) + validated = validate_workflow_path(candidate, root) with open(validated, "rb") as file: return hashlib.sha256(file.read()).hexdigest() - except (SecurityError, OSError, ValueError) as e: + except (SecurityError, OSError, ValueError, SubWorkflowNotFound) as e: logger.debug(f"No digest for sub-workflow {path}: {e}") return None diff --git a/dw/result.py b/dw/result.py index b837b9c1..4db86cd4 100644 --- a/dw/result.py +++ b/dw/result.py @@ -1,4 +1,5 @@ import os +import time import numpy import torch import soundfile @@ -12,7 +13,7 @@ is_av_available, ) from collections.abc import Mapping -from .events import emit_phase +from .events import emit_log, emit_phase, emit_warning from .security import ( SecurityError, validate_file_base_name, @@ -25,6 +26,68 @@ # Result saving constants MAX_BASE_NAME_LENGTH = 200 DEFAULT_AUDIO_SAMPLE_RATE = 44100 +# The rate a video is written at when neither the workflow nor the artifact +# says - a diffusers convention old enough that changing it would restate +# every existing workflow's output +DEFAULT_VIDEO_FPS = 8 + + +def _artifact_size(artifact): + """How much there is to write, said the way the thing itself counts - + frames for a video, samples for a waveform. Best effort: it is narration + beside a file name, so anything it cannot measure it does not mention.""" + try: + frames = getattr(artifact, "frames", None) + if frames is not None: + return f"{len(frames)} frames" + if hasattr(artifact, "__len__") and not isinstance(artifact, (str, bytes)): + return f"{len(artifact)} frames" + except Exception: + pass + return "" + + +def _file_size_mb(path): + try: + return os.path.getsize(path) / (1024 * 1024) + except OSError: + return 0.0 + + +def frames_for_encoding(frames): + """Generated frames in the form `encode_video` encodes without first + inspecting them. + + A pipeline that returns `output_type="np"` hands back float frames in + [0, 1], and diffusers' `encode_video` establishes that range with three + full-size temporaries - `np.zeros_like`, `np.ones_like` and the bool + mask - before converting. On a 121-frame 960x544 clip that is ~3 GB of + allocation and 16 s of wall clock on an idle box, against 2.4 s for the + encode itself, and it is the bulk of a 'saving' phase that ran for 53 s + with nothing else in it (#97, measured on lem 2026-09-14). + + Converting here is a pass and a half and hands back a torch tensor, + which `encode_video` takes as given - so the check never runs. Frames + outside [0, 1] are left exactly as they were: that is the branch where + diffusers warns and treats them as pixel values already, and it is not + a path any pipeline here produces or that this can be tested against. + + The source array is never written to - a later step may still read this + result through a `previous_result:` reference, and the step cache + retains it. + """ + if not isinstance(frames, numpy.ndarray) or frames.size == 0: + return frames + if not numpy.issubdtype(frames.dtype, numpy.floating): + return frames + if float(frames.min()) < 0.0 or float(frames.max()) > 1.0: + return frames + denormalized = numpy.empty(frames.shape, dtype=numpy.uint8) + # Frame by frame: the whole-array form allocates another copy the size + # of the video, which is the cost this exists to avoid + for index in range(frames.shape[0]): + denormalized[index] = numpy.round(frames[index] * 255.0) + return torch.from_numpy(denormalized) def output_file_path(output_dir, file_name): @@ -109,16 +172,24 @@ class AudioVideo: the result mux them into one file instead of dropping the audio on the floor. """ - def __init__(self, frames, audio, sample_rate): + def __init__(self, frames, audio, sample_rate, fps=None): """ Args: frames: The video, as PIL images or an array of frames audio: Waveform for this video, shaped (channels, samples) sample_rate: Sample rate of the waveform, or None if the pipeline did not report one + fps: Frame rate these frames are meant to play at, when something + knows it - a joined video's own rate, or the rate of the file + a task read. Carried for the same reason AudioTrack carries + its sample rate: `result.fps` defaults to 8, and a step that + joins 24 fps shots writing them at 8 is three times slow with + its audio still the right length (#84). A declared + `result.fps` still wins over this """ self.frames = frames self.audio = audio self.sample_rate = sample_rate + self.fps = fps class AudioTrack: @@ -396,19 +467,29 @@ def save_artifact( output_path = output_file_path(output_dir, f"{file_base_name}{extension}") logger.info(f"Saving artifact to {output_path}") + # Writing one file is the whole of the 'saving' phase's wall clock, + # and on a video it is minutes of it with nothing else to report - + # the denoise counter is frozen at its last step and there is no + # further event until step_end, so a healthy run is indistinguishable + # from a hung one (#97). Name the file as it starts and report what + # it cost as it finishes, the same shape the modular block lead-in + # got in #95 + emit_log( + f"writing {os.path.basename(output_path)}" + + (f" ({_artifact_size(artifact)})" if _artifact_size(artifact) else ""), + file=os.path.basename(output_path), + content_type=content_type, + ) + started = time.monotonic() try: if content_type.startswith("video"): if isinstance(artifact, AudioVideo): self.save_audio_video(artifact, output_path, content_type) else: - export_to_video( - artifact, output_path, fps=self.result_definition.get("fps", 8) - ) + export_to_video(artifact, output_path, fps=self.video_fps(artifact)) elif content_type == "image/gif": - export_to_gif( - artifact, output_path, fps=self.result_definition.get("fps", 8) - ) + export_to_gif(artifact, output_path, fps=self.video_fps(artifact)) elif content_type.startswith("audio"): waveforms = normalize_audio(artifact) # Declared rate > the rate a generated track carries > default @@ -470,8 +551,46 @@ def save_artifact( ) raise + emit_log( + f"wrote {os.path.basename(output_path)} in " + f"{time.monotonic() - started:.1f}s ({_file_size_mb(output_path):.1f} MB)", + file=os.path.basename(output_path), + seconds=round(time.monotonic() - started, 1), + ) return [output_path] + def video_fps(self, artifact): + """The frame rate this video is written at. + + Declared `result.fps` first, then the rate the artifact carries (a + join's own rate, or the rate of the files it read), then 8. + + The order matters more than it looks: `result.fps` and a task's own + `fps` argument are separate knobs, and the one an author thinks to + set is the task's. A step that told `concat_videos` its shots are 24 + fps and said nothing on `result` used to write them at 8 - the + picture three times long against an audio track still the right + length, with nothing said about it (#84). A workflow that does + declare `result.fps` still wins, so writing at a rate other than the + source's - a deliberate slow motion - stays available, and says so. + """ + declared = self.result_definition.get("fps") + carried = getattr(artifact, "fps", None) + if declared is None: + return carried or DEFAULT_VIDEO_FPS + if carried and abs(declared - carried) > 0.01: + emit_warning( + f"Writing video at {declared} fps, but the frames it was " + f"given run at {carried} fps - the file will play " + f"{declared / carried:.2g}x speed " + f"({carried / declared:.2g} times as long). Drop 'fps' from " + f"the step's result to keep the source rate", + kind="fps_mismatch", + declared_fps=declared, + source_fps=carried, + ) + return declared + def save_audio_video(self, artifact, output_path, content_type): """Write a video and the audio generated with it into a single file. @@ -484,7 +603,7 @@ def save_audio_video(self, artifact, output_path, content_type): output_path: Path of the file to write content_type: MIME type of the video being written """ - fps = self.result_definition.get("fps", 8) + fps = self.video_fps(artifact) # The pipeline reports the sample rate of what it generated - the result # definition can still override it sample_rate = self.result_definition.get( @@ -543,7 +662,7 @@ def save_audio_video(self, artifact, output_path, content_type): logger.debug(f"Muxing audio at {sample_rate}Hz into {output_path}") encode_video( - artifact.frames, + frames_for_encoding(artifact.frames), fps=fps, output_path=output_path, audio=as_audio_track(artifact.audio), diff --git a/dw/serve.py b/dw/serve.py index 59c1d543..caed92cf 100644 --- a/dw/serve.py +++ b/dw/serve.py @@ -192,10 +192,19 @@ def main(): # search path is what lets an example run as it shipped: the workspace's # own library is still searched first and is still the only one written # to. Pinned in the environment, so the worker resolves as the API does - from .workspace import ASSETS_SUBDIR, example_libraries, set_library_fallbacks + from .workspace import ( + ASSETS_SUBDIR, + WORKFLOWS_SUBDIR, + example_libraries, + set_library_fallbacks, + ) example_dirs = example_libraries(args.examples_dirs) set_library_fallbacks(PROMPTS_SUBDIR, example_dirs[PROMPTS_SUBDIR]) + # The workflow trees themselves, so a sub-workflow step can compose a + # stored template by the name list_workflows reports rather than a copy + # of it in this workspace (#90) + set_library_fallbacks(WORKFLOWS_SUBDIR, args.examples_dirs) # The shared library goes ahead of the examples and behind the # workspace's own, which is the order 'asset:' resolves in: a workspace # name shadows a shared one, and a shared one shadows an example's. diff --git a/dw/server/app.py b/dw/server/app.py index 371c589d..19611e35 100644 --- a/dw/server/app.py +++ b/dw/server/app.py @@ -1353,7 +1353,10 @@ def validate_workflow( "error": None, "errors": [], "warnings": workflow_argument_warnings(definition) - + entry_field_warnings(definition, request.arguments), + + entry_field_warnings(definition, request.arguments) + # An argument a sub-workflow step passes to a workflow that + # declares no variable for it - dropped in silence at run time + + candidate.sub_workflow_warnings(), } if request.arguments: # Naming what was checked is the difference between 'the stored @@ -1510,6 +1513,13 @@ def list_workflows( "sources": [source.to_dict() for source in sources], "workflows": sorted(details), "details": details, + # What a `cost` is, and so what a null one means. Curated: + # figures a maintainer measured once on the devices named and + # wrote into the workflow - nothing derives them from this + # server's own job history, so null means nobody wrote one + # down, not that the run is cheap or that this box has never + # run it (#91) + "cost_basis": "curated", } @app.put("/api/workflows/{name:path}") diff --git a/dw/server/jobs.py b/dw/server/jobs.py index 73f2d55d..c659c5dd 100644 --- a/dw/server/jobs.py +++ b/dw/server/jobs.py @@ -342,7 +342,9 @@ def __init__(self, spec): self.started_at = None self.finished_at = None self.manifest = [] - self.warnings = spec.get("warnings", []) + # A copy: run-time warnings are appended to this list (see + # _note_progress) and the spec is what a rerun is built from + self.warnings = list(spec.get("warnings", [])) self.error = None self.traceback = None # Which run this job turned out to be - reported by the worker's @@ -361,6 +363,7 @@ def __init__(self, spec): self.phase_detail = None self.phase_started_at = None self.step_name = None + self.parent_step = None self.step_index = None self.total_steps = None self.denoise_step = None @@ -400,10 +403,27 @@ def _note_progress(self, event): elif kind == "pipeline_step": self.denoise_step = event.get("step") self.denoise_total_steps = event.get("total_steps") + elif kind == "warning": + # Both channels, on purpose: the event log keeps the moment it + # happened, `warnings` keeps it where a caller who polled the + # finished job will actually look, since a warning about the + # artifact outlives the run that noticed it (#82). The step it + # fired in is the run's, not the warning's - the engine warns + # from inside a step without knowing which one it is + message = event.get("message") + if message: + named = f"{self.step_name}: {message}" if self.step_name else message + if named not in self.warnings: + self.warnings.append(named) elif kind == "step_start": self.step_name = event.get("step") - self.step_index = event.get("index") - self.total_steps = event.get("total_steps") + # A sub-workflow counts its own steps from zero; what a caller + # watching a composed run needs is where the run it queued has + # got to, so the parent's counter wins when the event carries + # one and the step name stays the child's (#90) + self.parent_step = event.get("parent_step") + self.step_index = event.get("parent_index", event.get("index")) + self.total_steps = event.get("parent_total_steps", event.get("total_steps")) # A new step's denoise loop has not started; the previous step's # count would read as this one's progress self.denoise_step = None @@ -418,6 +438,9 @@ def progress(self): now = time.time() summary = { "step": self.step_name, + # The step of the queued workflow the one above is running + # inside, for a composed run; null when they are the same thing + "parent_step": self.parent_step, "step_index": self.step_index, "total_steps": self.total_steps, "phase": self.phase, diff --git a/dw/tasks/audio_utils.py b/dw/tasks/audio_utils.py index 81d5f6b7..b0aba909 100644 --- a/dw/tasks/audio_utils.py +++ b/dw/tasks/audio_utils.py @@ -14,6 +14,7 @@ import soundfile import torch +from ..events import emit_warning from ..security import ( validate_path, validate_url, @@ -656,10 +657,17 @@ def warn_on_level_spread(waveforms, command="concat_videos", measure="rms"): return None spread = max(levels) - min(levels) if spread >= LEVEL_SPREAD_WARN_DB: - logger.warning( + # emit_warning rather than logger.warning: this is a property of the + # file the run is about to write, and the caller reading the job is + # the one who can act on it (#82) + emit_warning( f"{command}: the tracks being joined span {spread:.1f} dB " f"({measure} {min(levels):.1f} to {max(levels):.1f} dBFS) - the cut " - f"will be audible as a level jump. Pass match_levels to even them out" + f"will be audible as a level jump. Pass match_levels to even them out", + kind="level_spread", + command=command, + spread_db=round(spread, 1), + measure=measure, ) return spread diff --git a/dw/tasks/concat_videos.py b/dw/tasks/concat_videos.py index 829c7698..df9c60e5 100644 --- a/dw/tasks/concat_videos.py +++ b/dw/tasks/concat_videos.py @@ -61,7 +61,9 @@ def concat_videos( hard cut on tonal material, which a bleed would only stutter. It is the wrong tool for a continuous bed such as a laugh track or room tone: a fade only deepens the hole a bleed is there to cover - fps: Frame rate of the videos - required to join audio when trimming + fps: Frame rate of the videos - required to join audio when + trimming, and the rate the joined file is written at unless + the step's result.fps overrides it match_levels: Even the shots' loudness out before joining - "rms" matches perceived level (the measurement get_gallery_metadata reports as mean_dbfs), "peak" matches the @@ -147,4 +149,10 @@ def concat_videos( ) logger.debug(f"Concatenated {len(videos)} videos into {len(frames)} frames") - return AudioVideo(frames, audio, sample_rate) + # The rate the caller declared, else the rate the first input carries - + # either beats the result's 8 fps default (#84) + written_fps = fps or next( + (v.fps for v in videos if getattr(v, "fps", None)), + None, + ) + return AudioVideo(frames, audio, sample_rate, fps=written_fps) diff --git a/dw/tasks/dissolve_videos.py b/dw/tasks/dissolve_videos.py index 66841afd..2f281c92 100644 --- a/dw/tasks/dissolve_videos.py +++ b/dw/tasks/dissolve_videos.py @@ -108,7 +108,11 @@ def dissolve_videos( f"Dissolved {len(clips)} videos into {len(frames)} frames " f"({dissolve_frames}-frame seams)" ) - return AudioVideo(frames, audio, sample_rate) + written_fps = fps or next( + (v.fps for v in loaded if getattr(v, "fps", None)), + None, + ) + return AudioVideo(frames, audio, sample_rate, fps=written_fps) def _dissolve_join(previous, following, overlap): diff --git a/dw/tasks/interpolate_frames.py b/dw/tasks/interpolate_frames.py index 7513ab84..c0cfbe5f 100644 --- a/dw/tasks/interpolate_frames.py +++ b/dw/tasks/interpolate_frames.py @@ -49,6 +49,7 @@ def interpolate_frames(video, device="cpu", **kwargs): # An AudioVideo or a frame array unwraps to its frames; a PIL list passes # through by identity. The soundtrack does not survive - the frame count # changes, so pair_audio is how it comes back + source_fps = getattr(video, "fps", None) video = frames_as_pil_list(video) if len(video) < 2: raise ValueError(f"Need at least 2 frames to interpolate, got {len(video)}") @@ -69,7 +70,13 @@ def interpolate_frames(video, device="cpu", **kwargs): frames = _interpolate_2x(frames, model) logger.info(f"Interpolation complete: {len(video)} -> {len(frames)} frames") - return AudioVideo(frames, None, None) + # Multiplied, not carried: interpolation adds frames between the ones it + # was given, so playing them back at the source rate would run the clip + # `multiplier` times long. The rate that keeps the source's duration is + # the source's times the multiplier (#84) + return AudioVideo( + frames, None, None, fps=source_fps * multiplier if source_fps else None + ) def _interpolate_2x(frames, model): diff --git a/dw/tasks/pair_audio.py b/dw/tasks/pair_audio.py index ec21e3cd..8e19848f 100644 --- a/dw/tasks/pair_audio.py +++ b/dw/tasks/pair_audio.py @@ -68,4 +68,6 @@ def pair_audio(video, audio, sample_rate=None): # cost a copy of the whole thing for nothing frames = video.frames if isinstance(video, AudioVideo) else video logger.debug(f"Pairing frames with audio at {rate} Hz") - return AudioVideo(frames, as_channels_samples(waveform), rate) + return AudioVideo( + frames, as_channels_samples(waveform), rate, fps=getattr(video, "fps", None) + ) diff --git a/dw/tasks/stabilize.py b/dw/tasks/stabilize.py index e4b72ee8..e2fcc2e4 100644 --- a/dw/tasks/stabilize.py +++ b/dw/tasks/stabilize.py @@ -117,5 +117,5 @@ def stabilize_video(clip, smooth=0): ) if isinstance(clip, AudioVideo): - return AudioVideo(held, clip.audio, clip.sample_rate) + return AudioVideo(held, clip.audio, clip.sample_rate, fps=clip.fps) return held diff --git a/dw/tasks/task.py b/dw/tasks/task.py index eb32869b..dcdb2a92 100644 --- a/dw/tasks/task.py +++ b/dw/tasks/task.py @@ -270,7 +270,7 @@ def _per_frame(image, process): frames = [process(frame) for frame in frames_as_pil_list(image)] audio = getattr(image, "audio", None) sample_rate = getattr(image, "sample_rate", None) - return AudioVideo(frames, audio, sample_rate) + return AudioVideo(frames, audio, sample_rate, fps=getattr(image, "fps", None)) @register_command("upscale", implementation="dw.tasks.upscale.upscale_image") diff --git a/dw/tasks/video_utils.py b/dw/tasks/video_utils.py index fbbcd2ca..9626b143 100644 --- a/dw/tasks/video_utils.py +++ b/dw/tasks/video_utils.py @@ -288,7 +288,11 @@ def _decode_audio_video(handle): f"Decoded {len(frames)} frames and " f"{audio.shape[1] if audio is not None else 0} audio samples" ) - return AudioVideo(frames, audio, sample_rate if audio is not None else None) + # The file's own rate travels with it: a step that joins videos read + # from disk knows what to write them back at without being told (#84) + return AudioVideo( + frames, audio, sample_rate if audio is not None else None, fps=frame_rate + ) # How far a decoded track may be off the frames' own duration and still be diff --git a/dw/worker.py b/dw/worker.py index 1877ecde..35cd25a7 100644 --- a/dw/worker.py +++ b/dw/worker.py @@ -22,7 +22,19 @@ from dw.security import validate_output_path from dw.events import RunContext, WorkflowCancelled from dw import get_device_type, empty_device_cache, device_memory_stats -from dw.host_memory import host_memory_fields +from dw.host_memory import ( + host_memory_fields, + host_memory_stats, + pinned_host_memory_fields, + release_host_caches, +) + + +def _mb(value): + """A host memory reading for a message, or 'unknown' where the platform + could not take one.""" + return f"{value:.0f} MB" if value is not None else "unknown" + logger = logging.getLogger("dw.worker") @@ -199,7 +211,9 @@ def _handle_execute(self, command: Dict[str, Any]): "message": "Workflow changed - releasing cached models...", } ) - self._cleanup_all() + self.result_queue.put( + {"type": "output", "message": self._cleanup_all()} + ) self.workflow_identity = identity self.result_queue.put( @@ -426,6 +440,12 @@ def _cleanup_all(self): """ Complete cleanup - clear all cached models and components. Called when workflow changes or on shutdown. + + Returns: + One line saying what host memory looks like on the other side of + it. The caller decides whether that reaches the client: on a + workflow switch it is the answer to "what did the last job leave + behind", which is the question a later OOM is asked (#98). """ import gc from .tasks.model_cache import clear_model_cache @@ -467,7 +487,24 @@ def _cleanup_all(self): except Exception as e: logger.warning(f"Could not perform GPU cleanup: {e}") - logger.info("Full cleanup complete") + # Dropping the references above frees the weights inside this + # process; it does not hand the arenas they were read into back to + # the kernel. On a template that needs 96% of host RAM the residue of + # the *previous* job is what the OOM killer arrives for, minutes into + # a run that is itself perfectly legal (#98) - so ask for it back + # here, where a different model family is about to be loaded, and say + # what came back rather than leaving it to be inferred from a later + # reading + released = release_host_caches() + stats = host_memory_stats() + summary = ( + f"Released cached models: host RSS {_mb(stats.get('rss_mb'))}, " + f"{_mb(stats.get('available_mb'))} available" + + (f" ({released:.0f} MB returned to the OS)" if released else "") + ) + + logger.info(f"Full cleanup complete. {summary}") + return summary def _parent_is_dead(self) -> bool: """ @@ -519,6 +556,11 @@ def _get_memory_info(self) -> Dict[str, Any]: # else. Measured inside the worker, so the process figures are the # worker's own info.update(host_memory_fields()) + # And what torch's pinned-host allocator is sitting on, which + # `rss_mb` includes without saying so: on a group-offloaded workflow + # it is GB of staging buffers, and it is the half of a worker's host + # residency no figure named before (#98) + info.update(pinned_host_memory_fields()) try: stats = device_memory_stats() diff --git a/dw/workflow.py b/dw/workflow.py index 00b898a3..3c559509 100644 --- a/dw/workflow.py +++ b/dw/workflow.py @@ -73,6 +73,11 @@ InvalidInputError, UntrustedWorkflowError, ) +from .workflow_sources import ( + builtin_root, + resolve_sub_workflow, + SubWorkflowNotFound, +) logger = logging.getLogger("dw") @@ -270,6 +275,22 @@ class Workflow: # leaves no manifest of its own, since its steps are already rolled up # into the parent's _run_dir_inherited = False + # Where the parent step that delegated to this workflow sits in the + # run the caller queued: {"step", "index", "total_steps"}. A child + # counts its own steps from zero, so without this a composed run + # reported "step 1 of 1" from inside the first of the parent's three + # (#90) - and "is this nearly finished" is the whole question progress + # answers. Handed straight down to a grandchild, so the numbers always + # describe the run that was queued + _parent_progress = None + # Whether the parent step that composed this workflow declares a + # `result` of its own. It does the saving then, and this run's last step + # does not: the two used to write the same artifact twice, once under + # the parent step's name and subfolder and once under the child's, with + # the child's entry shadowing a manifest key the caller never wrote + # (#92). A child whose parent declares nothing still saves, since + # otherwise the output would exist nowhere + _final_save_owned_by_parent = False def __init__(self, workflow_definition, output_dir, file_spec, workflow_dir=None): self.workflow_definition = workflow_definition @@ -292,11 +313,37 @@ def argument_template(self): def variables(self): return self.workflow_definition.get("variables", {}) + def step_save_name(self, workflow_id, step_name, index): + """The base name a step's files are written under. + + Inside a composed child the parent step's name leads, so two steps + composing the same workflow do not both want one name and get told + apart by a '-2' suffix that says nothing about which step made it + (#92). + """ + base = f"{workflow_id}-{step_name}.{index}" + parent = self._parent_progress + return f"{parent['step']}.{base}" if parent else base + + def _parent_progress_fields(self): + """The queued run's own step counter, on an event a sub-workflow + emits - empty for a top-level run, whose index is already that.""" + parent = self._parent_progress + if not parent: + return {} + return { + "parent_step": parent["step"], + "parent_index": parent["index"], + "parent_total_steps": parent["total_steps"], + } + def step_file_prefix(self, step_name): """Naming prefix for files a step writes on its own (chain segment spills), matching the workflow-id-step naming its results are saved under.""" - return f"{self.name}-{step_name}" + prefix = f"{self.name}-{step_name}" + parent = self._parent_progress + return f"{parent['step']}.{prefix}" if parent else prefix @property def effective_output_dir(self): @@ -385,10 +432,136 @@ def expanded_definition(self, arguments=None, source_indices=None): definition = replace_variables(definition, variables) return expand_for_each(definition, source_indices) - def validation_errors(self, arguments=None): + def resolve_sub_workflow_path(self, path): + """Where one sub-workflow step's `path` resolves to, as + (path, root) - the same resolution create_step_action does, asked + ahead of the run so validation can answer for free what used to cost + a queued job to find out (#89). + + Raises SubWorkflowNotFound, SecurityError or InvalidInputError, + each carrying the message the run would have failed with. + """ + confine_to = self.workflow_dir + if path.startswith("builtin:"): + builtin_name = path.replace("builtin:", "") + if ( + not builtin_name.endswith(".json") + or "/" in builtin_name + or "\\" in builtin_name + ): + raise InvalidInputError( + f"Invalid builtin workflow name: {builtin_name}" + ) + confine_to = builtin_root() + resolved = os.path.join(confine_to, builtin_name) + if not os.path.isfile(resolved): + raise SubWorkflowNotFound(path, [resolved]) + return validate_workflow_path(resolved, confine_to), confine_to + if confine_to is None and not os.path.isabs(path): + confine_to = catalog_root_dir(self.file_spec) + resolved, confine_to = resolve_sub_workflow( + path, os.path.dirname(self.file_spec), confine_to + ) + return validate_workflow_path(resolved, confine_to), confine_to + + def sub_workflow_errors(self, expanded, source_indices=None, composing=None): + """Every sub-workflow step whose `path` names nothing this server can + reach, composes a workflow already on the chain, or resolves to a + workflow that does not itself validate. + + `composing` is the resolved path of every workflow above this one, + which is what makes a cycle an error here rather than a recursion + the run discovers. + """ + errors = [] + composing = list(composing or []) + for index, step in enumerate(expanded.get("steps", []) or []): + reference = step.get("workflow") + if not isinstance(reference, dict) or not isinstance( + reference.get("path"), str + ): + continue + source = source_indices[index] if source_indices else index + where = f"steps[{source}].workflow.path" + path = reference["path"] + try: + resolved, root = self.resolve_sub_workflow_path(path) + except (SubWorkflowNotFound, SecurityError, InvalidInputError) as e: + errors.append({"path": where, "message": str(e)}) + continue + if resolved in composing: + errors.append( + { + "path": where, + "message": ( + f"Sub-workflow '{path}' composes a workflow that " + "is already composing it - a cycle: " + + " -> ".join(composing + [resolved]) + ), + } + ) + continue + try: + child = workflow_from_file(resolved, self.output_dir, root) + except Exception as e: + errors.append({"path": where, "message": f"Sub-workflow '{path}': {e}"}) + continue + for error in child.validation_errors(composing=composing + [resolved]): + errors.append( + { + "path": f"{where} -> {error['path']}", + "message": f"Sub-workflow '{path}': {error['message']}", + } + ) + return errors + + def sub_workflow_warnings(self, expanded=None): + """An argument a sub-workflow step passes down that the workflow it + composes declares no variable for - dropped in silence at run time, + and composition is exactly where a name drifts (#89).""" + warnings = [] + try: + expanded = expanded if expanded is not None else self.expanded_definition() + except Exception: + return warnings + for index, step in enumerate(expanded.get("steps", []) or []): + reference = step.get("workflow") + if not isinstance(reference, dict): + continue + passed = reference.get("arguments") + if not isinstance(passed, dict) or not isinstance( + reference.get("path"), str + ): + continue + try: + resolved, root = self.resolve_sub_workflow_path(reference["path"]) + child = workflow_from_file(resolved, self.output_dir, root) + except Exception: + # An unresolvable path is an error, reported by + # sub_workflow_errors - not a second complaint here + continue + declared = child.workflow_definition.get("variables") or {} + for name in sorted(set(passed) - set(declared)): + warnings.append( + { + "path": f"steps[{index}].workflow.arguments.{name}", + "message": ( + f"'{reference['path']}' declares no variable " + f"'{name}' - the value is dropped. Declared: " + + (", ".join(sorted(declared)) or "") + ), + } + ) + return warnings + + def validation_errors(self, arguments=None, composing=None): """Every schema violation in the definition, as [{path, message}]; empty when it validates. `arguments` are the caller's, so a - for_each over a list the caller supplies is checked as it will run.""" + for_each over a list the caller supplies is checked as it will run. + + `composing` carries the chain of sub-workflows above this one, so a + workflow that composes itself is an error rather than a recursion. + """ errors = validate_data_all(self.workflow_definition, load_schema("workflow")) # Only once the shape is known good: the passes below walk the # steps array and a definition that fails the schema may have no @@ -415,9 +588,11 @@ def validation_errors(self, arguments=None): # reported against 'variables' as a whole rather than escaping # as an unhandled exception return [{"path": "variables", "message": str(e)}] - return previous_result_reference_errors( - expanded, source_indices - ) + subfolder_errors(expanded, source_indices) + return ( + previous_result_reference_errors(expanded, source_indices) + + subfolder_errors(expanded, source_indices) + + self.sub_workflow_errors(expanded, source_indices, composing) + ) def _undeclared_variable_errors(self, arguments=None): """Every 'variable:' reference naming nothing the workflow declares. @@ -701,6 +876,7 @@ def run( step=step_data["name"], index=i, total_steps=len(steps), + **self._parent_progress_fields(), ) # Seeds resolve most-specific-first: pipeline > step > workflow @@ -730,10 +906,21 @@ def run( # A sub-workflow step is never cacheable: its files roll up # from the child's own manifest, which a hit does not rebuild. is_cacheable = "workflow" not in step_data and cache_enabled_this_run + # The last step of a composed child whose parent does the + # saving (#92) - its files are the parent step's, written + # once, under the parent's name and subfolder + parent_saves_this = ( + self._final_save_owned_by_parent and i == len(steps) - 1 + ) step_data_snapshot = None if is_cacheable: try: step_data_snapshot = copy.deepcopy(step_data) + if parent_saves_this: + # Keyed apart from the same step run standalone: + # this entry's result was never saved here, so a + # standalone hit on it would report no files + step_data_snapshot["__saved_by_parent__"] = True except Exception as ex: # A realized argument that cannot be deep-copied (an # open handle, a live model object) just means this @@ -776,6 +963,24 @@ def run( step_seed, get_device(), ) + if isinstance(step_action, Workflow): + # The child reports into this run's counter rather than + # its own, and a grandchild reports into the same one + step_action._parent_progress = self._parent_progress or { + "step": step_data["name"], + "index": i, + "total_steps": len(steps), + } + # Only when the parent's own result would write + # something: a result block that names no content_type, + # or says save: false, saves nothing, and suppressing + # the child's save for it would lose the artifact + parent_result = step_data.get("result") + step_action._final_save_owned_by_parent = bool( + isinstance(parent_result, dict) + and parent_result.get("content_type") + and parent_result.get("save", True) + ) reused = cached_result is not None if reused: logger.info(f"Step '{step.name}' unchanged - reusing cached result") @@ -830,9 +1035,13 @@ def run( ) if not reused: - saved_files = result.save( - self.step_output_dir(step_data), - f"{workflow_id}-{step.name}.{i}", + saved_files = ( + [] + if parent_saves_this + else result.save( + self.step_output_dir(step_data), + self.step_save_name(workflow_id, step.name, i), + ) ) if is_cacheable: step_cache.put( @@ -857,7 +1066,11 @@ def run( } if reused: manifest_entry["reused"] = True - self.manifest.append(manifest_entry) + # No entry at all for a step the parent saves for: the + # parent's own entry names the same files, under the step + # name the caller wrote (#92) + if not parent_saves_this: + self.manifest.append(manifest_entry) # roll the child's saves up so job history and the gallery see # every file self.manifest.extend(sub_manifest) @@ -870,6 +1083,7 @@ def run( step=step.name, index=i, total_steps=len(steps), + **self._parent_progress_fields(), **step_end_data, ) logger.debug(f"Step {step.name} completed with result: {result}") @@ -1149,24 +1363,28 @@ def create_step_action( os.path.dirname(os.path.abspath(__file__)), "workflows" ) path = os.path.join(confine_to, builtin_name) - # Handle relative paths. A template under templates/ names a - # model config as '../models/x.json'; collapsing the '..' here - # is what lets the validator judge where the path actually - # lands rather than refusing the spelling - containment is - # still checked on the resolved path below - elif not os.path.isabs(path): - base_dir = os.path.dirname(self.file_spec) - path = os.path.normpath(os.path.join(base_dir, path)) - # An unconfined run (no workflow_dir - a bare CLI - # invocation) used to rely on the '..' regex alone to - # stop a relative reference from leaving the file's own - # directory; normalising the path removes that guard, so - # here confine it to the catalog root instead - the - # referencing file's nearest ancestor literally named - # 'workflows', which still lets it climb to a sibling - # folder like models/ but not out of the catalog - if confine_to is None: + # Everything else - a relative path, or a catalog name as + # list_workflows reports it - goes through the search path. + # A template under templates/ names a model config as + # '../models/x.json', so a path relative to the referencing + # file still resolves first and the '..' is collapsed here, + # which is what lets the validator judge where the path + # actually lands rather than refusing the spelling; + # containment is still checked on the resolved path below. + # An unconfined run (no workflow_dir - a bare CLI + # invocation) used to rely on the '..' regex alone to stop a + # relative reference from leaving the file's own directory; + # normalising the path removes that guard, so confine it to + # the catalog root instead - the referencing file's nearest + # ancestor literally named 'workflows', which still lets it + # climb to a sibling folder like models/ but not out of the + # catalog + else: + if confine_to is None and not os.path.isabs(path): confine_to = catalog_root_dir(self.file_spec) + path, confine_to = resolve_sub_workflow( + path, os.path.dirname(self.file_spec), confine_to + ) # Validate the resolved path - confined when this workflow # itself is (an inline/server-submitted run), so a diff --git a/dw/workflow_schema.json b/dw/workflow_schema.json index 3cefb295..3ba5338a 100644 --- a/dw/workflow_schema.json +++ b/dw/workflow_schema.json @@ -35,7 +35,7 @@ }, "cost": { "type": "array", - "description": "Measured runs, one per device the maintainer measured on. Never derived; absent means unknown.", + "description": "Measured runs, one per device the maintainer measured on. Never derived; absent means unknown. 'minutes' is the whole run's wall clock on a worker that has to load the models - what a consumer actually waits through, not the denoise loop alone; on a video template the two differ by minutes.", "items": { "type": "object", "required": ["device", "vram_gb", "minutes"], @@ -1090,7 +1090,7 @@ "type": "object", "properties": { "path": { - "description": "The path to the workflow file. Use 'builtin:' for built-in workflows.", + "description": "The workflow this step composes: a catalog name as list_workflows reports it (with or without '.json'), a path relative to the workflow that names it ('../models/x.json'), or 'builtin:name.json' for one of the packaged fragments. A name is resolved first beside the referencing file, then against this run's own workflows directory, then against each read-only source on the server's search path - so a stored template can be composed without copying it into the workspace. A path outside every source is refused. When the composing step declares a 'result', that is where the composed output is saved and the composed workflow's own last step does not save it again.", "type": "string" }, "arguments": { @@ -1119,7 +1119,7 @@ "type": "string" }, "fps": { - "description": "Frames per second - only used when output is video", + "description": "Frames per second - only used when output is video. Defaults to the rate the frames themselves carry (a chain's 'fps', a join's own rate, the rate of the file a task read) and to 8 only when nothing knows better. Set it to write at a rate other than the source's - a deliberate slow motion - which the run then warns about.", "type": "integer", "default": 8 }, diff --git a/dw/workflow_sources.py b/dw/workflow_sources.py index 94788c4f..9646bbdd 100644 --- a/dw/workflow_sources.py +++ b/dw/workflow_sources.py @@ -163,3 +163,105 @@ def listing(sources): for name in source.names(): found.setdefault(name, source) return dict(sorted(found.items())) + + +def fallback_roots(primary=None): + """The read-only workflow roots a sub-workflow name is resolved against + after the directory the run is confined to. + + Pinned in DW_WORKFLOW_PATH by dw.serve, the same way the prompt and + asset libraries are, so the worker resolves a composed template exactly + as the API would. + """ + from .workspace import WORKFLOWS_SUBDIR, library_fallbacks + + return library_fallbacks(WORKFLOWS_SUBDIR, primary) + + +def _candidate_names(name): + """A name as written, and with .json appended when it has no extension - + the catalog reports names without it, and run_workflow's workflow_path + takes either (#90).""" + names = [name] + if not name.endswith(".json"): + names.append(f"{name}.json") + return names + + +class SubWorkflowNotFound(Exception): + """A sub-workflow step's path names nothing on the search path.""" + + def __init__(self, path, tried): + self.path = path + # In order, each candidate once - two roots can resolve one name to + # the same file, and saying so twice reads as two failures + self.tried = list(dict.fromkeys(tried)) + detail = "\n ".join(self.tried) + super().__init__( + f"Sub-workflow '{path}' could not be resolved. It is read as a " + "catalog name from list_workflows (with or without .json), or a " + "path relative to the workflow that names it. Looked in:" + f"\n {detail}" + ) + + +def resolve_sub_workflow(path, base_dir, confine_to): + """Where a sub-workflow step's `path` resolves to, and the root the + child is confined to, as (path, root). + + Order, first hit wins: + + 1. relative to the directory of the workflow that names it - which is + how a template reaches '../models/x.json', and stays first so an + existing composition keeps meaning what it did + 2. the same, with '.json' supplied + 3. the run's own workflow root (a workspace's workflows/), by catalog + name, with or without '.json' + 4. each read-only root on the search path, the same way - which is + what lets a stored template be composed rather than copied (#90) + + An absolute path is taken as written and confined to whichever root + holds it, so the sandbox still refuses one that belongs to no source. + + Raises SubWorkflowNotFound, naming every candidate it looked at. + """ + roots = [] + if confine_to: + roots.append(os.path.abspath(os.path.expanduser(str(confine_to)))) + for root in fallback_roots(roots[0] if roots else None): + if root not in roots: + roots.append(root) + + tried = [] + if os.path.isabs(path): + candidate = os.path.normpath(path) + tried.append(candidate) + for root in roots: + source = WorkflowSource(root, EXAMPLES_ORIGIN, False) + if source.contains(candidate) and os.path.isfile(candidate): + return candidate, root + # No root holds it - hand it back confined as it was, so the + # security layer writes the refusal it always did + return candidate, confine_to + + if base_dir: + for name in _candidate_names(path): + candidate = os.path.normpath(os.path.join(base_dir, name)) + tried.append(candidate) + if os.path.isfile(candidate): + return candidate, confine_to + + for root in roots: + source = WorkflowSource(root, EXAMPLES_ORIGIN, False) + # allow_create, because what is being asked is where the name would + # land rather than whether something is there - None means the name + # traverses out of the root, and the file check is the next line + candidate = resolve_in_source(source, path, allow_create=True) + if candidate is None: + tried.append(f"{os.path.join(root, path)} (outside the root)") + continue + tried.append(candidate) + if os.path.isfile(candidate): + return candidate, root + + raise SubWorkflowNotFound(path, tried) diff --git a/dw/workspace.py b/dw/workspace.py index 756ddd64..0b0f8c65 100644 --- a/dw/workspace.py +++ b/dw/workspace.py @@ -292,10 +292,15 @@ def set_workspace(workspace): # DW_PROMPT_DIR and DW_ASSET_DIR PROMPT_PATH_ENV_VAR = "DW_PROMPT_PATH" ASSET_PATH_ENV_VAR = "DW_ASSET_PATH" +# The same idea for workflows, which a sub-workflow step names: a stored +# template lives in an examples tree the workspace's own workflows/ cannot +# reach, so composing one used to mean copying it in (#90) +WORKFLOW_PATH_ENV_VAR = "DW_WORKFLOW_PATH" LIBRARY_PATH_ENV_VARS = { PROMPTS_SUBDIR: PROMPT_PATH_ENV_VAR, ASSETS_SUBDIR: ASSET_PATH_ENV_VAR, + WORKFLOWS_SUBDIR: WORKFLOW_PATH_ENV_VAR, } diff --git a/dw_mcp/assets.py b/dw_mcp/assets.py index f8581f82..5468f450 100644 --- a/dw_mcp/assets.py +++ b/dw_mcp/assets.py @@ -62,7 +62,9 @@ def delete_asset(client, name): return client.delete_json(api_path("api", "assets", name)) -def keep_output(client, name, asset_name=None, overwrite=False, shared=False): +def keep_output( + client, name, asset_name=None, overwrite=False, shared=False, workspace=None +): """Keep a generated file as an input asset, under a stable name. A run's files are named by the run that made them, which is the wrong @@ -86,6 +88,7 @@ def keep_output(client, name, asset_name=None, overwrite=False, shared=False): "overwrite": overwrite, "shared": shared, }, + workspace=workspace, ) diff --git a/dw_mcp/catalog.py b/dw_mcp/catalog.py index 27997f13..a3c8b76c 100644 --- a/dw_mcp/catalog.py +++ b/dw_mcp/catalog.py @@ -13,7 +13,16 @@ def list_workflows( shape, traits, cost, output kinds and variable names per workflow - what choosing one needs and nothing that reading one needs. Templates only unless `include_models` or `configures` asks for the model configs - of one template. `get_workflow` has the full definition.""" + of one template. `get_workflow` has the full definition. + + `cost_basis` in the answer says what a `cost` is: `curated` means a + maintainer measured it once, on the devices the entry names, and wrote + it into the workflow. Nothing derives one from this server's own job + history, so `cost: null` means nobody wrote a figure down - not that + the run is cheap, and not that this box has never run it. For a + template with no figure, `list_jobs` on earlier runs of it carries + `started_at`/`finished_at`, which is the measurement this server + actually holds.""" params = {"view": "compact"} if shape: params["shape"] = shape @@ -137,17 +146,17 @@ def list_jobs(client, limit=20, status=None, workspace=None): return answer -def list_gallery(client, limit=50, subfolder=None): +def list_gallery(client, limit=50, subfolder=None, workspace=None): """Generated media in the output directory, newest first. `subfolder` narrows to one in-run subfolder ('final', 'intermediate', '' for files at a run's root); None means every file.""" params = {"limit": limit} if subfolder is not None: params["subfolder"] = subfolder - return client.get_json("/api/gallery", params=params) + return client.get_json("/api/gallery", params=params, workspace=workspace) -def get_gallery_metadata(client, name, envelope=False): +def get_gallery_metadata(client, name, envelope=False, workspace=None): """Metadata embedded in a saved file: the full workflow that made it, plus the job that produced it when history remembers one, plus for audio and video what the file holds - duration, sample rate, channels, @@ -160,6 +169,7 @@ def get_gallery_metadata(client, name, envelope=False): body = client.get_json( api_path("api", "gallery", name, "metadata"), params={"envelope": "true"} if envelope else None, + workspace=workspace, ) media = body.get("media") if media and media.get("kind") in ("audio", "video"): diff --git a/dw_mcp/client.py b/dw_mcp/client.py index 1dda9f4f..7fe20c1c 100644 --- a/dw_mcp/client.py +++ b/dw_mcp/client.py @@ -123,39 +123,51 @@ def close(self): # ------------------------------------------------------------- requests - def get_json(self, path, params=None): - return self._json(self._request("GET", path, params=params), path) + def get_json(self, path, params=None, workspace=None): + return self._json( + self._request("GET", path, params=params, workspace=workspace), path + ) - def post_json(self, path, payload=None, params=None): + def post_json(self, path, payload=None, params=None, workspace=None): """`params` is for a route whose options are query parameters rather than a body - the export route, which takes `overwrite` beside the workspace selector `_scoped` adds.""" return self._json( - self._request("POST", path, json=payload or {}, params=params), path + self._request( + "POST", path, json=payload or {}, params=params, workspace=workspace + ), + path, ) - def put_json(self, path, payload): - return self._json(self._request("PUT", path, json=payload), path) + def put_json(self, path, payload, workspace=None): + return self._json( + self._request("PUT", path, json=payload, workspace=workspace), path + ) - def delete_json(self, path, params=None): - return self._json(self._request("DELETE", path, params=params), path) + def delete_json(self, path, params=None, workspace=None): + return self._json( + self._request("DELETE", path, params=params, workspace=workspace), path + ) - def post_bytes(self, path, data, params=None): + def post_bytes(self, path, data, params=None, workspace=None): """Send a file's bytes as the request body - the shape POST /api/uploads takes, so a single file needs no multipart parser at either end.""" return self._json( - self._request("POST", path, content=data, params=params), path + self._request( + "POST", path, content=data, params=params, workspace=workspace + ), + path, ) - def get_bytes(self, path): + def get_bytes(self, path, workspace=None): """Raw body plus content type - for the output media served from the /outputs static mount rather than an /api route.""" - response = self._request("GET", path) + response = self._request("GET", path, workspace=workspace) self._raise_for_status(response, path) return response.content, response.headers.get("content-type", "") - def get_bytes_if(self, path, accept_content_type): + def get_bytes_if(self, path, accept_content_type, workspace=None): """Like `get_bytes`, but the body is only downloaded when `accept_content_type(content_type)` is true. @@ -166,7 +178,7 @@ def get_bytes_if(self, path, accept_content_type): on acceptance. An error status is still raised either way, since the body has to be read to report it. """ - response = self._stream_request("GET", path) + response = self._stream_request("GET", path, workspace=workspace) try: content_type = response.headers.get("content-type", "") if response.status_code < 400 and not accept_content_type(content_type): @@ -177,7 +189,7 @@ def get_bytes_if(self, path, accept_content_type): finally: response.close() - def stream_to_file(self, path, destination): + def stream_to_file(self, path, destination, workspace=None): """Stream `path`'s body straight to `destination` on disk, in chunks, rather than buffering it whole - for a body too large to hold in memory (the videos `get_bytes` can't return). Returns @@ -190,7 +202,7 @@ def stream_to_file(self, path, destination): a torn partial file at `destination` - which would otherwise "exist" for a later `overwrite=False` caller and mask the failure. """ - response = self._stream_request("GET", path) + response = self._stream_request("GET", path, workspace=workspace) try: if response.status_code >= 400: self._call_httpx(response.read, path) @@ -222,28 +234,36 @@ def write_chunks(): # ------------------------------------------------------------ internals - def _request(self, method, path, **kwargs): + def _request(self, method, path, workspace=None, **kwargs): return self._call_httpx( - lambda: self._http.request(method, path, **self._scoped(kwargs)), path + lambda: self._http.request(method, path, **self._scoped(kwargs, workspace)), + path, ) - def _scoped(self, kwargs): - """Add the session's workspace to a request's query string. + def _scoped(self, kwargs, workspace=None): + """Add the workspace a request is for to its query string. One place rather than a parameter on every handler: routes that are not workspace-scoped (prompts, models, system) ignore an unknown query parameter, and the server treats a missing selector as its default - so the default workspace sends nothing and every request looks exactly as it did before workspaces existed. + + `workspace` is the per-call pin - "for this one call, without + switching the session" - and wins over the session's own. Naming the + default explicitly is the way to reach it from a session that is + somewhere else, so it sends no selector rather than the session's + (#99). """ - if self.workspace == DEFAULT_WORKSPACE: + name = workspace or self.workspace + if name == DEFAULT_WORKSPACE: return kwargs params = dict(kwargs.get("params") or {}) - params.setdefault("workspace", self.workspace) + params.setdefault("workspace", name) return {**kwargs, "params": params} - def _stream_request(self, method, path, **kwargs): - scoped = self._scoped(kwargs) + def _stream_request(self, method, path, workspace=None, **kwargs): + scoped = self._scoped(kwargs, workspace) return self._call_httpx( lambda: self._http.send( self._http.build_request(method, path, **scoped), stream=True diff --git a/dw_mcp/diagnose.py b/dw_mcp/diagnose.py index 480faded..2c4c7c48 100644 --- a/dw_mcp/diagnose.py +++ b/dw_mcp/diagnose.py @@ -174,11 +174,21 @@ def wait_for_job(client, job_id, timeout_seconds=20): `seconds_since_event` climbs is a stuck one. `denoise_step: null` under `generating` is neither: it is the lead-in - the pipeline runs before the loop - encoding the prompt and any - reference image or audio - which emits nothing and is well over a - minute on a large video model (~90 s on MiniMax H3). Silence there is - expected; `seconds_since_event` only says something once - `denoise_step` is a number, or in any other phase.""" + the pipeline runs before the loop - encoding the prompt and every + reference - which emits nothing and is well over a minute on a large + video model. Its length follows what it has to encode: ~90 s on + MiniMax H3 for a prompt with an image or audio reference, ~10 min once + a *video* reference is among them (measured 629 s for one 5 s 960x544 + clip on an RTX 3090). Silence there is expected, and `get_job_events` + says which block it is inside while it lasts - one `log` line per + top-level block of a modular pipeline. `seconds_since_event` only says + something once `denoise_step` is a number, or in any other phase. + + Even then it is coarse: where a transformer block cache is configured + the denoise steps are uneven - several cheap ones, then a full one - + so on H3 a 140 s gap between steps is a healthy run. Read liveness as + `denoise_step` having moved between polls minutes apart rather than as + silence under a fixed threshold.""" requested = max(0.0, float(timeout_seconds)) applied = min(requested, float(MAX_WAIT_SECONDS)) capped = applied < requested diff --git a/dw_mcp/media.py b/dw_mcp/media.py index 6f3067a1..da19981b 100644 --- a/dw_mcp/media.py +++ b/dw_mcp/media.py @@ -30,14 +30,16 @@ MAX_RETURNED_CHARACTERS = 20000 -def get_output_image(client, name, max_dimension=768): +def get_output_image(client, name, max_dimension=768, workspace=None): """One image from the output directory, downscaled, as base64 plus the sizes it went in and came out at.""" def is_image(content_type): return not content_type or content_type.startswith("image/") - body, content_type = client.get_bytes_if(api_path("outputs", name), is_image) + body, content_type = client.get_bytes_if( + api_path("outputs", name), is_image, workspace=workspace + ) if body is None: raise DwApiError( f"{name} is {content_type}, not an image - this tool returns " @@ -101,7 +103,9 @@ def _fit(image, limit): ) -def get_output_text(client, name, max_characters=MAX_RETURNED_CHARACTERS): +def get_output_text( + client, name, max_characters=MAX_RETURNED_CHARACTERS, workspace=None +): """One text output from the output directory - the form a prompt enhancement and any `text/plain` result arrive in.""" @@ -109,7 +113,9 @@ def is_text(content_type): kind = content_type.split(";")[0].strip().lower() return kind.startswith("text/") or kind == "application/json" - body, content_type = client.get_bytes_if(api_path("outputs", name), is_text) + body, content_type = client.get_bytes_if( + api_path("outputs", name), is_text, workspace=workspace + ) if body is None: raise DwApiError( f"{name} is {content_type or 'of no declared type'}, not text - " @@ -129,13 +135,13 @@ def is_text(content_type): } -def delete_output(client, name): +def delete_output(client, name, workspace=None): """Remove one file from the output directory. The gallery is the output directory read back, so this is where a delete belongs.""" - return client.delete_json(api_path("api", "gallery", name)) + return client.delete_json(api_path("api", "gallery", name), workspace=workspace) -def download_output(client, name, destination=None, overwrite=False): +def download_output(client, name, destination=None, overwrite=False, workspace=None): """Fetch one output file and save it to local disk, for an agent that wants the artifact itself rather than a description of it. @@ -175,7 +181,7 @@ def download_output(client, name, destination=None, overwrite=False): if parent: os.makedirs(parent, exist_ok=True) content_type, bytes_written = client.stream_to_file( - api_path("outputs", name), destination + api_path("outputs", name), destination, workspace=workspace ) except OSError as e: # A client-side path handed to a `dw.serve --mcp` endpoint lands diff --git a/dw_mcp/server.py b/dw_mcp/server.py index a1c861de..2e0e1d82 100644 --- a/dw_mcp/server.py +++ b/dw_mcp/server.py @@ -167,9 +167,14 @@ def list_workflows( further (comma-separated, all must match): has-audio, chained, image-conditioned, identity-referenced, needs-input-media, composes-workflows. Each entry carries a one-line `summary`, its - `shape` and `traits` (what it needs supplied), `cost` (measured - runs per device; null means unknown - call `get_memory` and say - so), output kinds and variable names. `lists`, present for a + `shape` and `traits` (what it needs supplied), `cost` (curated: + figures a maintainer measured once on the devices named and wrote + into the workflow, never derived from this server's job history - + so null means nobody wrote one down, not that the run is cheap; + the answer's `cost_basis` says as much. For a null one, earlier + runs of the same template in `list_jobs` carry + `started_at`/`finished_at`, which is the measurement this box + actually holds), output kinds and variable names. `lists`, present for a list-driven workflow, names per list variable the fields an entry takes, the steps run over it and the default's length; there `cost[].per_entry`, when present, is the measured cost of one @@ -268,6 +273,14 @@ def get_memory() -> dict: show what a run is holding or failing to release. A host field is absent, rather than null, on a platform that cannot measure it. + `host_pinned_reserved_mb` / `host_pinned_allocated_mb`, when + present, are torch's pinned-host cache - the staging buffers group + offloading moves weights through. They are part of + `host_memory_rss_mb` and invisible in every `gpu_*` figure, so a + worker that has released every model and still holds GB is usually + holding them (#98); they are returned when the worker switches to a + different workflow. + `live: true` means `info` was measured now and is the worker's own memory - only these readings are comparable with each other. `live: false` means it was not: `info: null` (with `stale: false`) @@ -318,7 +331,9 @@ def list_jobs( client, limit=limit, status=status, workspace=workspace ) - def list_gallery(limit: int = 50, subfolder: str | None = None) -> dict: + def list_gallery( + limit: int = 50, subfolder: str | None = None, workspace: str | None = None + ) -> dict: """List generated output files, newest first. A name is //, where may itself sit in a subfolder the step chose (`final/episode.mp4`) - the form @@ -331,10 +346,19 @@ def list_gallery(limit: int = 50, subfolder: str | None = None) -> dict: the latter, so `subfolder="final"` is "what did these runs deliver". Each entry also carries a ready-made `url` for viewing the file over HTTP, already scoped to the right workspace; use it as - given rather than composing one from the name.""" - return catalog.list_gallery(client, limit=limit, subfolder=subfolder) + given rather than composing one from the name. + + `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 is reachable from + here without leaving this one (#99).""" + return catalog.list_gallery( + client, limit=limit, subfolder=subfolder, workspace=workspace + ) - def get_gallery_metadata(name: str, envelope: bool = False) -> dict: + def get_gallery_metadata( + name: str, envelope: bool = False, workspace: str | None = None + ) -> dict: """Get the metadata embedded in a generated file: the exact workflow, arguments and seed that produced it. Use this to reproduce a result, or to see what a run that went wrong actually @@ -347,8 +371,15 @@ def get_gallery_metadata(name: str, envelope: bool = False) -> dict: track something is: whether a shot is still sounding at its last frame, how deep the hole at a seam goes, where a score goes quiet. Leave it off unless you are asking a question about a position in - the track - a long track is a long list.""" - return catalog.get_gallery_metadata(client, name, envelope=envelope) + the track - a long track is a long list. + + `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 is reachable from + here without leaving this one (#99).""" + return catalog.get_gallery_metadata( + client, name, envelope=envelope, workspace=workspace + ) def list_guides() -> dict: """List the documentation the engine serves: each guide's @@ -395,7 +426,7 @@ def get_guide(name: str, section: str | None = None) -> dict: # --------------------------------------------------------------- media def get_output_image( - name: str, max_dimension: int = 768 + name: str, max_dimension: int = 768, workspace: str | None = None ) -> list[ImageContent | TextContent]: """Look at a generated image, named as `list_gallery` or a job's manifest reports it. Use this to judge output quality - it is the @@ -405,8 +436,15 @@ def get_output_image( `get_gallery_metadata` or hand the user the file. The image is downscaled to `max_dimension` on its longest side; the second part of the result reports the size it went in and came out at, so a - downscale is never silent.""" - result = media.get_output_image(client, name, max_dimension=max_dimension) + downscale is never silent. + + `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 is reachable from + here without leaving this one (#99).""" + result = media.get_output_image( + client, name, max_dimension=max_dimension, workspace=workspace + ) image = ImageContent( type="image", data=result["data"], mime_type=result["mime_type"] ) @@ -421,21 +459,38 @@ def get_output_image( ) return [image, telemetry] - def get_output_text(name: str, max_characters: int = 20000) -> dict: + def get_output_text( + name: str, max_characters: int = 20000, workspace: str | None = None + ) -> dict: """Read a text output - a prompt enhancement, or any step whose result is text/plain or JSON. Truncated to `max_characters`, and - the reply says how long the file really was.""" - return media.get_output_text(client, name, max_characters=max_characters) + the reply says how long the file really was. + + `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 is reachable from + here without leaving this one (#99).""" + return media.get_output_text( + client, name, max_characters=max_characters, workspace=workspace + ) - def delete_output(name: str) -> dict: + def delete_output(name: str, workspace: str | None = None) -> dict: """Permanently remove one generated file from the output directory. Not recoverable: rerunning the job that made it is the only way back, and any "output:" reference pointing at it stops resolving. - Prefer `keep_output` first if it is worth keeping.""" - return media.delete_output(client, name) + Prefer `keep_output` first if it is worth keeping. + + `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 is reachable from + here without leaving this one (#99).""" + return media.delete_output(client, name, workspace=workspace) def download_output( - name: str, destination: str | None = None, overwrite: bool = False + name: str, + destination: str | None = None, + overwrite: bool = False, + workspace: str | None = None, ) -> dict: """Save one output file to disk on the machine running the MCP server - for the stdio `dw-mcp` that is @@ -455,9 +510,18 @@ def download_output( full path, a directory, or omitted to save into the current working directory under the output's own name; a '..' path segment in it is refused. An existing file at the resolved path is left - alone unless `overwrite=True`.""" + alone unless `overwrite=True`. + + `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 is reachable from + here without leaving this one (#99).""" return media.download_output( - client, name, destination=destination, overwrite=overwrite + client, + name, + destination=destination, + overwrite=overwrite, + workspace=workspace, ) tool(get_output_image, READ_ONLY) @@ -499,6 +563,7 @@ def keep_output( asset_name: str | None = None, overwrite: bool = False, shared: bool = False, + workspace: str | None = None, ) -> dict: """Keep a generated file as an input asset under a stable "asset:" name, so later workflows can rely on it - a run's own name moves @@ -508,9 +573,19 @@ def keep_output( own. The copy happens on the server, inside the workspace: nothing is downloaded or re-uploaded. Pass `shared=true` to keep it in the library every workspace shares instead - where something a later - piece in its own workspace has to reach belongs.""" + piece in its own workspace has to reach 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 is reachable from + here without leaving this one (#99).""" return assets.keep_output( - client, name, asset_name=asset_name, overwrite=overwrite, shared=shared + client, + name, + asset_name=asset_name, + overwrite=overwrite, + shared=shared, + workspace=workspace, ) def delete_asset(name: str) -> dict: @@ -595,7 +670,15 @@ def validate_workflow( A `result.subfolder` or `file_base_name` that cannot be written (a `..`, a backslash, a separator in `file_base_name`) is reported here - at its JSON path, after `for_each` expansion.""" + at its JSON path, after `for_each` expansion. + + A sub-workflow step is resolved too: a `workflow.path` that names + nothing this server can reach is an error at + `steps[N].workflow.path` (the message lists where it looked), the + workflow it names is validated in turn under that path, a + composition cycle is refused, and an argument passed down that the + composed workflow declares no variable for comes back as a + warning.""" return authoring.validate_workflow( client, workflow=workflow, @@ -784,9 +867,18 @@ def wait_for_job(job_id: str, timeout_seconds: int = 20) -> dict: `denoise_step`/`denoise_total_steps`, null until the denoise loop starts - which is how a slow run and a stuck one tell apart between two otherwise identical polls. A null `denoise_step` under - `generating` is the pipeline's lead-in (encoding the prompt and any - reference image or audio, ~90 s on MiniMax H3), which emits - nothing: wait it out rather than reading the silence as a hang.""" + `generating` is the pipeline's lead-in - encoding the prompt and + every reference - which emits nothing, and its length depends on + what it has to encode: on MiniMax H3 ~90 s for a prompt with an + image or audio reference, but ~10 min once a *video* reference is + among them (measured 629 s for one 5 s 960x544 clip on an RTX + 3090). Wait it out rather than reading the silence as a hang. + Gaps between denoise steps are uneven too where a transformer + block cache is configured - most steps cheap, every few steps a + full one - so on H3 `seconds_since_event` of ~140 s with a number + in `denoise_step` is still healthy. The signal is whether + `denoise_step` has moved since a poll minutes ago, not silence + past a fixed threshold.""" return diagnose.wait_for_job(client, job_id, timeout_seconds=timeout_seconds) # The cap is a number a caller paces against, so the description states diff --git a/plugins/dw/skills/ltx-2.5/SKILL.md b/plugins/dw/skills/ltx-2.5/SKILL.md index 9fb4f78b..5c896642 100644 --- a/plugins/dw/skills/ltx-2.5/SKILL.md +++ b/plugins/dw/skills/ltx-2.5/SKILL.md @@ -125,21 +125,19 @@ AESTHETIC QUALITY (in addition to the above, without breaking the objective capt 1. `validate_workflow` first - free, and it catches arguments the pipeline rejects. -2. Quote the listing's `cost`. Only `templates/ltx2/text-to-video` and - `templates/ltx2/two-stage` declare one; for the other six say so and give - the shape of the spend instead - a 121-frame clip at 960x544 is under a - minute warm on a 24 GB card, the two-stage flow about eight, and extend and - chain multiply by their passes. Either way get the go-ahead before - `run_workflow` with `acknowledged_cost=true`. +2. Quote the listing's `cost` - the run's whole wall clock, model loading + included. Only `templates/ltx2/text-to-video` and `templates/ltx2/two-stage` + declare one; for the other six say so and give the shape of the spend + instead - a 121-frame clip at 960x544 is under two minutes cold on a 24 GB + card, of which a minute is loading, the two-stage flow about eight, and + extend and chain multiply by their passes. Either way get the go-ahead + before `run_workflow` with `acknowledged_cost=true`. 3. `wait_for_job`, then `get_job` for the manifest. The write-out runs after - the last step ends, reports nothing while it does, and is not free: roughly - 4 s per frame at 1536x896 - about 8 minutes for 121 frames, about half an - hour for 481. Scale by frame count from that rate rather than reading - "minutes" as a constant, and expect it to fall with the frame size. The job - is not stuck. -4. Every step that writes pays that, so only the ones worth writing should: - `"result": {"save": false}` on the rest, as `two-stage` does for `base` and - `upscale`. On a long chain it is the largest saving there is, and missing + the last step ends and names each file as it starts it - seconds for a + 121-frame clip since 2026-09-14, minutes before that (#97). +4. Writing is still not free on a long chain, so only the steps worth writing + should: `"result": {"save": false}` on the rest, as `two-stage` does for + `base` and `upscale`. It saves disk as much as time now, and missing it is silent. What does write carries a `subfolder` in the manifest: the step the user will be shown is `final` and every other saving step `intermediate`, the way `generative-upscale` keeps `upscaled` in `final` and its low-resolution diff --git a/plugins/dw/skills/minimax-h3/SKILL.md b/plugins/dw/skills/minimax-h3/SKILL.md index d5d1672e..1a7449b9 100644 --- a/plugins/dw/skills/minimax-h3/SKILL.md +++ b/plugins/dw/skills/minimax-h3/SKILL.md @@ -23,8 +23,8 @@ not from here. ## Which shape is the request - **One clip, up to 14.4 seconds, from text**: `templates/minimax/video-with-audio`. - From a one-line idea: `templates/minimax/enhance-prompt` writes the prompt - with the built-in Context-IR enhancer first. + From a one-line idea: `templates/minimax/enhance-prompt` writes it with the + built-in Context-IR enhancer first. - **Pinned to a picture**: first frame `templates/minimax/image-to-video`; first and last `templates/minimax/first-and-last-frame`; last only `templates/minimax/last-frame-only`; a one-line idea plus a picture @@ -32,14 +32,15 @@ not from here. - **A subject that must look the same**: `templates/minimax/reference-to-video` (an image fixes appearance, an audio clip fixes voice); `templates/minimax/composable-references` adds a video reference for framing - and camera; `templates/minimax/generated-subject-reference` draws the subject + and camera, at about 3.4x the cost; + `templates/minimax/generated-subject-reference` draws the subject with Z-Image first and references it in the same workflow; `templates/minimax/voice-timbre-reference` fixes a voice from a Bark-spoken line. - **Several boards in one generation, one unbroken score**: `templates/minimax/storyboard` - H3 cuts between the boards inside a single generation, which no concat of separate clips can match for continuous audio. - It is one beat with fixed cut points, not a building block: four of them - concatenated give twelve equal-length shots and a cast redrawn four times. + It is one beat with fixed cut points, not a building block: four + concatenated give twelve equal shots and a cast redrawn four times. Past one beat with a recurring cast, use the cuts pattern below. - **Longer than 14.4 seconds**: decide first whether the seam is a cut or a continuation. Chain when the same action or line of speech has to cross the @@ -78,8 +79,8 @@ not from here. drifts. Each entry's `num_frames` is its own, so pace the cut. The reference carries delivery as well as timbre: a flat read gives a flat performance. Bark's presets are conversational; for a narrator with - gravitas, `upload_asset` a recorded read in that register and reference - the same file in every shot. + gravitas, `upload_asset` a read in that register and reference it in + every shot. - **Music alone**: `templates/minimax/music` (Music3); the `minimax-music3` skill. If none fits, compose from `list_tasks` before authoring a new workflow, and @@ -88,9 +89,9 @@ read the `workflows` guide's authoring section first. ## Hard rules - `num_frames` is `17n + 5`, from 124 to 345, at a fixed 24 fps: 5.17 to 14.4 - seconds in one clip. Most templates default to 124 for fast iteration (storyboard uses 192); `num_frames=345` - is the full length and fits the same 24 GB configuration. The 5-second floor - is diffusers'; the model card says 4. + seconds in one clip. Most default to 124 for fast iteration (storyboard 192); + `num_frames=345` is the full length and fits the same 24 GB configuration. + The 5-second floor is diffusers'; the model card says 4. - Canvas: a 768-pixel short edge, at most 768x1344 pixels, dimensions in multiples of 32, aspect from 1:4 to 4:1. Output audio is 32 kHz stereo. - The text- and frame-conditioned templates render at 960x544 with the 544p @@ -100,8 +101,7 @@ read the `workflows` guide's authoring section first. `generated-subject-reference`, `chain-matched-to-audio`, `chain-video-continuity`) carry no LoRA and run 20 steps, because the turbo LoRA is distilled against the base transformer and they load the reference - one; such a run is about twice the time of a turbo one at the same length. - `storyboard`, `dialogue-short`, `music-video` and + one. `storyboard`, `dialogue-short`, `music-video` and `chain-matched-and-aligned` pass references *and* keep the turbo LoRA at nine steps; say nine for those, not 20. - Nothing carries between generations except what is passed as a reference: @@ -112,14 +112,14 @@ read the `workflows` guide's authoring section first. - H3 is guidance-distilled: no `guidance_scale`, no negative prompt. Say what is there, never what is not. - When deriving a variant, keep `release_pipeline` on the step the template - puts it on: it frees the Z-Image boards before H3 loads. A run killed by - SIGKILL near the end, in a worker warm from a previous job, that succeeds - on a retry in a fresh worker is host memory, not the prompt. + puts it on: it frees the Z-Image boards before H3 loads. A run SIGKILLed near + the end in a warm worker that succeeds on a retry in a fresh one is host + memory, not the prompt. - Ref2VA limits: at most 9 images, 3 videos, 3 audio clips, 12 files; audio can never be the only reference. References are labelled in the order passed. - Music3 reads `audio_duration` as a ceiling, not a target: ask for more than - the song needs and trim with `templates/audio-trim-fade`. The `minimax-music3` - skill has the rest of that family. + the song needs and trim with `templates/audio-trim-fade`; the + `minimax-music3` skill has the rest. - Write the prompt for the length being generated: shot timestamps should span the duration, or a five-second script conditions a five-second story whatever the frame count. @@ -130,8 +130,8 @@ H3 wants Context-IR, MiniMax's own format. Do not invent it and do not paraphrase it from examples: 1. If the `h3-prompt-writing` skill is installed (MiniMax ships it in - https://github.com/MiniMax-AI/MiniMax-H3 under `skills/`), use it. If it - is not, tell the user once that + https://github.com/MiniMax-AI/MiniMax-H3 under `skills/`), use it. If not, + say once that `npx skills add MiniMax-AI/MiniMax-H3 --skill h3-prompt-writing` installs it - only that skill; the repo's other eight are style packs - and go on without it. @@ -157,14 +157,16 @@ inherits the portrait's composition. 1. `validate_workflow` first - free, and it catches arguments the pipeline does not accept. 2. Quote the listing's `cost` (warm minutes on the card it was measured on; - a first load is longer). When the listing declares none, say so and give the - shape of the spend instead: a 124-frame turbo clip is a few minutes on a - 24 GB card, the full 345 frames about three times that, a - reference-conditioned clip about twice a turbo one, and a chain multiplies - by its segment count. Get the user's go-ahead before `run_workflow` with - `acknowledged_cost=true`. + a first load is longer). When it declares none, say so and give the shape + instead: a 124-frame turbo clip is a few minutes on a 24 GB card, 345 + frames three times that, an image reference twice a turbo clip, a video + reference beside it 3.4x again, and a chain multiplies by its segments. + Get the user's go-ahead before `run_workflow` with `acknowledged_cost=true`. 3. `wait_for_job`, then `get_job` for the manifest. A cancelled H3 job runs - on to its next step boundary, minutes on this model. Each entry carries + on to its next step boundary, minutes on this model. Silence is not a hang: + `denoise_step` is null through the reference encode (~90 s, ~10 min with a + video reference), and the block cache makes later steps uneven - two-minute + gaps are healthy. Each entry carries `subfolder`: `final` is the deliverable (`episode`, `music_video`, `voyage`), `intermediate` the scratch; keep that split in anything you compose. @@ -183,13 +185,12 @@ inherits the portrait's composition. a voice-over without affect (the reference's delivery came through), every shot the same length, a look word repeated on every board (shallow depth of field) softening every shot. -5. After an inline run worth keeping, `get_job_workflow` and `save_workflow` it, - so the next run is by name rather than by pasting JSON; `export_job` bundles - the run — workflow, manifest, job row and media — for git. The bundle is on - the server: fetch its zip URL and unpack it into `exports/` under the - session's working directory, never a temp directory, and do not make a - folder named after the job id first, since the archive already unpacks - into one. +5. After an inline run worth keeping, `get_job_workflow` and `save_workflow` + it, so the next run is by name rather than pasted JSON; `export_job` bundles + the run — workflow, manifest, job row and media — for git. It is on the + server: fetch its zip URL and unpack it into `exports/` under the session's + working directory, never a temp directory; the archive already unpacks into + a job-id folder, so do not make one first. ## Sources diff --git a/tests/test_catalog_structure.py b/tests/test_catalog_structure.py index 7785ea73..c11a107d 100644 --- a/tests/test_catalog_structure.py +++ b/tests/test_catalog_structure.py @@ -498,6 +498,7 @@ def test_every_readme_link_resolves(path): COSTED = { "workflows/templates/minimax/music-video.json": 35, "workflows/templates/minimax/dialogue-short.json": 42, + "workflows/templates/minimax/composable-references.json": 27.4, "workflows/templates/assemble-and-score.json": 0.2, "workflows/templates/dissolve-between-shots.json": 0.2, } @@ -505,8 +506,11 @@ def test_every_readme_link_resolves(path): @pytest.mark.parametrize("path,minutes", sorted(COSTED.items())) def test_the_cut_templates_quote_a_measured_cost(path, minutes): - """Measured on an RTX 3090, 2026-09-10; without a figure an agent - cannot quote a price before spending 40 minutes of GPU.""" + """Measured on an RTX 3090 (the cut templates 2026-09-10, + composable-references 2026-09-13); without a figure an agent cannot + quote a price before spending 40 minutes of GPU. A video reference is + the expensive one - the same 124-frame shot is 7.8 min with an image + reference alone and 27.4 with a video reference beside it.""" definition = json.load(open(os.path.join(REPO_ROOT, path), encoding="utf-8")) entry = definition["cost"][0] assert entry["name"] == "RTX 3090" diff --git a/tests/test_concat_videos.py b/tests/test_concat_videos.py index 115e95e6..a4567213 100644 --- a/tests/test_concat_videos.py +++ b/tests/test_concat_videos.py @@ -398,3 +398,68 @@ def test_shots_already_at_one_level_draw_no_warning(self, caplog): concat_videos([audio_video(4, 0.5), audio_video(4, 0.45)]) assert "level jump" not in caplog.text + + +class TestWarningsReachTheCaller: + """A warning that only reaches the server's log does not exist from + outside it - see issue #82, where match_levels verified but the spread + warning was invisible over MCP.""" + + def test_the_level_spread_warning_is_emitted_as_an_event(self): + from dw.events import RunContext, activate_context, deactivate_context + + events = [] + token = activate_context(RunContext(on_event=events.append)) + try: + concat_videos([audio_video(4, 0.5), audio_video(4, 0.05)]) + finally: + deactivate_context(token) + + warnings = [e for e in events if e["event"] == "warning"] + assert len(warnings) == 1 + assert warnings[0]["kind"] == "level_spread" + assert warnings[0]["command"] == "concat_videos" + assert warnings[0]["spread_db"] == pytest.approx(20.0, abs=0.2) + assert "match_levels" in warnings[0]["message"] + + def test_matched_shots_emit_nothing(self): + from dw.events import RunContext, activate_context, deactivate_context + + events = [] + token = activate_context(RunContext(on_event=events.append)) + try: + concat_videos( + [audio_video(4, 0.5), audio_video(4, 0.05)], match_levels="rms" + ) + finally: + deactivate_context(token) + + assert [e for e in events if e["event"] == "warning"] == [] + + +class TestFrameRateTravelsWithTheJoin: + """result.fps defaults to 8, so a 24 fps cut that says nothing there + used to be written three times slow against audio of the right length - + see issue #84.""" + + def test_the_tasks_fps_is_carried_to_the_result(self): + result = concat_videos([frames(4), frames(4)], fps=24) + + assert result.fps == 24 + + def test_an_input_videos_rate_is_carried_when_the_task_is_told_nothing(self): + first = AudioVideo(frames(4), None, None, fps=30) + + result = concat_videos([first, frames(4)]) + + assert result.fps == 30 + + def test_the_tasks_own_fps_wins_over_its_inputs(self): + first = AudioVideo(frames(4), None, None, fps=30) + + result = concat_videos([first, frames(4)], fps=24) + + assert result.fps == 24 + + def test_nothing_is_carried_when_nothing_knows(self): + assert concat_videos([frames(4), frames(4)]).fps is None diff --git a/tests/test_dissolve_videos.py b/tests/test_dissolve_videos.py index d7935797..c5949007 100644 --- a/tests/test_dissolve_videos.py +++ b/tests/test_dissolve_videos.py @@ -127,3 +127,15 @@ def test_off_by_default_but_a_wide_spread_warns(self, caplog): assert abs(result.audio[0][-1]) == pytest.approx(0.05) assert "level jump" in caplog.text + + +class TestFrameRateTravelsWithTheDissolve: + """As with concat_videos - the rate the step was told is the rate the + file is written at, rather than result.fps's default of 8 (#84).""" + + def test_the_tasks_fps_is_carried_to_the_result(self): + result = dissolve_videos( + [frames(8, 0), frames(8, 255)], dissolve_frames=2, fps=24 + ) + + assert result.fps == 24 diff --git a/tests/test_host_memory.py b/tests/test_host_memory.py index 2c1433da..9b69fd26 100644 --- a/tests/test_host_memory.py +++ b/tests/test_host_memory.py @@ -95,3 +95,115 @@ def test_the_repl_prints_host_memory_either_way(capsys, gpu_available): out = capsys.readouterr().out assert "Worker RSS: 1234.5 MB" in out assert "32000.0 MB of 64000.0 MB" in out + + +def test_the_peak_is_never_below_the_resident_figure(monkeypatch): + """peak - rss is what the pair exists to answer; a small negative there + reads as 'these fields are not comparable' - see issue #83.""" + monkeypatch.setattr(host_memory, "_peak_rss_mb", lambda: 764.1484375) + monkeypatch.setattr( + host_memory, + "_psutil_stats", + lambda: {"rss_mb": 764.79296875, "total_mb": 64000.0, "available_mb": 32000.0}, + ) + + stats = host_memory.host_memory_stats() + + assert stats["peak_rss_mb"] == stats["rss_mb"] == 764.79296875 + + +def test_a_genuine_peak_is_left_alone(monkeypatch): + monkeypatch.setattr(host_memory, "_peak_rss_mb", lambda: 33044.98) + monkeypatch.setattr( + host_memory, + "_psutil_stats", + lambda: {"rss_mb": 2561.69, "total_mb": 64000.0, "available_mb": 32000.0}, + ) + + assert host_memory.host_memory_stats()["peak_rss_mb"] == 33044.98 + + +class TestReleasingHostCaches: + """What a full cleanup hands back to the OS, and what it reports (#98). + + A worker that had released every model still sat on 14.5 GB of anonymous + memory on the box this was measured on, which is the whole margin a + template needing 96% of host RAM has. + """ + + def test_it_empties_the_pinned_cache_and_trims_the_heap(self, monkeypatch): + called = [] + + class FakeC: + @staticmethod + def _host_emptyCache(): + called.append("pinned") + + monkeypatch.setitem( + __import__("sys").modules, "torch", type("torch", (), {"_C": FakeC}) + ) + monkeypatch.setattr( + host_memory, "trim_host_memory", lambda: called.append("trim") + ) + readings = iter([20000.0, 14000.0]) + monkeypatch.setattr( + host_memory, "host_memory_stats", lambda: {"rss_mb": next(readings)} + ) + + released = host_memory.release_host_caches() + + assert called == ["pinned", "trim"] + assert released == 6000.0 + + def test_a_reading_it_cannot_take_is_not_a_number_it_invents(self, monkeypatch): + monkeypatch.setattr(host_memory, "trim_host_memory", lambda: 0.0) + monkeypatch.setattr(host_memory, "host_memory_stats", lambda: {"rss_mb": None}) + + assert host_memory.release_host_caches() == 0.0 + + def test_a_torch_without_the_hook_is_not_an_error(self, monkeypatch): + monkeypatch.setitem( + __import__("sys").modules, "torch", type("torch", (), {"_C": object}) + ) + monkeypatch.setattr(host_memory, "trim_host_memory", lambda: 0.0) + monkeypatch.setattr(host_memory, "host_memory_stats", lambda: {"rss_mb": 100.0}) + + assert host_memory.release_host_caches() == 0.0 + + def test_the_pinned_figures_are_reported_in_mb(self, monkeypatch): + class FakeCuda: + @staticmethod + def host_memory_stats(): + return { + "allocated_bytes.all.current": 512 * 1024 * 1024, + "reserved_bytes.all.current": 2048 * 1024 * 1024, + } + + monkeypatch.setitem( + __import__("sys").modules, "torch", type("torch", (), {"cuda": FakeCuda}) + ) + + assert host_memory.pinned_host_memory_fields() == { + "host_pinned_allocated_mb": 512.0, + "host_pinned_reserved_mb": 2048.0, + } + + def test_no_pinned_allocator_reports_nothing_rather_than_zero(self, monkeypatch): + """A key that is absent says 'not measurable here'; a zero would say + 'measured, and there is none' - the same rule the host fields follow.""" + + class FakeCuda: + @staticmethod + def host_memory_stats(): + raise RuntimeError("no CUDA") + + monkeypatch.setitem( + __import__("sys").modules, "torch", type("torch", (), {"cuda": FakeCuda}) + ) + + assert host_memory.pinned_host_memory_fields() == {} + + def test_trim_is_a_no_op_off_linux(self, monkeypatch): + monkeypatch.setattr(__import__("sys"), "platform", "darwin") + + assert host_memory.trim_host_memory() == 0.0 diff --git a/tests/test_job_progress.py b/tests/test_job_progress.py index b3f8b664..aa93aafd 100644 --- a/tests/test_job_progress.py +++ b/tests/test_job_progress.py @@ -170,3 +170,46 @@ def test_a_queued_event_is_stamped_against_creation_until_the_job_starts(): job.add_event({"event": "job_status", "status": "queued"}) assert job.events_after(-1)[0]["at"] >= 0 + + +class TestRuntimeWarnings: + """A warning a step raises about what it is writing has to reach the + caller, not just the server's log - see issue #82, where `match_levels` + verified over MCP but the spread warning it replaces did not exist out + there at all.""" + + def test_a_warning_event_lands_on_the_jobs_warnings(self): + job = running_job( + {"event": "step_start", "step": "join", "index": 0, "total_steps": 1}, + {"event": "warning", "message": "the tracks span 9.9 dB", "kind": "x"}, + ) + + assert job.warnings == ["join: the tracks span 9.9 dB"] + assert slim_job(job.detail())["warnings"] == ["join: the tracks span 9.9 dB"] + + def test_it_keeps_its_place_in_the_event_stream_too(self): + job = running_job({"event": "warning", "message": "a spread"}) + + assert [e["event"] for e in job.events_after(-1)] == ["warning"] + + def test_a_warning_before_any_step_is_carried_unnamed(self): + job = running_job({"event": "warning", "message": "a spread"}) + + assert job.warnings == ["a spread"] + + def test_the_same_warning_twice_is_carried_once(self): + job = running_job( + {"event": "step_start", "step": "join", "index": 0, "total_steps": 1}, + {"event": "warning", "message": "a spread"}, + {"event": "warning", "message": "a spread"}, + ) + + assert job.warnings == ["join: a spread"] + + def test_the_argument_warnings_a_job_was_queued_with_survive(self): + job = Job({"workflow_name": "shot", "warnings": ["unknown argument 'fsp'"]}) + job.add_event({"event": "warning", "message": "a spread"}) + + assert job.warnings == ["unknown argument 'fsp'", "a spread"] + # the spec is what a rerun is built from - appending must not edit it + assert job.spec["warnings"] == ["unknown argument 'fsp'"] diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 81366b37..3abb72c3 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -623,6 +623,22 @@ async def test_wait_for_job_names_the_cap_it_applies(): assert "timeout_capped" in description +@pytest.mark.asyncio +async def test_wait_for_job_scales_the_lead_in_to_the_references(): + """#95: the description is the documented way to tell a slow run from a + hung one, and a consumer following its ~90 s figure would have been + entitled to cancel a healthy video-reference run at the 3 minute mark - + that lead-in measured 629 s. It has to name the video reference and its + own order of magnitude, and say that the denoise steps are uneven once + they start, or the silence between them reads as a stall too.""" + tools = await tools_of(server_over(ok({}))) + + description = tools["wait_for_job"].description + assert "video" in description + assert "90 s" in description and "10 min" in description + assert "140 s" in description + + @pytest.mark.asyncio async def test_export_job_warns_the_copy_costs_disk_and_names_total_bytes(): tools = await tools_of(server_over(ok({}))) @@ -747,6 +763,7 @@ def refusing(request): "download_output": (media, "download_output"), "get_gallery_metadata": (catalog, "get_gallery_metadata"), "get_workflow": (catalog, "get_workflow"), + "delete_output": (media, "delete_output"), } diff --git a/tests/test_mcp_workspaces.py b/tests/test_mcp_workspaces.py index 3e4630ed..b08ea210 100644 --- a/tests/test_mcp_workspaces.py +++ b/tests/test_mcp_workspaces.py @@ -215,3 +215,77 @@ def handler(request): assert result["directories"]["prompts"] == "/home/user/prompts" assert result["device"] == "cuda" assert result["version"] == "0.1.0" + + +class TestPerCallPin: + """`workspace=` on an output-side tool: for this one call, without + switching the session (#99). A job pinned there with + `run_workflow(workspace=...)` is otherwise unreachable - its files + resolve against the session's workspace and answer "does not exist".""" + + def pinned(self, call): + client, seen = recording() + call(client) + return seen[-1].url + + def test_every_output_side_tool_carries_the_pin(self): + from dw_mcp import assets, catalog, media + + calls = ( + lambda client: catalog.list_gallery(client, workspace="qa"), + lambda client: catalog.get_gallery_metadata( + client, "run/out.mp4", workspace="qa" + ), + lambda client: media.delete_output(client, "run/out.mp4", workspace="qa"), + lambda client: assets.keep_output( + client, "run/out.mp4", asset_name="shot.mp4", workspace="qa" + ), + ) + for call in calls: + assert self.pinned(call).params.get("workspace") == "qa" + + def test_the_pin_reaches_the_outputs_route_too(self): + """/outputs is a route, not an /api one, and streams rather than + returning JSON - the two places a query parameter is easiest to + drop.""" + from dw_mcp import media + + client, seen = recording() + + def handler(request): + seen.append(request) + return httpx.Response( + 200, content=b"hello", headers={"content-type": "text/plain"} + ) + + client = DwClient(transport=httpx.MockTransport(handler)) + media.get_output_text(client, "run/out.txt", workspace="qa") + assert seen[-1].url.params.get("workspace") == "qa" + + def test_the_pin_does_not_switch_the_session(self): + from dw_mcp import catalog + + client, seen = recording() + catalog.list_gallery(client, workspace="qa") + assert client.workspace == DEFAULT_WORKSPACE + catalog.list_gallery(client) + assert seen[-1].url.params.get("workspace") is None + + def test_the_pin_wins_over_the_session_s_own(self): + from dw_mcp import catalog + + client, seen = recording() + client.workspace = "shots" + catalog.list_gallery(client, workspace="qa") + assert seen[-1].url.params.get("workspace") == "qa" + + def test_naming_the_default_reaches_it_from_a_named_session(self): + """The one spelling that cannot be a missing selector: a session in + 'shots' asking for the default sends none, which is what the server + reads as its default.""" + from dw_mcp import catalog + + client, seen = recording() + client.workspace = "shots" + catalog.list_gallery(client, workspace=DEFAULT_WORKSPACE) + assert seen[-1].url.params.get("workspace") is None diff --git a/tests/test_modular_progress.py b/tests/test_modular_progress.py index 700ba18a..f60dd949 100644 --- a/tests/test_modular_progress.py +++ b/tests/test_modular_progress.py @@ -14,6 +14,8 @@ from PIL import Image from tqdm.auto import tqdm +from diffusers.modular_pipelines.modular_pipeline import SequentialPipelineBlocks + from dw.events import RunContext, WorkflowCancelled from .test_phase_events import _pipeline_workflow, _run @@ -171,3 +173,153 @@ def test_the_pipeline_is_handed_back_unpatched(): assert "progress_bar" not in vars(fake.denoise) assert isinstance(fake.denoise.progress_bar(total=1), tqdm) + + +class FakeSequentialBlocks(SequentialPipelineBlocks): + """The dispatch a real `SequentialPipelineBlocks` performs: it walks its + named sub-blocks in order, calling each with the pipeline and the state. + `__call__` lives on the class, which is why the patch cannot be + per-instance the way the progress-bar one is - and the real base class + is what it subclasses, because only a sequence may be narrated by + walking its sub-blocks.""" + + def __init__(self, sub_blocks): + self.sub_blocks = sub_blocks + + def __call__(self, pipeline, state): + for block in self.sub_blocks.values(): + pipeline, state = block(pipeline, state) + return pipeline, state + + +class FakeNamedBlock: + def __init__(self, on_call=None): + self.on_call = on_call + + def __call__(self, pipeline, state): + if self.on_call is not None: + self.on_call() + return pipeline, state + + +class FakeBlockedPipeline: + """A modular pipeline that runs its blocks the way the real one does: + text encode, then reference encode, then the denoise loop.""" + + def __init__(self, steps=3, on_encode=None): + self.denoise = FakeDenoiseBlock(steps) + self._blocks = FakeSequentialBlocks( + { + "text_encoder": FakeNamedBlock(), + "vae_encoder": FakeNamedBlock(on_encode), + "denoise": FakeNamedBlock(self.denoise.run), + } + ) + + @property + def blocks(self): + return copy.deepcopy(self._blocks) + + def __call__(self, prompt=None, num_inference_steps=None, generator=None): + self._blocks(self, None) + return FakeOutput() + + +def _logs(events): + return [event["message"] for event in events if event["event"] == "log"] + + +def test_each_block_of_the_lead_in_says_it_started(): + """#95: the encode that runs before the denoise loop emitted nothing, so + a video reference - which takes minutes of it - looked exactly like a + hang. The blocks have names, and naming each one as it starts is the + difference between silence and 'it is encoding the reference'.""" + events = _events(FakeBlockedPipeline()) + + assert _logs(events) == [ + "acme/model: text_encoder", + "acme/model: vae_encoder", + "acme/model: denoise", + ] + + +def test_a_block_is_named_before_it_runs_rather_than_after(): + """After is no use: the whole point is the event that lands while the + long block is still going, which is where the ten minutes go.""" + timeline = [] + pipeline = FakeBlockedPipeline(on_encode=lambda: timeline.append("encoding")) + + def record(context, event): + if event["event"] == "log": + timeline.append(event["message"]) + + _events(pipeline, record) + + assert timeline == [ + "acme/model: text_encoder", + "acme/model: vae_encoder", + "encoding", + "acme/model: denoise", + ] + + +def test_the_block_dispatch_is_handed_back_unpatched(): + """The patch is on the class, so leaving it in place would outlive the + run and report blocks into whatever context came next.""" + pipeline = FakeBlockedPipeline() + original = FakeSequentialBlocks.__call__ + + _events(pipeline) + + assert FakeSequentialBlocks.__call__ is original + + +def test_a_pipeline_with_no_blocks_still_runs(): + """Not every pipeline without a step callback is modular.""" + assert _steps(_events(FakeModularPipeline())) == [(1, 3), (2, 3), (3, 3)] + + +class FakeConditionalBlocks: + """What `AutoPipelineBlocks` does: it *picks* one sub-block on its + inputs rather than running them all. Not a `SequentialPipelineBlocks`, + and that is the whole point.""" + + def __init__(self, sub_blocks, chosen): + self.sub_blocks = sub_blocks + self.chosen = chosen + + def __call__(self, pipeline, state): + return self.sub_blocks[self.chosen](pipeline, state) + + +class FakeConditionalPipeline: + def __init__(self): + self.ran = [] + self.denoise = FakeDenoiseBlock(3) + self._blocks = FakeConditionalBlocks( + { + "image_branch": FakeNamedBlock(lambda: self.ran.append("image")), + "video_branch": FakeNamedBlock(lambda: self.ran.append("video")), + }, + chosen="video_branch", + ) + + @property + def blocks(self): + return copy.deepcopy(self._blocks) + + def __call__(self, prompt=None, num_inference_steps=None, generator=None): + self._blocks(self, None) + return FakeOutput() + + +def test_a_container_that_chooses_one_block_is_left_alone(): + """Narrating by walking `sub_blocks` is only correct for a sequence. A + conditional container runs one branch, so the same walk would run every + branch of it - a wrong answer bought with a progress message.""" + pipeline = FakeConditionalPipeline() + + events = _events(pipeline) + + assert pipeline.ran == ["video"] + assert _logs(events) == [] diff --git a/tests/test_result.py b/tests/test_result.py index e02ca748..227a79e5 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -1093,3 +1093,181 @@ def test_a_non_mp4_content_type_raises(self, tmp_path): if __name__ == "__main__": pytest.main([__file__, "-v"]) + + +class TestVideoFrameRate: + """`result.fps` and a task's own `fps` are separate knobs, and the one an + author sets is the task's - so the rate the frames carry decides when the + result declares none. See issue #84.""" + + def save(self, result_definition, artifact): + result = Result(result_definition) + result.add_result(artifact) + with ( + patch("dw.result.encode_video") as encode, + patch("dw.result.export_to_video") as export, + patch("dw.result.is_av_available", return_value=True), + ): + with tempfile.TemporaryDirectory() as temp_dir: + result.save(temp_dir, "test") + return encode, export + + def test_the_carried_rate_is_used_when_the_result_declares_none(self): + artifact = AudioVideo("frames", torch.zeros((2, 100)), 48000, fps=24) + + encode, _ = self.save({"content_type": "video/mp4"}, artifact) + + assert encode.call_args.kwargs["fps"] == 24 + + def test_a_declared_rate_still_wins(self): + artifact = AudioVideo("frames", torch.zeros((2, 100)), 48000, fps=24) + + encode, _ = self.save({"content_type": "video/mp4", "fps": 12}, artifact) + + assert encode.call_args.kwargs["fps"] == 12 + + def test_the_old_default_holds_when_nothing_knows_the_rate(self): + artifact = AudioVideo("frames", torch.zeros((2, 100)), 48000) + + encode, _ = self.save({"content_type": "video/mp4"}, artifact) + + assert encode.call_args.kwargs["fps"] == 8 + + def test_a_plain_frame_list_still_defaults_to_eight(self): + result = Result({"content_type": "video/mp4"}) + result.add_result([Image.new("RGB", (8, 8))]) + with ( + patch("dw.result.export_to_video") as export, + tempfile.TemporaryDirectory() as temp_dir, + ): + result.save(temp_dir, "test") + + assert export.call_args.kwargs["fps"] == 8 + + def test_declaring_a_rate_the_frames_contradict_warns(self): + from dw.events import RunContext, activate_context, deactivate_context + + events = [] + token = activate_context(RunContext(on_event=events.append)) + try: + self.save( + {"content_type": "video/mp4", "fps": 8}, + AudioVideo("frames", torch.zeros((2, 100)), 48000, fps=24), + ) + finally: + deactivate_context(token) + + warning = next(e for e in events if e["event"] == "warning") + assert warning["kind"] == "fps_mismatch" + assert warning["declared_fps"] == 8 + assert warning["source_fps"] == 24 + # 24 fps frames written at 8 play in slow motion, not fast - the + # factor is declared/source, and it pointed the other way (#88) + assert "0.33x speed" in warning["message"] + assert "3 times as long" in warning["message"] + + def test_declaring_the_rate_the_frames_carry_warns_about_nothing(self): + from dw.events import RunContext, activate_context, deactivate_context + + events = [] + token = activate_context(RunContext(on_event=events.append)) + try: + self.save( + {"content_type": "video/mp4", "fps": 24}, + AudioVideo("frames", torch.zeros((2, 100)), 48000, fps=24), + ) + finally: + deactivate_context(token) + + assert [e for e in events if e["event"] == "warning"] == [] + + +class TestFramesForEncoding: + """What `encode_video` is handed, and why it is not the float array the + pipeline returned (#97).""" + + def test_float_frames_in_zero_to_one_become_a_uint8_tensor(self): + from dw.result import frames_for_encoding + + frames = numpy.zeros((2, 4, 4, 3), dtype=numpy.float32) + frames[1] = 1.0 + frames[0, 0, 0] = 0.5 + + converted = frames_for_encoding(frames) + + assert isinstance(converted, torch.Tensor) + assert converted.dtype == torch.uint8 + assert converted[1].max().item() == 255 + assert converted[0, 0, 0].tolist() == [128, 128, 128] + + def test_the_source_array_is_left_alone(self): + """A later step can still read this result through a + 'previous_result:' reference, and the step cache retains it.""" + from dw.result import frames_for_encoding + + frames = numpy.full((2, 2, 2, 3), 0.5, dtype=numpy.float32) + frames_for_encoding(frames) + + assert frames.max() == 0.5 and frames.dtype == numpy.float32 + + def test_frames_outside_the_range_are_handed_over_untouched(self): + """That is diffusers' own 'assume they are pixel values' branch - + left to it rather than reproduced here.""" + from dw.result import frames_for_encoding + + frames = numpy.full((1, 2, 2, 3), 255.0, dtype=numpy.float32) + + assert frames_for_encoding(frames) is frames + + def test_anything_that_is_not_a_float_array_is_passed_through(self): + from dw.result import frames_for_encoding + + already_uint8 = numpy.zeros((1, 2, 2, 3), dtype=numpy.uint8) + assert frames_for_encoding(already_uint8) is already_uint8 + assert frames_for_encoding("frames") == "frames" + tensor = torch.zeros((1, 2, 2, 3)) + assert frames_for_encoding(tensor) is tensor + + def test_a_muxed_save_converts_before_it_encodes(self): + result = Result({"content_type": "video/mp4", "fps": 24}) + frames = numpy.ones((1, 2, 2, 3), dtype=numpy.float32) + result.add_result(AudioVideo(frames, torch.zeros((2, 100)), 48000)) + + with ( + patch("dw.result.encode_video") as encode, + patch("dw.result.is_av_available", return_value=True), + tempfile.TemporaryDirectory() as temp_dir, + ): + result.save(temp_dir, "test") + + handed = encode.call_args.args[0] + assert isinstance(handed, torch.Tensor) and handed.dtype == torch.uint8 + + +class TestSavingIsNarrated: + """The 'saving' phase used to emit nothing at all - on a video template + that is minutes with the denoise counter frozen at its last step, which + is indistinguishable from a hang (#97).""" + + def events_of_a_save(self): + from dw.events import RunContext, activate_context, deactivate_context + + events = [] + token = activate_context(RunContext(on_event=events.append)) + try: + result = Result({"content_type": "text/plain"}) + result.add_result("some text") + with tempfile.TemporaryDirectory() as temp_dir: + result.save(temp_dir, "test") + finally: + deactivate_context(token) + return [e for e in events if e["event"] == "log"] + + def test_the_file_is_named_as_it_starts_and_costed_as_it_finishes(self): + logs = self.events_of_a_save() + + assert len(logs) == 2 + assert logs[0]["message"].startswith("writing test-0.0.txt") + assert logs[1]["message"].startswith("wrote test-0.0.txt in ") + assert logs[0]["file"] == logs[1]["file"] == "test-0.0.txt" + assert isinstance(logs[1]["seconds"], float) diff --git a/tests/test_runs.py b/tests/test_runs.py index fb4aece9..18713d9a 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -697,8 +697,11 @@ def test_a_parents_subfolder_does_not_move_a_childs_files( } Workflow(parent, str(tmp_path / "out"), str(tree / "Parent.json")).run({}) (run,) = (tmp_path / "out" / "Parent").iterdir() - assert (run / "runs_test-gen0.0-0.0.png").is_file() - assert not (run / "final" / "runs_test-gen0.0-0.0.png").exists() + # The parent step's name leads a composed child's file names, so two + # steps composing one workflow are told apart by the step that made + # them rather than by a '-2' suffix (#92) + assert (run / "child.runs_test-gen0.0-0.0.png").is_file() + assert not (run / "final" / "child.runs_test-gen0.0-0.0.png").exists() def test_a_chain_spill_lands_in_the_steps_subfolder(self, tmp_path, fake_pipeline): # save_segments writes through the pipeline wrapper's output_dir, diff --git a/tests/test_server.py b/tests/test_server.py index 7c4f0679..74604d72 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -516,6 +516,60 @@ def test_validate_accepts_a_stored_workflow_name(server, tmp_path): assert any("guidance_scael" in w for w in result["warnings"]) +def test_the_listing_says_what_a_cost_is(server): + """`cost: null` covered both "nobody measured it" and "not measured for + your device"; the listing now says which kind of figure a cost is + (#91).""" + with server(success_script) as client: + answer = client.get("/api/workflows").json() + + assert answer["cost_basis"] == "curated" + + +def test_validate_reports_a_sub_workflow_path_that_resolves_nowhere(server): + """The pre-flight is documented as "this will run", and a composed step + naming a workflow the server cannot reach used to come back valid and + fail 0.6 s into the job (#89).""" + with server(success_script) as client: + workflow = { + "id": "QaSubPathProbe", + "steps": [ + { + "name": "sub", + "workflow": { + "path": "templates/does-not-exist-at-all", + "arguments": {}, + }, + "result": {"content_type": "image/jpeg"}, + } + ], + } + + result = client.post("/api/validate", json={"workflow": workflow}).json() + + assert result["valid"] is False + assert [e["path"] for e in result["errors"]] == ["steps[0].workflow.path"] + assert "does-not-exist-at-all" in result["errors"][0]["message"] + + +def test_validate_accepts_a_sub_workflow_step_naming_a_stored_workflow(server): + with server(success_script) as client: + workflow = { + "id": "QaSubPathProbe", + "steps": [ + { + "name": "sub", + "workflow": {"path": "Basic", "arguments": {}}, + "result": {"content_type": "image/jpeg"}, + } + ], + } + + result = client.post("/api/validate", json={"workflow": workflow}).json() + + assert result["valid"] is True, result + + def test_validate_requires_exactly_one_workflow_source(server): with server(success_script) as client: assert client.post("/api/validate", json={}).status_code == 400 diff --git a/tests/test_video_utils.py b/tests/test_video_utils.py index 76430ca6..4c7850bb 100644 --- a/tests/test_video_utils.py +++ b/tests/test_video_utils.py @@ -307,6 +307,15 @@ def test_frames_and_audio_come_back_together(self, tmp_path): assert video.sample_rate == 8000 assert video.audio.shape[0] == 2 + def test_the_files_own_frame_rate_comes_back_with_it(self, tmp_path): + """A step that joins videos read from disk knows what to write them + back at without being told - see issue #84.""" + from dw.tasks.video_utils import load_audio_video + + path = self.write_video(tmp_path / "shot.mp4", fps=24, num_frames=24) + + assert load_audio_video(path).fps == 24 + def test_audio_is_fitted_to_the_frames_own_duration(self, tmp_path): """The codec pads the last block; joined shot after shot that padding would walk the sound off the picture.""" diff --git a/tests/test_workflow.py b/tests/test_workflow.py index 8f21ff18..6ef16b0e 100644 --- a/tests/test_workflow.py +++ b/tests/test_workflow.py @@ -863,3 +863,361 @@ def test_a_variable_cycle_is_a_validation_error_at_variables(tmp_path): assert [e["path"] for e in errors] == ["variables"] assert "a -> b -> a" in errors[0]["message"] + + +class TestSubWorkflowNameResolution: + """A sub-workflow step's path reads like run_workflow's workflow_path: + a catalog name, with or without .json, resolved across the same search + path the server lists (#90).""" + + def _catalog(self, tmp_path, parent_path_value): + import json + + workflows = tmp_path / "workflows" + (workflows / "minimax").mkdir(parents=True) + child = { + "id": "child", + "steps": [ + { + "name": "noop", + "task": { + "command": "get_dict_value", + "arguments": {"dict": {"k": 1}, "key": "k"}, + }, + } + ], + } + (workflows / "minimax" / "ref2va.json").write_text(json.dumps(child)) + parent = { + "id": "parent", + "steps": [ + { + "name": "sub", + "workflow": {"path": parent_path_value, "arguments": {}}, + } + ], + } + parent_path = workflows / "parent.json" + parent_path.write_text(json.dumps(parent)) + return workflows, parent_path + + def _resolve(self, tmp_path, path_value, workflow_dir=None): + from dw.workflow import workflow_from_file + + workflows, parent_path = self._catalog(tmp_path, path_value) + workflow = workflow_from_file( + str(parent_path), + str(tmp_path / "outputs"), + str(workflow_dir) if workflow_dir else str(workflows), + ) + return workflow.create_step_action( + workflow.workflow_definition["steps"][0], + shared_components={}, + previous_pipelines={}, + default_seed=42, + device="cpu", + ) + + def test_a_catalog_name_without_the_extension_resolves(self, tmp_path): + action = self._resolve(tmp_path, "minimax/ref2va") + + assert action.name == "child" + + def test_a_name_in_a_read_only_source_resolves(self, tmp_path, monkeypatch): + """The examples tree list_workflows reports as a source - reachable + without copying the template into the workspace.""" + import json + + from dw.workspace import WORKFLOW_PATH_ENV_VAR + + examples = tmp_path / "examples" + (examples / "templates").mkdir(parents=True) + (examples / "templates" / "stored.json").write_text( + json.dumps( + { + "id": "stored", + "steps": [ + { + "name": "noop", + "task": { + "command": "get_dict_value", + "arguments": {"dict": {"k": 1}, "key": "k"}, + }, + } + ], + } + ) + ) + workspace = tmp_path / "space" / "workflows" + workspace.mkdir(parents=True) + parent = { + "id": "parent", + "steps": [ + { + "name": "sub", + "workflow": {"path": "templates/stored", "arguments": {}}, + } + ], + } + parent_path = workspace / "parent.json" + parent_path.write_text(json.dumps(parent)) + monkeypatch.setenv(WORKFLOW_PATH_ENV_VAR, str(examples)) + + from dw.workflow import workflow_from_file + + workflow = workflow_from_file( + str(parent_path), str(tmp_path / "outputs"), str(workspace) + ) + action = workflow.create_step_action( + workflow.workflow_definition["steps"][0], + shared_components={}, + previous_pipelines={}, + default_seed=42, + device="cpu", + ) + + assert action.name == "stored" + # confined to the root it was read from, not to the workspace + assert action.workflow_dir == str(examples) + + def test_a_name_that_resolves_nowhere_says_where_it_looked(self, tmp_path): + from dw.workflow_sources import SubWorkflowNotFound + + with pytest.raises(SubWorkflowNotFound) as exc_info: + self._resolve(tmp_path, "minimax/does-not-exist") + + message = str(exc_info.value) + assert "does-not-exist" in message + assert "Looked in" in message + # every candidate is a real path it looked at, '.json' supplied - + # not a name reported as refused when it was simply absent + assert "does-not-exist.json" in message + assert "outside the root" not in message + + def test_a_relative_path_beside_the_file_still_wins(self, tmp_path): + """The '../models/x.json' form every template uses is unchanged.""" + action = self._resolve(tmp_path, "minimax/ref2va.json") + + assert action.name == "child" + + +class TestComposedStepSavesOnce: + """A sub-workflow step that declares a result owns the file: the child's + last step used to save the same artifact a second time, under its own + step name, into the run root (#92).""" + + def _compose(self, tmp_path, parent_result=True): + import json + + workflows = tmp_path / "workflows" + workflows.mkdir() + child = { + "id": "child", + "steps": [ + { + "name": "write", + "task": { + "command": "compose_text", + "arguments": {"parts": ["hello"]}, + }, + "result": {"content_type": "text/plain"}, + } + ], + } + (workflows / "child.json").write_text(json.dumps(child)) + step = {"name": "sub", "workflow": {"path": "child.json", "arguments": {}}} + if parent_result: + step["result"] = {"content_type": "text/plain", "subfolder": "final"} + parent = {"id": "parent", "steps": [step]} + parent_path = workflows / "parent.json" + parent_path.write_text(json.dumps(parent)) + + from dw.workflow import workflow_from_file + + workflow = workflow_from_file( + str(parent_path), str(tmp_path / "outputs"), str(workflows) + ) + workflow.run({}, {}) + return workflow + + def _written(self, tmp_path): + return sorted( + os.path.relpath(os.path.join(directory, name), str(tmp_path / "outputs")) + for directory, _dirs, files in os.walk(str(tmp_path / "outputs")) + for name in files + if name.endswith(".txt") + ) + + def test_the_artifact_is_written_once(self, tmp_path): + self._compose(tmp_path) + + written = self._written(tmp_path) + assert len(written) == 1, written + assert "final" in written[0] + + def test_the_manifest_names_only_the_step_the_caller_wrote(self, tmp_path): + workflow = self._compose(tmp_path) + + assert [entry["step"] for entry in workflow.manifest] == ["sub"] + + def test_a_parent_that_declares_no_result_leaves_the_child_saving(self, tmp_path): + workflow = self._compose(tmp_path, parent_result=False) + + written = self._written(tmp_path) + assert len(written) == 1, written + assert [entry["step"] for entry in workflow.manifest] == ["sub", "write"] + + def test_a_composed_file_carries_the_parent_step_name(self, tmp_path): + self._compose(tmp_path, parent_result=False) + + assert os.path.basename(self._written(tmp_path)[0]).startswith( + "sub.child-write" + ) + + +class TestSubWorkflowValidation: + """A sub-workflow path that cannot resolve is a validation error, not a + run that fails 0.6 s in after the pre-flight said valid (#89).""" + + def _tree(self, tmp_path): + workflows = tmp_path / "workflows" + workflows.mkdir() + return workflows + + def _parent(self, workflows, path_value, arguments=None): + import json + + parent = { + "id": "parent", + "steps": [ + { + "name": "sub", + "workflow": {"path": path_value, "arguments": arguments or {}}, + "result": {"content_type": "image/jpeg"}, + } + ], + } + parent_path = workflows / "parent.json" + parent_path.write_text(json.dumps(parent)) + + from dw.workflow import workflow_from_file + + return workflow_from_file( + str(parent_path), str(workflows.parent / "outputs"), str(workflows) + ) + + def test_a_path_that_resolves_nowhere_is_an_error(self, tmp_path): + workflow = self._parent(self._tree(tmp_path), "templates/does-not-exist-at-all") + + errors = workflow.validation_errors() + + assert [e["path"] for e in errors] == ["steps[0].workflow.path"] + assert "does-not-exist-at-all" in errors[0]["message"] + + def test_a_path_outside_the_root_is_an_error(self, tmp_path): + import json + + workflows = self._tree(tmp_path) + outside = tmp_path / "outside.json" + outside.write_text(json.dumps({"id": "x", "steps": []})) + workflow = self._parent(workflows, str(outside)) + + errors = workflow.validation_errors() + + assert [e["path"] for e in errors] == ["steps[0].workflow.path"] + + def test_a_workflow_that_composes_itself_is_a_cycle(self, tmp_path): + import json + + workflows = self._tree(tmp_path) + definition = { + "id": "loop", + "steps": [ + { + "name": "sub", + "workflow": {"path": "loop.json", "arguments": {}}, + "result": {"content_type": "image/jpeg"}, + } + ], + } + path = workflows / "loop.json" + path.write_text(json.dumps(definition)) + + from dw.workflow import workflow_from_file + + workflow = workflow_from_file( + str(path), str(tmp_path / "outputs"), str(workflows) + ) + errors = workflow.validation_errors() + + assert any("cycle" in e["message"] for e in errors), errors + + def test_a_child_that_does_not_validate_is_reported_under_the_step(self, tmp_path): + import json + + workflows = self._tree(tmp_path) + (workflows / "child.json").write_text( + json.dumps({"id": "child", "steps": [{"name": "broken"}]}) + ) + workflow = self._parent(workflows, "child") + + errors = workflow.validation_errors() + + assert errors + assert errors[0]["path"].startswith("steps[0].workflow.path -> ") + + def test_a_resolvable_child_validates_clean(self, tmp_path): + import json + + workflows = self._tree(tmp_path) + (workflows / "child.json").write_text( + json.dumps( + { + "id": "child", + "variables": {"prompt": "a cat"}, + "steps": [ + { + "name": "noop", + "task": { + "command": "compose_text", + "arguments": {"parts": ["variable:prompt"]}, + }, + "result": {"content_type": "text/plain"}, + } + ], + } + ) + ) + workflow = self._parent(workflows, "child", {"prompt": "a dog"}) + + assert workflow.validation_errors() == [] + assert workflow.sub_workflow_warnings() == [] + + def test_an_argument_the_child_does_not_declare_warns(self, tmp_path): + import json + + workflows = self._tree(tmp_path) + (workflows / "child.json").write_text( + json.dumps( + { + "id": "child", + "variables": {"prompt": "a cat"}, + "steps": [ + { + "name": "noop", + "task": { + "command": "compose_text", + "arguments": {"parts": ["variable:prompt"]}, + }, + "result": {"content_type": "text/plain"}, + } + ], + } + ) + ) + workflow = self._parent(workflows, "child", {"promt": "a dog"}) + + warnings = workflow.sub_workflow_warnings() + + assert [w["path"] for w in warnings] == ["steps[0].workflow.arguments.promt"] + assert "declares no variable" in warnings[0]["message"] diff --git a/todos.md b/todos.md deleted file mode 100644 index ae81cad9..00000000 --- a/todos.md +++ /dev/null @@ -1,108 +0,0 @@ -# todos - -## chaining - -- Keyframe pre-planning: since fl2va takes first and last keyframes, generate a storyboard of keyframes first, then fill each segment between consecutive pairs. Segments become independent — no cumulative drift, and parallelizable. This is arguably a better long-video strategy than sequential chaining -- Anti-drift correction: histogram/color matching each segment's frames back to segment 0 — cheap, addresses the best-known failure mode of autoregressive video chaining -- Per-segment prompt scheduling: "prompt": ["intro shot...", "then the camera...", ...] — one prompt per segment, for narrative arcs (falls out of the chain loop almost for free) -- Chained prompt-embed reuse — LTX2I2VChained only. chain.py:255 re-runs the full prompt through the 14 GB Gemma once per segment; at 3 segments that's two redundant encodes per run. Engine change, not config. - -## performance - -- Save a pre-quantized checkpoint. The 45 s SDNQ pass re-quantizes identical weights on every cold start. Save once locally, point model_name at it, and cold starts drop to plain weight loading. Also speeds the REPL's first load. This is the one real remaining structural win for non-REPL use. -- save compiled checkpoint like above -- torch.compile with repeated_blocks. Attacks the 25 s denoise across 48 repeated blocks. It only became viable when the transformer went resident — compile and group-offload hooks fight each other, and that's gone now. But first-run compilation costs more than it saves, so it only pays off paired with #1, where the graph survives between runs. - -## ltx 2.5 round-out - -Priority ranked. Assessment: we shipped t2v + i2v + chaining out of ~11 native -capabilities. Most of the gap was two generic engine limits, not per-model work. - -1. [x] **Cache blocks for LTX-2.5.** `LTX2VideoTransformerBlock` registered in - [cache_blocks.json](dw/cache_blocks.json). Its second returned stream is audio, not - `encoder_hidden_states`, so the registry grew an `encoder_hidden_states_argument_name` - remap - without it a skipped block feeds the text embeddings back as the audio - stream (pinned by a test that reproduces exactly that). Of little use on the - distilled model's 8-step schedule; it is there for anything longer. -2. [x] **Frames and audio across a step boundary.** `Result.get_artifact_properties` - reads object artifacts by attribute, so `previous_result:step.frames` works on the - AudioVideo a task or a chain produces. Two tasks carry the pieces: `video_frames` - (frames as one 0-255 array, the shape conditions want) and `pair_audio` (puts a - soundtrack back beside frames a frames-only step returned). Ships LTX2TwoStage.json. -3. [x] **Argument objects built from named fields.** `from_arguments` in - [arguments.py](dw/arguments.py) constructs a type from the arguments it names, for - types with no `from_file()` and no media `kind` - LTX-2's conditions and IC-LoRA - references. Defers to `build_objects` when one of those arguments names a step. - Ships LTX2Keyframes.json, LTX2Extend.json, LTX2ICLora.json. -4. [-] **Diffusion decoder.** Built, run, dropped. `LTX2VideoDiffusionDecodePipeline` - works, but two things make it a bad deal on 24GB: its first three stages run on the - full volume by design (only stage 4 and the diffusion blocks tile) and the attention - mask they build is quadratic in the output grid - 70GiB at 1536x896x121, unaffected - by tile size - and a step run with `output_type: "{latent}"` also returns audio - latents that nothing outside a pipeline call can vocode, so the flow is silent. - Base resolution would fit, but that is a silent clip at the same size as LTX2.json. - Documented in RECIPES_24GB. The per-component `enable_tiling` this exposed is kept: - the engine could only tile a component literally named 'vae' before. -5. [x] **Prompt enhancement.** LTX2I2VEnhancePrompt.json declares `google/gemma-4-E2B-it` - as the `prompt_enhancer` plus its `processor`, image-conditioned off the reference - frame. Quantized `uint4` because the pipeline moves the enhancer onto the accelerator - and never moves it back. -6. [-] **Non-distilled example.** Dropped: `transformer_full` is ~38GB in bf16 and its - guidance knobs cost three transformer passes per step, so it is not a 24GB - configuration. The knobs are documented in RECIPES_24GB; no example ships them. -7. [x] **Auto duration.** LTX2I2VEnhancePrompt.json omits `num_frames` and gives the - duration head `min_seconds` / `max_seconds` instead. -8. [x] **Docs.** LTX-2.5 section in RECIPES_24GB, `from_arguments` and the - frames/audio hand-off in WORKFLOW_GUIDE, both tasks in TASKS, the dual-stream cache - block in ACCELERATION. - -### left over - -- **Stage-2 refine.** The full LTX flow re-denoises the upsampled latents at - `STAGE_2_DISTILLED_SIGMA_VALUES` with `noise_scale: 0.909375` (the standard pipeline - defaults it to 0.0, so it must be passed). Blocked on the batch dimension: a step run - with `output_type: "{latent}"` pairs its latents with the audio latents per batch item, - which drops the leading axis every pipeline's `latents` argument expects. The - `previous_result:step.frames` route keeps it, but then the audio latents have no - decoder of their own - `audio_vae` + `vocoder` are only reachable inside a pipeline - call. Needs either a standalone audio decode step or batch-preserving latent artifacts. -- **HDR** (`LTX2HDRPipeline`). Still deferred - needs pre-computed connector embeddings - from a safetensors file we have no path to produce. -- [x] **Released pipelines did not give their VRAM back.** Found and fixed. - `populate_from_pretrained_arguments` loaded each declared sub-component *into the - step's own definition dict* (`from_pretrained_arguments[name] = component`), and the - definition belongs to the workflow, which outlives every step - so `release_pipeline` - freed the wrapper while the weights stayed reachable. LTX2ICLora.json OOMed with - 22.6GiB allocated: two 22B transformers alive at once. It only showed up on workflows - that declare sub-components, which is why an sd15 pipeline (model_name only) released - cleanly and hid it. Both mutation sites now copy; a second load also keeps its - 'model_name', which load_component used to consume out of the definition. - Confirmed end to end: LTX2TwoStage.json now drops from 13.3GB to 2.8GB at its - release boundary, where it used to hold 12.2GB. LTX2ICLora.json keeps sharing its - transformer between the two steps - that is now a preference (it skips a second - 38GB load) rather than the workaround it was. - -- **Gated repo.** `Lightricks/LTX-2.5-22b-IC-LoRA-Pixel-Spatial-Upscaler` is behind a - license click-through, accepted on this machine as of 2026-08-18. - `google/gemma-4-E2B-it` needs nothing. - -## introspection - -- [x] **Task argument discovery.** Resolved 2026-08-30 with a third option - neither of the two on the table: the handlers were already thin shims - forwarding `**arguments` into implementation functions with real - signatures, so `@register_command` now records each implementation's - dotted path and the introspection layer reads that function's signature - - the same function the dispatch calls, so the schema cannot drift (and a - registry-integrity test fails if a path rots). `describe_task` serves - `/api/tasks/{command}` in describe_class's shape; the editor's task forms - consume it, and `/api/validate` flags task-argument typos like pipeline - ones. `provided=` hides dispatch-supplied parameters; `device` is always - offered. gather_inputs and the image processors stay declared free-form. - -## deferred - -- **Multi-GPU workers.** Deferred 2026-08-30: no multi-GPU hardware to test - against at the moment. The seams are already in place when it returns - - device overrides accept `cuda:N` throughout, and the server's JobManager - is the natural place to grow a worker pool. diff --git a/ui/src/lib/editor/FlowView.svelte b/ui/src/lib/editor/FlowView.svelte index 7299f932..0c7d18cb 100644 --- a/ui/src/lib/editor/FlowView.svelte +++ b/ui/src/lib/editor/FlowView.svelte @@ -23,6 +23,40 @@ const graph = $derived(dataFlowGraph(workflow)) + // SVG text neither wraps nor takes text-overflow, so a label longer than + // the box ran out of its right edge. Budgets are characters at the box's + // inner width (BOX_W less the 10px inset each side) for each line's font + // - bold 12px sans for the name, 10px mono for the detail - and the + // clipPath below catches what a wider glyph set still pushes past. + const NAME_CHARS = 20 + const NAME_CHARS_WITH_TAG = 15 // the entry tag sits in the top-right corner + const DETAIL_CHARS = 26 + + /** The text cut to `max` characters with an ellipsis where it was cut: a + * name is told apart by how it starts, a path by how it ends. */ + function fit(text: string, max: number, keep: 'head' | 'tail'): string { + if (text.length <= max) return text + return keep === 'head' + ? text.slice(0, max - 1) + '…' + : '…' + text.slice(text.length - max + 1) + } + /** The parts of a node's labels that did not fit, in full, for its tooltip. */ + function overflowTitle(node: FlowNode): string { + const nameShown = fit( + node.name, + node.isEntryPoint ? NAME_CHARS_WITH_TAG : NAME_CHARS, + 'head', + ) + return [ + nameShown === node.name ? null : node.name, + fit(node.detail, DETAIL_CHARS, 'tail') === node.detail + ? null + : node.detail, + ] + .filter(Boolean) + .join('\n') + } + // Layered left-to-right layout: a node's layer is one past the deepest // producer that feeds it directly, so entry points (no previous_result // input) sit in the first column and depth reads as real dependency @@ -157,6 +191,9 @@ > + + + {#each layout.edgeLines ?? [] as edge, i (i)} @@ -180,14 +217,26 @@ class:active={node.name === activeStep} class:done={stateOf(node.name) === 'done'} transform={`translate(${pos.x}, ${pos.y})`} + clip-path="url(#flow-nodebox)" aria-label={`step ${node.name}, ${kindLabel(node.kind)}${stateOf(node.name) ? ', ' + stateOf(node.name) : ''}${node.isEntryPoint ? ', entry point' : ''}${fanIn ? ', fan-in: ' + fanIn.label : ''}`} {...nodeAttributes(node.name)} > + {#if overflowTitle(node)} + {overflowTitle(node)} + {/if} - {node.name} + {fit( + node.name, + node.isEntryPoint ? NAME_CHARS_WITH_TAG : NAME_CHARS, + 'head', + )} {kindLabel(node.kind)} {#if node.detail} - {node.detail} + {fit(node.detail, DETAIL_CHARS, 'tail')} {/if} {#if node.isEntryPoint} { expect(node.classList.contains('active')).toBe(false) } }) + +it('keeps a long detail inside its box, showing the tail and the whole on hover', () => { + const composed = { + steps: [ + { + name: 'shot1_inventory', + workflow: { path: 'templates/minimax/composable-reference-shot.json' }, + }, + ], + } + const { container } = render(FlowView, { workflow: composed }) + const node = nodeFor(container, 'shot1_inventory') + const detail = node.querySelector('.stepdetail')! + // A path is told apart by its end, so that is the part kept + expect(detail.textContent).toMatch(/^…/) + expect(detail.textContent).toMatch(/reference-shot\.json$/) + expect(detail.textContent!.length).toBeLessThan( + 'templates/minimax/composable-reference-shot.json'.length, + ) + expect(node.querySelector('title')?.textContent).toBe( + 'templates/minimax/composable-reference-shot.json', + ) + // The box clips whatever a wider glyph set still pushes past its edge + expect(node.getAttribute('clip-path')).toBe('url(#flow-nodebox)') +}) + +it('shortens a long step name from the end, leaving room for the entry tag', () => { + const long = { + steps: [ + { + name: 'reference_to_video_audio_with_lipsync', + pipeline: { configuration: { component_type: 'Fake' } }, + }, + ], + } + const { container } = render(FlowView, { workflow: long }) + const node = nodeFor(container, 'reference_to_video_audio_with_lipsync') + const name = node.querySelector('.stepname')! + expect(name.textContent).toMatch(/…$/) + expect(name.textContent!.length).toBeLessThan( + 'reference_to_video_audio_with_lipsync'.length, + ) + // A name that fits gets no tooltip at all + expect( + nodeFor(render(FlowView, { workflow }).container, 'gen').querySelector( + 'title', + ), + ).toBeNull() + expect(node.querySelector('title')?.textContent).toBe( + 'reference_to_video_audio_with_lipsync', + ) +}) diff --git a/ui/src/lib/pages/JobPage.svelte b/ui/src/lib/pages/JobPage.svelte index af543af6..ad91dc09 100644 --- a/ui/src/lib/pages/JobPage.svelte +++ b/ui/src/lib/pages/JobPage.svelte @@ -10,7 +10,12 @@ import { ApiError, api, outputUrl, streamJobEvents } from '../api' import { confirmDialog } from '../confirm.svelte' import { go } from '../router.svelte' - import { groupResultFiles, sectionBySubfolder } from '../results' + import { + groupResultFiles, + sectionBySubfolder, + unsavedSteps, + } from '../results' + import { finishedNodes, flowNodeName } from '../runstate' import { stepProgress } from '../progress' import FlowView from '../editor/FlowView.svelte' import CopyButton from '../CopyButton.svelte' @@ -157,15 +162,27 @@ const steps = $derived( (events.find((e) => e.event === 'workflow_start')?.steps as string[]) ?? [], ) - const currentStep = $derived( - [...events].reverse().find((e) => e.event === 'step_start')?.step as - string | undefined, + // The step_start the run is on, as the engine named it: a for_each member + // keeps its '@', a sub-workflow's inner step its own name. The Progress + // list is written in these names, so it reads off this one. + const stepStart = $derived( + [...events].reverse().find((e) => e.event === 'step_start'), ) - const finishedSteps = $derived( - new Set( - events.filter((e) => e.event === 'step_end').map((e) => e.step as string), + const currentStep = $derived(stepStart?.step as string | undefined) + // The same step in the name the definition gives it, which is what the + // flow graph's nodes are called - see runstate.ts for why the two differ + const activeNode = $derived( + flowNodeName( + stepStart?.step as string | undefined, + stepStart?.parent_step as string | undefined, ), ) + // A sub-workflow's inner step has no row of its own in the Progress list, + // so what lights up while one runs is the composed step that queued it + const listStep = $derived( + steps.includes(currentStep ?? '') ? currentStep : activeNode, + ) + const finishedSteps = $derived(finishedNodes(events as JobEvent[], steps)) // Scoped to the step now running: its phase, and its own denoise counter const progress = $derived(stepProgress(events as JobEvent[])) const denoise = $derived(progress.denoise) @@ -246,6 +263,13 @@ const sectioned = $derived( sections.length > 1 || sections[0].subfolder !== '', ) + // Steps that ran and wrote no file, with the definition's reason for each. + // Without this a workflow that keeps most of its steps in memory shows a + // short Results list and no word about the rest, which reads as outputs + // that went missing rather than as a choice the workflow made + const unsaved = $derived( + unsavedSteps(job?.manifest, events as JobEvent[], definition), + ) const running = $derived(job !== null && !TERMINAL.includes(job.status)) // A cancel requested while loading a model or running a task step has no // checkpoint to catch it until that phase finishes - without this the UI @@ -352,13 +376,13 @@
- {step} - {#if step === currentStep && running} + {#if step === listStep && running} {#if denoise}
Workflow {/if} - {#if fileGroups.length} + {#if fileGroups.length || unsaved.length}

Results

{#if allReused} @@ -491,6 +515,27 @@ {/each} {/each} {/each} + {#if unsaved.length} +

Steps that wrote nothing

+
    + {#each unsaved as entry (entry.node)} +
  • + {entry.node} + {#if entry.members.length} + × {entry.members.length} + {/if} + + {#if entry.reason}{entry.reason.key} + {entry.reason.detail}{:else}no file written{/if} + +
    • + {/each} +
+ {/if}
{/if} @@ -682,4 +727,25 @@ .subhead + .stephead { margin-top: 0; } + /* The steps whose files are deliberately absent: the step name is what + the engine resolves, the reason is written for a person */ + .unsaved { + list-style: none; + margin: 0; + padding: 0; + font-size: var(--t-sm); + } + .unsaved li { + display: flex; + align-items: baseline; + flex-wrap: wrap; + gap: 0.5rem; + padding: 0.15rem 0; + } + .unsaved code { + font-size: 0.8rem; + } + .unsaved .why { + min-width: 0; + } diff --git a/ui/src/lib/pages/JobPage.test.ts b/ui/src/lib/pages/JobPage.test.ts index 84a20d16..bf4282b3 100644 --- a/ui/src/lib/pages/JobPage.test.ts +++ b/ui/src/lib/pages/JobPage.test.ts @@ -13,13 +13,17 @@ const stream = vi.hoisted(() => ({ const metadata = vi.hoisted(() => ({ byFile: {} as Record>, })) +// The definition the job ran, for the flow view and the unsaved reasons +const ran = vi.hoisted(() => ({ + definition: null as Record | null, +})) vi.mock('../api', () => ({ ApiError: class ApiError extends Error {}, api: { getJob: vi.fn(() => Promise.resolve(detail.job)), getJobWorkflow: vi.fn(() => - Promise.resolve({ definition: null, seed_variable: null }), + Promise.resolve({ definition: ran.definition, seed_variable: null }), ), galleryMetadata: vi.fn((name: string) => Promise.resolve({ @@ -62,9 +66,24 @@ afterEach(() => { cleanup() stream.onEvent = null metadata.byFile = {} + ran.definition = null vi.mocked(api.galleryMetadata).mockClear() }) +/** The flow view's box for one step of the definition. */ +function nodeFor(container: HTMLElement, name: string) { + return [...container.querySelectorAll('g.node')].find((node) => + node.getAttribute('aria-label')?.startsWith(`step ${name},`), + )! +} + +/** The page's "wrote nothing" rows, whitespace flattened. */ +function unsavedRows(container: HTMLElement) { + return [...container.querySelectorAll('.unsaved li')].map((li) => + li.textContent?.replace(/\s+/g, ' ').trim(), + ) +} + it('shows each image beside what made it, with a download link, and never probes a video', async () => { metadata.byFile['a.png'] = { model_name: 'org/model', @@ -149,3 +168,120 @@ it('places a live step_end under its subfolder before the manifest arrives', asy ).toBeTruthy(), ) }) + +it('lights the for_each step in the flow chart while one of its members runs', async () => { + // The graph is drawn from the definition, where for_each is one step; the + // engine reports the members, so the two only meet at the group name + ran.definition = { + steps: [ + { + name: 'shot', + pipeline: { configuration: { component_type: 'Fake' } }, + for_each: 'variable:shots', + }, + { name: 'episode', task: { command: 'mux' } }, + ], + } + detail.job = { ...job([]), status: 'running', finished_at: null } + const { container } = render(JobPage, { jobId: 'j1' }) + await waitFor(() => expect(stream.onEvent).not.toBeNull()) + stream.onEvent!({ + seq: 1, + event: 'workflow_start', + steps: ['shot@open', 'shot@reveal', 'episode'], + }) + stream.onEvent!({ seq: 2, event: 'step_start', step: 'shot@open' }) + await waitFor(() => + expect(nodeFor(container, 'shot').classList.contains('active')).toBe(true), + ) + + // One member of two down: the group is not behind us yet + stream.onEvent!({ seq: 3, event: 'step_end', step: 'shot@open', files: [] }) + await waitFor(() => + expect(nodeFor(container, 'shot').classList.contains('active')).toBe(true), + ) + expect(nodeFor(container, 'shot').classList.contains('done')).toBe(false) + + stream.onEvent!({ seq: 4, event: 'step_end', step: 'shot@reveal', files: [] }) + stream.onEvent!({ seq: 5, event: 'step_start', step: 'episode' }) + await waitFor(() => + expect(nodeFor(container, 'shot').classList.contains('done')).toBe(true), + ) + expect(nodeFor(container, 'episode').classList.contains('active')).toBe(true) +}) + +it('keeps the Progress list on the composed step while its child runs', async () => { + ran.definition = { + steps: [ + { name: 'shot1', workflow: { path: 'child.json' } }, + { name: 'episode', task: { command: 'mux' } }, + ], + } + detail.job = { ...job([]), status: 'running', finished_at: null } + const { container } = render(JobPage, { jobId: 'j1' }) + await waitFor(() => expect(stream.onEvent).not.toBeNull()) + stream.onEvent!({ + seq: 1, + event: 'workflow_start', + steps: ['shot1', 'episode'], + }) + // What a child emits: its own step name, with the parent's alongside + stream.onEvent!({ + seq: 2, + event: 'step_start', + step: 'reference_to_video_audio', + parent_step: 'shot1', + }) + await waitFor(() => + expect(nodeFor(container, 'shot1').classList.contains('active')).toBe(true), + ) + const dotFor = (name: string) => + [...container.querySelectorAll('.step')] + .find( + (row) => + row.querySelector('span:nth-child(2)')?.textContent?.trim() === name, + )! + .querySelector('.dot')! + expect(dotFor('shot1').classList.contains('active')).toBe(true) + expect(dotFor('episode').classList.contains('active')).toBe(false) +}) + +it('says why a step wrote nothing, so a deliberate non-output is not a missing one', async () => { + ran.definition = { + steps: [ + { name: 'base', pipeline: {}, result: { save: false } }, + { name: 'edit', task: { command: 'mux' } }, + { name: 'film', pipeline: {}, result: { content_type: 'video/mp4' } }, + ], + } + detail.job = job([ + { step: 'base', files: [] }, + { step: 'edit', files: [] }, + { step: 'film', files: ['final/film.mp4'], subfolder: 'final' }, + ]) + const { container } = render(JobPage, { jobId: 'j1' }) + await waitFor(() => + expect(screen.getByText('Steps that wrote nothing')).toBeTruthy(), + ) + expect(unsavedRows(container)).toEqual([ + 'base result.save is false, so the step is kept in memory and never written', + 'edit result.content_type is not declared, so there is no file type to write', + ]) + // The step that did write is in the results above, not in this list + expect(screen.getByRole('heading', { level: 3, name: 'final/' })).toBeTruthy() +}) + +it('explains a run that wrote nothing at all rather than showing no results', async () => { + ran.definition = { steps: [{ name: 'base', result: { save: false } }] } + detail.job = job([{ step: 'base', files: [] }]) + const { container } = render(JobPage, { jobId: 'j1' }) + await waitFor(() => + expect(screen.getByText('Steps that wrote nothing')).toBeTruthy(), + ) + expect( + screen.getByRole('heading', { level: 2, name: 'Results' }), + ).toBeTruthy() + expect(unsavedRows(container)).toEqual([ + 'base result.save is false, so the step is kept in memory and never written', + ]) +}) diff --git a/ui/src/lib/results.test.ts b/ui/src/lib/results.test.ts index 681eddd1..4681dbe4 100644 --- a/ui/src/lib/results.test.ts +++ b/ui/src/lib/results.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from 'vitest' -import { groupResultFiles, sectionBySubfolder } from './results' +import { + groupResultFiles, + sectionBySubfolder, + unsavedReason, + unsavedSteps, +} from './results' import type { JobEvent } from './types' const stepEnd = ( @@ -160,3 +165,116 @@ describe('sectionBySubfolder', () => { expect(sections.map((s) => s.subfolder)).toEqual(['final', 'shots/act-1']) }) }) + +describe('unsavedReason', () => { + it('names result.save when the workflow asked for no file', () => { + expect( + unsavedReason({ result: { save: false, content_type: 'image/png' } }), + ).toEqual({ + key: 'result.save', + detail: 'is false, so the step is kept in memory and never written', + }) + }) + + it('names result.content_type when there is no type to write', () => { + expect(unsavedReason({ task: { command: 'mux' } })).toEqual({ + key: 'result.content_type', + detail: 'is not declared, so there is no file type to write', + }) + }) + + it('has no reason for a step that declares a file', () => { + expect( + unsavedReason({ result: { content_type: 'video/mp4', save: true } }), + ).toBeNull() + }) +}) + +describe('unsavedSteps', () => { + const definition = (steps: Array>) => ({ steps }) + const steps = definition([ + { name: 'base', result: { save: false } }, + { name: 'edit', task: { command: 'mux' } }, + { name: 'film', result: { content_type: 'video/mp4' } }, + ]) + + it('names every step that ran and wrote nothing, with the reason', () => { + const unsaved = unsavedSteps( + [ + { step: 'base', files: [] }, + { step: 'edit', files: [] }, + { step: 'film', files: ['final/film.mp4'], subfolder: 'final' }, + ], + [], + steps, + ) + expect(unsaved).toEqual([ + { + node: 'base', + members: [], + reason: { + key: 'result.save', + detail: 'is false, so the step is kept in memory and never written', + }, + }, + { + node: 'edit', + members: [], + reason: { + key: 'result.content_type', + detail: 'is not declared, so there is no file type to write', + }, + }, + ]) + }) + + it('collapses a for_each group into the one step the workflow declares', () => { + const unsaved = unsavedSteps( + [ + { step: 'base@open', files: [] }, + { step: 'base@reveal', files: [] }, + ], + [], + steps, + ) + expect(unsaved).toEqual([ + { + node: 'base', + members: ['base@open', 'base@reveal'], + reason: { + key: 'result.save', + detail: 'is false, so the step is kept in memory and never written', + }, + }, + ]) + }) + + it('lists only steps that have finished, so a running job is not accused', () => { + const events = [ + { seq: 1, event: 'workflow_start', steps: ['base', 'edit', 'film'] }, + { seq: 2, event: 'step_end', step: 'base', files: [] }, + ] as unknown as JobEvent[] + expect(unsavedSteps(undefined, events, steps).map((s) => s.node)).toEqual([ + 'base', + ]) + }) + + it("ignores a sub-workflow's inner steps, which are no node of this graph", () => { + const composed = definition([ + { name: 'shot1_amnesty', workflow: { path: 'child.json' } }, + ]) + const unsaved = unsavedSteps( + [ + { step: 'reference_to_video_audio', files: [] }, + { step: 'shot1_amnesty', files: ['intermediate/a.mp4'] }, + ], + [], + composed, + ) + expect(unsaved).toEqual([]) + }) + + it('has nothing to say without a definition to read the reasons off', () => { + expect(unsavedSteps([{ step: 'base', files: [] }], [], null)).toEqual([]) + }) +}) diff --git a/ui/src/lib/results.ts b/ui/src/lib/results.ts index c84e946c..0f3db546 100644 --- a/ui/src/lib/results.ts +++ b/ui/src/lib/results.ts @@ -1,3 +1,4 @@ +import { flowNodeName } from './runstate' import type { JobEvent, ManifestEntry, StepEndEvent } from './types' /** One step's output files, with where in the run directory they landed. */ @@ -86,3 +87,103 @@ export function sectionBySubfolder(groups: StepGroup[]): SubfolderSection[] { groups: bySubfolder.get(subfolder)!, })) } + +/** One step of a run that finished without writing a file. */ +export interface UnsavedStep { + /** The step's name in the definition. One entry covers a whole for_each + * group, since that is the one step the workflow declares. */ + node: string + /** The member names the engine ran it under, one per for_each entry - + * empty for a step that has no `for_each`, which is its own name. */ + members: string[] + /** Why nothing was written, read off the definition - null when the + * definition does not explain it. */ + reason: UnsavedReason | null +} + +/** The JSON key a step writes nothing because of, and what that means - + * kept apart so the page can set the key in the type the engine resolves + * literally. */ +export interface UnsavedReason { + key: string + detail: string +} + +/** Why a step's result block saves no file: the two ways the engine skips + * the save, both in `Result.save` (dw/result.py). */ +export function unsavedReason(step: Record): UnsavedReason | null { + const result = step.result + if (result && result.save === false) { + return { + key: 'result.save', + detail: 'is false, so the step is kept in memory and never written', + } + } + if (!result || typeof result.content_type !== 'string') { + return { + key: 'result.content_type', + detail: 'is not declared, so there is no file type to write', + } + } + return null +} + +/** The steps that ran and wrote nothing, with the reason the definition + * gives for each. + * + * The page groups a run's files by producing step and drops the empty + * groups, which made a workflow whose steps deliberately write nothing + * indistinguishable from a run whose outputs had gone missing: the steps + * were there, their files were not, and nothing said why. A step counts as + * having run when the manifest carries its entry - every step that finished + * gets one - or a top-level step_end named it, so a job still in flight + * never lists a step it has not reached yet, and a historical job, which + * has no events at all, reads correctly off the manifest alone. */ +export function unsavedSteps( + manifest: ManifestEntry[] | undefined, + events: JobEvent[], + definition: Record | null, +): UnsavedStep[] { + const byNode = new Map>() + for (const step of (definition?.steps ?? []) as Array>) { + if (typeof step?.name === 'string') byNode.set(step.name, step) + } + if (!byNode.size) return [] + + // An inner step of a sub-workflow is no node of the parent's graph, so + // the entry it rolls up into the manifest is no evidence about anything + // here - only a definition step, or a member of one, names a node + const nodeOf = (step: string): string | undefined => { + const node = flowNodeName(step) + return node && byNode.has(node) ? node : undefined + } + + const wrote = new Set() + for (const group of groupResultFiles(manifest, events)) { + const node = nodeOf(group.step) + if (node) wrote.add(node) + } + + const ran = new Map() + const note = (step: string) => { + const node = nodeOf(step) + if (!node) return + const members = ran.get(node) ?? [] + if (!members.includes(step)) members.push(step) + ran.set(node, members) + } + for (const entry of manifest ?? []) note(entry.step) + for (const event of events) { + if (event.event === 'step_end' && !event.parent_step && event.step) { + note(event.step as string) + } + } + + return [...ran] + .filter(([node]) => !wrote.has(node)) + .map(([node, members]) => ({ + node, + members: members.filter((member) => member !== node), + reason: unsavedReason(byNode.get(node)!), + })) +} diff --git a/ui/src/lib/runstate.test.ts b/ui/src/lib/runstate.test.ts new file mode 100644 index 00000000..43990901 --- /dev/null +++ b/ui/src/lib/runstate.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest' +import { finishedNodes, flowNodeName } from './runstate' +import type { JobEvent } from './types' + +const ended = (step: string, parent_step?: string): JobEvent => + ({ seq: 0, event: 'step_end', step, parent_step }) as JobEvent + +describe('flowNodeName', () => { + it('reduces a for_each member to the group the definition declares', () => { + expect(flowNodeName('base@open')).toBe('base') + }) + + it("reduces a sub-workflow's inner step to the composed step that queued it", () => { + expect(flowNodeName('reference_to_video_audio', 'shot1_amnesty')).toBe( + 'shot1_amnesty', + ) + }) + + it('takes the parent even when the child step is a for_each group itself', () => { + expect(flowNodeName('base@open', 'shot@reveal')).toBe('shot') + }) + + it('leaves an ordinary step alone, and has nothing for a missing name', () => { + expect(flowNodeName('film')).toBe('film') + expect(flowNodeName(undefined)).toBeUndefined() + }) +}) + +describe('finishedNodes', () => { + const members = ['base@open', 'base@reveal', 'film'] + + it('finishes a for_each group only once every member has ended', () => { + expect(finishedNodes([ended('base@open')], members)).toEqual([]) + expect( + finishedNodes([ended('base@open'), ended('base@reveal')], members), + ).toEqual(['base']) + }) + + it('finishes a plain step on its own end', () => { + expect(finishedNodes([ended('film')], members)).toEqual(['film']) + }) + + it("does not finish a composed step for one of its child's inner steps", () => { + const names = ['shot1_amnesty', 'episode'] + const events = [ended('inner_a', 'shot1_amnesty')] + expect(finishedNodes(events, names)).toEqual([]) + events.push(ended('shot1_amnesty')) + expect(finishedNodes(events, names)).toEqual(['shot1_amnesty']) + }) + + it('has nothing finished before the run has reported anything', () => { + expect(finishedNodes([], members)).toEqual([]) + }) +}) diff --git a/ui/src/lib/runstate.ts b/ui/src/lib/runstate.ts new file mode 100644 index 00000000..8bd0fc9c --- /dev/null +++ b/ui/src/lib/runstate.ts @@ -0,0 +1,61 @@ +/** Where a run's event stream meets the definition it is running. + * + * The flow graph is drawn from the definition, where `for_each` is left + * unexpanded and a sub-workflow is one step, but the engine *runs* the + * definition expanded: it reports `base@open` where the definition says + * `base`, and a sub-workflow's inner step under its own name where the + * definition says the composed step. Nothing the page highlights matches + * until both sides are reduced to the same name - which is what broke the + * flow view's run state when list-driven steps arrived. */ + +import type { JobEvent } from './types' + +/** The for_each step/member separator - `dw/for_each.py`'s + * MEMBER_SEPARATOR, reserved in every step name, so the first one in a + * name always splits a member from its group. */ +const MEMBER_SEPARATOR = '@' + +/** The name the definition gives the step the engine ran. + * + * A for_each member reduces to its group (`base@open` -> `base`); anything + * a sub-workflow emitted reduces to the step that queued it, which the + * engine puts on every event a child emits as `parent_step` + * (`_parent_progress_fields`, dw/workflow.py). A plain step is its own + * name, returned as it is. */ +export function flowNodeName( + step?: string, + parentStep?: string, +): string | undefined { + const name = parentStep || step + if (!name) return undefined + const at = name.indexOf(MEMBER_SEPARATOR) + return at === -1 ? name : name.slice(0, at) +} + +/** The nodes a run has finished, given the step names its `workflow_start` + * listed. + * + * A for_each group is one node, so it is done only once every member is - + * greening it on the first would say the group is behind us while its + * remaining entries are still queued. A sub-workflow is done when the + * composed step itself ends, not when one of the child's inner steps does, + * so only the top-level step_end events count: a child's carry the parent's + * name in `parent_step`, which is exactly the marker to leave out. */ +export function finishedNodes( + events: JobEvent[], + stepNames: string[], +): string[] { + const ended = new Set( + events + .filter((event) => event.event === 'step_end' && !event.parent_step) + .map((event) => event.step as string), + ) + const members = new Map() + for (const name of stepNames) { + const node = flowNodeName(name) + if (node) members.set(node, [...(members.get(node) ?? []), name]) + } + return [...members] + .filter(([, names]) => names.every((name) => ended.has(name))) + .map(([node]) => node) +} diff --git a/workflows/templates/ltx2/text-to-video.json b/workflows/templates/ltx2/text-to-video.json index 0f8c574a..7cc9cf88 100644 --- a/workflows/templates/ltx2/text-to-video.json +++ b/workflows/templates/ltx2/text-to-video.json @@ -2,7 +2,7 @@ "id": "LTX2", "description": "The baseline for the LTX-2.5 family: text to video with a soundtrack generated alongside it, on a single 24GB card. The distilled transformer runs a fixed eight-step schedule with guidance off, quantized per component so it stays resident while the Gemma text encoder and the connectors are group offloaded around it. First load adds about 10 minutes.", "cost": [ - {"device": "cuda", "name": "RTX 3090", "vram_gb": 24, "minutes": 0.6} + {"device": "cuda", "name": "RTX 3090", "vram_gb": 24, "minutes": 1.8} ], "variables": { "transformer_weights_dtype": "{uint4}", diff --git a/workflows/templates/minimax/composable-references.json b/workflows/templates/minimax/composable-references.json index 20982024..ffaaa845 100644 --- a/workflows/templates/minimax/composable-references.json +++ b/workflows/templates/minimax/composable-references.json @@ -2,6 +2,9 @@ "id": "MiniMaxH3Ref2VAVideo", "description": "Reference types are composable, and each one contributes a different attribute. Here an image reference supplies the subject and a video reference supplies the shot - its framing, lighting and camera movement - so the two are recombined into a clip that copies neither wholesale. The prompt's retention analysis is what assigns those roles; the reference list only declares what is available.", "summary": "H3 video: an image reference supplies the subject, a video reference the framing and camera.", + "cost": [ + {"device": "cuda", "name": "RTX 3090", "vram_gb": 24, "minutes": 27.4} + ], "seed": 12345, "variables": { "prompt": "subject_definitions:\n is the astronaut in , in a white hard-shell suit with a gold-mirrored visor, an orange chest harness and a soft-sided life-support pack.\n