From 4f4f41f39f55eb2347dce8b70f460dba6b021b2c Mon Sep 17 00:00:00 2001 From: Don Kackman Date: Sat, 12 Sep 2026 09:32:32 -0500 Subject: [PATCH 01/55] docs: output-folders proposal reviewed into a design (subfolder on every surface) Co-Authored-By: Claude Opus 5 (1M context) --- docs/proposals/output-folders.md | 430 ++++++++++++++++++------------- 1 file changed, 248 insertions(+), 182 deletions(-) diff --git a/docs/proposals/output-folders.md b/docs/proposals/output-folders.md index b080168a..2c743673 100644 --- a/docs/proposals/output-folders.md +++ b/docs/proposals/output-folders.md @@ -1,6 +1,8 @@ -# Proposal: folders for a job's outputs +# Design: subfolders for a run's outputs -Status: **proposed, not implemented**. Written for MCP feedback ticket T016. +Status: **designed, not implemented**. Written for MCP feedback ticket T016; +reviewed against the code on 2026-09-12. Decisions taken in that review are +marked *decided*. ## The ask, as filed @@ -22,39 +24,39 @@ each file name. The consumer of that run - an agent deciding what to show the user, or a person opening the gallery - has to know the workflow to know which file is the deliverable. -## Why this is a proposal and not an edit +## Why this needed a design and not an edit -A destination folder is a new field in the workflow schema, which means it -is a concept every consumer has to learn and every surface has to carry. But -the reason it is a proposal rather than an edit is narrower than that: **the -output tree already has a layer with the meaning this wants.** +A destination folder is a new field in the workflow schema, so every +consumer has to learn it and every surface has to carry it. But the reason +it needed deciding is narrower: **the output tree already has a layer with +the meaning this wants**, and three things read that tree by position. -A run writes to `///`, and three -separate things read that shape by position: +A run writes to `///`: - `strip_run_id` (`dw/runs.py`) turns `ltx2/Gyre/20260905-181530-a1b2c3d4/ still-0.png` into the gallery folder `ltx2/Gyre`, which is how a workflow - run fifty times is one entry in the folder filter rather than fifty. + run fifty times is one entry in the folder filter rather than fifty. It + does this by testing whether the *last* directory segment is a run id. - `output:` references are `//`, with `latest` allowed in the run-id position (`resolve_output_reference`). - The manifest, and `workflow.json` beside it, sit at the root of the run directory and describe everything in it. -Adding a folder *inside* a run directory puts a segment between the run id -and the file name, and every one of those three readers has to be told which -segment is which. Getting that wrong is not a cosmetic bug: `strip_run_id` -would start reporting `ltx2/Gyre/final` and `ltx2/Gyre/intermediate` as two -unrelated workflows in the gallery filter, and an `output:` reference that -worked yesterday would resolve to nothing. That is the part that needs -deciding before anything is written. +A folder *inside* a run directory puts a segment between the run id and the +file name. As the code stands, `strip_run_id` on +`ltx2/Gyre//final/x.mp4` returns `ltx2/Gyre//final` - the run id +is no longer the last segment, so the gallery filter would gain one entry +per run. That is the part that had to be decided before anything was +written. The other two readers turn out to need nothing (below). There is also a question the ticket settles one way and the existing code -settles the other, covered under [Open questions](#open-questions): the -workflow and prompt libraries do not actually enforce one level. +settles the other: the workflow and prompt libraries do not enforce one +level, so "consistent with them" means not enforcing one here either. -## The shape +## The field -A step's `result` block names the folder its files are written into: +A step's `result` block names the subfolder of the run directory its files +are written into: ```json { @@ -62,34 +64,95 @@ A step's `result` block names the folder its files are written into: "task": { "command": "concat_videos", "arguments": { "...": "..." } }, "result": { "content_type": "video/mp4", - "folder": "final" + "subfolder": "final" } } ``` -- `folder` is optional. Without it, files are written where they are written - today: at the root of the run directory. **Every existing workflow keeps - working unchanged, and a workflow that never sets `folder` is - indistinguishable from one written before this existed.** -- The value is a relative path validated the way every other - workflow-supplied path segment is - through `validate_output_path` against - the run directory, so `../` and absolute paths are refused rather than - escaping into another run. -- It may carry a `variable:` reference, so a caller can route a run's - outputs without editing the workflow. -- It is per step, not per workflow: the point is that one run's steps land - in different places. +- **Named `subfolder`, on every surface** (*decided*). It is literally a + subfolder of the run directory. The word `folder` was rejected because a + gallery entry's `folder` already means the workflow identity, and an + author who set `"folder": "final"` would have read back + `folder: "ltx2/Gyre"` beside some other key; `group` was rejected because + `for_each` documentation already uses "group" for what `gather:` reads. + One word, same meaning, in the schema, the manifests, the gallery and the + query string. +- Optional. Without it, files are written where they are written today: at + the root of the run directory. **Every existing workflow keeps working + unchanged, and a workflow that never sets `subfolder` writes + byte-identical paths to one written before this existed.** +- A relative path of any depth (*decided*; see the first open question, + now closed). Validated through `validate_output_path` against the run + directory, so `../`, absolute paths and symlink escapes are refused + rather than reaching another run. `\` is refused like `/../`. +- Substitution applies as it does to the rest of the step: `variable:` so a + caller can route a run's outputs without editing the workflow, and + `item:` inside a `for_each` template so members can land in their own + places (`"subfolder": "item:subfolder"`). This is the case that argues + against a depth limit - `shots/act-1` is a natural thing to want. +- Per step, not per workflow: the point is that one run's steps land in + different places. +- **No default, ever** (*decided*). A one-step workflow's output is by + definition the deliverable, but defaulting it to `final` would move + every such workflow's files on upgrade and silently stop any stored + `output:` reference from advancing. The convention the tooling steers toward is two names, `final` and `intermediate` - see [Steering](#steering-consumers-toward-it). The engine does not know those names or treat them specially; a workflow that wants `shots/` and `audio/` gets them. +### `file_base_name` may no longer contain a separator + +`"file_base_name": "final/"` passes `validate_string_input` and +`validate_output_path` today and then fails at `open()` because the +directory does not exist - a latent crash, not a feature. With a real +placement field it becomes a validation error naming `subfolder` as the way +to do it. No shipped workflow (`workflows/`, `dw/workflows/`, `plugins/`) +uses a separator there. + ## What each surface does with it -### The run directory and the manifest +### The engine (`dw/result.py`, `dw/workflow.py`, `dw/runs.py`) + +- `Result.save(output_dir, base_name)` reads `subfolder` from the result + definition, joins it onto `output_dir`, validates the joined directory + with `validate_output_path(joined, output_dir)`, creates it, and saves + into it. Nothing else in `save` changes: names stay + `{workflow}-{step}.{i}-{j}.{k}.ext`, deduplication stays per path. +- The manifest entry `workflow.py` appends per step gains + `"subfolder": `. A step-cache hit keeps the *definition's* + subfolder while its files stay the earlier run's absolute paths, exactly + as `reused` entries work now. Sub-workflow entries roll up as they do + now; a sub-workflow's subfolder is relative to the run directory it + inherits, which is the right reading. +- `dw/runs.py` gains `split_run_path(relative) -> (identity, run_id, + subfolder)`: locate the first directory segment matching + `RUN_ID_PATTERN` (`^\d{8}-\d{6}-[0-9a-f]{8}(-\d+)?$`, specific enough + that no identity segment matches it); identity is everything before, + subfolder everything after. `strip_run_id` becomes a thin wrapper + returning the identity, so its callers and tests hold. A path with no + run id (flat layout) returns its directory as identity and `""` as + subfolder, as today. + +### The two manifests + +There are two, and they record files differently: + +- `manifest.json` in the run directory records files relative to the *run + directory* (`manifest_relative_files`), so a foldered file appears as + `final/dialogue_short-assemble.0-0.0.mp4`. +- The job's manifest in `jobs.sqlite`, returned by `get_job`, records files + relative to the *output root* (`_relative_output_names`), so the same + file appears as `dialogue-short//final/dialogue_short-assemble.0-0.0.mp4`. + `job_for_file` matches on the tail of that name, so attribution of a + foldered file keeps working. + +Both already carry directory segments, so neither needs a schema change. +Both gain `subfolder` on each step entry so a consumer groups without +parsing paths. -``` +```text outputs/dialogue-short/20260911-205805-a1b2c3d4/ manifest.json workflow.json @@ -98,171 +161,174 @@ outputs/dialogue-short/20260911-205805-a1b2c3d4/ final/dialogue_short-assemble.0-0.0.mp4 ``` -The manifest already records each step's files as paths relative to the run -directory (`manifest_relative_files`), so a foldered file appears as -`final/dialogue_short-assemble.0-0.0.mp4` with no schema change to the -manifest at all. Each manifest entry additionally carries `folder` (the -empty string for an unfoldered step), so a consumer can group without -parsing paths. - ### `get_job` and `list_jobs` -`get_job`'s manifest gains the `folder` on each entry, which is additive. On -top of that, a finished job's reply carries: - -```json -"outputs": { "final": ["final/...mp4"], "intermediate": ["intermediate/...mp4", "..."] } -``` - -- a grouping of the manifest's files by folder, so "what did this produce" - is answerable without walking the manifest. A job whose workflow uses no - folders reports `{"": [...]}`, which is the honest answer rather than a - guess at which file is the deliverable. - -`list_jobs` stays a listing: it gains nothing per job, because a row that -carried file lists is what T018 just finished cutting down. The `total`/ -`truncated` shape is untouched. - -### The gallery listing (`GET /api/gallery`, MCP `list_gallery`) - -This is the surface where the decision above bites. The rule: - -- `folder` on a gallery entry stays **the workflow identity**, exactly as - today - `ltx2/Gyre`, not `ltx2/Gyre/final`. The folder filter continues to - group a workflow's runs, which is what it is for. -- A new field, `group` (`final`, `intermediate`, or `""`), carries the - in-run folder, and `GET /api/gallery` takes an optional `?group=` filter - beside the existing `?folder=`. -- `strip_run_id` is extended to drop everything after the run id rather than - just the file name: `ltx2/Gyre//final/x.mp4` -> `ltx2/Gyre`, and the - discarded segments become the `group`. Today's paths are unaffected - because they have nothing after the run id. - -This keeps the two axes separate - *which workflow* and *which part of the -run* - rather than multiplying them into one filter list that grows by a -factor of two for every workflow. - -### `asset:` and `previous_result:` - -- **`previous_result:` is unaffected.** It names a step, and results are - passed in memory between steps; where a step's files landed on disk has - never been part of it. A foldered step is read back exactly as it is now. -- **`output:` gains one segment.** The name is - `//`, where `` is already allowed - to be a relative path - `_resolve_segments` walks segments and - `validate_output_reference` permits `/`. So - `output:dialogue-short/latest/final/episode.mp4` works with **no change to - the resolver**, and the `latest` search (newest run that holds the file) - keeps working because it is a per-run existence check on the whole - remainder of the path. -- **`asset:` is unaffected.** The asset library is a separate tree and - already takes nested names (`asset:gyre/frames/web.mp4`). -- **`keep_output`** takes the output's relative name, which now may contain - the folder, and writes into the asset library under `asset_name` as - before. The output's folder is not copied into the asset name - the two - trees mean different things, and a promoted file is by definition final. +`get_job`'s manifest entries carry `subfolder`. **Nothing else is added** +(*decided*): an earlier draft bucketed the files into an `outputs` map, +which would have needed `""` as a JSON key for unfoldered steps and added +a second way to read what the entries already say. The tool description +carries the reading instead: *a step's `subfolder` says what kind of +output it is; by convention the deliverable is `final`*. + +`list_jobs` stays a listing and gains nothing per job - a row that carried +file lists is what T018 just finished cutting down. + +### The gallery (`GET /api/gallery`, MCP `list_gallery`) + +- `folder` on an entry stays **the workflow identity**, exactly as today - + `ltx2/Gyre`, not `ltx2/Gyre/final`. The folder filter continues to group + a workflow's runs, which is what it is for. +- Each entry gains `subfolder` (`final`, `intermediate`, `""`, or whatever + the workflow wrote), from `split_run_path`. +- `GET /api/gallery` takes an optional `?subfolder=` beside `?folder=`; the + reply's `folders` list is unchanged and gains a sibling `subfolders` + (distinct values over the whole tree, for the UI control). +- MCP `list_gallery(limit, subfolder=None)`. Today it takes only `limit`, + so this is a new parameter, not a mirror of an existing one. + +The two axes - *which workflow* and *which part of the run* - stay +separate rather than multiplying into one filter list. + +**Flat layout.** With `--output-layout flat` there is no run id to anchor +on, so `subfolder` is `""` and a foldered file's directory - `ltx/final` - +appears in the workflow filter. Accepted: flat is the legacy layout for +callers whose scripts glob the output directory, and the on-disk placement +is still honoured, which is what such a caller wants. + +### `output:`, `asset:`, `previous_result:`, `keep_output` - no change + +Verified against the code: + +- **`previous_result:`** names a step and results pass in memory; where a + step's files landed has never been part of it. +- **`output:`** names are `//` where + `` is already a relative path: `_resolve_segments` walks every + remaining segment and `validate_output_reference` permits `/`. So + `output:dialogue-short/latest/final/episode.mp4` resolves with no change, + and `latest` (newest run *holding the file*) keeps working because it is a + per-run existence check on the whole remainder. +- **`asset:`** is a separate tree that already takes nested names. +- **`keep_output`** / `POST /api/assets/keep` take the output's relative + name, which may now contain the subfolder, and write into the asset + library under `asset_name` as before. The subfolder is not copied into + the asset name - a promoted file is by definition final. +- `delete_output`, `get_output_image`, `download_output`, `/outputs/` + serving all take the whole remainder as a path. ### The web UI -One change, mapped onto what the gallery already has: beside the existing -folder filter (workflow identity), a second, smaller control for `group`, -defaulting to showing everything. A run that used no folders shows exactly -what it shows today. The job page groups the manifest it already renders -under the same headings. - -The editor's form for a `result` block gains `folder` the way it gains every -other schema field - from the schema, with no editor-specific work. +- Gallery: a second, smaller filter for `subfolder` beside the folder + filter, fed by `subfolders`, defaulting to everything. `grouping.ts` is + unchanged - the server still supplies `folder`. +- Job page: the manifest it already renders is grouped under headings by + `subfolder` when any entry has a non-empty one; unchanged otherwise. +- Editor: `result.subfolder` appears from the schema, no editor work. ### Breaking changes -**None over MCP or REST.** Everything above is additive: a new optional -workflow field, new keys on existing replies, a new optional query -parameter. Two things a scripted consumer should know: - -1. A manifest file name may now contain a `/` where it previously could not. - Anything that assumed a manifest entry was a bare file name - splitting - on `/` and taking the last part, or joining it to a directory by hand - - keeps working, but a consumer that wants the folder should read the - `folder` field rather than parsing the path. -2. `gallery` entries gain `group`; `folder` keeps its current meaning. A - consumer filtering on `folder` sees no change. +**None over MCP or REST, and none to existing workflows or workspaces.** +Everything is additive: a new optional workflow field, new keys on existing +replies, a new optional query parameter, no migration of `jobs.sqlite` or +of run directories. Three things a consumer should know: + +1. A manifest file name may now contain a `/` after the run id where it + previously could not. A consumer that wants the subfolder should read + the `subfolder` field rather than parse the path. +2. `file_base_name` containing a separator is refused at validation. It + failed at write time before, so no working workflow changes behaviour. +3. **The steering stage moves template outputs.** Once a template marks + steps `final`/`intermediate`, its files land in `/final/x.mp4`. + A user workflow chained off a *template's* output by run path + (`output: