Conversation
Clarify unsupported Iterate readiness versus materializer ownership. Add regression coverage for the generic and forced compatibility scheduler split.
Hide runtime ledgers from public serialization while retaining explicit persistence dumps. Make failed pending workflow calls terminal and remove branch-local generated frontend artifacts.
- Keep internal execution ledgers out of the public schema while preserving durable snapshots.\n- Make workflow-call publication and sibling completion transactional.\n- Align execution-engine documentation and focused regression coverage.
Remove unused generic child queue scaffolding and align execution-engine documentation with the current runtime.\nPreserve the existing queue adapter and frontend/backend contract boundaries.
Treat lifecycle effects as pending only while their execution is unfinished. Preserve durable completed-call effects and cover sequential workflow-call reload.
Preserve the refactored graph implementation while adopting main's invocation module discovery.
Review: PR #217 "Execution engine refactoring" (head 1ce2ffc, base bdfa4f4)Two High findings: graphs that main runs to completion now stall or crash when an If sits inside a For/Iterate body. I would not merge until those are fixed. The nested-If and Collect-order findings (3 and 4) change behaviour and need either a fix or a documented decision. FindingsHigh1. An If with a constant condition inside a loop body stalls the session with no error
2. An If whose condition comes from outside the loop, with branches inside the body, crashes with
Medium3. Nested If: a side consumer of the middle If is silently skipped
4. Collect output order changes for admitted fan-in shapes and differs at the admission boundary
5. Persisted sessions are much larger and slower to load, and webv2 queue listings load every row
6. Per-item scheduling is about 1.4x to 3x slower, and the scaling test is narrower
Low7. The quarantine catch misses non-ValueError load errors, so a single bad row stalls the queue (not a regression)
8.
9. The new facade shims turn the usual wrap/spy pattern into infinite recursion
10. Every Call Saved Workflow call keeps a full copy of the workflow in the parent session, never pruned
11. Call Saved Workflow failures lose their exception type and traceback
Open Questions
|
|
@Pfannkuchensack Thanks for reviewing. Findings 1 and 2 are confirmed merge blockers. High findings
Medium findings
Low findings
Open questions
|
Summary
This is a backend execution-engine refactor. The goal is to make workflow execution easier to extend and maintain by giving each control-flow type clear ownership of how it schedules its next work, without changing the frontend/backend contract.
invokeai/app/services/shared/.If,For,Iterate/Collect, nested control flow, and saved-workflow execution through generic execution paths.invokeai/frontend/...changed.This incorporates the linear
Forscheduling requirements from #225:Forcollection between iterations without quadratic deep-copying.ForandIterate.The implementation is adapted into this branch rather than being an exact cherry-pick of PR 225.
Deferred work:
I deferred removing the old control-flow-specific scheduling and materialization paths that are still needed for unsupported graphs, forced compatibility mode, and legacy snapshots.
Each removal must first prove that the generic execution path produces identical behavior, persistence, recovery, and queue lifecycle results. The frontend contract must remain unchanged, and every deletion requires focused tests and adversarial review.
This PR is already large, and I decided to hit the brakes on more changes.
Related Issues / Discussions
Closes #225: linear
For-loop scheduling.QA Instructions
Passed at the time of writing:
Contract checks:
invokeai/frontend/...changes.Manual testing:
Load existing workflows with
For,Call Saved Workflow, andIteratenodes and make sure they execute as before. Then, create similar ones from scratch and see that behavior is preserved.Merge Plan
Checklist
What's Newcopy (if doing a release after this PR)