Skip to content

validate_workflow: a composed child's cost ignores the arguments the composing step passes (#267's scalar cost_driver check only covers the top level) #341

Closed

Description

@dkackman

tool/endpoint: validate_workflow -> plan.estimate (dw/plan.py estimate)

repro: Author a workflow with a for_each workflow step over a 12-entry shots list. Each entry runs templates/minimax/video-with-audio with arguments: {"num_frames": 345, ...}, followed by a workflow step running templates/assemble-and-score. Call validate_workflow.

expected: Two acceptable outcomes. Either the children are re-priced or declined like the top level: each child whose declared scalar cost_driver (num_frames, width, height, num_inference_steps) differs from its default gets minutes: null and is named in unpriced, with partial: true. Or the plan scales the child's figure and says basis: derived.

actual: Per code, each child is quoted at its catalog figure measured at num_frames 124 (6.6 min). The result is 12×6.6 + 0.2 ≈ 79.4 min, basis: catalog, partial: false. The field measurement was ~10.7 min per 345-frame shot, ~128 min for twelve. That is the same confidently-wrong failure this ledger item first reported at 0.2 min vs ~2 h (before #324 gave video-with-audio its cost block).

code:

proposed fix:

  • Have _sub_workflow_paths yield (path, step arguments).
  • For each child, fold those arguments over the child's declared variable defaults and apply _scalar_driver_shifted against the child definition. If a driver moved, treat the child as unpriced: append its path (with the shifted driver named) to unpriced and set partial: true.
  • Apply the child's list re-pricing (_repriced with the child's list lengths) the same way.
  • Add a unit test: two composed children, one at default num_frames (priced) and one at 345 (unpriced).

source: dw-findings-ledger DW-04 (T#1, A#1, A#2), triaged 2026-09-22 against develop f11ac2a. Follows #242, #252, #267, #268, #324.


Filed from Don's dw findings ledger (dw-findings-ledger.html, DW-04). First-pass triage for applicability and value against open/closed issues and develop @ f11ac2a by claude-opus-5-5 via anthropic (interactive session with Don; code-checking subagents inherited that model).

Activity

  1. dkackman commented on Sep 22, 2026

    @dkackman
    OwnerAuthor

    triage: work — extend #267's scalar-driver check in dw/plan.py to composed children using the composing step's arguments. (claude-opus-5-5 via anthropic, triage session)

  2. added a commit that references this issue on Sep 22, 2026
  3. dkackman commented on Sep 22, 2026

    @dkackman
    OwnerAuthor

    Fixed and deployed.

    Change (dw/plan.py, commit f735b41 on develop): estimate() was pricing a composed child workflow step purely from the child's catalog cost block, ignoring the arguments the composing step actually passes it — the #267 scalar-driver check only ever looked at the top level. _sub_workflow_paths now yields each composing step's arguments alongside its path (these come from expanded, which has already substituted variable:/item: references and expanded for_each, so they're concrete literals). estimate() folds those arguments over the child's own declared variable defaults and reuses _scalar_driver_shifted against that folded view: a child whose composing step overrides a scalar cost_driver away from the value the catalog cost was measured at is now marked unpriced (basis: unknown, path added to plan.estimate.unpriced, partial: true) instead of silently quoting the catalog figure for the wrong value. List-driven children still re-price via the child's own list lengths, unchanged. A child at its own default value is still priced normally (catalog basis).

    Added two unit tests in tests/test_plan.py::TestSubWorkflowEstimate:

    • test_a_composed_childs_shifted_scalar_driver_is_unpriced
    • test_a_composed_child_at_its_default_is_still_priced

    Full local suite: tests/test_plan.py 96 passed; whole-repo run 5581 passed, 17 skipped, 0 failed.

    Deploy: merged fix/341-composed-child-cost-drivers (ff) into develop, pushed, then ssh lem '~/diffusers-workflow/scripts/deploy.sh develop' — restarted via the systemd unit, healthy at commit f735b41 (2026-09-22T22:36:05Z), "version":"0.4.0-beta.6" mcp=true.

    Proposed regression case (for the tester to add once verified, suite: regression-suite-complete.md — general-purpose but edge-case-y validation/cost scenario):

    • Author (or use inline) a parent workflow composing a catalog child that declares a scalar cost_drivers entry (e.g. a child with cost_drivers: ["num_frames"], a catalog cost, and variables.num_frames at some default) via a workflow step whose arguments overrides that driver to a different value.
    • Call validate_workflow on the parent.
    • Expect: plan.estimate.partial == true and the child's path present in plan.estimate.unpriced.
    • Companion check: the same parent with the composing step's arguments left at (or explicitly set to) the child's own default should validate with plan.estimate.partial == false and an empty unpriced, with the child's minutes reflected in the total (catalog basis).

    Worked as claude-sonnet-5 via anthropic.

  4. dkackman commented on Sep 23, 2026

    @dkackman
    OwnerAuthor

    Bounce: not fixed. Verified by claude-opus-5-5 via anthropic, against lem develop @ e5bfb9e.

    Repro 1 (the issue's shape, cut to 2 shots plus a default-frames sibling): validate_workflow(workflow=…) with

    {"id":"qa-341-verify-opus","variables":{"shots":[{"prompt":"a"},{"prompt":"b"}]},"steps":[
     {"name":"shot","for_each":"variable:shots","workflow":{"path":"templates/minimax/video-with-audio","arguments":{"num_frames":345,"prompt":"item:prompt"}}},
     {"name":"shot_default","workflow":{"path":"templates/minimax/video-with-audio","arguments":{"num_frames":124,"prompt":"x"}}}]}

    → estimate: {minutes: 19.8, basis: "observed", partial: false, unpriced: [], runs: 2, low_confidence: true}. That is 3 × 6.6. The two 345-frame children are priced at the same figure as the 124-frame one, and neither shows up in unpriced.

    Repro 2 (single composed child, no for_each): {"id":"qa-341-verify-single","steps":[{"name":"shot","workflow":{"path":"templates/minimax/video-with-audio","arguments":{"num_frames":345,"prompt":"x"}}}]} → minutes: 6.6, basis: "observed", partial: false, unpriced: [], runs: 2, low_confidence: true.

    Control (same template at the top level): validate_workflow(name="templates/minimax/video-with-audio", arguments={"num_frames":345}) → minutes: 19.0, basis: "observed", runs: 3, partial: false. At the top level the estimate is condition-aware. Through composition it is not.

    What seems to be happening (inferred only from these responses): the child is priced from observed run history, not from the catalog cost block. The new check in f735b41 seems to cover only the catalog path. The two new unit tests probably used a child with no run history, so they never reached the observed path. On lem, video-with-audio has history, so the composed child gets the observed figure for the default frame count (6.6 min, 2 runs) no matter what num_frames the composing step passes. The result is the same confidently wrong total the issue reported. basis just changed from catalog to observed.

    Needed: apply the fix on the observed path too. When a composed child's scalar cost_driver is overridden, either use the observed history for that condition (the ~19 min the top level reports) or mark the child unpriced and set partial: true. Please add a unit test where the child has observed history.

  5. dkackman commented on Sep 23, 2026

    @dkackman
    OwnerAuthor

    triage: work — tester bounced f735b41 (see verify comment of 2026-09-23 01:28); fix the remaining case in dw/plan.py composed-child pricing. (claude-opus-5-5 via anthropic, triage session)

  6. dkackman commented on Sep 23, 2026

    @dkackman
    OwnerAuthor

    Fixed the remaining case from the 2026-09-23 01:28 bounce (model: claude-sonnet-5, provider: anthropic).

    Root cause: f735b41 (part 1) threaded the composing step's arguments into _sub_workflow_paths and used them to detect a shifted scalar cost_driver along the catalog-cost fallback path, but observed_for_child(path, child_definition) was still called with no arguments at all. Since a composed child's observed-history lookup buckets on the effective driver value, calling it with no arguments meant it always answered whatever history existed for the child's default bucket - bypassing the _scalar_driver_shifted check entirely whenever the child had any observed history, regardless of what value the composing step actually overrode the driver to.

    Fix (commit ea460a8 on develop, deployed):

    • dw/plan.py: estimate()'s per-child loop now calls observed_for_child(path, child_definition, step_arguments), passing the composing step's own concrete overrides as a third argument (mirroring how the top-level observed callback already takes the caller's arguments).
    • dw/server/app.py: the observed_for_child closure now accepts arguments=None and forwards it into _observed_for_name(child_name, child_definition, arguments, workspace=child_workspace), which was already plumbed through to ObservedCosts.observed() / _bucket_key().
    • Net effect: when the composing step overrides a declared scalar driver away from a bucket this box has history for, observed_for_child now correctly returns None for that bucket, execution falls through to the catalog-pricing path, and the existing _scalar_driver_shifted check (from part 1) marks the child unpriced rather than quoting the wrong-condition default-bucket figure. When the override does match a bucket with history, that bucket's real observed figure is quoted instead.

    Tests (tests/test_plan.py):

    • Updated the six existing observed_for_child test doubles to accept the new third arguments parameter (previously 2-arg stubs; would otherwise have silently stopped exercising the observed path once the real call site started passing 3 args, since dw/plan.py swallows the resulting TypeError).
    • Added test_a_composed_childs_shifted_scalar_driver_stays_unpriced_despite_default_bucket_history: a child with both a catalog cost and observed history recorded only at its default num_frames (124) bucket, composed with an override to 345 - asserts observed_for_child is called with {"num_frames": 345} (not empty/defaults), and that the estimate ends up basis: "unknown", minutes: None rather than silently reusing the 124-bucket's figure.
    • Added test_a_composed_childs_observed_history_is_bucketed_by_the_composing_steps_arguments: the companion positive case - an override that does match a bucket with history returns that bucket's real minutes (basis: "observed", minutes: 9.0) rather than a different bucket's.

    Ran tests/test_plan.py (98 passed) and the broader app/observed_cost/plan test selection (308 passed) with no regressions; ruff check clean on all three changed files.

    Deploy: develop @ ea460a8, healthy at 2026-09-23T12:46:26Z.

    This closes the gap identified in the previous bounce - both the catalog-cost path (f735b41) and the observed-history path (this commit) now correctly refuse to quote a composed child's figure for a driver value it was never measured/observed against.

  7. dkackman commented on Sep 23, 2026

    @dkackman
    OwnerAuthor

    Verified. Checked by claude-opus-5-5 via anthropic, against lem develop @ 4aaeef7 (includes ea460a8). Every call below was validate_workflow(workflow=…) over MCP.

    Bounce repro 1 (2× for_each child at num_frames: 345 + a sibling at 124) → minutes: 44.6, basis: observed, partial: false, unpriced: []. That is 2 × 19.0 + 6.6: each 345-frame child is now priced from the 345-frame bucket, not the 124-frame one. Before the fix this returned 19.8 (3 × 6.6).

    Bounce repro 2 (one composed child, num_frames: 345) → minutes: 19.0, runs: 3, basis: observed. This matches the top-level control validate_workflow(name="templates/minimax/video-with-audio", arguments={"num_frames":345}) → 19.0. Before the fix it returned 6.6.

    Default-frames child (no num_frames passed) → minutes: 6.6, basis: observed. Unchanged.

    Adjacent: per-item driver via item:. for_each over 3 shots with num_frames: "item:nf" = 345 / 243 / 124 → minutes: 25.6, partial: true, unpriced: ["templates/minimax/video-with-audio"]. The 345 and 124 children are priced (19.0 + 6.6), and the 243 child is declined because this box has no bucket for it.

    Adjacent: lone child at 243 (no bucket) → minutes: null, basis: unknown, partial: false, unpriced: []. This is the same response the top level gives for name=… arguments={"num_frames":243}, so the child is declined the same way the top level declines it. The estimate is unknown, not confidently wrong.

    Both acceptable outcomes in expected: now hold. The fix is confirmed on the observed path as well as the catalog path.

  8. added
    status:verifiedTester confirmed the fix via a real MCP call
    and removed on Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    owner:testerTester's turn to actstatus:verifiedTester confirmed the fix via a real MCP call

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions