Mark/easystitch

Fork 0

Decompose: Make EmbroideryPipeline.process() async with progress reporting #34

New issue

Closed

opened 2026-06-22 16:23:19 +00:00 by Mark · 1 comment

Mark commented

2026-06-22 16:23:19 +00:00

Owner

EmbroideryPipeline.process() is sync + CPU-bound — needs async/executor semantics

Context

EmbroideryPipeline.process() is fully synchronous and CPU-bound. For
the upcoming HTTP service (issue #31), this is a scaling problem: a
single request blocks the worker thread for 200ms–2s+.

Goal

Add async + progress-callback semantics so the HTTP service can:

Run multiple requests concurrently without one blocking all others
Emit progress events the client can render in a UI

Decomposition

The original ticket offered two options (A: run_in_executor-only, B:
progress_cb + run_in_executor). The right answer is "both" (option C
in the new framing), but the choice has consequences for the API surface
and needs to be locked before any code is written.

P1: Design decision — pick the concurrency primitive

Goal: Lock the design before code. Two sub-decisions:

Async wrapper strategy: run_in_executor (offload to thread pool,
no progress events) vs progress_cb parameter on process() (allows
yield-between-stages progress reporting, requires process() to be
refactored to call the callback).
Progress event shape: what does the callback receive? Plain log
strings? Structured dicts with stage name + percentage? SSE event
format if going through FastAPI?

Files:

New: docs/design/async-pipeline.md with the decision, the rejected
alternatives, and the API contract.

Acceptance:

The decision is committed to a doc with a clear yes/no on each of
the two sub-decisions.
The API contract is spelled out (signature of process_async or
process(progress_cb=...), what the callback receives, when it
fires, how cancellation works).
The ThreadCraft team signs off on the choice (their progress UI
depends on it).

Why first: the API shape of P2/P3 follows from this. Doing P2/P3
without a locked decision risks rework.

P2: Implement the chosen primitive

Implement the design from P1. Whichever path is chosen, this is the
substance of the change.

Files:

Modify: src/main_pipeline.py (refactor process() to support the
chosen API)
New: src/main_pipeline_async.py (if the choice was a separate
async wrapper instead of refactoring process())
Modify: src/service/handlers/digitize.py (issue #31 P2 handler
consumes the new API)

Acceptance:

The chosen API works as documented in P1.
5 concurrent digitize requests on a 4-core box complete in < 6× the
serial time per request (parallelism is real, not serialized).
Cancellation: if the caller cancels mid-request, the worker thread
is interrupted (or the result is discarded) within 1 second.

Depends on: P1 (decision).

P3: Progress callback wiring

If P1 chose progress_cb, implement the per-stage progress
notifications. If P1 chose run_in_executor-only, this phase is
unnecessary (mark as N/A in the issue tracker).

Files:

Modify: src/main_pipeline.py (add progress_cb parameter, call
between stages)
Modify: src/service/handlers/digitize.py (forward progress events
to SSE response)
New: tests/test_progress_callback.py (asserts callback fires
for each stage, with correct stage names)

Acceptance:

Calling process(progress_cb=cb) invokes cb once per stage
transition with {stage, status, duration_ms?}.
The service handler emits SSE events that the ThreadCraft client
can render as a progress bar.
A test asserts the callback fires for the 5 stages in order:
raster_to_vector, semantic_annotation, dom_resolution,
physical_optimization, format_emission.

Depends on: P1 (decision), P2 (chosen primitive).

Out of scope

The actual thread-pool sizing / queue depth / timeout values are
covered by issue #43 (concurrency contract). This ticket is about
the API shape, not the operational parameters.
The progress_cb is for stage-level progress, not pixel-level
progress within a stage. That would require deeper refactoring
per stage.

# EmbroideryPipeline.process() is sync + CPU-bound — needs async/executor semantics ## Context `EmbroideryPipeline.process()` is fully synchronous and CPU-bound. For the upcoming HTTP service (issue #31), this is a scaling problem: a single request blocks the worker thread for 200ms–2s+. ## Goal Add async + progress-callback semantics so the HTTP service can: - Run multiple requests concurrently without one blocking all others - Emit progress events the client can render in a UI ## Decomposition The original ticket offered two options (A: `run_in_executor`-only, B: `progress_cb` + `run_in_executor`). The right answer is "both" (option C in the new framing), but the choice has consequences for the API surface and needs to be locked before any code is written. ### P1: Design decision — pick the concurrency primitive **Goal:** Lock the design before code. Two sub-decisions: 1. **Async wrapper strategy:** `run_in_executor` (offload to thread pool, no progress events) vs `progress_cb` parameter on `process()` (allows yield-between-stages progress reporting, requires `process()` to be refactored to call the callback). 2. **Progress event shape:** what does the callback receive? Plain log strings? Structured dicts with stage name + percentage? SSE event format if going through FastAPI? **Files:** - New: `docs/design/async-pipeline.md` with the decision, the rejected alternatives, and the API contract. **Acceptance:** - The decision is committed to a doc with a clear yes/no on each of the two sub-decisions. - The API contract is spelled out (signature of `process_async` or `process(progress_cb=...)`, what the callback receives, when it fires, how cancellation works). - The ThreadCraft team signs off on the choice (their progress UI depends on it). **Why first:** the API shape of P2/P3 follows from this. Doing P2/P3 without a locked decision risks rework. --- ### P2: Implement the chosen primitive Implement the design from P1. Whichever path is chosen, this is the substance of the change. **Files:** - Modify: `src/main_pipeline.py` (refactor `process()` to support the chosen API) - New: `src/main_pipeline_async.py` (if the choice was a separate async wrapper instead of refactoring `process()`) - Modify: `src/service/handlers/digitize.py` (issue #31 P2 handler consumes the new API) **Acceptance:** - The chosen API works as documented in P1. - 5 concurrent digitize requests on a 4-core box complete in < 6× the serial time per request (parallelism is real, not serialized). - Cancellation: if the caller cancels mid-request, the worker thread is interrupted (or the result is discarded) within 1 second. **Depends on:** P1 (decision). --- ### P3: Progress callback wiring If P1 chose `progress_cb`, implement the per-stage progress notifications. If P1 chose `run_in_executor`-only, this phase is unnecessary (mark as N/A in the issue tracker). **Files:** - Modify: `src/main_pipeline.py` (add `progress_cb` parameter, call between stages) - Modify: `src/service/handlers/digitize.py` (forward progress events to SSE response) - New: `tests/test_progress_callback.py` (asserts callback fires for each stage, with correct stage names) **Acceptance:** - Calling `process(progress_cb=cb)` invokes `cb` once per stage transition with `{stage, status, duration_ms?}`. - The service handler emits SSE events that the ThreadCraft client can render as a progress bar. - A test asserts the callback fires for the 5 stages in order: `raster_to_vector`, `semantic_annotation`, `dom_resolution`, `physical_optimization`, `format_emission`. **Depends on:** P1 (decision), P2 (chosen primitive). ## Out of scope - The actual thread-pool sizing / queue depth / timeout values are covered by issue #43 (concurrency contract). This ticket is about the API shape, not the operational parameters. - The `progress_cb` is for stage-level progress, not pixel-level progress within a stage. That would require deeper refactoring per stage.

Mark added the

feature-request

triage-approved

labels

2026-06-22 16:23:19 +00:00

Mark changed title from ~~EmbroideryPipeline.process() is sync + CPU-bound; needs async or executor semantics for the HTTP layer~~ to Decompose: Make EmbroideryPipeline.process() async with progress reporting

2026-06-22 16:30:52 +00:00

Mark referenced this issue

2026-06-22 16:31:16 +00:00

Decompose: Make EmbroideryPipeline.process() async with progress reporting — Phase 1: Design decision — pick the concurrency primitive #51

Mark referenced this issue

2026-06-22 16:31:17 +00:00

Decompose: Make EmbroideryPipeline.process() async with progress reporting — Phase 2: Implement the chosen primitive #52

Mark referenced this issue

2026-06-22 16:31:18 +00:00

Decompose: Make EmbroideryPipeline.process() async with progress reporting — Phase 3: Progress callback wiring #53

Mark commented

2026-06-22 16:31:19 +00:00

Author

Owner

This issue has been decomposed into the following sub-issues:

#	Title	Phase Scope
#51	Decompose: Make EmbroideryPipeline.process() async with prog	Goal: Lock the design before code. Two sub-decisions: 1. **Async wrapper st
#52	Decompose: Make EmbroideryPipeline.process() async with prog	Implement the design from P1. Whichever path is chosen, this is the substance of
#53	Decompose: Make EmbroideryPipeline.process() async with prog	If P1 chose `progress_cb`, implement the per-stage progress notifications. If P1

Each sub-issue has been labeled with triage-approved and is ready for queue routing.

This issue has been decomposed into the following sub-issues: | # | Title | Phase Scope | |---|-------|-------------| | #51 | Decompose: Make EmbroideryPipeline.process() async with prog | **Goal:** Lock the design before code. Two sub-decisions: 1. **Async wrapper st | | #52 | Decompose: Make EmbroideryPipeline.process() async with prog | Implement the design from P1. Whichever path is chosen, this is the substance of | | #53 | Decompose: Make EmbroideryPipeline.process() async with prog | If P1 chose `progress_cb`, implement the per-stage progress notifications. If P1 | Each sub-issue has been labeled with `triage-approved` and is ready for queue routing.

Mark closed this issue

2026-06-22 16:31:19 +00:00

Mark referenced this issue

2026-06-26 20:38:30 +00:00

Design decision — pick the concurrency primitive #89

Mark referenced this issue

2026-06-26 20:50:52 +00:00

Implement the chosen primitive #90

Mark referenced this issue

2026-06-26 21:37:52 +00:00

Progress callback wiring #91