Stream lookahead: concurrent remote/API step execution for video workflows (10-12x FPS, outputs identical to sequential)

[balthazur]

Stream lookahead: concurrent remote/API step execution for video workflows, with strict frame ordering for stateful steps

Note

Started as an exploration POC for the video-streaming workflows effort (follow-up to the remote-execution benchmarking discussed in #p-video-streaming-workflows and to #2538); now hardened through three adversarial reviews and the repo's .claude/skills review guidelines, and open for review. The feature is opt-in and default-off; the default path is behavior-identical to main. Profiling-skill trace per the perf-PR requirement is being produced and will be shared alongside.

Video workflows in remote execution mode — the mode the app's video previews will run on, and the substrate for any video processing service that doesn't own a GPU — today process one frame per round trip: the whole pipeline waits ~200-600 ms of network for every frame, capping throughput at 2-4 FPS regardless of how fast serverless actually is. This PR teaches the Execution Engine to keep N frames' remote model requests (and external-API calls like Gemini/Claude/OCR) in flight concurrently while trackers, counters, and sinks still see frames strictly in stream order — 10-12x FPS (3.5 → 44 on a laptop, 5 → 60 colocated), with outputs bit-identical to sequential execution, behind a default-off env var.

The mechanism is a two-pass scheduler over the existing compiled DAG (blocks declare two facts, the engine does the rest — details below); it composes with, and does not touch, the local RF-DETR stream pipeline, and it is the "parallel frame processing in the context of full workflow execution" direction the core team has already declared, built on the per-block statefulness metadata Pawel proposed.

What this does

With the new WORKFLOWS_STREAM_LOOKAHEAD_DEPTH=N (default 1 = off; old name WORKFLOWS_REMOTE_EXECUTION_PIPELINE_DEPTH honored as fallback), the stream scheduler keeps up to N frames' long-latency step executions in flight concurrently — remote model requests in remote execution mode, and external-API calls (Gemini, Claude, GPT, OCR services) in ANY execution mode — while stateful steps (trackers, counters) run strictly in frame order on each frame's own inputs. Tracking output is bit-identical to sequential execution (tracker-ID parity verified in every benchmark).

Architecture: the engine owns all mechanics; blocks declare two facts

1. WorkflowBlockManifest.is_stateful_for_video_processing() (default: stateful, conservative) — whether the block must observe frames in stream order. One override on the visualization base covers the stateless visualizers; trace and heatmap explicitly remain stateful. Dynamic Custom Python manifests carry no declaration and are always treated as stateful.
2. WorkflowBlock.is_async_stream_step() (absent/false by default) — the block certifies its run() is a long-latency, re-entrant call the scheduler may execute ahead of stream order. That is the entire per-block surface: 1-3 lines.

The engine does everything else, generically: it partitions the compiled DAG once into a frontier (steps whose ancestors are all stateless; async steps are launch-only sinks) and a remainder; per frame, a deferred pass runs the frontier — assembling each async step's inputs on the stream thread, executing its whole run() on a lookahead worker pool, and registering per-output future placeholders built from the block's own describe_outputs() — and buffers the frame's live ExecutionDataManager. At ordered emission, a resume pass runs the remainder on that same EDM; the pre-existing future resolution at input assembly performs the ordered wait. Guards (all falling back to sequential with a warning): async step outside the frontier (stateful-fed, chained-async), multi-source pipelines, mixing with the local RF-DETR pipeline (untouched), missing execution graph, scalar/wildcard-output/dimension-offset steps.

Adoption: 59 blocks — every Roboflow model family and version (object detection v1-v3, instance segmentation v1-v4, keypoints v1-v3, both classification families v1-v3, semantic segmentation, SAM2, SAM3 family) and all external-API blocks (Claude v1-3, OpenAI v2-4, Gemini v1-3, Gemma, Llama-Vision, Qwen family, OpenRouter variants, Moondream2, SmolVLM, OCR/EasyOCR/GLM-OCR, seg-preview, LMM-classifier, visual-search). Excluded with reasons: wildcard-output blocks (openai v1, lmm), deprecated blocks (cogvlm, gaze), stateful video trackers (sam2/sam3 video), all sinks (side effects), scalar blocks pending batch support (clip, perception-encoder, google-vision-ocr, stability-ai), and openai_compatible (mutates a client cache in the request path — needs a fix first).

Benchmarks (all numbers from this branch's final architecture)

vehicles-2.mp4, 300 measured frames (SAM3: 100), rfdetr-nano on serverless.roboflow.com, laptop ~100 ms RTT. Reproduce: development/benchmark_scripts/benchmark_remote_stream_pipeline.py --workflow tracking|sam3|two-models|preprocessed-tracking.

Workflow (640x360)	Sequential	Depth 8	Depth 16
tracking (model -> ByteTrack -> viz)	3.52 FPS	35.5 (10.1x)	43.8 (12.4x)
preprocessed-tracking (stateless crop + side branch)	3.79 FPS	37.6 (9.9x)	45.0 (11.9x)
two models -> consensus -> ByteTrack	2.14 FPS	22.8 (10.7x)	—
SAM3 text-prompt segmentation -> ByteTrack	1.51 FPS	12.7 (8.4x)	—

• Tracker-ID parity: 0 mismatched frames in every pipelined run, all shapes, emissions always in frame order.

us-central1 worker (n2-standard-2, colocated with serverless, ~12 ms RTT), tracking workflow, 300 frames, this exact architecture:

Clip	Sequential	Depth 8	Depth 16
640x360	5.0 FPS	59.6 (11.9x)	56.5 (11.3x)
1920x1080	3.6 FPS	—	33.9 (9.3x)

Parity 0/300 mismatched frames on the worker too. Laptop (~100 ms RTT) and colocated worker converge on similar pipelined throughput — pipelining makes throughput nearly RTT-independent (the ceiling becomes serverless capacity, not distance).

• Reference ceilings: main sequential 2.9 FPS; stateless-parallel c16 hack (broken tracking semantics) 70.4 FPS laptop / 38.7 worker.
• Re-validated on the review-hardened head (laptop, same clip): tracking 3.33 -> 33.3 (d8) -> 39.0 FPS (d16), SAM3 1.61 -> 15.1 (d8), parity 0 mismatches everywhere; scheduler-overhead bench net -0.15 ms/frame — the review fixes are perf-neutral (run-to-run deltas are network variance).
• Rule of thumb depth ~ target_fps x per-request latency; sensible default 8-16. First emission is delayed by buffer fill (~depth/fps) — relevant for live streams, irrelevant for files. Heavier hosted models (SAM3) saturate at lower depth.

Scheduler overhead (benchmark_remote_stream_pipeline_overhead.py, mocked instant HTTP, production log level): the two-pass machinery costs ~+0.1 ms/frame in isolation (zero-encode control, size-independent); in practice the lookahead pass measures 0.1-0.2 ms/frame FASTER than the sequential pass because the frame's GIL-releasing JPEG encode moves off the scheduling thread (the effect scales with encode cost: ~0.7 ms/frame on high-entropy 640x360 content). Compilation is untouched (compiler diff vs main is empty). An earlier ~1 ms/frame figure was a DEBUG-logging artifact (dev .env; asymmetric log-line counts) and is retracted. Client re-instantiation per request: 0.005 ms (the historical cost was TLS sessions, solved by #2538, which the long-lived pool threads compound).

Multi-pipeline: two concurrent pipelines in one process — both pipelined, byte-identical tracker IDs across pipelines (no shared state).

Context: why this is not the depth-2 null result

The team's RFDETR_PIPELINE_DEPTH=2 experiment concluded frame pipelining "didn't give any speedup once the futures were materialized" (Damian; nsys showed ~13k cudasync calls vs 406 — every materialization forced a same-frame CUDA stream sync). That regime was local-GPU compute-bound: ~15% hideable time. This PR targets the opposite regime: remote workflow execution spends ~85-92% of every frame waiting on the network (~30 ms server compute inside 180-420 ms frame time, measured). Materializing our future is a Future.result() on an HTTP response that arrived depth / fps seconds earlier — a no-op read, not a GPU sync. Same technique, opposite physics: ~1.0x there, 8-12x here.

Failure & operational semantics

A failed request surfaces at its frame's emission (SDK retries apply first). On end-of-stream drain, remaining buffered frames are emitted best-effort and any emission failure is re-raised afterwards (StreamLookaheadDrainError, carrying the frames that did drain) so a failed tail frame surfaces through the normal inference error path instead of disappearing as an empty drain. Offloaded run() calls execute with the stream thread's execution/debug/OTel context rebound and failures wrapped in StepExecutionError with a BlockTraceback, matching the sync path; batch-contract violations (wrong output count) fail at the model step with counts, not as an index error on the consumer. Hung-request lifecycle: pool threads are daemon and close() detaches them from concurrent.futures' interpreter-shutdown join (which ignores the daemon flag) — verified a 30 s hung request no longer blocks process exit. Future resolution at resume has a WORKFLOWS_ASYNC_FUTURE_RESULT_TIMEOUT (default 60 s) deadline that sequential mode doesn't have: a degraded backend that stalls a single request past it ends the stream rather than just slowing it — raise the env var for flaky links (see Follow-ups for SDK-side request timeouts, which sequential lacks too).

Usage accounting caveat (flagging explicitly for the usage-tracking owners): usage is recorded exactly once per frame with api key/workflow id/fps intact, but execution_duration in steady state reflects the resume-pass compute, not the remote inference wall time — that wait is hidden behind newer frames' work, which is the point of the lookahead. Any consumer of per-frame duration (duration minimums, billing analytics) sees smaller values for lookahead streams than sequential ones doing identical work.

Memory: depth x (frame + frontier outputs) buffered (~100 MB at depth 16/1080p). Known cosmetics: profiler counts two run-brackets per frame; watchdog started/ready pairing skews under buffering (shared with RF-DETR). Nothing outside the stream runner offloads — one-shot HTTP run() calls never enter the lookahead path even with the env var set (verified: run_workflow never passes async selectors).

Versioning & repo review-guidelines compliance

Audited against the repo's own .claude/skills/ review suite (review-workflows-execution-engine, review-topic-concurrency-and-resource-safety, review-topic-workflow-state-management, review-topic-backward-compat-and-versioning, review-topic-test-hygiene, review-workflows-blocks, review-topic-local-vs-remote-execution, review-topic-external-contract-and-silent-fallback):

• EE version companions: EXECUTION_ENGINE_V1_VERSION bumped 1.12.0 -> 1.13.0 (new capability, minor), changelog entry added to docs/workflows/execution_engine_changelog.md, and both mirrored version-assertion tests updated (test_workflow_endpoints.py, hosted_platform_tests/test_workflows.py). Default run(...) behavior is unchanged with the env var unset. inference/core/version.py bump deferred to release per the skill's carve-out.
• Concurrency rules: futures resolve through the shared executor/utils.py helpers with WORKFLOWS_ASYNC_FUTURE_RESULT_TIMEOUT; the lookahead pool is injected (never created inside run()), bounded, has an explicit close() AND a weakref.finalize GC fallback (the Add weakref finalizers for RF-DETR stream pipeline executors #2491 pattern), and max_concurrent_steps is honored.
• Block rules: all block edits are additive classmethods or in-place bug fixes (no type/kind/I-O changes, so no vN+1 siblings needed); output contracts verified across all 61 declared blocks; no new kinds/loader wiring.
• State rules: the new statefulness declaration complements (does not replace) get_restrictions(); default is conservative-stateful; Custom Python dynamic blocks are treated as stateful.

Tests

25 focused test functions / 51 parametrized cases owned by this PR (plus main's pre-existing suites restored and passing): async-step eligibility matrix, frontier shapes (incl. the Custom-Python-manifest case), generic offload with per-output futures, run/resume round trip (no step executed twice), wrap-selection matrix (incl. RF-DETR regression + all fallbacks), runner ordering/drain/error-isolation, gating, statefulness metadata, block declarations — and the adversarial reversed-delay real-ExecutionEngine test asserting frame order, per-frame payload identity, and tracker parity against a sequential reference. Review-hardening additions: drain-error propagation at runner + pipeline level, worker-side context rebinding, usage-decorator placement, daemon pool threads, class_names copy semantics for SAM3 v1/v2/seg_preview, buffered-dispatch gating keyed on the actual handler (not the depth env var), and a registry-wide adoption guard (no per-video-stateful block may be declared stateless-for-lookahead).

Follow-ups

• Scalar A2 blocks (clip, stability-ai, google-vision-ocr) need batch support before declaring; openai_compatible needs its client cache made thread-safe.
• A stateless: true declaration for Custom Python blocks (aligned with the sandbox/WS execution work) — until then they are stateful by construction.
• Per-model depth guidance; serverless batch endpoint would compose (B frames/request x D in flight); stateful/stateless workflow-breakdown analysis from platform data (Pawel's ask, 2026-06-25).
• Profiling-skill trace file to be attached per the new perf-PR requirement before undrafting; coordinate with Try to build jetson-utils for JP 5.1 #2357 (tensor-native variants would mirror the two declarations).
• Full live-model depth>1-vs-depth-1 parity matrix over every adopted block (post-merge — needs model-serving/mocked-HTTP infra per family). The declaration-level slice is landed: a registry-wide guard asserts no per-video-stateful block is declared stateless-for-lookahead (the automated net for the class_names class of mis-adoption; 26/211 loaded blocks are subjects). A third review also ran 7 previously-unexercised families (keypoints, both classifications, instance-seg v1/v2/v4, semantic-seg, OpenAI v3) through real lookahead streams with reversed completion order and both serialization modes: 16/16 exact parity.
• Client-side request timeouts in the SDK executors (session.post / requests.post carry no timeout=): a dead TCP peer hangs a request forever in BOTH sequential and lookahead modes today; lookahead's exit-safety now contains the blast radius, but per-request socket timeouts are the real fix and belong in inference_sdk.
• Align the steady-state execution_duration semantics (see the accounting caveat above) with the usage-tracking owners before GA.

How this PR got here (3 iterations)

1. v1 — flush-based per-block shim: the OD block itself queued remote requests and emitted buffered frames through the existing flush_stream_pipeline path. Worked (13.6x on the worker), but each block needed ~40 lines of shim and the block owned scheduling.
2. v2 — frontier + EDM checkpoint/resume: introduced the per-block statefulness metadata and the DAG-partition scheduler (deferred pass buffers the frame's live ExecutionDataManager; resume pass runs the remainder at ordered emission). Blocks still carried a request-queueing shim.
3. v3 (this PR) — engine-generic offload: the engine offloads any declared step's whole run() and builds output futures from describe_outputs(). The shared shim was deleted; per-block surface shrank to the two declarations; 59 blocks adopted. Each iteration was benchmarked against serverless with tracker-parity checks, and the design went through several adversarial review passes, including three independent external agent reviews: four lifecycle/observability findings (tail-frame drain error swallowing, async-step context loss, usage-duration timing, non-daemon pool threads) fixed in d2f4a36b4; one shared-scalar mutation finding (SAM3 v1/v2 + seg_preview class_names) fixed in 7b4801a9e; and the third review proving the daemon-only lifecycle fix insufficient (concurrent.futures joins pool workers at shutdown regardless of daemonness) plus a late-blamed batch-contract failure — both fixed in 1e86fa6b6. Details and each review's cleared-attack record (output contracts across all 61 declared blocks, 7-family lookahead parity incl. serialization, re-entrancy hammer, thread-leak cycles, frontier guards) are in the review-response comments below.

Written by Claude Fable 5. Generated with Claude Code

[github-actions]

👋 Thanks for the pull request! Here is how automated Claude review works here, so you spend credits (and reviewer time) wisely.

🚦 This PR is marked Ready for review, so automated Claude review will run — and every pass spends real credits.

Warning

💸 The Claude reviewer bills in credits, not vibes

Automated review spins up a real agent that reads real code and spends real credits on every pass. It is glad to help — but it is not a rubber duck, a linter you poke in a loop, or a substitute for reading the contributing guide. Treat it like an expensive senior reviewer whose time you booked, and show up prepared.

Draft when unsure, Ready when you mean it:

• 🌱 Not sure the PR is in good shape yet? Keep it (or set it back) as a draft — drafts pause review, so you can push and iterate without burning credits on a moving target.
• 💪 Feel strong about the contents? Mark it Ready for review and the reviewer will take a look.

However you get there, arrive prepared:

• 🧱 Bring a SOLID, thorough PR. Point your local agent at our skills/ to tune it to our guidelines first — or, if you are one of those fabled carbon-based contributors, read them yourself. A half-baked diff costs exactly the same to review as a finished one.
• ✅ Resolve every comment before you re-request review. Re-requesting with threads still open means paying twice for the same conversation.
• 🔁 Do not use CI review as an inner loop for a local agent. The reviewer is not a step-by-step debugger — do the unfolding locally and arrive with the answer, not the search.
• 🙋 If something looks off, ask a human. One question to a maintainer is cheaper and faster than three rounds of agent re-review chasing a misread.

Reviews are not free. A draft costs nothing to review; a Ready PR is a promise that it is worth reviewing.

• Prefer to skip automated review entirely? Add the skip-claude-review label.

[balthazur]

Response to external adversarial review (Codex 5.5, extra-high effort)

We ran an independent agent review against this branch with instructions to prove errors with repros, not just flag them. It confirmed the offload/ordering/output contracts hold (61 declared blocks audited clean, focused suites green, no serialization bug), and proved four real defects. All four are fixed in d2f4a36:

1. Critical — final buffered frame errors were swallowed on drain. LookaheadPipelinedWorkflowRunner.flush() caught every emission failure and returned the remaining results, so a failed tail-frame request could be logged, dropped, and the pipeline would end normally.
Fix: flush() still drains every buffered frame best-effort, but now re-raises afterwards via StreamLookaheadDrainError, which carries the frames that did drain. InferencePipeline._drain_inference_handler dispatches those results, then lets the error propagate through the normal inference error path (INFERENCE_ERROR status update). Regression tests at both the runner and the InferencePipeline level.

2. Async steps lost execution/debug/trace context. The offloaded run() was submitted bare, bypassing the ContextVar/OTel rebinding that safe_execute_step does for the sync path — worker threads saw execution_id=None, no remote-timing collector, no span parent.
Fix: the launch captures the stream thread's context (execution id, remote-timing + debug collectors, OTel context) and the worker rebinds it before run(), mirroring safe_execute_step; failures are wrapped in StepExecutionError with the block id, same as the sync path. Test asserts the worker observes the caller's execution_id.

3. Usage duration was recorded before async work finished. @usage_collector sat on the deferred pass, which returns as soon as requests are launched — recorded execution_duration was ~0.
Fix: the decorator moved to the resume pass, which waits on the in-flight futures at input assembly, so duration covers actual step execution. Still exactly once per frame (api key / workflow id resolve from the same workflow kwarg). Round-trip test pins the placement.

4. Hung requests could block interpreter exit after close(). ThreadPoolExecutor threads are non-daemon; shutdown(wait=False) left a hung remote request keeping the process alive.
Fix: all pool workers are pre-spawned from a short-lived daemon thread (worker threads inherit the creating thread's daemon flag), so they can never block exit; close() additionally cancels queued work (cancel_futures=True). Test asserts every pool thread exists up front and is daemon.

Reviewer's cleared attacks, for the record: declared-output contract audit across all 61 async-declared blocks (0 problems), the three focused pytest suites (95 passed), and a serialize_results=True trace (futures resolve before serialization).

Written by Claude Fable 5 · 🤖 Generated with Claude Code

[balthazur]

Response to second adversarial review (Opus 4.8 Ultra)

A second independent agent review ran against the PR tip 749f10f5d and re-verified the first review's findings plus one novel defect. Verdict quoted: "Yes, mergeable behind the default-off flag … the invariants that matter — emission order and tracker-ID parity — hold." Status of its findings:

F2 (novel, MEDIUM) — SAM3 v1/v2 + seg_preview v1 mutated a shared cross-frame class_names list. Scalar list parameters are the same object across every frame's run() (the runner only shallow-copies the params dict), so under lookahead the in-place if len(class_names)==0: class_names.append(None) raced across concurrent frames — the shared list could become [None, None, …] and stay corrupted for the rest of the stream (duplicated prompts → duplicated masks + duplicated remote cost). The v3 block had already been given a fresh-copy fix; the reviewer proved v1/v2/seg_preview were missed.
Fixed in 7b4801a: the same class_names = [] if class_names is None else list(class_names) copy applied at all 7 remaining sites (SAM3 v1 run_locally/run_remotely/run_via_request, SAM3 v2 same three, seg_preview v1 run_via_request). Per the reviewer's "audit every adopted block" ask, we ran an AST-based audit of all 61 async-declared blocks for in-place mutation (append/extend/pop/update/del/item-assign) of any passed-in parameter: the 10 class_names.append(None) sites (7 now fixed + 3 already fixed in v3) were the only ones; everything else mutates fresh locals or per-frame batch data. Three regression tests added asserting the caller's list survives run() un-mutated (verified they fail on the unfixed code).

F1 (contextvar loss), F3 (usage duration), F4 (non-daemon workers), F5 (drain swallow) — independently corroborate the first review; all four were already fixed in d2f4a36b4 (see the previous review-response comment for details). The reviewer confirmed that commit addresses them.

Cleared attacks worth keeping on record: declared-output contracts across all 61 blocks (0 mismatches), serialize_results=True byte-parity at depth 4, bounded thread growth (peak 22 threads, 2-model workflow @ depth 8), OpenAI client-cache re-entrancy, frontier guards (async-fed-async and stateful-fed-async correctly fall back to sequential), and exact emission order / tracker parity.

On the suggested depth>1-vs-depth-1 parity matrix over all 60 adopted blocks: agreed that's the single highest-value guard for future adoptions, noted in Follow-ups. It needs either live models or a per-family mock harness, so we're keeping it out of this exploration PR; the shared-scalar-mutation class specifically is now covered by the AST audit + the copy-semantics regression tests.

Process note: this review ran concurrently in the same checkout as the fix session and hard-reset the branch twice while diagnosing tree contamination; all work was recovered from the reflog and the branch history is clean (749f10f5d → d2f4a36b4 → 7b4801a9e, fast-forward only). Future agent reviews will run on a separate clone.

Written by Claude Fable 5 · 🤖 Generated with Claude Code

[balthazur]

Response to third adversarial review (Cursor / Fable, extra-high effort)

Third independent agent review. Verdict quoted: "I would merge this behind its default-off flag. The correctness core — frontier partitioning, per-output future chaining, ordered resume on a per-frame EDM — survived every attack I could mount." Its five findings, and what we did (1e86fa6b6):

1 (HIGH, proven) — the daemon-prespawn lifecycle fix was ineffective by construction. Correct, and worth being blunt about: our previous fix shipped a documented guarantee that was false. concurrent.futures registers a shutdown hook that joins every pool worker regardless of the daemon flag, so a request hung past close() still blocked interpreter exit (reviewer's repro: exit blocked for exactly the hang duration with all workers daemon=True).
Fixed: close() now also detaches the workers from that shutdown hook (the reviewer's suggested fix); the daemon flag remains as the second half so threading's own shutdown doesn't wait on them either. Verified with the reviewer's scenario: a 30 s hung task submitted before close() no longer delays exit at all. The prespawn docstring no longer over-claims. The underlying root cause — the SDK's session.post(...) carries no client-side timeout=, in sequential mode too — is now an explicit follow-up (belongs in inference_sdk, affects both modes).

2 (MEDIUM, measured) — steady-state execution_duration under-reports. Confirmed and inherent: once the buffer is full, a frame's futures resolved while newer frames were being processed, so the resume pass records only its own compute. Count stays exactly once per frame with api key/workflow id/fps intact (reviewer verified, including drained frames). Action: we are not silently shipping it — the resume docstring now states the approximation precisely, and the PR description gained an explicit "usage accounting caveat" section addressed to the usage-tracking owners, plus a follow-up item to align semantics before GA.

3 (MEDIUM, traced) — the 60 s future-resolution deadline is a stream-killing mode sequential doesn't have. Acknowledged and documented in the failure-semantics section: WORKFLOWS_ASYNC_FUTURE_RESULT_TIMEOUT (env-tunable, default 60 s) bounds the resume-side wait; a degraded-backend brownout past it ends a lookahead stream that sequential would have survived (sequential just stalls indefinitely — see finding 1's missing SDK timeout). A retry-once-at-resume story is a fair pre-GA ask; keeping it out of this exploration PR.

4 (LOW, proven) — batch-contract violations surfaced late, blamed on the consumer, without a traceback. Fixed: the offload wrapper now validates the returned element count against the input indices and raises at the model step with exact counts; async StepExecutionErrors now attach the same BlockTraceback as safe_execute_step. Regression test asserts block id, message, and traceback presence.

5 (INFO) — stale artifacts. Fixed: the frontier docstring no longer references the deleted remote_stream_pipeline module, and the PR body's stale "one-shot HTTP requests still offload" claim is replaced with the reviewer-verified truth (nothing outside the stream runner offloads).

Cleared attacks now on record (this review's coverage was the deepest yet): 7 previously-unexercised block families through real lookahead streams with reversed completion order and both serialization modes (16/16 exact parity — including semantic-seg RLE masks and a VLM), output-contract AST audit (0 mismatches across 61 files), OpenAI client-cache + SAM3 re-entrancy hammer (8×100, zero contamination), bounded pool-in-pool growth, frontier shapes e2e (consensus correctly excluded as async-fed; continue_if falls back), 50 init/close cycles with no thread leak.

Benchmarks re-validated on the review-hardened head (laptop): tracking 3.33 → 33.3 (d8) → 39.0 FPS (d16), SAM3 1.61 → 15.1 (d8), 0 parity mismatches; scheduler-overhead net −0.15 ms/frame. The fixes are perf-neutral.

Written by Claude Fable 5 · 🤖 Generated with Claude Code

[github-actions]

🤖 Claude review started at commit 0cf9194ca4359136a301883feaff568fd2b8ac02.

New commits are not auto-reviewed. Add the claude-review label (remove & re-add it to trigger again) when you want another review.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 0cf9194. Configure here.}

[github-actions]

@balthazur — thanks for the unusually thorough writeup and the prior adversarial-review trail. I traced the lookahead scheduler, the frontier/async partition, the executor lifecycle, future chaining/resolution, and the block declarations. I did not find a high-confidence correctness blocker in the code itself: the feature is default-off, the EE version companions are all present (1.13.0 + changelog + both version-assertion tests), output futures resolve through the shared timeout-bounded helpers, the pool lifecycle is handled, and the blanket stateless-visualization default is safe (trace/heatmap correctly re-declare stateful; the _cache-bearing zone visualizers are order-independent memoizers).

⏳ This PR is on hold pending answers below. The review will not advance to sign-off and the PR should not be enabled/GA'd until the IMPORTANT item is acknowledged by the owners it affects. Behind the default-off flag the code is low-risk; the hold is about the billing-semantics decision, not a code defect.

IMPORTANT — needs an owner decision, not just code

1. execution_duration under-reporting is a billing/usage-accounting change and needs usage-tracking-owner acknowledgment before this is ever enabled. You've documented it (resume_stream_lookahead_workflow docstring + PR 'usage accounting caveat'), and it is exactly-once per frame with api-key/workflow-id/fps intact — but in steady state the recorded duration reflects only resume-pass compute, not the remote inference wall time, so any consumer of per-frame duration (duration minimums, billing analytics) sees systematically smaller values for lookahead streams doing identical work. Because this is a product/billing decision the code can't resolve, escalating: @PawelPeczek-Roboflow @grzegorz-roboflow @dkosowski87 — please confirm this semantics change is acceptable (and whether a follow-up must land before the flag is turned on anywhere), or state what the accounting must instead record. Until an owner confirms, I'm keeping this open.

Questions (please answer; they may hide a defect)

2. New stream-terminating failure mode vs. sequential (IMPORTANT). In lookahead mode a single request that stalls past WORKFLOWS_ASYNC_FUTURE_RESULT_TIMEOUT (default 60s) makes the resume pass raise and ends the stream, whereas sequential mode with no SDK-side timeout= merely slows down and keeps running. For the intended first consumer (the app's video previews in remote mode), is ending the stream on one degraded request the desired behavior, or should a per-frame timeout degrade to dropping/erroring that single frame and continuing? Please confirm the intended operator contract.

3. CPython-internal reliance in _shutdown_lookahead_executor (optional). executor._threads and concurrent.futures.thread._threads_queues are private and unversioned; the detach trick is correct on the currently-targeted interpreters and degrades safely (.pop(..., None)), but it will silently stop providing its exit-safety guarantee if CPython reworks that shutdown hook. Worth a comment pinning the Python versions you validated — you mention a 30s-hung-task manual check; is there any automated guard, or is it manual-only?

4. Adopted-block parity coverage (optional). You list a depth>1-vs-depth-1 parity matrix over every adopted block as a follow-up, and note 7 families were spot-checked. Since the correctness of each of the 59 adoptions rests on 'this block run() is truly a pure per-frame function with no cross-frame shared-object mutation' (the class_names bug is the proof this can be missed), is landing that automated parity harness a merge gate for you, or explicitly post-merge? An owner may want to weigh in given the blast radius.

Re-review is not automatic: once you've answered and/or pushed changes, add the claude-review label (remove and re-add to re-trigger) to request a fresh pass. Unanswered items may keep this out of a release.

Commands that informed this review: gh pr diff, gh api on the PR comments/reviews/commits, and git diff against the base SHA over the executor, stream handlers, engine cores, env.py, prototypes/block.py, and the visualization/block declarations.

Reviewed at HEAD: 0cf9194

[balthazur]

Thanks for the careful trace — and for separating "code defect" from "owner decision," which is exactly the right cut here. Status on all four, plus the Cursor Bugbot finding:

Code-addressable items — fixed and pushed:

• Cursor Bugbot (buffered dispatch keyed on env, not the actual runner) — real bug, fixed in f906bd35c. With WORKFLOWS_STREAM_LOOKAHEAD_DEPTH>1 set but the workflow not qualifying for lookahead (no async steps / multi-source / stateful-fed → wrap falls back to a plain WorkflowRunner), the pipeline still capped the predictions queue to 4 and ran the buffered dispatch path with nothing buffered. Now the queue cap, drain/close, and late future-resolution all key on whether the handler actually buffers (exposes flush() — the same signal _drain_inference_handler/_close_inference_handler already duck-type). Plain runners keep the normal path. Regression test added.

• (3) CPython-internal reliance in _shutdown_lookahead_executor — 6f7958114 pins the validated versions (3.9–3.12) in the comment, notes it degrades safely to the pre-existing "hung request blocks exit" behavior (never a crash) via pop(..., None), and points at the guard: test_lookahead_runner_pool_threads_are_daemon asserts both the daemon flag AND that the workers are detached from concurrent.futures.thread._threads_queues. So it's automated, not manual-only.

• (4) Adopted-block parity coverage — landed the CI-runnable slice now (6f7958114): a registry-wide consistency guard asserting that no block keeping per-video HTTP state (STATEFUL_VIDEO_HTTP_SOFT_RESTRICTION — 26 of 211 loaded blocks: trackers, counters, motion/background, delta-filter) is ever declared stateless-for-video, since that contradiction is exactly what would place a stateful block in the offload frontier and corrupt it. That's the automated declaration-level net for the class_names class of mis-adoption. The full live-model depth>1-vs-depth-1 matrix over all 59 adoptions genuinely needs model-serving infra (real weights / mocked HTTP per family), so I'm keeping that one explicitly post-merge and would value an owner's call on whether it should gate GA — flagging rather than deciding.

Owner decisions — I've documented, not resolved (agree these aren't mine to close):

• (1) execution_duration under-reporting — no code change; this is the billing/usage-accounting semantics call you escalated. It's documented (resume-pass docstring + PR "usage accounting caveat"), exactly-once per frame with api-key/workflow-id/fps intact. Deferring to the usage-tracking owners on whether the steady-state semantics are acceptable or must record differently before the flag is ever enabled. Holding as you have it.

• (2) Stream-terminating failure vs. sequential on the 60s deadline — also an operator-contract call rather than a defect. Today: WORKFLOWS_ASYNC_FUTURE_RESULT_TIMEOUT (env-tunable, default 60s) bounds the resume-side wait; past it the stream ends, whereas sequential (no SDK-side timeout=) just slows. For the video-previews consumer I'd lean toward "degrade that single frame (drop/error) and continue" rather than ending the stream, but since it changes failure semantics I don't want to pick it unilaterally — happy to implement whichever contract the owners confirm, and the real fix (client-side session.post timeouts) belongs in inference_sdk and helps sequential too (already in Follow-ups).

Net: the two remaining items are genuinely the owners' to decide and the hold is correct. I'll add the claude-review label to get a fresh pass on the three pushed fixes. Full context for reviewers: this began as a POC to confirm remote-execution pipelining is viable for the app's video previews; the ask is a high-level "does the approach make sense" read, not a GA sign-off.

Written by Claude Fable 5 · 🤖 Generated with Claude Code

[github-actions]

🤖 Claude review started at commit 6f7958114de677000f9a25148113ea5b57218f5e.

New commits are not auto-reviewed. Add the claude-review label (remove & re-add it to trigger again) when you want another review.

[github-actions]

@balthazur — thanks for the three pushed fixes; I re-reviewed the delta (f906bd35c, 6f7958114) against the base and your prior review responses.

⏳ This PR remains ON HOLD, PENDING owner answers. The review will not advance to sign-off, and the flag should not be enabled/GA'd, until the two IMPORTANT items below are acknowledged by the owners they affect. Behind the default-off flag the code is low-risk; the hold is about two product/operator decisions the code cannot resolve, not a code defect.

Delta verified clean (no new findings):

• f906bd35c (buffered dispatch keyed on the handler's flush(), not the depth env var) — traced against base _rfdetr_stream_pipeline_enabled(): RF-DETR local pipelining (instance_segmentation/v3.can_activate_stream_pipeline) still returns a PipelinedWorkflowRunner (has flush() → buffered), and a fallback plain WorkflowRunner now correctly skips the queue cap/drain. No RF-DETR regression; strictly more correct than the env-keyed gate.
• 6f7958114 — test_all_registry_manifests_declare_boolean_statefulness and test_cross_frame_state_blocks_are_never_declared_stateless_for_lookahead are real CI unit tests; _shutdown_lookahead_executor internals are version-pinned (3.9–3.12) with test_lookahead_runner_pool_threads_are_daemon guarding the detach. This resolves optional questions UDP Interface Fix #3 and Remove Old Benchmark Data #4 (declaration-level slice) with code evidence.
• EE version companions confirmed present: 1.13.0 + changelog + both version-assertion tests.

IMPORTANT — still open, need an owner decision (not a code change):

1. execution_duration under-reporting is a billing/usage-accounting semantics change. In steady state the recorded per-frame execution_duration reflects only resume-pass compute, not the remote inference wall time (resume_stream_lookahead_workflow docstring confirms this is intentional/approximate). Count stays exactly-once per frame with api-key/workflow-id/fps intact, but any consumer of per-frame duration (duration minimums, billing analytics) sees systematically smaller values for lookahead streams than sequential ones doing identical work. @PawelPeczek-Roboflow @grzegorz-roboflow @dkosowski87 — please confirm this semantics change is acceptable (and whether a follow-up must land before the flag is turned on anywhere), or state what the accounting must instead record. The contributor has documented it but correctly declined to close it unilaterally.

2. New stream-terminating failure mode vs. sequential. A single request that stalls past WORKFLOWS_ASYNC_FUTURE_RESULT_TIMEOUT (default 60s) makes the resume pass raise and ends the stream, whereas sequential mode (no SDK-side timeout=) merely slows and keeps running. For the intended first consumer (the app's video previews in remote mode), is ending the stream on one degraded request the desired operator contract, or should a per-frame timeout drop/error that single frame and continue? Please confirm the intended behavior. (The contributor leans toward "degrade the single frame and continue" but has not picked it unilaterally — an owner call is needed.)

Call to action: these two items are genuinely the owners' to decide. Until an owner confirms each, the PR stays on hold and may be kept out of a release.

Re-review is not automatic: once the questions are answered and/or you push changes, add the claude-review label (remove and re-add to re-trigger) to request a fresh pass.

Commands that informed this review: gh pr diff/gh pr view, gh api on the PR issue-comments/review-comments/reviews/commits, and git show <base>:<path> over inference_pipeline.py, model_handlers/workflows.py, executor/core.py, executor/utils.py, prototypes/block.py, env.py, the EE version/changelog, and the new stream-lookahead tests.

Reviewed at HEAD: 6f79581