Skip to content

validate_workflow: enforce candidates/scores and previous_result/for_each list-identity consistency #213

Closed

Description

@dkackman

Split from #208 (approved by Don, scoped by triage's edge-case list to keep it bounded).

tool/endpoint: validate_workflow

background: #119's hand-off comment claimed dw/select_validation.py enforces "candidates/scores gather-consistency," but what actually shipped is only a shape check (both gather: or both plain lists — select_errors line ~87). The claimed list-identity consistency was never implemented.

scope for this fix (per Don, to keep this from sprawling into every edge case triage flagged):

  1. select's candidates/scores gather-consistency: when a select step's candidates and scores are both gather: references, compare the resolved expansion set (post arguments override, not just static length) each reference drew from. Refuse if they don't match — the two for_each groups must have expanded from the same list. Report at steps[N].task.arguments.scores (or .candidates), naming the mismatch (e.g. "candidates has 2 entries, scores has 1").

  2. previous_result: into a for_each step: same rule from the other direction — a previous_result: reference into a for_each step must come from a for_each step over the same list. (The Deterministic N→1 selection between a fan-out and an expensive stage (select reducer + judge scorer) #119 error text already states this for the non-for_each case; this closes the gap for the for_each-into-for_each case the repro below exercises.)

Out of scope for this issue (real, but separate — don't fold in): a gather from a non-for_each step, a literal list vs variable: list distinction, member-scoped previous_result:. If those turn out to need their own rules, file separately once this lands.

repro:

{"id": "qa119_mismatch",
 "variables": {
   "candidates": [{"name":"a","prompt":"a red apple"},{"name":"b","prompt":"a green pear"}],
   "other": [{"name":"x","prompt":"a blue bowl"}]},
 "seed": 7,
 "steps": [
  {"name":"still","for_each":"variable:candidates","release_pipeline":true,
   "pipeline":{"configuration":{"component_type":"ZImagePipeline","offload":"sequential"},
     "from_pretrained_arguments":{"model_name":"Tongyi-MAI/Z-Image-Turbo","torch_dtype":"torch.bfloat16","low_cpu_mem_usage":true},
     "arguments":{"prompt":"item:prompt","num_inference_steps":9,"guidance_scale":0,"width":512,"height":512}},
   "result":{"content_type":"image/jpeg","subfolder":"intermediate"}},
  {"name":"judge","for_each":"variable:other",
   "task":{"command":"judge","arguments":{"image":"previous_result:still","rubric":"How red is this?","scale":[0,10],"device":"cuda"}}},
  {"name":"pick","task":{"command":"select","arguments":{"candidates":"gather:still","scores":"gather:judge","rule":"argmax"}},
   "result":{"content_type":"image/jpeg","subfolder":"final"}}]}

expected: valid: false before queueing, flagging both the judge step's previous_result:still (drawn from a different list than judge's own for_each:variable:other) and the pick step's scores/candidates size mismatch.

actual: valid: true with a plan; only diagnostic is an unrelated unused-field warning. The mismatch surfaces only at run time (after the fan-out), or not at all if select silently zips the shorter list.

Filed by triage per Don's approval to release this scoped version of #208.

Activity

  1. dkackman commented on Sep 18, 2026

    @dkackman
    OwnerAuthor

    triage: work — self-contained and already scoped/approved by Don as the bounded release of #208 (closed), so no escalation (model opus via provider anthropic). Not a duplicate: #208 is the parent, #119 shipped only the shape check (dw/select_validation.py, both-gather:-or-both-lists). Fix lands in dw/select_validation.py for rule 1 and beside previous_result_reference_errors (dw/previous_results.py) / dw/for_each.py's expansion (which already records source indices and the list each group came from) for rule 2 — the "resolved expansion set" the issue asks for is what expand_for_each produces after the caller's arguments are folded in validation_errors, so compare the expanded member name sets (shot@x) rather than static lengths. Respect the issue's out-of-scope list; file new issues for anything outside it. Kept separate from #212 (lighter, different file) so this session has its full budget.

  2. dkackman commented on Sep 18, 2026

    @dkackman
    OwnerAuthor

    triage: work — re-confirmed on 2026-09-18 re-triage; nothing on develop or any branch touches it yet (git log develop / git branch -a show no #213 work). The earlier triage comment's pointers stand (dw/select_validation.py for rule 1; dw/previous_results.py + dw/for_each.py expansion for rule 2, comparing expanded member-name sets post-arguments). Not a duplicate of #212 — different files; the #212 branch does not overlap this one. (model opus via provider anthropic)

  3. dkackman commented on Sep 18, 2026

    @dkackman
    OwnerAuthor

    Fixed on develop (commit f6eba42, merged bf6cc58), deployed to lem 2026-09-18 12:47 UTC.

    Rule 1 (candidates/scores gather-consistency) — fixed. select_errors in dw/select_validation.py never compared resolved gather-expansion sizes: by the time it runs (after for_each expansion in Workflow.validation_errors()), a gather:<group> reference has already been rewritten by _gather() (dw/for_each.py) into an explicit list of previous_result:<group>@<key> strings — so the pre-existing _is_gather() literal-string check never fired on a real, integration-level call. Added _is_expanded_gather() plus a size-comparison branch that operates on that post-expansion list shape, reporting at steps[N].task.arguments.scores (or .candidates) with a message naming both sizes, e.g.:

    select: 'candidates' and 'scores' gather from for_each groups of different sizes: candidates has 2 entries, scores has 1.

    Verified against a purpose-built repro isolating rule 1 (judge step referencing a fixed asset instead of previous_result:, so only the gather-size mismatch trips) — empty [] before the fix, single correctly-worded error after. Two new unit tests added in tests/test_select_validation.py. Full suite: 5041 passed, 16 skipped, no regressions.

    Rule 2 (previous_result: into a for_each step must come from a for_each over the same list) — already covered, no change needed. This is _rewrite_reference in dw/for_each.py, which already raises a ForEachError when a previous_result: reference names a for_each group whose entries differ from the referencing member's own entries. git log -S traces this to commit 3b7e856, predating both #119 and this issue's filing. I ran the issue's own repro JSON (qa119_mismatch) directly against current develop and got valid: false, with:

    'still' names the for_each step 'still' from inside for_each step 'judge', which runs over a different list...

    This contradicts the issue's stated "actual: valid: true" — that must reflect an earlier revision. No code change made for rule 2; closing it out as already resolved.

    Note on the issue's original repro: because for_each expansion is fail-fast in Workflow.validation_errors() (an expansion failure short-circuits and returns only that one error), a single repro that trips rule 2 cannot also surface rule 1 in the same validation call — they were verified with two separate repros for that reason.

    Proposed regression-suite-smoke case:

    • Call validate_workflow on a workflow whose select step gathers candidates from a for_each group of size 2 and scores from a for_each group of size 1 (both groups otherwise valid, no rule-2 violation).
    • Expected: valid: false, one error at steps[N].task.arguments.scores, message containing "different sizes" and both entry counts ("2 entries", "1").

    Model/provider: sonnet / anthropic.

  4. dkackman commented on Sep 18, 2026

    @dkackman
    OwnerAuthor

    Verified over MCP (validate_workflow, live dw server on lem) — model opus via provider anthropic.

    Ran 5 calls:

    1. Issue's original repro (qa119_mismatch) → valid: false, one error at steps[1].task.arguments.image: "'still' names the for_each step 'still' from inside for_each step 'judge', which runs over a different list. Use 'gather:still' …". Rule 2 fires. Consistent with the implementer's fail-fast note — rule 1 can't co-surface in the same call, so it was checked separately.
    2. Rule 1 isolated (judge reads asset:qa-cast/hal-portrait.jpg instead of previous_result:; candidates group 2, scores group 1) → valid: false, one error at steps[2].task.arguments.scores: "select: 'candidates' and 'scores' gather from for_each groups of different sizes: candidates has 2 entries, scores has 1."
    3. Control (both groups size 2) → valid: true, plan list_entries: {candidates: 2, other: 2}, only the unrelated unused-prompt warnings. No false positive.
    4. arguments override path (static lists 2/2, arguments: {"other": [one entry]}) → valid: false, same rule-1 error with "candidates has 2 entries, scores has 1". The issue's "resolved expansion set, post arguments override, not just static length" requirement holds.
    5. Mirror (candidates 1, scores 2) → valid: false, error at .scores: "candidates has 1 entries, scores has 2". Reported at .scores in both directions rather than .candidates — fine per the issue's ".scores (or .candidates)" wording; noting it only so nobody expects the path to flip.

    Both rules match expected:. Adding the implementer's proposed smoke case as S-F055 (case 2 above, plus the override variant). Closing as verified.

  5. added
    status:verifiedTester confirmed the fix via a real MCP call
    and removed on Sep 18, 2026
  6. added 2 commits that reference this issue on Sep 18, 2026
    f6eba42
    8e1398e
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

    bugSomething isn't workingowner: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