From b53a57f0a5ad3f6f50a812078f0077d7ba43e80a Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 18 Oct 2026 14:09:18 -0400 Subject: [PATCH 01/18] bench(flow): exclusive-time profiler for one replay run Co-Authored-By: Claude Opus 5.5 --- .../benchmark/openemr_throughput/profiler.py | 320 ++++++++++++++++++ 1 file changed, 320 insertions(+) create mode 100644 packages/flow/benchmark/openemr_throughput/profiler.py diff --git a/packages/flow/benchmark/openemr_throughput/profiler.py b/packages/flow/benchmark/openemr_throughput/profiler.py new file mode 100644 index 0000000000..4c4619f829 --- /dev/null +++ b/packages/flow/benchmark/openemr_throughput/profiler.py @@ -0,0 +1,320 @@ +"""Exclusive-time profiler for one replay run. + +Wraps a fixed list of Replayer, backend and vision functions. Each wrapped +call's time is split into its own (exclusive) time and the time spent in +other wrapped calls it made, so the buckets add up to the wall time of the +run with no double counting. Time not inside any wrapped call is engine +bookkeeping ("engine other"). + +The profiler only measures. It changes no behavior: every wrapper calls the +original function with the same arguments and returns its result. + +Usage:: + + prof = RunProfiler() + prof.install(backend) # wraps this backend instance + classes + with prof.span("task"): + report = replayer.run(...) + prof.uninstall() + prof.summary() # {name: {"n":..., "excl_s":..., "incl_s":...}} +""" + +from __future__ import annotations + +import functools +import inspect +import threading +import time +from collections import defaultdict +from contextlib import contextmanager +from typing import Any, Callable, Iterator + +# Replayer methods worth naming. Each is wrapped on the class. +REPLAYER_METHODS = ( + "_run_step", + "_resolve_step", + "_apply_step_gates", + "_wait_starting_state_settled", + "_verify_identity", + "_verify_identity_ocr", + "_verify_dedicated_identity_signal", + "_surface_identity_check", + "_act", + "_wait_for_dialog", + "_handle_interstitials", + "_verify_typed_input", + "_verify_selected_option", + "_check_postconditions", + "_poll_postconditions", + "_verify_effects", + "_settle_seeded", + "_save_step_png", + "_wait_for_scroll_transition_and_settle", + "_revalidate_consequential_actuation", + "_finalize_report", +) + +# Backend methods. Wrapped on the instance so other backends are untouched. +BACKEND_METHODS = ( + "screenshot", + "page_quiescent", + "page_change_marker", + "click", + "right_click", + "type_text", + "type_text_guarded", + "press", + "press_guarded", + "scroll", + "locate_structural", + "act_structural", + "act_guarded_coordinate", + "structured_text_at", + "text_value_at", + "displayed_text_at", + "focused_text_value", + "focused_field_label", + "focused_select_label_at", + "visible_text_present", + "take_dialogs", + "arm_dialog", + "structural_locator_at", + "submit_binding_at", + "workflow_state_identity", + "session_identity", + "application_identity", +) + +# (module, function) pairs in the vision layer. +VISION_FUNCS = ( + ("openadapt_flow.vision.settle", "wait_settled"), + ("openadapt_flow.vision.settle", "wait_settled_result"), + ("openadapt_flow.vision.settle", "wait_settled_seeded"), + ("openadapt_flow.vision.ocr", "ocr"), + ("openadapt_flow.vision.match", "find_template"), + ("openadapt_flow.vision.match", "find_text"), + ("openadapt_flow.vision.match", "find_text_candidates"), + ("openadapt_flow.vision.hashing", "phash_png"), +) + +# Report buckets: which named calls count toward each. +BUCKETS: dict[str, tuple[str, ...]] = { + "waits: fixed sleeps (poll intervals)": ("time.sleep",), + "waits: page signals (DOM/network quiet)": ("be.page_quiescent", "be.page_change_marker"), + "waits: screen settle (frame compare)": ( + "vision.wait_settled", + "vision.wait_settled_result", + "vision.wait_settled_seeded", + "rp._settle_seeded", + "rp._wait_starting_state_settled", + "rp._wait_for_scroll_transition_and_settle", + "vision.phash_png", + ), + "screenshots (capture)": ("be.screenshot",), + "step evidence PNGs (write)": ("rp._save_step_png",), + "identity: structure (DOM)": ( + "rp._verify_identity", + "rp._verify_dedicated_identity_signal", + "rp._surface_identity_check", + "be.structured_text_at", + "be.workflow_state_identity", + "be.session_identity", + "be.application_identity", + ), + "identity: OCR": ("rp._verify_identity_ocr", "vision.ocr@identity"), + "OCR (other)": ("vision.ocr",), + "target resolution": ( + "rp._resolve_step", + "be.locate_structural", + "be.structural_locator_at", + "be.submit_binding_at", + "vision.find_template", + "vision.find_text", + "vision.find_text_candidates", + ), + "typing and keys": ("be.type_text", "be.type_text_guarded", "be.press", "be.press_guarded"), + "clicks and scrolls": ( + "be.click", + "be.right_click", + "be.act_structural", + "be.act_guarded_coordinate", + "be.scroll", + ), + "input read-back (field value)": ( + "rp._verify_typed_input", + "rp._verify_selected_option", + "be.text_value_at", + "be.displayed_text_at", + "be.focused_text_value", + "be.focused_field_label", + "be.focused_select_label_at", + ), + "postconditions and dialogs": ( + "rp._check_postconditions", + "rp._poll_postconditions", + "be.visible_text_present", + "rp._wait_for_dialog", + "be.take_dialogs", + "be.arm_dialog", + "rp._handle_interstitials", + ), + "save read-back (system of record)": ("rp._verify_effects",), + "engine other": ( + "rp._run_step", + "rp._act", + "rp._apply_step_gates", + "rp._revalidate_consequential_actuation", + "rp._finalize_report", + "span.task", + ), +} + + +class RunProfiler: + """Stack-based exclusive-time profiler (main thread only).""" + + def __init__(self) -> None: + self._thread = threading.get_ident() + self._stack: list[list[Any]] = [] # [name, start, child_time] + self.stats: dict[str, dict[str, float]] = defaultdict( + lambda: {"n": 0, "excl_s": 0.0, "incl_s": 0.0} + ) + self.steps: dict[str, dict[str, float]] = defaultdict(lambda: defaultdict(float)) + self.by_parent: dict[str, dict[str, float]] = defaultdict(lambda: defaultdict(float)) + self._current_step: str | None = None + self._undo: list[Callable[[], None]] = [] + + # -- core ------------------------------------------------------------- + + def _enter(self, name: str) -> None: + self._stack.append([name, time.perf_counter(), 0.0]) + + def _exit(self) -> None: + name, start, child = self._stack.pop() + incl = time.perf_counter() - start + excl = incl - child + if name == "vision.ocr" and any( + f[0] in ("rp._verify_identity_ocr", "rp._verify_identity") for f in self._stack + ): + name = "vision.ocr@identity" + parent = next((f[0] for f in reversed(self._stack) if f[0].startswith("rp.")), "-") + self.by_parent[name][parent] += excl + st = self.stats[name] + st["n"] += 1 + st["excl_s"] += excl + st["incl_s"] += incl + if self._current_step: + self.steps[self._current_step][name] += excl + if self._stack: + self._stack[-1][2] += incl + + def _wrap( + self, name: str, fn: Callable[..., Any], *, static: bool = False + ) -> Callable[..., Any]: + prof = self + + @functools.wraps(fn) + def wrapper(*a: Any, **k: Any) -> Any: + if threading.get_ident() != prof._thread: + return fn(*a, **k) + prev_step = prof._current_step + if name == "rp._run_step": + step = a[0 if static else 1] if len(a) > (0 if static else 1) else k.get("step") + prof._current_step = getattr(step, "id", None) or prev_step + prof._enter(name) + try: + return fn(*a, **k) + finally: + prof._exit() + if name == "rp._run_step": + prof._current_step = prev_step + + return wrapper + + @contextmanager + def span(self, name: str) -> Iterator[None]: + self._enter(f"span.{name}") + try: + yield + finally: + self._exit() + + # -- install ---------------------------------------------------------- + + def install(self, backend: Any) -> None: + import importlib + + from openadapt_flow.runtime.replayer import Replayer + + for m in REPLAYER_METHODS: + raw = inspect.getattr_static(Replayer, m, None) + if raw is None: + continue + if isinstance(raw, staticmethod): + wrapped: Any = staticmethod(self._wrap(f"rp.{m}", raw.__func__, static=True)) + else: + wrapped = self._wrap(f"rp.{m}", raw) + setattr(Replayer, m, wrapped) + self._undo.append(lambda m=m, raw=raw: setattr(Replayer, m, raw)) + for m in BACKEND_METHODS: + orig = getattr(backend, m, None) + if orig is None or not callable(orig): + continue + setattr(backend, m, self._wrap(f"be.{m}", orig)) + self._undo.append(lambda m=m: backend.__dict__.pop(m, None)) + for mod_name, fn_name in VISION_FUNCS: + mod = importlib.import_module(mod_name) + orig = getattr(mod, fn_name, None) + if orig is None: + continue + wrapped = self._wrap(f"vision.{fn_name}", orig) + setattr(mod, fn_name, wrapped) + self._undo.append(lambda mod=mod, fn_name=fn_name, orig=orig: setattr(mod, fn_name, orig)) + # Modules that imported the function by name keep their own + # reference; patch those too. + for other in ("openadapt_flow.runtime.replayer", "openadapt_flow.vision"): + om = importlib.import_module(other) + if getattr(om, fn_name, None) is orig: + setattr(om, fn_name, wrapped) + self._undo.append( + lambda om=om, fn_name=fn_name, orig=orig: setattr(om, fn_name, orig) + ) + orig_sleep = time.sleep + time.sleep = self._wrap("time.sleep", orig_sleep) + self._undo.append(lambda: setattr(time, "sleep", orig_sleep)) + + def uninstall(self) -> None: + while self._undo: + self._undo.pop()() + + # -- output ----------------------------------------------------------- + + def summary(self) -> dict[str, dict[str, float]]: + return {k: {kk: round(vv, 4) for kk, vv in v.items()} for k, v in self.stats.items()} + + def parents(self) -> dict[str, dict[str, float]]: + """Exclusive time of each call, split by the nearest Replayer caller.""" + return { + n: {p: round(v, 4) for p, v in sorted(per.items(), key=lambda x: -x[1])} + for n, per in self.by_parent.items() + } + + def buckets(self) -> dict[str, float]: + out: dict[str, float] = {} + named = set() + for bucket, names in BUCKETS.items(): + out[bucket] = round(sum(self.stats[n]["excl_s"] for n in names if n in self.stats), 4) + named.update(names) + unnamed = [n for n in self.stats if n not in named] + out["unassigned"] = round(sum(self.stats[n]["excl_s"] for n in unnamed), 4) + return out + + def step_buckets(self) -> dict[str, dict[str, float]]: + rev = {n: b for b, names in BUCKETS.items() for n in names} + out: dict[str, dict[str, float]] = {} + for step, per in self.steps.items(): + row: dict[str, float] = defaultdict(float) + for n, v in per.items(): + row[rev.get(n, "unassigned")] += v + out[step] = {k: round(v, 4) for k, v in row.items() if v > 0.0005} + return out From 76243015a5e6a332880b9c67e1d84f64ae195f3e Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 18 Oct 2026 14:19:40 -0400 Subject: [PATCH 02/18] feat(flow): worklist pool with one claim per item and pipelined read-back Co-Authored-By: Claude Opus 5.5 --- .../runtime/readback_pipeline.py | 89 +++ .../flow/openadapt_flow/runtime/replayer.py | 221 +++++- .../openadapt_flow/runtime/worklist_pool.py | 728 ++++++++++++++++++ packages/flow/tests/test_readback_pipeline.py | 137 ++++ packages/flow/tests/test_worklist_pool.py | 320 ++++++++ 5 files changed, 1493 insertions(+), 2 deletions(-) create mode 100644 packages/flow/openadapt_flow/runtime/readback_pipeline.py create mode 100644 packages/flow/openadapt_flow/runtime/worklist_pool.py create mode 100644 packages/flow/tests/test_readback_pipeline.py create mode 100644 packages/flow/tests/test_worklist_pool.py diff --git a/packages/flow/openadapt_flow/runtime/readback_pipeline.py b/packages/flow/openadapt_flow/runtime/readback_pipeline.py new file mode 100644 index 0000000000..d2ffb43e73 --- /dev/null +++ b/packages/flow/openadapt_flow/runtime/readback_pipeline.py @@ -0,0 +1,89 @@ +"""Pipelined read-back: verify item n's save while item n+1 runs. + +A worklist worker with a read-back window above 0 gives its Replayer a +:class:`ReadbackPipeline`. When the LAST action of an item has a +system-of-record effect, the Replayer submits that effect's read-back to the +pipeline's side thread instead of waiting for it, finishes the item, and +returns. The worker then starts the next item on the same browser while the +read-back runs. + +What still holds: + +- The read-back is the same check with the same verifier; only its timing + moves. The item's ``report.json`` is rewritten with the verdict (and its + outcome re-stamped) when it finishes. +- Only the final action of an item is deferred. If the item runs any further + step, the Replayer first waits for the read-back and halts on a failure, as + it would inline. +- Only read-backs that don't use the screen are deferred (an on-screen + read-back needs the browser, which the next item is using). Durable and + idempotency-ledger runs always verify inline, because their terminal state + is written once. +- The worker bounds how many read-backs may wait (the window), and the pool + stops every worker on the first failed one. +""" + +from __future__ import annotations + +import threading +from concurrent.futures import Future, ThreadPoolExecutor +from dataclasses import dataclass, field +from typing import Any, Callable, Optional + + +@dataclass +class PipelinedReadback: + """One deferred read-back: its verdict future and how to settle it.""" + + step_id: str + #: Resolves to the read-back's error text (None = confirmed). + verdict: "Future[Optional[str]]" + #: Applies the verdict to the item's report and saves it. Set by the + #: Replayer when the item finishes. + settle: Optional[Callable[[Optional[str]], Any]] = None + #: Resolves to whatever ``settle`` returned (the settled report). + settled: "Future[Any]" = field(default_factory=Future) + + +class ReadbackPipeline: + """One side thread per worker for deferred read-backs.""" + + def __init__(self) -> None: + self._executor = ThreadPoolExecutor(max_workers=1, thread_name_prefix="readback") + self._lock = threading.Lock() + self._handoff: Optional[PipelinedReadback] = None + #: Why the last item verified inline instead (for the run report). + self.inline_reason: Optional[str] = None + + def submit(self, step_id: str, verify: Callable[[], Optional[str]]) -> PipelinedReadback: + entry = PipelinedReadback(step_id=step_id, verdict=self._executor.submit(verify)) + return entry + + def hand_off(self, entry: PipelinedReadback, settle: Callable[[Optional[str]], Any]) -> None: + """Called by the Replayer as the item finishes: settle on completion.""" + + entry.settle = settle + + def _done(fut: "Future[Optional[str]]") -> None: + try: + error = fut.result() + except Exception as exc: # noqa: BLE001 - a raising verifier is a failure + error = f"read-back raised {type(exc).__name__}; reconciliation is required" + try: + entry.settled.set_result(settle(error)) + except Exception as exc: # noqa: BLE001 + entry.settled.set_exception(exc) + + with self._lock: + self._handoff = entry + entry.verdict.add_done_callback(_done) + + def take(self) -> Optional[PipelinedReadback]: + """The read-back the last item handed off, if any (clears it).""" + + with self._lock: + entry, self._handoff = self._handoff, None + return entry + + def close(self) -> None: + self._executor.shutdown(wait=True) diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 7d9e12e6c1..b26c6f0e1d 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -221,6 +221,10 @@ QualificationCampaignGuard, ) from openadapt_flow.runtime.control_overlay import RuntimeControlOverlayEmitter + from openadapt_flow.runtime.readback_pipeline import ( + PipelinedReadback, + ReadbackPipeline, + ) from openadapt_flow.transaction import IdempotencyLedger # REGION_STABLE template check: how far the expected content may shift from @@ -751,7 +755,16 @@ def __init__( layout_drift: Optional[bool] = None, layout_drift_tolerances: Optional[Any] = None, layout_drift_advisor: Optional[Callable[[Any], Optional[str]]] = None, + secret_env_overrides: Optional[Mapping[str, str]] = None, + readback_pipeline: Optional["ReadbackPipeline"] = None, ) -> None: + # Per-run secret sources (worklist workers): secret param name -> the + # NAME of the env var holding its value, so each worker signs in with + # its own account. Values are never stored here. + self.secret_env_overrides: dict[str, str] = dict(secret_env_overrides or {}) + # Pipelined read-back (runtime/readback_pipeline.py); None = inline. + self.readback_pipeline = readback_pipeline + self._pipelined_readback: Optional["PipelinedReadback"] = None # A JIT draft (openadapt_flow.jit) runs only once admitted, or attended # by a named person (the canary). Checked in run(). self._attended_draft_by = attended_draft_by @@ -2406,6 +2419,16 @@ def _finalize_report( ) -> RunReport: """Persist exact evidence first, then project its terminal outcome.""" + pending = getattr(self, "_pipelined_readback", None) + handed_off = False + if pending is not None: + if persist and report.success and self.readback_pipeline is not None: + handed_off = True + else: + # The run ended some other way: settle the read-back inline + # before the report is written. + self._apply_pipelined_verdict(report, self._join_pipelined_readback(keep=True)) + self._pipelined_readback = None self._end_surfaces() if self._surface_environment_observations: report.observed_surface_environments = dict( @@ -2441,6 +2464,168 @@ def _finalize_report( clear_guard(None) if project_terminal: self._emit_control_overlay_terminal(report.execution_outcome) + if handed_off: + assert pending is not None and self.readback_pipeline is not None + self._pipelined_readback = None + self.readback_pipeline.hand_off( + pending, + lambda error, entry=pending: self._settle_pipelined_readback( + report, workflow, run_dir, entry, error + ), + ) + return report + + # -- pipelined read-back (runtime/readback_pipeline.py) ------------------ + + def _may_pipeline_readback(self, verifier: Any) -> bool: + """Whether this effect's read-back may run on the side thread.""" + + pipeline = getattr(self, "readback_pipeline", None) + if pipeline is None: + return False + reason = None + if self.durable or getattr(self, "_program_durable", None) is not None: + reason = "durable runs verify inline" + elif getattr(self, "idempotency_ledger", None) is not None: + reason = "idempotency-ledger runs verify inline" + elif getattr(self, "_pipelined_readback", None) is not None: + reason = "an earlier read-back in this item is still pending" + else: + from openadapt_flow.runtime.effects.onscreen import ( + OnScreenReadbackVerifier, + ) + + target = verifier if verifier is not None else self.effect_verifier + if target is None or isinstance(target, OnScreenReadbackVerifier): + reason = "on-screen read-backs need the browser" + elif getattr(target, "backend", None) is not None: + reason = "the read-back uses the screen" + pipeline.inline_reason = reason + return reason is None + + def _defer_readback( + self, + step: Step, + before: EffectState, + result: StepResult, + *, + effects: Optional[list["Effect"]], + verifier: Any, + ) -> None: + assert self.readback_pipeline is not None + scratch = StepResult(step_id=step.id, intent=step.intent, ok=True) + + def verify() -> Optional[str]: + return self._verify_effects( + step, before, scratch, effects=effects, verifier=verifier + ) + + entry = self.readback_pipeline.submit(step.id, verify) + entry.scratch = scratch # type: ignore[attr-defined] + entry.result = result # type: ignore[attr-defined] + entry.intent = step.intent # type: ignore[attr-defined] + result.effect_verified = None + result.effect_results.append( + "[pipelined] system-of-record read-back pending" + ) + self._pipelined_readback = entry + + def _join_pipelined_readback(self, *, keep: bool = False) -> Optional[str]: + """Wait for a pending read-back, apply it to its step, return its error.""" + + entry = getattr(self, "_pipelined_readback", None) + if entry is None: + return None + try: + error = entry.verdict.result() + except Exception as exc: # noqa: BLE001 - a raising verifier fails closed + error = ( + f"read-back raised {type(exc).__name__}; reconciliation is required" + ) + self._copy_pipelined_evidence(entry, error) + if not keep: + self._pipelined_readback = None + return error + + def _copy_pipelined_evidence(self, entry: Any, error: Optional[str]) -> None: + result: StepResult = entry.result + scratch: StepResult = entry.scratch + result.effect_results = [ + r for r in result.effect_results if not r.startswith("[pipelined]") + ] + result.effect_results.extend(scratch.effect_results) + result.effect_evidence.extend(scratch.effect_evidence) + result.effect_field_checks.extend(scratch.effect_field_checks) + result.effect_verified = scratch.effect_verified + if scratch.failure_category is not None: + result.failure_category = scratch.failure_category + if error is not None: + result.ok = False + result.error = error + result.safety_halt = True + if result.failure_category is None: + result.failure_category = ( + "governed_refusal" + if self.governed_authorization is not None + else "safety_halt" + ) + else: + result.safety_halt = result.safety_halt or scratch.safety_halt + + def _apply_pipelined_verdict( + self, + report: RunReport, + error: Optional[str], + *, + entry: Optional["PipelinedReadback"] = None, + ) -> None: + if error is None: + return + report.success = False + report.execution_completed = False + report.terminal_outcome = "halt" + report.results.append( + StepResult( + step_id="", + intent="program halt", + ok=False, + error=error, + safety_halt=True, + ) + ) + if entry is None: + entry = getattr(self, "_pipelined_readback", None) + report.halt = HaltObservation( + state_id=getattr(entry, "step_id", "") or "", + intent=getattr(entry, "intent", "") or "", + reason=error, + outcome="halt", + # The screen now belongs to the next item; nothing is probed. + observed_texts=[], + completed_intents=[r.intent for r in report.results if r.ok and not r.skipped], + ) + + def _settle_pipelined_readback( + self, + report: RunReport, + workflow: Workflow, + run_dir: Path, + entry: "PipelinedReadback", + error: Optional[str], + ) -> RunReport: + """Apply a finished deferred read-back to the saved item report.""" + + self._copy_pipelined_evidence(entry, error) + if error is not None: + self._apply_pipelined_verdict(report, error, entry=entry) + else: + # The first stamp saw an unconfirmed effect; re-derive from the + # completed execution now that it is confirmed. + if report.execution_completed: + report.success = True + self._stamp_execution_outcome(report, workflow, run_dir) + self._stamp_approval(report) + report.save(run_dir) return report def _project_idempotency_outcome_after_persist( @@ -6403,6 +6588,19 @@ def _run_step( new page halts. """ + pending_error = self._join_pipelined_readback() + if pending_error is not None: + # The previous action's deferred read-back failed: refuse to act + # further in this run, exactly as an inline failure would. + refused = StepResult( + step_id=step.id, + intent=step.intent, + ok=False, + error=pending_error, + safety_halt=True, + failure_category="safety_halt", + ) + return refused router = self._surface_router if router is None: if step.action is ActionKind.SWITCH_SURFACE: @@ -7554,7 +7752,24 @@ def _run_step_on_active_surface( # save, a phantom optimistic-UI success, a duplicate write, or a # lost update -- the EffectVerifier reads the REAL record and HALTs # on any non-CONFIRMED verdict (docs/design/EFFECT_VERIFIER.md). - if (error is None or delivery_uncertain) and effect_pre_state is not None: + if ( + error is None + and not delivery_uncertain + and effect_pre_state is not None + and self._may_pipeline_readback(active_verifier) + ): + # Pipelined read-back (runtime/readback_pipeline.py): the same + # check, run on the side thread. Any later step in this run + # waits for it first; the item's report is settled with the + # verdict when the check finishes. + self._defer_readback( + step, + effect_pre_state, + result, + effects=resolved_effects, + verifier=active_verifier, + ) + elif (error is None or delivery_uncertain) and effect_pre_state is not None: effect_error = self._verify_effects( step, effect_pre_state, @@ -11498,7 +11713,9 @@ def _act_without_dialog( if step.secret: # Secret value is never in the bundle/params: inject it from # the environment, failing fast with an actionable message. - env_var = secret_env_var(step.param or "") + env_var = getattr(self, "secret_env_overrides", {}).get( + step.param or "" + ) or secret_env_var(step.param or "") text = os.environ.get(env_var, "") if not step.param: return ( diff --git a/packages/flow/openadapt_flow/runtime/worklist_pool.py b/packages/flow/openadapt_flow/runtime/worklist_pool.py new file mode 100644 index 0000000000..ab10cf13df --- /dev/null +++ b/packages/flow/openadapt_flow/runtime/worklist_pool.py @@ -0,0 +1,728 @@ +"""Parallel worklist runs: N workers, one signed-in session each. + +A worklist run with ``--workers N`` replays the same compiled program once per +worklist item, spread over N workers. Every item is still a complete governed +run with every check (identity, input read-back, postconditions, the +system-of-record read-back). Only the scheduling changes. + +Rules this module enforces: + +- **One claim per item.** Items live in a durable SQLite queue in the run + directory. A worker claims the next pending item in one ``BEGIN IMMEDIATE`` + transaction, so no two workers can hold the same item, and an item that was + claimed is never handed out again, even after a crash. A claimed item with + no recorded result after a restart becomes ``uncertain`` and needs a + person: something may have been written. +- **One stop for all.** A halt, a failure, an error, or a + RECONCILIATION_REQUIRED outcome on any worker sets a stop flag in the same + database. Every worker checks it before each claim, so no new item starts. + Items already in flight finish their own checks. +- **Pipelined read-back is bounded.** With a read-back window ``k > 0`` a + worker may start its next item while up to ``k`` earlier items still wait + for their read-back. When a read-back fails the run stops, and the report + names the items (at most ``k`` per worker) that the same worker wrote after + the failed one. + +The pool does not know how to sign in or replay; :class:`PoolWorker` objects +do. The CLI builds web workers (one browser context and one account each), and +tests use fakes. +""" + +from __future__ import annotations + +import hashlib +import json +import sqlite3 +import threading +import time +from collections import deque +from concurrent.futures import Future +from dataclasses import asdict, dataclass, field +from pathlib import Path +from typing import Any, Callable, Iterable, Mapping, Optional, Protocol, Sequence + +QUEUE_FILENAME = "worklist_queue.sqlite3" +POOL_REPORT_FILENAME = "worklist_pool.json" +POOL_REPORT_MD = "WORKLIST.md" + +#: Item states. ``pending`` and ``claimed`` are live; the rest are final. +PENDING = "pending" +CLAIMED = "claimed" +VERIFIED = "verified" +COMPLETED = "completed" # finished, effects not certified (no production profile) +HALTED = "halted" +FAILED = "failed" +RECONCILIATION_REQUIRED = "reconciliation_required" +ERROR = "error" # the worker raised; whether anything was written is unknown +UNCERTAIN = "uncertain" # claimed before a crash, no result recorded +READBACK_PENDING = "readback_pending" + +FINAL_STATES = frozenset( + {VERIFIED, COMPLETED, HALTED, FAILED, RECONCILIATION_REQUIRED, ERROR, UNCERTAIN} +) +SUCCESS_STATES = frozenset({VERIFIED, COMPLETED}) + +#: ``on_halt`` policies. ``stop`` (default): any non-success item stops new +#: claims everywhere. ``skip_unwritten``: an item that halted before any write +#: (transaction outcome HALTED_BEFORE_EFFECT) is recorded and the run goes on; +#: every other non-success outcome still stops the run. +ON_HALT_STOP = "stop" +ON_HALT_SKIP_UNWRITTEN = "skip_unwritten" +ON_HALT_POLICIES = (ON_HALT_STOP, ON_HALT_SKIP_UNWRITTEN) + + +class PoolRefusal(ValueError): + """The pool configuration is unsafe; nothing was executed.""" + + +def rows_fingerprint(rows: Sequence[Mapping[str, Any]], *, salt: str = "") -> str: + """Stable digest of the worklist, so a resume can't swap the items.""" + + digest = hashlib.sha256(salt.encode()) + for row in rows: + digest.update(json.dumps(row, sort_keys=True, separators=(",", ":")).encode()) + digest.update(b"\n") + return digest.hexdigest() + + +def _now() -> float: + return time.time() + + +class WorklistQueue: + """Durable, exactly-once item queue shared by every worker. + + Each call opens its own SQLite connection, so worker threads never share + one. Claims use ``BEGIN IMMEDIATE`` (one writer at a time). + """ + + def __init__(self, path: Path) -> None: + self.path = Path(path) + + # -- setup ------------------------------------------------------------ + + def _connect(self) -> sqlite3.Connection: + conn = sqlite3.connect(self.path, timeout=60.0, isolation_level=None) + conn.execute("PRAGMA journal_mode=WAL") + conn.execute("PRAGMA synchronous=FULL") + return conn + + @classmethod + def open( + cls, + path: Path, + rows: Sequence[Mapping[str, Any]], + *, + fingerprint: str, + ) -> "WorklistQueue": + """Create the queue, or reopen one for the same worklist. + + Reopening marks every item still ``claimed`` as ``uncertain`` (its + worker died mid-item) and keeps any stop flag. A queue made for a + different worklist is refused. + """ + + queue = cls(path) + path.parent.mkdir(parents=True, exist_ok=True) + conn = queue._connect() + try: + conn.execute("BEGIN IMMEDIATE") + conn.execute( + "CREATE TABLE IF NOT EXISTS meta (key TEXT PRIMARY KEY, value TEXT)" + ) + conn.execute( + "CREATE TABLE IF NOT EXISTS items (" + " idx INTEGER PRIMARY KEY," + " row_json TEXT NOT NULL," + " state TEXT NOT NULL," + " worker TEXT," + " claimed_at REAL," + " finished_at REAL," + " result_json TEXT)" + ) + found = conn.execute( + "SELECT value FROM meta WHERE key='fingerprint'" + ).fetchone() + if found is None: + conn.execute( + "INSERT INTO meta (key, value) VALUES ('fingerprint', ?)", + (fingerprint,), + ) + conn.executemany( + "INSERT INTO items (idx, row_json, state) VALUES (?, ?, ?)", + [ + (i, json.dumps(dict(row), sort_keys=True), PENDING) + for i, row in enumerate(rows) + ], + ) + elif found[0] != fingerprint: + conn.execute("ROLLBACK") + raise PoolRefusal( + f"{path} belongs to a different worklist; use a new run directory" + ) + else: + conn.execute( + "UPDATE items SET state=?, finished_at=? WHERE state IN (?, ?)", + (UNCERTAIN, _now(), CLAIMED, READBACK_PENDING), + ) + conn.execute("COMMIT") + finally: + conn.close() + return queue + + # -- claims ----------------------------------------------------------- + + def claim(self, worker: str) -> Optional[tuple[int, dict[str, Any]]]: + """Claim the next pending item, or ``None`` when stopped or empty.""" + + conn = self._connect() + try: + conn.execute("BEGIN IMMEDIATE") + if conn.execute("SELECT 1 FROM meta WHERE key='stop'").fetchone(): + conn.execute("ROLLBACK") + return None + found = conn.execute( + "SELECT idx, row_json FROM items WHERE state=? ORDER BY idx LIMIT 1", + (PENDING,), + ).fetchone() + if found is None: + conn.execute("ROLLBACK") + return None + idx, row_json = found + changed = conn.execute( + "UPDATE items SET state=?, worker=?, claimed_at=? " + "WHERE idx=? AND state=?", + (CLAIMED, worker, _now(), idx, PENDING), + ).rowcount + if changed != 1: # pragma: no cover - guarded by BEGIN IMMEDIATE + conn.execute("ROLLBACK") + raise RuntimeError(f"item {idx} claim raced") + conn.execute("COMMIT") + return int(idx), json.loads(row_json) + finally: + conn.close() + + def set_state( + self, idx: int, worker: str, state: str, result: Mapping[str, Any] + ) -> None: + """Record an item's state; only the worker holding the claim may.""" + + conn = self._connect() + try: + conn.execute("BEGIN IMMEDIATE") + changed = conn.execute( + "UPDATE items SET state=?, finished_at=?, result_json=? " + "WHERE idx=? AND worker=? AND state IN (?, ?)", + ( + state, + _now() if state in FINAL_STATES else None, + json.dumps(dict(result), sort_keys=True, default=str), + idx, + worker, + CLAIMED, + READBACK_PENDING, + ), + ).rowcount + conn.execute("COMMIT") + finally: + conn.close() + if changed != 1: + raise RuntimeError(f"item {idx} is not claimed by {worker}") + + # -- stop flag -------------------------------------------------------- + + def request_stop(self, reason: str, *, worker: str, idx: Optional[int]) -> None: + """Stop new claims on every worker. The first reason wins.""" + + conn = self._connect() + try: + conn.execute( + "INSERT OR IGNORE INTO meta (key, value) VALUES ('stop', ?)", + ( + json.dumps( + {"reason": reason, "worker": worker, "idx": idx, "at": _now()} + ), + ), + ) + finally: + conn.close() + + def stop_reason(self) -> Optional[dict[str, Any]]: + conn = self._connect() + try: + found = conn.execute("SELECT value FROM meta WHERE key='stop'").fetchone() + finally: + conn.close() + return json.loads(found[0]) if found else None + + # -- reading ---------------------------------------------------------- + + def items(self) -> list[dict[str, Any]]: + conn = self._connect() + try: + rows = conn.execute( + "SELECT idx, state, worker, claimed_at, finished_at, result_json " + "FROM items ORDER BY idx" + ).fetchall() + finally: + conn.close() + return [ + { + "idx": r[0], + "state": r[1], + "worker": r[2], + "claimed_at": r[3], + "finished_at": r[4], + "result": json.loads(r[5]) if r[5] else None, + } + for r in rows + ] + + +@dataclass +class ItemRun: + """What a worker hands back for one item. + + ``state`` is the item's state as far as the worker knows. With pipelined + read-back, ``pending_readback`` is a future that resolves to the final + ``(state, detail)`` once the read-back finishes; until then the item is + ``readback_pending``. + """ + + state: str + detail: dict[str, Any] = field(default_factory=dict) + pending_readback: Optional["Future[tuple[str, dict[str, Any]]]"] = None + + +class PoolWorker(Protocol): + """One worker: its own signed-in session, used by one thread only.""" + + name: str + + def open(self) -> None: + """Start the session (browser, sign-in). Runs on the worker thread.""" + + def session_fingerprint(self) -> Optional[str]: + """A digest of the session's credentials (e.g. its cookies), or None. + + The pool refuses to run when two workers report the same digest: + they would share one signed-in session. + """ + + def run_item(self, idx: int, row: dict[str, Any], item_dir: Path) -> ItemRun: + """Replay one item. Runs on the worker thread.""" + + def close(self) -> None: + """End the session. Runs on the worker thread.""" + + +@dataclass(frozen=True) +class PoolPolicy: + workers: int + readback_window: int = 0 + on_halt: str = ON_HALT_STOP + items_independent: bool = False + #: The app's declared concurrency limit (sessions at once). + max_workers: Optional[int] = None + + def refusal(self) -> Optional[str]: + if self.workers < 1: + return "--workers must be at least 1" + if self.max_workers is not None and self.workers > self.max_workers: + return ( + f"--workers {self.workers} is above the deployment's limit of " + f"{self.max_workers} concurrent sessions for this app " + "(worklist.max_workers)" + ) + if self.workers > 1 and not self.items_independent: + return ( + "--workers above 1 runs items out of order and at the same " + "time; the deployment must declare worklist.items_independent: " + "true" + ) + if self.readback_window < 0: + return "the read-back window must be 0 or more" + if self.readback_window > 0 and not self.items_independent: + return ( + "a read-back window above 0 starts the next item before the " + "last read-back finishes; the deployment must declare " + "worklist.items_independent: true" + ) + if self.on_halt not in ON_HALT_POLICIES: + return f"worklist.on_halt must be one of {', '.join(ON_HALT_POLICIES)}" + return None + + +def item_state_from_report(report: Any) -> str: + """Map a finished item's RunReport to a pool item state.""" + + transaction = getattr(report, "transaction_outcome", None) + transaction = getattr(transaction, "value", transaction) + if transaction == "reconciliation_required": + return RECONCILIATION_REQUIRED + outcome = getattr(report, "execution_outcome", None) + outcome = getattr(outcome, "value", outcome) + if outcome is not None: + return { + "verified": VERIFIED, + "completed_unverified": COMPLETED, + "halted": HALTED, + "rolled_back": FAILED, + "failed": FAILED, + }.get(str(outcome).lower(), FAILED) + return COMPLETED if getattr(report, "success", False) else HALTED + + +def item_detail_from_report(report: Any) -> dict[str, Any]: + def _v(x: Any) -> Any: + return getattr(x, "value", x) + + halt = getattr(report, "halt", None) + return { + "success": bool(getattr(report, "success", False)), + "execution_outcome": _v(getattr(report, "execution_outcome", None)), + "transaction_outcome": _v(getattr(report, "transaction_outcome", None)), + "halt_reason": getattr(halt, "reason", None) if halt is not None else None, + "total_ms": getattr(report, "total_ms", None), + } + + +def _stops_run(state: str, detail: Mapping[str, Any], on_halt: str) -> bool: + if state in SUCCESS_STATES: + return False + if ( + on_halt == ON_HALT_SKIP_UNWRITTEN + and state == HALTED + and detail.get("transaction_outcome") == "halted_before_effect" + ): + return False + return True + + +@dataclass +class WorkerSummary: + name: str + items: list[int] = field(default_factory=list) + states: dict[str, int] = field(default_factory=dict) + busy_s: float = 0.0 + open_s: float = 0.0 + error: Optional[str] = None + + +@dataclass +class PoolReport: + workers: int + readback_window: int + on_halt: str + started_at: float + ended_at: float + stop: Optional[dict[str, Any]] + items: list[dict[str, Any]] + per_worker: list[WorkerSummary] + written_after_failed_readback: dict[str, list[int]] = field(default_factory=dict) + + @property + def wall_s(self) -> float: + return self.ended_at - self.started_at + + def counts(self) -> dict[str, int]: + out: dict[str, int] = {} + for item in self.items: + out[item["state"]] = out.get(item["state"], 0) + 1 + return out + + @property + def success(self) -> bool: + return self.stop is None and all( + item["state"] in SUCCESS_STATES for item in self.items + ) + + def entries_per_hour(self) -> Optional[float]: + done = sum(1 for item in self.items if item["state"] in SUCCESS_STATES) + return round(done * 3600.0 / self.wall_s, 1) if self.wall_s > 0 else None + + def to_json(self) -> dict[str, Any]: + return { + "schema": "openadapt.worklist_pool/1", + "workers": self.workers, + "readback_window": self.readback_window, + "on_halt": self.on_halt, + "started_at": self.started_at, + "ended_at": self.ended_at, + "wall_s": round(self.wall_s, 3), + "success": self.success, + "counts": self.counts(), + "entries_per_hour": self.entries_per_hour(), + "stop": self.stop, + "written_after_failed_readback": self.written_after_failed_readback, + "per_worker": [asdict(w) for w in self.per_worker], + "items": self.items, + } + + def render_md(self) -> str: + lines = [ + "# Worklist run", + "", + f"- Workers: {self.workers}; read-back window: {self.readback_window}; " + f"on halt: {self.on_halt}", + f"- Wall time: {self.wall_s:.1f} s; finished correctly: " + f"{sum(1 for i in self.items if i['state'] in SUCCESS_STATES)} of " + f"{len(self.items)}", + ] + if self.stop: + where = f"item {self.stop.get('idx')}" if self.stop.get("idx") is not None else "the pool" + lines.append( + f"- **Stopped** by {self.stop.get('worker')} at {where}: " + f"{self.stop.get('reason')}" + ) + for idx, after in sorted(self.written_after_failed_readback.items()): + lines.append( + f"- Read-back of item {idx} failed. Items written after it by " + f"the same worker: {', '.join(map(str, after)) or 'none'}" + ) + lines += [ + "", + "## Per worker", + "", + "| Worker | Items | Results | Busy (s) | Sign-in (s) |", + "|---|---:|---|---:|---:|", + ] + for w in self.per_worker: + states = ", ".join(f"{k} {v}" for k, v in sorted(w.states.items())) or "-" + if w.error: + states += f" (worker error: {w.error})" + lines.append( + f"| {w.name} | {len(w.items)} | {states} | {w.busy_s:.1f} | {w.open_s:.1f} |" + ) + lines += ["", "## Per item", "", "| Item | Worker | State | Seconds |", "|---:|---|---|---:|"] + for item in self.items: + res = item.get("result") or {} + secs = res.get("wall_s") + shown = f"{secs:.1f}" if isinstance(secs, (int, float)) else "-" + lines.append( + f"| {item['idx']} | {item.get('worker') or '-'} | {item['state']} | {shown} |" + ) + return "\n".join(lines) + "\n" + + def save(self, run_dir: Path) -> Path: + run_dir.mkdir(parents=True, exist_ok=True) + path = run_dir / POOL_REPORT_FILENAME + tmp = path.with_suffix(".tmp") + tmp.write_text(json.dumps(self.to_json(), indent=1, default=str)) + tmp.replace(path) + (run_dir / POOL_REPORT_MD).write_text(self.render_md()) + return path + + +def run_worklist_pool( + rows: Sequence[Mapping[str, Any]], + workers: Sequence[PoolWorker], + *, + run_dir: Path, + policy: PoolPolicy, + fingerprint_salt: str = "", + on_event: Optional[Callable[[str], None]] = None, +) -> PoolReport: + """Run every row on the pool and return the report (also saved). + + Raises :class:`PoolRefusal` before anything runs when the policy is + unsafe, the number of workers differs from ``policy.workers``, or two + workers share a session. + """ + + refusal = policy.refusal() + if refusal is None and len(workers) != policy.workers: + refusal = f"{policy.workers} workers requested but {len(workers)} configured" + names = [w.name for w in workers] + if refusal is None and len(set(names)) != len(names): + refusal = "worker names must be distinct" + if refusal is not None: + raise PoolRefusal(refusal) + + say = on_event or (lambda _msg: None) + run_dir = Path(run_dir) + queue = WorklistQueue.open( + run_dir / QUEUE_FILENAME, + rows, + fingerprint=rows_fingerprint(rows, salt=fingerprint_salt), + ) + started = _now() + summaries = {w.name: WorkerSummary(name=w.name) for w in workers} + written_after: dict[str, list[int]] = {} + lock = threading.Lock() + + # Phase 1: every worker opens its own session; the pool then checks that + # no two share one before any item is claimed. + opened = threading.Barrier(len(workers) + 1) + go = threading.Event() + abort = threading.Event() + fingerprints: dict[str, Optional[str]] = {} + + def stop(reason: str, worker: str, idx: Optional[int]) -> None: + queue.request_stop(reason, worker=worker, idx=idx) + say(f"STOP ({worker}, item {idx}): {reason}") + + def finish(worker: PoolWorker, idx: int, state: str, detail: dict[str, Any]) -> None: + queue.set_state(idx, worker.name, state, detail) + with lock: + s = summaries[worker.name] + s.states[state] = s.states.get(state, 0) + 1 + say(f"{worker.name}: item {idx} {state}") + if _stops_run(state, detail, policy.on_halt): + stop( + detail.get("halt_reason") or f"item {idx} ended {state}", + worker.name, + idx, + ) + + def worker_main(worker: PoolWorker) -> None: + summary = summaries[worker.name] + t0 = time.monotonic() + try: + worker.open() + fingerprints[worker.name] = worker.session_fingerprint() + except Exception as exc: # noqa: BLE001 - recorded, run aborted + summary.error = f"sign-in failed: {type(exc).__name__}" + abort.set() + summary.open_s = time.monotonic() - t0 + try: + opened.wait() + except threading.BrokenBarrierError: # pragma: no cover + abort.set() + go.wait() + pending: deque[tuple[int, Future, list[int]]] = deque() + + def settle(entry: tuple[int, Future, list[int]]) -> None: + idx, fut, after = entry + try: + state, detail = fut.result() + except Exception as exc: # noqa: BLE001 + state, detail = ERROR, {"halt_reason": f"read-back raised {type(exc).__name__}"} + if state not in SUCCESS_STATES: + with lock: + written_after[str(idx)] = list(after) + detail = {**detail, "written_after": list(after)} + finish(worker, idx, state, detail) + + try: + if abort.is_set(): + return + while True: + while len(pending) > policy.readback_window: + settle(pending.popleft()) + claimed = queue.claim(worker.name) + if claimed is None: + break + idx, row = claimed + summary.items.append(idx) + for entry in pending: + entry[2].append(idx) + item_dir = run_dir / "items" / f"{idx:05d}" + t_item = time.monotonic() + try: + out = worker.run_item(idx, row, item_dir) + except Exception as exc: # noqa: BLE001 - fail closed + out = ItemRun( + state=ERROR, + detail={"halt_reason": f"worker raised {type(exc).__name__}: {exc}"[:300]}, + ) + wall = time.monotonic() - t_item + summary.busy_s += wall + detail = {**out.detail, "wall_s": round(wall, 3), "worker": worker.name} + if out.pending_readback is not None and out.state in SUCCESS_STATES: + queue.set_state(idx, worker.name, READBACK_PENDING, detail) + fut: Future = Future() + + def _chain(src: Future, dst: Future = fut, base: dict = detail) -> None: + try: + st, more = src.result() + dst.set_result((st, {**base, **more, "readback": "pipelined"})) + except Exception as exc: # noqa: BLE001 + dst.set_exception(exc) + + out.pending_readback.add_done_callback(_chain) + pending.append((idx, fut, [])) + else: + finish(worker, idx, out.state, detail) + while pending: + settle(pending.popleft()) + except Exception as exc: # noqa: BLE001 - never leave peers running blind + summary.error = f"{type(exc).__name__}: {exc}"[:300] + stop(f"worker {worker.name} crashed", worker.name, None) + while pending: + settle(pending.popleft()) + finally: + try: + worker.close() + except Exception: # noqa: BLE001 + pass + + threads = [ + threading.Thread(target=worker_main, args=(w,), name=f"worklist-{w.name}", daemon=True) + for w in workers + ] + for t in threads: + t.start() + opened.wait() + shared = _shared_sessions(fingerprints) + if shared: + abort.set() + go.set() + for t in threads: + t.join() + raise PoolRefusal( + "workers " + + " and ".join(shared) + + " share one signed-in session; each worker needs its own account" + ) + if abort.is_set(): + go.set() + for t in threads: + t.join() + failed = [s.name for s in summaries.values() if s.error] + raise PoolRefusal(f"sign-in failed for {', '.join(failed)}; nothing was executed") + go.set() + for t in threads: + t.join() + + report = PoolReport( + workers=policy.workers, + readback_window=policy.readback_window, + on_halt=policy.on_halt, + started_at=started, + ended_at=_now(), + stop=queue.stop_reason(), + items=queue.items(), + per_worker=list(summaries.values()), + written_after_failed_readback=written_after, + ) + report.save(run_dir) + return report + + +def _shared_sessions(fingerprints: Mapping[str, Optional[str]]) -> list[str]: + seen: dict[str, str] = {} + for name, fp in fingerprints.items(): + if fp is None: + continue + if fp in seen: + return [seen[fp], name] + seen[fp] = name + return [] + + +def cookie_fingerprint(cookies: Iterable[Mapping[str, Any]]) -> Optional[str]: + """Digest of a browser context's cookies (names, domains and values). + + Two contexts signed in as different users have different session cookie + values; two that share one session have the same ones. Returns None when + there are no cookies (nothing to compare). + """ + + parts = sorted( + f"{c.get('domain', '')}\t{c.get('path', '')}\t{c.get('name', '')}\t{c.get('value', '')}" + for c in cookies + ) + if not parts: + return None + return hashlib.sha256("\n".join(parts).encode()).hexdigest() diff --git a/packages/flow/tests/test_readback_pipeline.py b/packages/flow/tests/test_readback_pipeline.py new file mode 100644 index 0000000000..b84b13b9ea --- /dev/null +++ b/packages/flow/tests/test_readback_pipeline.py @@ -0,0 +1,137 @@ +"""Pipelined read-back in the real Replayer: same check, later timing.""" + +from __future__ import annotations + +import json + +import pytest + +from openadapt_flow.compiler.loop_authoring import author_data_driven_loop +from openadapt_flow.mockmed.fault_server import serve as fault_serve +from openadapt_flow.runtime.effects import RestRecordVerifier +from openadapt_flow.runtime.readback_pipeline import ReadbackPipeline +from openadapt_flow.runtime.replayer import Replayer +from tests.test_loop_authoring import ( + _encounter_body, + _RowWritingBackend, + _vision_confirms_saved, +) +from tests.test_replayer import make_png + + +@pytest.fixture() +def dirs(tmp_path): + bundle = tmp_path / "bundle" + (bundle / "templates").mkdir(parents=True) + (bundle / "templates" / "btn.png").write_bytes(make_png((50, 20))) + return bundle, tmp_path / "run" + + +def _run(records, dirs, *, corrupt=None, pipeline=None, verifier_factory=None): + bundle, run_dir = dirs + looped = author_data_driven_loop(_encounter_body(), records) + url, db, stop = fault_serve() + backend = _RowWritingBackend(url, corrupt=corrupt) + replayer = Replayer( + backend, + vision=_vision_confirms_saved(), + effect_verifier=(verifier_factory or RestRecordVerifier)(url), + poll_interval_s=0.01, + readback_pipeline=pipeline, + ) + try: + report = replayer.run(looped, bundle_dir=bundle, run_dir=run_dir) + handoff = pipeline.take() if pipeline is not None else None + settled = handoff.settled.result(timeout=30) if handoff is not None else None + finally: + stop() + return report, settled, backend, run_dir + + +def test_confirmed_readback_settles_the_saved_report(dirs) -> None: + pipeline = ReadbackPipeline() + report, settled, backend, run_dir = _run( + [{"patient_id": "alice", "note": "alice triage"}], dirs, pipeline=pipeline + ) + assert settled is report + assert report.success is True + save = [r for r in report.results if r.step_id == "save"][-1] + assert save.effect_verified is True + assert any("CONFIRMED" in line for line in save.effect_results) + assert not any(line.startswith("[pipelined]") for line in save.effect_results) + saved = json.loads((run_dir / "report.json").read_text()) + saved_save = [r for r in saved["results"] if r["step_id"] == "save"][-1] + assert saved_save["effect_verified"] is True + pipeline.close() + + +def test_refuted_readback_halts_the_item_after_the_fact(dirs) -> None: + pipeline = ReadbackPipeline() + report, settled, backend, run_dir = _run( + [{"patient_id": "bob", "note": "bob triage"}], + dirs, + corrupt={"bob": "mallory"}, + pipeline=pipeline, + ) + assert settled is report + assert report.success is False + assert report.terminal_outcome == "halt" + assert report.halt is not None and "HALTED" in report.halt.reason + save = [r for r in report.results if r.step_id == "save"][-1] + assert save.effect_verified is False and save.ok is False + saved = json.loads((run_dir / "report.json").read_text()) + assert saved["success"] is False + pipeline.close() + + +def test_a_later_step_waits_for_the_pending_readback_and_halts(dirs) -> None: + """Inside one run, the next record's first step joins the deferred check.""" + + pipeline = ReadbackPipeline() + report, settled, backend, _ = _run( + [ + {"patient_id": "alice", "note": "alice triage"}, + {"patient_id": "bob", "note": "bob triage"}, + {"patient_id": "cara", "note": "cara triage"}, + ], + dirs, + corrupt={"bob": "mallory"}, + pipeline=pipeline, + ) + # bob's wrong write was caught before cara's first keystroke. + assert backend.posted == [("alice", "alice triage"), ("mallory", "bob triage")] + assert report.success is False + assert settled is None # nothing left pending: the run settled inline + saves = [r for r in report.results if r.step_id == "save"] + assert [s.effect_verified for s in saves] == [True, False] + # The refusal is cara's first step, refused on bob's deferred verdict. + refused = [r for r in report.results if r.step_id == "type_patient" and not r.ok] + assert len(refused) == 1 and "HALTED" in (refused[0].error or "") + pipeline.close() + + +def test_without_a_pipeline_the_readback_stays_inline(dirs) -> None: + report, settled, backend, _ = _run( + [{"patient_id": "alice", "note": "alice triage"}], dirs + ) + assert settled is None + assert report.success is True + save = [r for r in report.results if r.step_id == "save"][-1] + assert save.effect_verified is True + + +def test_screen_bound_verifier_is_never_deferred(dirs) -> None: + class ScreenVerifier(RestRecordVerifier): + backend = object() # reads the screen: needs the browser + + pipeline = ReadbackPipeline() + report, settled, backend, _ = _run( + [{"patient_id": "alice", "note": "alice triage"}], + dirs, + pipeline=pipeline, + verifier_factory=ScreenVerifier, + ) + assert settled is None + assert pipeline.inline_reason == "the read-back uses the screen" + assert report.success is True + pipeline.close() diff --git a/packages/flow/tests/test_worklist_pool.py b/packages/flow/tests/test_worklist_pool.py new file mode 100644 index 0000000000..b6c89f3852 --- /dev/null +++ b/packages/flow/tests/test_worklist_pool.py @@ -0,0 +1,320 @@ +"""Parallel worklist pool: one claim per item, one stop for all workers.""" + +from __future__ import annotations + +import json +import threading +import time +from concurrent.futures import Future +from pathlib import Path +from typing import Any, Callable, Optional + +import pytest + +from openadapt_flow.runtime.worklist_pool import ( + COMPLETED, + ERROR, + HALTED, + POOL_REPORT_FILENAME, + POOL_REPORT_MD, + QUEUE_FILENAME, + RECONCILIATION_REQUIRED, + UNCERTAIN, + VERIFIED, + ItemRun, + PoolPolicy, + PoolRefusal, + WorklistQueue, + cookie_fingerprint, + item_state_from_report, + rows_fingerprint, + run_worklist_pool, +) + + +class FakeWorker: + """Runs items by calling ``behave(idx)``; records what it did.""" + + def __init__( + self, + name: str, + behave: Optional[Callable[[int], ItemRun]] = None, + *, + delay_s: float = 0.01, + fingerprint: Optional[str] = None, + fail_open: bool = False, + ) -> None: + self.name = name + self.behave = behave or (lambda idx: ItemRun(state=VERIFIED)) + self.delay_s = delay_s + self.fingerprint = fingerprint if fingerprint is not None else f"fp-{name}" + self.fail_open = fail_open + self.ran: list[int] = [] + self.opened = False + self.closed = False + self.thread: Optional[int] = None + + def open(self) -> None: + self.thread = threading.get_ident() + if self.fail_open: + raise RuntimeError("bad password") + self.opened = True + + def session_fingerprint(self) -> Optional[str]: + return self.fingerprint + + def run_item(self, idx: int, row: dict[str, Any], item_dir: Path) -> ItemRun: + assert threading.get_ident() == self.thread, "item ran off the worker thread" + self.ran.append(idx) + time.sleep(self.delay_s) + return self.behave(idx) + + def close(self) -> None: + self.closed = True + + +def _rows(n: int) -> list[dict[str, str]]: + return [{"name": f"patient-{i}"} for i in range(n)] + + +def _policy(workers: int, **kw: Any) -> PoolPolicy: + kw.setdefault("items_independent", True) + return PoolPolicy(workers=workers, **kw) + + +def test_each_item_is_claimed_exactly_once(tmp_path: Path) -> None: + workers = [FakeWorker(f"w{i}", delay_s=0.002) for i in range(8)] + report = run_worklist_pool(_rows(200), workers, run_dir=tmp_path, policy=_policy(8)) + ran = [idx for w in workers for idx in w.ran] + assert sorted(ran) == list(range(200)) + assert len(ran) == len(set(ran)) + assert report.success + assert report.counts() == {VERIFIED: 200} + # Work is actually spread: every worker ran something. + assert all(w.ran for w in workers) + assert all(w.closed for w in workers) + saved = json.loads((tmp_path / POOL_REPORT_FILENAME).read_text()) + assert saved["counts"] == {VERIFIED: 200} + assert {w["name"] for w in saved["per_worker"]} == {f"w{i}" for i in range(8)} + assert sum(len(w["items"]) for w in saved["per_worker"]) == 200 + assert "## Per worker" in (tmp_path / POOL_REPORT_MD).read_text() + + +def test_forced_halt_on_one_worker_stops_the_others(tmp_path: Path) -> None: + """A halt on worker w1 stops new claims on every worker; in-flight items finish.""" + + halted = threading.Event() + in_flight_finished: list[int] = [] + + def halting(idx: int) -> ItemRun: + if idx == 5: + halted.set() + return ItemRun( + state=HALTED, + detail={"halt_reason": "identity check failed", "transaction_outcome": "halted_before_effect"}, + ) + return ItemRun(state=VERIFIED) + + def slow_peer(idx: int) -> ItemRun: + # A peer item that is mid-flight when the halt lands still finishes + # and records its own result. + if halted.is_set(): + in_flight_finished.append(idx) + return ItemRun(state=VERIFIED) + + w1 = FakeWorker("w1", halting, delay_s=0.01) + peers = [FakeWorker(f"w{i}", slow_peer, delay_s=0.03) for i in (2, 3, 4)] + report = run_worklist_pool(_rows(100), [w1, *peers], run_dir=tmp_path, policy=_policy(4)) + + assert not report.success + assert report.stop is not None + assert report.stop["worker"] == "w1" and report.stop["idx"] == 5 + assert "identity check failed" in report.stop["reason"] + states = {item["idx"]: item["state"] for item in report.items} + assert states[5] == HALTED + # Far fewer than 100 items ran: claims stopped everywhere. + ran = [idx for w in (w1, *peers) for idx in w.ran] + assert len(ran) < 40 + # Every item that ran has a final result; nothing is left "claimed". + for idx in ran: + assert states[idx] in {VERIFIED, HALTED} + assert "pending" in states.values() + # No worker claimed anything after the stop flag was set. + stop_at = report.stop["at"] + for item in report.items: + if item["claimed_at"] is not None and item["idx"] != 5: + assert item["claimed_at"] <= stop_at + 0.001 + + +def test_reconciliation_required_stops_even_when_skipping_halts(tmp_path: Path) -> None: + def behave(idx: int) -> ItemRun: + if idx == 0: + return ItemRun( + state=HALTED, detail={"transaction_outcome": "halted_before_effect"} + ) + if idx == 3: + return ItemRun(state=RECONCILIATION_REQUIRED, detail={"halt_reason": "write not confirmed"}) + return ItemRun(state=VERIFIED) + + w = FakeWorker("w1", behave) + report = run_worklist_pool( + _rows(10), [w], run_dir=tmp_path, policy=_policy(1, on_halt="skip_unwritten") + ) + # Item 0 halted before any write and was skipped; item 3 stopped the run. + assert w.ran == [0, 1, 2, 3] + assert report.stop is not None and report.stop["idx"] == 3 + + +def test_worker_exception_is_an_error_and_stops(tmp_path: Path) -> None: + def boom(idx: int) -> ItemRun: + if idx == 2: + raise RuntimeError("browser crashed") + return ItemRun(state=COMPLETED) + + report = run_worklist_pool(_rows(10), [FakeWorker("w1", boom)], run_dir=tmp_path, policy=_policy(1)) + states = {item["idx"]: item["state"] for item in report.items} + assert states[2] == ERROR + assert report.stop is not None and report.stop["idx"] == 2 + assert states[3] == "pending" + + +def test_refuses_more_workers_than_the_app_allows(tmp_path: Path) -> None: + workers = [FakeWorker(f"w{i}") for i in range(4)] + with pytest.raises(PoolRefusal, match="limit of 2"): + run_worklist_pool(_rows(4), workers, run_dir=tmp_path, policy=_policy(4, max_workers=2)) + assert not any(w.opened for w in workers) + assert not (tmp_path / QUEUE_FILENAME).exists() + + +def test_refuses_parallel_items_not_declared_independent(tmp_path: Path) -> None: + with pytest.raises(PoolRefusal, match="items_independent"): + run_worklist_pool( + _rows(4), + [FakeWorker("a"), FakeWorker("b")], + run_dir=tmp_path, + policy=PoolPolicy(workers=2), + ) + with pytest.raises(PoolRefusal, match="items_independent"): + run_worklist_pool( + _rows(4), [FakeWorker("a")], run_dir=tmp_path, policy=PoolPolicy(workers=1, readback_window=1) + ) + + +def test_refuses_workers_that_share_one_session(tmp_path: Path) -> None: + a = FakeWorker("a", fingerprint="same") + b = FakeWorker("b", fingerprint="same") + with pytest.raises(PoolRefusal, match="share one signed-in session"): + run_worklist_pool(_rows(4), [a, b], run_dir=tmp_path, policy=_policy(2)) + assert a.ran == [] and b.ran == [] + assert a.closed and b.closed + + +def test_sign_in_failure_runs_nothing(tmp_path: Path) -> None: + a = FakeWorker("a") + b = FakeWorker("b", fail_open=True) + with pytest.raises(PoolRefusal, match="sign-in failed for b"): + run_worklist_pool(_rows(4), [a, b], run_dir=tmp_path, policy=_policy(2)) + assert a.ran == [] + + +def test_reopen_marks_claimed_items_uncertain_and_never_reclaims(tmp_path: Path) -> None: + rows = _rows(3) + fp = rows_fingerprint(rows) + q = WorklistQueue.open(tmp_path / QUEUE_FILENAME, rows, fingerprint=fp) + assert q.claim("w1") == (0, rows[0]) + # Crash: item 0 was claimed but never finished. Reopen for a resume. + q2 = WorklistQueue.open(tmp_path / QUEUE_FILENAME, rows, fingerprint=fp) + assert q2.items()[0]["state"] == UNCERTAIN + assert q2.claim("w2") == (1, rows[1]) + with pytest.raises(PoolRefusal, match="different worklist"): + WorklistQueue.open(tmp_path / QUEUE_FILENAME, _rows(4), fingerprint=rows_fingerprint(_rows(4))) + + +def test_only_the_claiming_worker_records_a_result(tmp_path: Path) -> None: + rows = _rows(1) + q = WorklistQueue.open(tmp_path / QUEUE_FILENAME, rows, fingerprint=rows_fingerprint(rows)) + q.claim("w1") + with pytest.raises(RuntimeError, match="not claimed by w2"): + q.set_state(0, "w2", VERIFIED, {}) + + +class PipelinedWorker(FakeWorker): + """Read-back runs on a side thread and resolves after ``readback_s``.""" + + def __init__(self, name: str, fail_idx: Optional[int], readback_s: float, **kw: Any) -> None: + super().__init__(name, **kw) + self.fail_idx = fail_idx + self.readback_s = readback_s + self.max_outstanding = 0 + self._outstanding = 0 + self._lock = threading.Lock() + + def run_item(self, idx: int, row: dict[str, Any], item_dir: Path) -> ItemRun: + self.ran.append(idx) + time.sleep(self.delay_s) + fut: Future = Future() + with self._lock: + self._outstanding += 1 + self.max_outstanding = max(self.max_outstanding, self._outstanding) + + def readback() -> None: + time.sleep(self.readback_s) + with self._lock: + self._outstanding -= 1 + if idx == self.fail_idx: + fut.set_result((RECONCILIATION_REQUIRED, {"halt_reason": "saved record differs"})) + else: + fut.set_result((VERIFIED, {})) + + threading.Thread(target=readback, daemon=True).start() + return ItemRun(state=VERIFIED, pending_readback=fut) + + +@pytest.mark.parametrize("window", [1, 2]) +def test_pipelined_readback_failure_names_items_written_after(tmp_path: Path, window: int) -> None: + w = PipelinedWorker("w1", fail_idx=3, readback_s=0.05, delay_s=0.01) + report = run_worklist_pool( + _rows(20), [w], run_dir=tmp_path, policy=_policy(1, readback_window=window) + ) + assert report.stop is not None and report.stop["idx"] == 3 + after = report.written_after_failed_readback["3"] + assert 1 <= len(after) <= window + assert after == list(range(4, 4 + len(after))) + # At most `window` read-backs wait while the next item is written. + assert w.max_outstanding <= window + 1 + states = {item["idx"]: item["state"] for item in report.items} + assert states[3] == RECONCILIATION_REQUIRED + # The items written after it still got their own read-back. + for idx in after: + assert states[idx] == VERIFIED + assert max(w.ran) <= 3 + window + md = (tmp_path / POOL_REPORT_MD).read_text() + assert "Read-back of item 3 failed" in md + + +def test_window_zero_waits_for_each_readback(tmp_path: Path) -> None: + w = PipelinedWorker("w1", fail_idx=None, readback_s=0.01, delay_s=0.0) + report = run_worklist_pool(_rows(5), [w], run_dir=tmp_path, policy=_policy(1)) + assert report.success + assert w.max_outstanding == 1 + + +def test_item_state_from_report() -> None: + class R: + def __init__(self, **kw: Any) -> None: + self.__dict__.update(kw) + + assert item_state_from_report(R(transaction_outcome="reconciliation_required", execution_outcome="halted")) == RECONCILIATION_REQUIRED + assert item_state_from_report(R(execution_outcome="verified")) == VERIFIED + assert item_state_from_report(R(execution_outcome="completed_unverified")) == COMPLETED + assert item_state_from_report(R(execution_outcome="halted")) == HALTED + assert item_state_from_report(R(success=True)) == COMPLETED + assert item_state_from_report(R(success=False)) == HALTED + + +def test_cookie_fingerprint_tells_sessions_apart() -> None: + a = [{"domain": "emr.test", "path": "/", "name": "OpenEMR", "value": "s1"}] + b = [{"domain": "emr.test", "path": "/", "name": "OpenEMR", "value": "s2"}] + assert cookie_fingerprint(a) != cookie_fingerprint(b) + assert cookie_fingerprint(a) == cookie_fingerprint(list(a)) + assert cookie_fingerprint([]) is None From 4fd89d1042b0002ac54a1d7c88fdee360e5f2345 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 18 Oct 2026 14:27:28 -0400 Subject: [PATCH 03/18] feat(flow): --workers N and --readback-window K for worklist runs Co-Authored-By: Claude Opus 5.5 --- .../benchmark/openemr_throughput/profiler.py | 53 ++- packages/flow/openadapt_flow/__main__.py | 270 ++++++++++++- .../flow/openadapt_flow/backends/factory.py | 6 + .../flow/openadapt_flow/cli_worklist_pool.py | 370 ++++++++++++++++++ packages/flow/openadapt_flow/deployment.py | 65 ++- .../runtime/readback_pipeline.py | 20 +- .../flow/openadapt_flow/runtime/replayer.py | 16 +- .../openadapt_flow/runtime/worklist_pool.py | 45 ++- packages/flow/tests/test_cli_worklist_pool.py | 267 +++++++++++++ packages/flow/tests/test_worklist_pool.py | 62 ++- 10 files changed, 1123 insertions(+), 51 deletions(-) create mode 100644 packages/flow/openadapt_flow/cli_worklist_pool.py create mode 100644 packages/flow/tests/test_cli_worklist_pool.py diff --git a/packages/flow/benchmark/openemr_throughput/profiler.py b/packages/flow/benchmark/openemr_throughput/profiler.py index 4c4619f829..dd412e81d7 100644 --- a/packages/flow/benchmark/openemr_throughput/profiler.py +++ b/packages/flow/benchmark/openemr_throughput/profiler.py @@ -100,7 +100,10 @@ # Report buckets: which named calls count toward each. BUCKETS: dict[str, tuple[str, ...]] = { "waits: fixed sleeps (poll intervals)": ("time.sleep",), - "waits: page signals (DOM/network quiet)": ("be.page_quiescent", "be.page_change_marker"), + "waits: page signals (DOM/network quiet)": ( + "be.page_quiescent", + "be.page_change_marker", + ), "waits: screen settle (frame compare)": ( "vision.wait_settled", "vision.wait_settled_result", @@ -132,7 +135,12 @@ "vision.find_text", "vision.find_text_candidates", ), - "typing and keys": ("be.type_text", "be.type_text_guarded", "be.press", "be.press_guarded"), + "typing and keys": ( + "be.type_text", + "be.type_text_guarded", + "be.press", + "be.press_guarded", + ), "clicks and scrolls": ( "be.click", "be.right_click", @@ -179,8 +187,12 @@ def __init__(self) -> None: self.stats: dict[str, dict[str, float]] = defaultdict( lambda: {"n": 0, "excl_s": 0.0, "incl_s": 0.0} ) - self.steps: dict[str, dict[str, float]] = defaultdict(lambda: defaultdict(float)) - self.by_parent: dict[str, dict[str, float]] = defaultdict(lambda: defaultdict(float)) + self.steps: dict[str, dict[str, float]] = defaultdict( + lambda: defaultdict(float) + ) + self.by_parent: dict[str, dict[str, float]] = defaultdict( + lambda: defaultdict(float) + ) self._current_step: str | None = None self._undo: list[Callable[[], None]] = [] @@ -194,10 +206,13 @@ def _exit(self) -> None: incl = time.perf_counter() - start excl = incl - child if name == "vision.ocr" and any( - f[0] in ("rp._verify_identity_ocr", "rp._verify_identity") for f in self._stack + f[0] in ("rp._verify_identity_ocr", "rp._verify_identity") + for f in self._stack ): name = "vision.ocr@identity" - parent = next((f[0] for f in reversed(self._stack) if f[0].startswith("rp.")), "-") + parent = next( + (f[0] for f in reversed(self._stack) if f[0].startswith("rp.")), "-" + ) self.by_parent[name][parent] += excl st = self.stats[name] st["n"] += 1 @@ -219,7 +234,11 @@ def wrapper(*a: Any, **k: Any) -> Any: return fn(*a, **k) prev_step = prof._current_step if name == "rp._run_step": - step = a[0 if static else 1] if len(a) > (0 if static else 1) else k.get("step") + step = ( + a[0 if static else 1] + if len(a) > (0 if static else 1) + else k.get("step") + ) prof._current_step = getattr(step, "id", None) or prev_step prof._enter(name) try: @@ -251,7 +270,9 @@ def install(self, backend: Any) -> None: if raw is None: continue if isinstance(raw, staticmethod): - wrapped: Any = staticmethod(self._wrap(f"rp.{m}", raw.__func__, static=True)) + wrapped: Any = staticmethod( + self._wrap(f"rp.{m}", raw.__func__, static=True) + ) else: wrapped = self._wrap(f"rp.{m}", raw) setattr(Replayer, m, wrapped) @@ -269,7 +290,9 @@ def install(self, backend: Any) -> None: continue wrapped = self._wrap(f"vision.{fn_name}", orig) setattr(mod, fn_name, wrapped) - self._undo.append(lambda mod=mod, fn_name=fn_name, orig=orig: setattr(mod, fn_name, orig)) + self._undo.append( + lambda mod=mod, fn_name=fn_name, orig=orig: setattr(mod, fn_name, orig) + ) # Modules that imported the function by name keep their own # reference; patch those too. for other in ("openadapt_flow.runtime.replayer", "openadapt_flow.vision"): @@ -277,7 +300,9 @@ def install(self, backend: Any) -> None: if getattr(om, fn_name, None) is orig: setattr(om, fn_name, wrapped) self._undo.append( - lambda om=om, fn_name=fn_name, orig=orig: setattr(om, fn_name, orig) + lambda om=om, fn_name=fn_name, orig=orig: setattr( + om, fn_name, orig + ) ) orig_sleep = time.sleep time.sleep = self._wrap("time.sleep", orig_sleep) @@ -290,7 +315,9 @@ def uninstall(self) -> None: # -- output ----------------------------------------------------------- def summary(self) -> dict[str, dict[str, float]]: - return {k: {kk: round(vv, 4) for kk, vv in v.items()} for k, v in self.stats.items()} + return { + k: {kk: round(vv, 4) for kk, vv in v.items()} for k, v in self.stats.items() + } def parents(self) -> dict[str, dict[str, float]]: """Exclusive time of each call, split by the nearest Replayer caller.""" @@ -303,7 +330,9 @@ def buckets(self) -> dict[str, float]: out: dict[str, float] = {} named = set() for bucket, names in BUCKETS.items(): - out[bucket] = round(sum(self.stats[n]["excl_s"] for n in names if n in self.stats), 4) + out[bucket] = round( + sum(self.stats[n]["excl_s"] for n in names if n in self.stats), 4 + ) named.update(names) unnamed = [n for n in self.stats if n not in named] out["unassigned"] = round(sum(self.stats[n]["excl_s"] for n in unnamed), 4) diff --git a/packages/flow/openadapt_flow/__main__.py b/packages/flow/openadapt_flow/__main__.py index 3effbf62de..a09356c331 100644 --- a/packages/flow/openadapt_flow/__main__.py +++ b/packages/flow/openadapt_flow/__main__.py @@ -802,9 +802,11 @@ def _build_and_run_replayer( qualification_fault_driver=None, surface_backends=None, deployment_surface: Optional[str] = None, + readback_pipeline=None, + secret_env_overrides: Optional[Mapping[str, str]] = None, ): """Build the shared Replayer configuration and execute one workflow.""" - return _configured_replayer( + replayer = _configured_replayer( backend, workflow=workflow, allow_egress=allow_egress, @@ -824,7 +826,12 @@ def _build_and_run_replayer( qualification_fault_driver=qualification_fault_driver, surface_backends=surface_backends, deployment_surface=deployment_surface, - ).run( + ) + # Worklist workers (cli_worklist_pool): a per-worker read-back pipeline + # and per-account secret sources. Both default to the serial behavior. + replayer.readback_pipeline = readback_pipeline + replayer.secret_env_overrides = dict(secret_env_overrides or {}) + return replayer.run( workflow, params=params, worklists=worklists, @@ -2172,6 +2179,25 @@ def _cmd_replay(args: argparse.Namespace) -> int: policy_name=getattr(args, "policy", None) or cfg.policy.policy, ) fault_driver = getattr(args, "_qualification_fault_driver", None) + from openadapt_flow.cli_worklist_pool import requested_pool + + pool_policy = requested_pool(args, cfg) + if pool_policy is not None: + return _replay_worklist_pool( + args, + cfg, + backend_cfg, + pool_policy, + workflow=workflow, + params=params, + worklists=worklists, + bundle=bundle, + run_dir=run_dir, + allow_egress=allow_egress, + durable=durable, + fault_driver=fault_driver, + surface_cfgs=surface_cfgs, + ) if _normalize_kind(backend_cfg.kind) != "web": return _replay_desktop( args, @@ -2354,6 +2380,182 @@ def run_on_page(page, *, entry_url: Optional[str]): ) +def _replay_worklist_pool( + args: argparse.Namespace, + cfg, + backend_cfg, + policy, + *, + workflow, + params, + worklists, + bundle: Path, + run_dir: Path, + allow_egress: bool, + durable: bool, + fault_driver, + surface_cfgs, +) -> int: + """Run a worklist program's items on N workers (docs/THROUGHPUT.md).""" + import hashlib + import os + from functools import partial + + from openadapt_flow import cli_worklist_pool as cwp + from openadapt_flow.backends.factory import _normalize_kind, build_backend + from openadapt_flow.runtime.worklist_pool import PoolRefusal + + kind = _normalize_kind(backend_cfg.kind) + url = args.url or cfg.backend.url + try: + if ( + fault_driver is not None + or getattr(args, "_qualification_case_execution", None) is not None + ): + raise PoolRefusal("qualification runs are serial") + if surface_cfgs: + raise PoolRefusal("multi-surface bundles run on one session") + if ( + getattr(args, "drift", None) + or getattr(args, "record_video", None) is not None + or float(getattr(args, "hold_on_halt", 0.0) or 0.0) > 0 + or getattr(args, "save_healed_to", None) + ): + raise PoolRefusal( + "--drift, --record-video, --hold-on-halt and --save-healed-to " + "apply to a single session" + ) + if kind not in ("web", "rdp"): + raise PoolRefusal(f"--workers supports web and RDP sessions, not {kind}") + if kind == "web" and not url: + raise PoolRefusal("--workers needs --url or backend.url") + if kind == "rdp" and not backend_cfg.rdp_host: + raise PoolRefusal("--workers over RDP needs backend.rdp_host") + if policy.refusal() is not None: + raise PoolRefusal(policy.refusal()) + relation = cwp.loop_relation(workflow) + rows = cwp.pool_rows(workflow, worklists, relation) + accounts = cwp.check_accounts(policy, cfg.worklist, kind) + except PoolRefusal as exc: + print(f"REFUSED: {exc}. Nothing was executed.") + return 2 + + item_authorization = getattr(args, "_item_authorization", None) + policy_name = getattr(args, "policy", None) or cfg.policy.policy + entry_url = cfg.worklist.entry_url or url + + def run_item(wiring, source, idx, row, item_dir, pipeline, *, page_mode: bool): + _cfg, effect_verifier, api_actuator, _durable, _egress = wiring + backend = build_backend(backend_cfg, page=source) if page_mode else source + item_dir.mkdir(parents=True, exist_ok=True) + origin = None + if page_mode: + origin = f"{urlsplit(source.url).scheme}://{urlsplit(source.url).netloc}" + return _build_and_run_replayer( + backend, + workflow=workflow, + params=params, + worklists={relation: [dict(row)]}, + bundle=bundle, + run_dir=item_dir, + save_healed_to=None, + allow_egress=allow_egress, + effect_verifier=effect_verifier, + api_actuator=api_actuator, + durable=durable, + use_structural=True, + pixel_verify_enabled=cfg.runtime.pixel_verify_enabled, + governed_authorization=( + item_authorization([dict(row)]) if item_authorization else None + ), + production_qualification_guard=getattr( + args, "_production_qualification_guard", None + ), + runtime_config=cfg.runtime, + execution_target_kind=( + "web" if page_mode else _report_backend_kind(backend_cfg.kind) + ), + surface_override=bool(getattr(args, "_surface_override", False)), + execution_origin=origin, + execution_entry_url=entry_url if page_mode else None, + approval_gate=_approval_gate_for_run( + args, item_dir, policy_name=policy_name + ), + deployment_surface=backend_cfg.deployment_surface, + readback_pipeline=pipeline, + ) + + # One effect verifier / API actuator per worker: they hold HTTP sessions + # and are not shared across threads. + wirings = [_deployment_runtime(args, params=params) for _ in accounts] + salt = getattr(getattr(workflow, "manifest", None), "content_digest", "") or "" + + if kind == "web": + workers = [] + for i, account in enumerate(accounts): + name = account.name if account is not None else "worker-1" + sign_in = None + if account is None: + sign_in = partial(lambda page, u: page.goto(u), u=url) + elif not account.browser_cdp_endpoint: + sign_in = partial( + cwp.sign_in_with_bundle, + bundle=Path(cfg.worklist.sign_in_bundle), + account=account, + backend_factory=lambda page: build_backend(backend_cfg, page=page), + url=url, + run_dir=run_dir / "sign-in" / name, + ) + workers.append( + cwp.WebPoolWorker( + name=name, + entry_url=entry_url, + run_item_on_page=partial(run_item, wirings[i], page_mode=True), + sign_in=sign_in, + cdp_endpoint=account.browser_cdp_endpoint if account else None, + headed=bool(args.headed or cfg.backend.headed), + readback_window=policy.readback_window, + ) + ) + return cwp.run_pool(rows, workers, run_dir=run_dir, policy=policy, salt=salt) + + # RDP: one FreeRDP session per account, connected together by the pool. + from openadapt_flow.backends.xfreerdp import ( + RdpTarget, + SessionPool, + XFreeRDPTransport, + ) + + transports = [ + XFreeRDPTransport( + RdpTarget( + host=backend_cfg.rdp_host, + port=backend_cfg.rdp_port, + username=account.rdp_username, + domain=backend_cfg.rdp_domain, + ), + password=(lambda env=account.rdp_password_env: os.environ[env]), + ) + for account in accounts + ] + with SessionPool(transports) as session_pool: + workers = [ + cwp.BackendPoolWorker( + name=account.name, + backend=build_backend( + backend_cfg, rdp_transport=session, rdp_connect=False + ), + run_item_on_backend=partial(run_item, wirings[i], page_mode=False), + fingerprint=hashlib.sha256( + f"{backend_cfg.rdp_host}|{account.rdp_username}".encode() + ).hexdigest(), + readback_window=policy.readback_window, + ) + for i, (account, session) in enumerate(zip(accounts, session_pool.sessions)) + ] + return cwp.run_pool(rows, workers, run_dir=run_dir, policy=policy, salt=salt) + + def _refuse_unverified_managed_qualification_admission( authorization: object, workflow: object | None = None, @@ -3098,6 +3300,45 @@ def _cmd_run(args: argparse.Namespace) -> int: if refused is not None: return refused + from openadapt_flow.cli_worklist_pool import loop_relation, requested_pool + from openadapt_flow.runtime.worklist_pool import PoolRefusal + + if requested_pool(args, cfg) is not None: + # Parallel worklist (docs/THROUGHPUT.md): every item gets its own + # single-use authorization, bound to that item's one-row worklist and + # otherwise identical to the run's (same gate, policy, approvals). + if dispatch_file or qualification_case is not None: + print( + "run REFUSED: --workers and --readback-window apply to local " + "runs, not a managed dispatch or a qualification case. Nothing " + "was executed." + ) + return 2 + try: + relation = loop_relation(workflow) + except PoolRefusal as exc: + print(f"run REFUSED: {exc}. Nothing was executed.") + return 2 + base_authorization = args._governed_run_authorization + + def _item_authorization(rows, base=base_authorization): + fresh = build_runtime_authorization( + workflow, + report, + params=runtime_params, + worklists={relation: rows}, + ) + keep = {"authorization_id", "created_at", "runtime_inputs_digest"} + return fresh.model_copy( + update={ + name: getattr(base, name) + for name in type(base).model_fields + if name not in keep + } + ) + + args._item_authorization = _item_authorization + if qualification_case is not None and not _claim_qualification_case_attempt(args): return 2 @@ -6740,6 +6981,31 @@ def _add_deployment_flags( "sole loop relation." ), ) + p.add_argument( + "--workers", + type=int, + default=None, + metavar="N", + help=( + "Run a worklist program's items on N workers at once, each in " + "its own signed-in session (docs/THROUGHPUT.md). The deployment " + "must declare worklist.max_workers >= N, one account per " + "worker, and items_independent: true. Each item is claimed " + "once; a halt on any worker stops new items on all of them." + ), + ) + p.add_argument( + "--readback-window", + type=int, + default=None, + metavar="K", + help=( + "Let a worker start its next item while up to K earlier saves " + "still wait for their system-of-record read-back (default: " + "worklist.readback_window, normally 0). A failed read-back " + "stops the run and names the items written after it." + ), + ) def _add_attest_flags(p: argparse.ArgumentParser) -> None: diff --git a/packages/flow/openadapt_flow/backends/factory.py b/packages/flow/openadapt_flow/backends/factory.py index cd9b6860db..900c6d648b 100644 --- a/packages/flow/openadapt_flow/backends/factory.py +++ b/packages/flow/openadapt_flow/backends/factory.py @@ -51,6 +51,7 @@ def build_backend( window_client: Optional["WindowClient"] = None, macos_client: Optional["MacOSClient"] = None, linux_client: Optional["LinuxClient"] = None, + rdp_connect: bool = True, ) -> "Backend": """Construct the backend selected by ``cfg.kind``. @@ -68,6 +69,8 @@ def build_backend( client is used (macOS Quartz or Win32). Primarily a test seam. macos_client: An injected native macOS client for ``kind: macos``. linux_client: An injected native Linux/AT-SPI client for ``kind: linux``. + rdp_connect: False when ``rdp_transport`` is already connected (a + :class:`~openadapt_flow.backends.xfreerdp.SessionPool` slot). Returns: A live backend implementing the :class:`Backend` protocol. @@ -147,6 +150,7 @@ def build_backend( rdp_transport=rdp_transport, window_client=window_client, citrix=(cfg.kind or "").strip().lower() == "citrix", + connect=rdp_connect, ) raise ValueError( @@ -161,6 +165,7 @@ def _build_rdp_backend( rdp_transport: Optional["RDPTransport"], window_client: Optional["WindowClient"], citrix: bool = False, + connect: bool = True, ) -> "Backend": """Build the ``rdp`` backend: network RDP (FreeRDP) or local client window. @@ -220,6 +225,7 @@ def _build_rdp_backend( ) return FreeRDPBackend( transport, + connect=connect or rdp_transport is None, max_frame_age_s=cfg.rdp_max_frame_age_s, readiness_probe=readiness_probe, application_marker=cfg.rdp_application_marker, diff --git a/packages/flow/openadapt_flow/cli_worklist_pool.py b/packages/flow/openadapt_flow/cli_worklist_pool.py new file mode 100644 index 0000000000..d132ed61bf --- /dev/null +++ b/packages/flow/openadapt_flow/cli_worklist_pool.py @@ -0,0 +1,370 @@ +"""CLI glue for parallel worklist runs (``--workers N``). + +``replay`` and ``run`` with ``--workers N`` (or a read-back window above 0) +run a worklist program once per item across N workers. See +``runtime/worklist_pool.py`` for the scheduling rules and docs/THROUGHPUT.md +for how to set it up. + +Each item is the same governed run a serial ``--worklist`` loop would make +for that row: the program with a one-row worklist, its own run directory +(``/items/NNNNN``), its own authorization bound to that row, and +every check. Workers differ only in their session. +""" + +from __future__ import annotations + +import os +import threading +from concurrent.futures import Future +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Callable, Mapping, Optional, Sequence + +from openadapt_flow.runtime.readback_pipeline import ReadbackPipeline +from openadapt_flow.runtime.worklist_pool import ( + ItemRun, + PoolPolicy, + PoolRefusal, + PoolReport, + cookie_fingerprint, + item_detail_from_report, + item_state_from_report, + run_worklist_pool, +) + +#: Called by a worker to replay one item on its own backend: +#: ``(backend_source, idx, row, item_dir, pipeline) -> RunReport``. +RunItem = Callable[[Any, int, dict[str, Any], Path, Optional[ReadbackPipeline]], Any] + + +def requested_pool(args: Any, cfg: Any) -> Optional[PoolPolicy]: + """The pool policy this run asked for, or None for a plain serial run.""" + + workers = getattr(args, "workers", None) + window = getattr(args, "readback_window", None) + section = cfg.worklist + if window is None: + window = section.readback_window + if workers is None and window == 0: + return None + return PoolPolicy( + workers=int(workers or 1), + readback_window=int(window), + on_halt=section.on_halt, + items_independent=section.items_independent, + max_workers=section.max_workers, + ) + + +def loop_relation(workflow: Any) -> str: + """The relation of a single-LOOP program (the ``for-each`` shape). + + Parallel workers split the loop's rows, so the program must be exactly + one LOOP state followed by terminals: anything before or after the loop + would run once per item instead of once per run. + """ + + from openadapt_flow.ir import StateKind + + program = getattr(workflow, "program", None) + if program is None: + raise PoolRefusal( + "--workers needs a worklist program (a for-each bundle); this " + "bundle is a single linear run" + ) + entry = program.states.get(program.entry) + others = [ + s + for sid, s in program.states.items() + if sid != program.entry and s.kind is not StateKind.TERMINAL + ] + if entry is None or entry.kind is not StateKind.LOOP or others: + raise PoolRefusal( + "--workers needs a program that is one loop over a worklist (the " + "shape `for-each` writes); this program has other states" + ) + for tr in entry.transitions: + target = program.states.get(tr.target) + if target is None or target.kind is not StateKind.TERMINAL: + raise PoolRefusal("--workers needs the loop to end the program") + return entry.loop.relation + + +def pool_rows( + workflow: Any, worklists: Mapping[str, list[dict[str, str]]], relation: str +) -> list[dict[str, str]]: + if relation in worklists: + return list(worklists[relation]) + source = (getattr(workflow, "data_sources", None) or {}).get(relation) + if source is None: + raise PoolRefusal(f"no rows for the loop's worklist {relation!r}") + return list(source.rows) + + +def check_accounts(policy: PoolPolicy, section: Any, backend_kind: str) -> list[Any]: + """The accounts the workers will use; refuse unsafe declarations.""" + + accounts = list(section.accounts) + if policy.workers == 1 and not accounts: + return [None] + if len(accounts) < policy.workers: + raise PoolRefusal( + f"--workers {policy.workers} needs {policy.workers} accounts in " + f"worklist.accounts (found {len(accounts)}); workers never share " + "a signed-in session" + ) + accounts = accounts[: policy.workers] + names = [a.name for a in accounts] + if len(set(names)) != len(names): + raise PoolRefusal("worklist.accounts names must be distinct") + seen: dict[tuple, str] = {} + for account in accounts: + missing = [ + env + for env in [ + *account.params_env.values(), + *account.secrets_env.values(), + *([account.rdp_password_env] if account.rdp_password_env else []), + ] + if not os.environ.get(env) + ] + if missing: + raise PoolRefusal( + f"account {account.name}: environment variable(s) " + f"{', '.join(sorted(missing))} are not set" + ) + who = ( + tuple(sorted((k, os.environ[v]) for k, v in account.params_env.items())), + account.rdp_username, + account.browser_cdp_endpoint, + ) + if who != ((), None, None) and who in seen: + raise PoolRefusal( + f"accounts {seen[who]} and {account.name} sign in as the same " + "user; many apps lock a record per user, so each worker needs " + "its own" + ) + seen[who] = account.name + if backend_kind == "web" and not ( + account.browser_cdp_endpoint or section.sign_in_bundle + ): + raise PoolRefusal( + f"account {account.name} has no way to sign in: set " + "worklist.sign_in_bundle or the account's browser_cdp_endpoint" + ) + if backend_kind == "rdp" and not ( + account.rdp_username and account.rdp_password_env + ): + raise PoolRefusal( + f"account {account.name} needs rdp_username and rdp_password_env" + ) + return accounts + + +@dataclass +class WebPoolWorker: + """One browser context, one account, one thread. + + ``sign_in(page)`` signs the fresh context in (replaying the sign-in + bundle with this worker's account, or a caller's own function). With + ``cdp_endpoint`` the worker instead opens a tab in that account's own + running browser and never closes it. + """ + + name: str + entry_url: str + run_item_on_page: RunItem + sign_in: Optional[Callable[[Any], None]] = None + cdp_endpoint: Optional[str] = None + headed: bool = False + readback_window: int = 0 + viewport: Mapping[str, int] = field( + default_factory=lambda: {"width": 1280, "height": 800} + ) + _pw: Any = None + _browser: Any = None + _context: Any = None + _page: Any = None + _pipeline: Optional[ReadbackPipeline] = None + + def open(self) -> None: + from playwright.sync_api import sync_playwright + + self._pw = sync_playwright().start() + if self.cdp_endpoint: + self._browser = self._pw.chromium.connect_over_cdp(self.cdp_endpoint) + contexts = self._browser.contexts + if not contexts: + raise RuntimeError("the attached browser has no signed-in context") + self._context = contexts[0] + self._page = self._context.new_page() + self._page.set_viewport_size(dict(self.viewport)) + self._page.goto(self.entry_url) + else: + from openadapt_flow._browser_setup import launch_chromium + + self._browser = launch_chromium(self._pw.chromium, headless=not self.headed) + self._context = self._browser.new_context( + viewport=dict(self.viewport), device_scale_factor=1 + ) + self._page = self._context.new_page() + if self.sign_in is not None: + self.sign_in(self._page) + if self.readback_window > 0: + self._pipeline = ReadbackPipeline() + + def session_fingerprint(self) -> Optional[str]: + return cookie_fingerprint(self._context.cookies()) + + def run_item(self, idx: int, row: dict[str, Any], item_dir: Path) -> ItemRun: + self._page.goto(self.entry_url) + report = self.run_item_on_page(self._page, idx, row, item_dir, self._pipeline) + return _item_run(report, self._pipeline) + + def close(self) -> None: + try: + if self._pipeline is not None: + self._pipeline.close() + if self.cdp_endpoint: + if self._page is not None: + self._page.close() + else: + if self._context is not None: + self._context.close() + if self._browser is not None: + self._browser.close() + finally: + if self._pw is not None: + self._pw.stop() + + +@dataclass +class BackendPoolWorker: + """A worker over an already-connected non-web session (RDP pool slot).""" + + name: str + backend: Any + run_item_on_backend: RunItem + fingerprint: Optional[str] = None + readback_window: int = 0 + _pipeline: Optional[ReadbackPipeline] = None + + def open(self) -> None: + if self.readback_window > 0: + self._pipeline = ReadbackPipeline() + + def session_fingerprint(self) -> Optional[str]: + return self.fingerprint + + def run_item(self, idx: int, row: dict[str, Any], item_dir: Path) -> ItemRun: + report = self.run_item_on_backend( + self.backend, idx, row, item_dir, self._pipeline + ) + return _item_run(report, self._pipeline) + + def close(self) -> None: + if self._pipeline is not None: + self._pipeline.close() + + +def _item_run(report: Any, pipeline: Optional[ReadbackPipeline]) -> ItemRun: + state = item_state_from_report(report) + detail = item_detail_from_report(report) + handoff = pipeline.take() if pipeline is not None else None + if handoff is None: + if pipeline is not None and pipeline.inline_reason: + detail["readback"] = f"inline ({pipeline.inline_reason})" + return ItemRun(state=state, detail=detail) + result: Future = Future() + + def _settled(fut: Future) -> None: + try: + settled = fut.result() + result.set_result( + (item_state_from_report(settled), item_detail_from_report(settled)) + ) + except Exception as exc: # noqa: BLE001 + result.set_exception(exc) + + handoff.settled.add_done_callback(_settled) + return ItemRun(state=state, detail=detail, pending_readback=result) + + +def sign_in_with_bundle( + page: Any, + *, + bundle: Path, + account: Any, + backend_factory: Callable[[Any], Any], + url: str, + run_dir: Path, +) -> None: + """Replay the recorded sign-in bundle with this account's credentials.""" + + from openadapt_flow.ir import Workflow + from openadapt_flow.runtime.replayer import Replayer + + page.goto(url) + workflow = Workflow.load(bundle) + params = {k: os.environ[v] for k, v in account.params_env.items()} + report = Replayer( + backend_factory(page), + secret_env_overrides=dict(account.secrets_env), + ).run(workflow, params=params, bundle_dir=bundle, run_dir=run_dir) + if not report.success: + raise RuntimeError(f"sign-in bundle did not finish for {account.name}") + + +def print_pool_report(report: PoolReport, run_dir: Path) -> int: + counts = ", ".join(f"{k} {v}" for k, v in sorted(report.counts().items())) + print( + f"Worklist: {len(report.items)} items on {report.workers} workers in " + f"{report.wall_s:.1f} s ({counts})." + ) + rate = report.entries_per_hour() + if rate is not None: + print(f"Finished items per hour, this run: {rate}") + for w in report.per_worker: + states = ", ".join(f"{k} {v}" for k, v in sorted(w.states.items())) or "nothing" + print(f" {w.name}: {len(w.items)} items ({states})") + if report.stop: + print( + f"STOPPED by {report.stop.get('worker')} at item {report.stop.get('idx')}: " + f"{report.stop.get('reason')}" + ) + for idx, after in sorted(report.written_after_failed_readback.items()): + print( + f"Read-back of item {idx} failed; written after it by the same " + f"worker: {', '.join(map(str, after)) or 'none'}" + ) + print(f"Per-item reports: {run_dir / 'items'}; summary: {run_dir / 'WORKLIST.md'}") + return 0 if report.success else 1 + + +def run_pool( + rows: Sequence[Mapping[str, Any]], + workers: Sequence[Any], + *, + run_dir: Path, + policy: PoolPolicy, + salt: str, +) -> int: + lock = threading.Lock() + + def say(msg: str) -> None: + with lock: + print(msg, flush=True) + + try: + report = run_worklist_pool( + rows, + workers, + run_dir=run_dir, + policy=policy, + fingerprint_salt=salt, + on_event=say, + ) + except PoolRefusal as exc: + print(f"REFUSED: {exc}. No item was run.") + return 2 + return print_pool_report(report, run_dir) diff --git a/packages/flow/openadapt_flow/deployment.py b/packages/flow/openadapt_flow/deployment.py index 1655e64ff9..b218454ad1 100644 --- a/packages/flow/openadapt_flow/deployment.py +++ b/packages/flow/openadapt_flow/deployment.py @@ -29,6 +29,7 @@ from pydantic import ( BaseModel, + ConfigDict, Field, field_validator, model_serializer, @@ -777,6 +778,62 @@ class IdempotencySection(BaseModel): key_env: Optional[str] = None +class WorklistAccount(BaseModel): + """One signed-in session for a parallel worklist worker. + + Holds env var NAMES only, never a credential value. + """ + + model_config = ConfigDict(extra="forbid") + + #: Label shown in the run report (for example ``clerk-1``). + name: str = Field(min_length=1, max_length=64) + #: Sign-in bundle parameter -> NAME of the env var holding its value + #: (for example ``{username: EMR_USER_1}``). + params_env: dict[str, str] = Field(default_factory=dict) + #: Secret sign-in parameter (a step marked ``secret``) -> NAME of the env + #: var holding it (for example ``{password: EMR_PASS_1}``). + secrets_env: dict[str, str] = Field(default_factory=dict) + #: Attach to this account's own signed-in Chromium (CDP) instead of + #: launching a browser and replaying the sign-in bundle. + browser_cdp_endpoint: Optional[str] = None + #: RDP: the user this session connects as, and the NAME of the env var + #: holding the password (FreeRDP session pool). + rdp_username: Optional[str] = None + rdp_password_env: Optional[str] = None + + +class WorklistSection(BaseModel): + """Parallel worklist runs (``--workers N``, docs/THROUGHPUT.md). + + The defaults run one item at a time, as before. ``--workers`` above + ``max_workers`` is refused, so the deployment states how many sessions + the app tolerates at once. + """ + + model_config = ConfigDict(extra="forbid") + + #: The app's concurrency limit: how many signed-in sessions may enter + #: items at once. ``--workers`` above this is refused. + max_workers: int = Field(default=1, ge=1, le=64) + #: Items touch disjoint records and may run in any order and at the same + #: time. Required for ``--workers`` above 1 and a read-back window above 0. + items_independent: bool = False + #: Pipelined read-back window k (default 0 = verify each save before the + #: next item starts). See ``runtime/readback_pipeline.py``. + readback_window: int = Field(default=0, ge=0, le=16) + #: ``stop`` (default): any halt stops new claims on every worker. + #: ``skip_unwritten``: an item that halted before writing anything is + #: recorded and the run goes on; anything else still stops it. + on_halt: Literal["stop", "skip_unwritten"] = "stop" + #: Recorded sign-in bundle each worker replays once with its account. + sign_in_bundle: Optional[str] = None + #: Page each item starts from (default: ``backend.url``). + entry_url: Optional[str] = None + #: One account per worker. Workers never share a session. + accounts: list[WorklistAccount] = Field(default_factory=list) + + #: Sentinel written by ``config init`` for fields the operator must fill. UNRESOLVED = "UNRESOLVED" @@ -805,14 +862,20 @@ class DeploymentConfig(BaseModel): #: declares. ``--surface NAME=...`` flags apply over these entries. Empty #: (and omitted when serialized) otherwise. surfaces: dict[str, BackendConfig] = Field(default_factory=dict) + #: Parallel worklist runs (``--workers``). Omitted when serialized at its + #: defaults (one worker). + worklist: WorklistSection = Field(default_factory=WorklistSection) @model_serializer(mode="wrap") def _serialize_compatible(self, handler: Any) -> dict[str, Any]: - """Keep configs without surface bindings byte-identical.""" + """Keep configs without surface bindings or worklist settings + byte-identical.""" data: dict[str, Any] = handler(self) if not self.surfaces: data.pop("surfaces", None) + if self.worklist == WorklistSection(): + data.pop("worklist", None) return data diff --git a/packages/flow/openadapt_flow/runtime/readback_pipeline.py b/packages/flow/openadapt_flow/runtime/readback_pipeline.py index d2ffb43e73..1d7400b3b9 100644 --- a/packages/flow/openadapt_flow/runtime/readback_pipeline.py +++ b/packages/flow/openadapt_flow/runtime/readback_pipeline.py @@ -49,17 +49,25 @@ class ReadbackPipeline: """One side thread per worker for deferred read-backs.""" def __init__(self) -> None: - self._executor = ThreadPoolExecutor(max_workers=1, thread_name_prefix="readback") + self._executor = ThreadPoolExecutor( + max_workers=1, thread_name_prefix="readback" + ) self._lock = threading.Lock() self._handoff: Optional[PipelinedReadback] = None #: Why the last item verified inline instead (for the run report). self.inline_reason: Optional[str] = None - def submit(self, step_id: str, verify: Callable[[], Optional[str]]) -> PipelinedReadback: - entry = PipelinedReadback(step_id=step_id, verdict=self._executor.submit(verify)) + def submit( + self, step_id: str, verify: Callable[[], Optional[str]] + ) -> PipelinedReadback: + entry = PipelinedReadback( + step_id=step_id, verdict=self._executor.submit(verify) + ) return entry - def hand_off(self, entry: PipelinedReadback, settle: Callable[[Optional[str]], Any]) -> None: + def hand_off( + self, entry: PipelinedReadback, settle: Callable[[Optional[str]], Any] + ) -> None: """Called by the Replayer as the item finishes: settle on completion.""" entry.settle = settle @@ -68,7 +76,9 @@ def _done(fut: "Future[Optional[str]]") -> None: try: error = fut.result() except Exception as exc: # noqa: BLE001 - a raising verifier is a failure - error = f"read-back raised {type(exc).__name__}; reconciliation is required" + error = ( + f"read-back raised {type(exc).__name__}; reconciliation is required" + ) try: entry.settled.set_result(settle(error)) except Exception as exc: # noqa: BLE001 diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index b26c6f0e1d..9a526ce95a 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -2427,7 +2427,9 @@ def _finalize_report( else: # The run ended some other way: settle the read-back inline # before the report is written. - self._apply_pipelined_verdict(report, self._join_pipelined_readback(keep=True)) + self._apply_pipelined_verdict( + report, self._join_pipelined_readback(keep=True) + ) self._pipelined_readback = None self._end_surfaces() if self._surface_environment_observations: @@ -2525,9 +2527,7 @@ def verify() -> Optional[str]: entry.result = result # type: ignore[attr-defined] entry.intent = step.intent # type: ignore[attr-defined] result.effect_verified = None - result.effect_results.append( - "[pipelined] system-of-record read-back pending" - ) + result.effect_results.append("[pipelined] system-of-record read-back pending") self._pipelined_readback = entry def _join_pipelined_readback(self, *, keep: bool = False) -> Optional[str]: @@ -2539,9 +2539,7 @@ def _join_pipelined_readback(self, *, keep: bool = False) -> Optional[str]: try: error = entry.verdict.result() except Exception as exc: # noqa: BLE001 - a raising verifier fails closed - error = ( - f"read-back raised {type(exc).__name__}; reconciliation is required" - ) + error = f"read-back raised {type(exc).__name__}; reconciliation is required" self._copy_pipelined_evidence(entry, error) if not keep: self._pipelined_readback = None @@ -2602,7 +2600,9 @@ def _apply_pipelined_verdict( outcome="halt", # The screen now belongs to the next item; nothing is probed. observed_texts=[], - completed_intents=[r.intent for r in report.results if r.ok and not r.skipped], + completed_intents=[ + r.intent for r in report.results if r.ok and not r.skipped + ], ) def _settle_pipelined_readback( diff --git a/packages/flow/openadapt_flow/runtime/worklist_pool.py b/packages/flow/openadapt_flow/runtime/worklist_pool.py index ab10cf13df..6887100d2b 100644 --- a/packages/flow/openadapt_flow/runtime/worklist_pool.py +++ b/packages/flow/openadapt_flow/runtime/worklist_pool.py @@ -470,7 +470,11 @@ def render_md(self) -> str: f"{len(self.items)}", ] if self.stop: - where = f"item {self.stop.get('idx')}" if self.stop.get("idx") is not None else "the pool" + where = ( + f"item {self.stop.get('idx')}" + if self.stop.get("idx") is not None + else "the pool" + ) lines.append( f"- **Stopped** by {self.stop.get('worker')} at {where}: " f"{self.stop.get('reason')}" @@ -494,7 +498,13 @@ def render_md(self) -> str: lines.append( f"| {w.name} | {len(w.items)} | {states} | {w.busy_s:.1f} | {w.open_s:.1f} |" ) - lines += ["", "## Per item", "", "| Item | Worker | State | Seconds |", "|---:|---|---|---:|"] + lines += [ + "", + "## Per item", + "", + "| Item | Worker | State | Seconds |", + "|---:|---|---|---:|", + ] for item in self.items: res = item.get("result") or {} secs = res.get("wall_s") @@ -562,7 +572,9 @@ def stop(reason: str, worker: str, idx: Optional[int]) -> None: queue.request_stop(reason, worker=worker, idx=idx) say(f"STOP ({worker}, item {idx}): {reason}") - def finish(worker: PoolWorker, idx: int, state: str, detail: dict[str, Any]) -> None: + def finish( + worker: PoolWorker, idx: int, state: str, detail: dict[str, Any] + ) -> None: queue.set_state(idx, worker.name, state, detail) with lock: s = summaries[worker.name] @@ -597,7 +609,10 @@ def settle(entry: tuple[int, Future, list[int]]) -> None: try: state, detail = fut.result() except Exception as exc: # noqa: BLE001 - state, detail = ERROR, {"halt_reason": f"read-back raised {type(exc).__name__}"} + state, detail = ( + ERROR, + {"halt_reason": f"read-back raised {type(exc).__name__}"}, + ) if state not in SUCCESS_STATES: with lock: written_after[str(idx)] = list(after) @@ -624,7 +639,11 @@ def settle(entry: tuple[int, Future, list[int]]) -> None: except Exception as exc: # noqa: BLE001 - fail closed out = ItemRun( state=ERROR, - detail={"halt_reason": f"worker raised {type(exc).__name__}: {exc}"[:300]}, + detail={ + "halt_reason": f"worker raised {type(exc).__name__}: {exc}"[ + :300 + ] + }, ) wall = time.monotonic() - t_item summary.busy_s += wall @@ -633,10 +652,14 @@ def settle(entry: tuple[int, Future, list[int]]) -> None: queue.set_state(idx, worker.name, READBACK_PENDING, detail) fut: Future = Future() - def _chain(src: Future, dst: Future = fut, base: dict = detail) -> None: + def _chain( + src: Future, dst: Future = fut, base: dict = detail + ) -> None: try: st, more = src.result() - dst.set_result((st, {**base, **more, "readback": "pipelined"})) + dst.set_result( + (st, {**base, **more, "readback": "pipelined"}) + ) except Exception as exc: # noqa: BLE001 dst.set_exception(exc) @@ -658,7 +681,9 @@ def _chain(src: Future, dst: Future = fut, base: dict = detail) -> None: pass threads = [ - threading.Thread(target=worker_main, args=(w,), name=f"worklist-{w.name}", daemon=True) + threading.Thread( + target=worker_main, args=(w,), name=f"worklist-{w.name}", daemon=True + ) for w in workers ] for t in threads: @@ -680,7 +705,9 @@ def _chain(src: Future, dst: Future = fut, base: dict = detail) -> None: for t in threads: t.join() failed = [s.name for s in summaries.values() if s.error] - raise PoolRefusal(f"sign-in failed for {', '.join(failed)}; nothing was executed") + raise PoolRefusal( + f"sign-in failed for {', '.join(failed)}; nothing was executed" + ) go.set() for t in threads: t.join() diff --git a/packages/flow/tests/test_cli_worklist_pool.py b/packages/flow/tests/test_cli_worklist_pool.py new file mode 100644 index 0000000000..5a4afe14db --- /dev/null +++ b/packages/flow/tests/test_cli_worklist_pool.py @@ -0,0 +1,267 @@ +"""`replay`/`run --workers N`: refusals and the per-item wiring. + +The browser and the Replayer are faked; the pool, the queue and the CLI +glue are real. +""" + +from __future__ import annotations + +import json +import threading +import time +from pathlib import Path +from typing import Any + +import pytest + +from openadapt_flow.__main__ import main +from openadapt_flow.compiler.loop_authoring import author_data_driven_loop +from openadapt_flow.ir import ActionKind, ParamSpec, Step, Workflow +from tests.test_cli_new_commands import _loop_program + + +def _for_each_bundle(tmp_path: Path, n: int = 6) -> tuple[Path, str]: + body = Workflow( + name="note", + steps=[ + Step( + id="type_note", + intent="type ", + action=ActionKind.TYPE, + param="note", + ) + ], + param_specs={"note": ParamSpec(name="note", example="x")}, + ) + looped = author_data_driven_loop(body, [{"note": f"n{i}"} for i in range(n)]) + bundle = tmp_path / "bundle" + looped.save(bundle) + (rel,) = tuple(looped.data_sources) + return bundle, rel + + +def _config( + tmp_path: Path, + *, + max_workers: int = 4, + accounts: int = 2, + same_user=False, + independent=True, +) -> Path: + lines = [ + "backend:\n url: http://app.example/form\n", + "worklist:\n", + f" max_workers: {max_workers}\n", + f" items_independent: {'true' if independent else 'false'}\n", + " accounts:\n", + ] + for i in range(accounts): + lines.append( + f" - name: clerk-{i}\n" + f" browser_cdp_endpoint: http://127.0.0.1:{9300 + (0 if same_user else i)}\n" + ) + path = tmp_path / "deploy.yaml" + path.write_text("".join(lines)) + return path + + +class _Page: + def __init__(self, ctx: "_Context") -> None: + self.ctx = ctx + self.url = "about:blank" + + def goto(self, url: str) -> None: + self.url = url + + def set_viewport_size(self, size: Any) -> None: + pass + + def close(self) -> None: + pass + + +class _Context: + def __init__(self, endpoint: str) -> None: + self.endpoint = endpoint + + def cookies(self) -> list[dict[str, str]]: + # Each endpoint is its own signed-in browser: its own session cookie. + return [ + { + "domain": "app.example", + "path": "/", + "name": "sid", + "value": self.endpoint, + } + ] + + def new_page(self) -> _Page: + return _Page(self) + + +class _Browser: + def __init__(self, endpoint: str) -> None: + self.contexts = [_Context(endpoint)] + + +class _Chromium: + def connect_over_cdp(self, endpoint: str) -> _Browser: + return _Browser(endpoint) + + +class _PW: + chromium = _Chromium() + + def start(self) -> "_PW": + return self + + def stop(self) -> None: + pass + + +class _Report: + def __init__(self, success: bool) -> None: + self.success = success + self.screenshots_may_leave_box = False + self.execution_outcome = None + self.transaction_outcome = None + self.halt = None + self.total_ms = 1.0 + + +def _install(monkeypatch, calls: list, *, halt_note: str | None = None) -> None: + lock = threading.Lock() + + class _Replayer: + def __init__(self, backend, **kwargs): + self.backend = backend + self.kwargs = kwargs + + def run(self, workflow, **kwargs): + note = next(iter(kwargs["worklists"].values()))[0]["note"] + with lock: + calls.append( + { + "note": note, + "endpoint": self.backend.ctx.endpoint, + "worklists": kwargs["worklists"], + "run_dir": kwargs["run_dir"], + "auth": self.kwargs.get("governed_authorization"), + } + ) + time.sleep(0.05) # long enough for both workers to claim items + return _Report(success=note != halt_note) + + import playwright.sync_api as psa + + import openadapt_flow.backends.playwright_backend as pwb + import openadapt_flow.runtime as runtime_mod + import openadapt_flow.runtime.grounder as grounder_mod + import openadapt_flow.runtime.remote_vlm as remote_mod + + monkeypatch.setattr(psa, "sync_playwright", lambda: _PW()) + monkeypatch.setattr(pwb, "PlaywrightBackend", lambda page, **kw: page) + monkeypatch.setattr(grounder_mod, "build_grounder", lambda fallback=None: None) + monkeypatch.setattr(remote_mod, "appliance_from_env", lambda: None) + monkeypatch.setattr(runtime_mod, "Replayer", _Replayer) + + +def _replay(bundle: Path, cfg: Path, run_dir: Path, *extra: str) -> int: + return main( + ["replay", str(bundle), "--config", str(cfg), "--run-dir", str(run_dir), *extra] + ) + + +def test_two_workers_split_the_items_one_row_each( + tmp_path, monkeypatch, capsys +) -> None: + bundle, rel = _for_each_bundle(tmp_path) + calls: list = [] + _install(monkeypatch, calls) + rc = _replay(bundle, _config(tmp_path), tmp_path / "run", "--workers", "2") + assert rc == 0, capsys.readouterr().out + assert sorted(c["note"] for c in calls) == [f"n{i}" for i in range(6)] + for c in calls: + # Each item is its own run over a one-row worklist, in its own dir. + assert c["worklists"] == {rel: [{"note": c["note"]}]} + assert c["run_dir"].parent.name == "items" + # Both sessions did work, and no item ran twice. + assert {c["endpoint"] for c in calls} == { + "http://127.0.0.1:9300", + "http://127.0.0.1:9301", + } + pool = json.loads((tmp_path / "run" / "worklist_pool.json").read_text()) + assert pool["counts"] == {"completed": 6} + assert {w["name"] for w in pool["per_worker"]} == {"clerk-0", "clerk-1"} + assert (tmp_path / "run" / "WORKLIST.md").exists() + + +def test_forced_halt_on_one_worker_stops_new_items(tmp_path, monkeypatch) -> None: + bundle, _ = _for_each_bundle(tmp_path, n=40) + calls: list = [] + _install(monkeypatch, calls, halt_note="n1") + rc = _replay(bundle, _config(tmp_path), tmp_path / "run", "--workers", "2") + assert rc == 1 + pool = json.loads((tmp_path / "run" / "worklist_pool.json").read_text()) + assert pool["stop"]["idx"] == 1 + assert len(calls) < 40 + assert pool["counts"]["halted"] == 1 + assert pool["counts"]["pending"] > 0 + + +@pytest.mark.parametrize( + "cfg_kw,flag,needle", + [ + ({"max_workers": 2}, "4", "limit of 2"), + ({"accounts": 1}, "2", "needs 2 accounts"), + ({"same_user": True}, "2", "sign in as the same user"), + ({"independent": False}, "2", "items_independent"), + ], +) +def test_unsafe_pool_is_refused_before_any_item( + tmp_path, monkeypatch, capsys, cfg_kw, flag, needle +) -> None: + bundle, _ = _for_each_bundle(tmp_path) + calls: list = [] + _install(monkeypatch, calls) + rc = _replay( + bundle, _config(tmp_path, **cfg_kw), tmp_path / "run", "--workers", flag + ) + assert rc == 2 + assert needle in capsys.readouterr().out + assert calls == [] + + +def test_no_deployment_means_one_worker_only(tmp_path, monkeypatch, capsys) -> None: + bundle, _ = _for_each_bundle(tmp_path) + calls: list = [] + _install(monkeypatch, calls) + rc = main( + [ + "replay", + str(bundle), + "--url", + "http://app.example", + "--run-dir", + str(tmp_path / "r"), + "--workers", + "2", + ] + ) + assert rc == 2 + assert "limit of 1" in capsys.readouterr().out + assert calls == [] + + +def test_program_with_steps_outside_the_loop_is_refused( + tmp_path, monkeypatch, capsys +) -> None: + wf = _loop_program() # an induced program: a click, then the loop + bundle = tmp_path / "bundle" + wf.save(bundle) + calls: list = [] + _install(monkeypatch, calls) + rc = _replay(bundle, _config(tmp_path), tmp_path / "run", "--workers", "2") + assert rc == 2 + assert "one loop over a worklist" in capsys.readouterr().out + assert calls == [] diff --git a/packages/flow/tests/test_worklist_pool.py b/packages/flow/tests/test_worklist_pool.py index b6c89f3852..52130b1247 100644 --- a/packages/flow/tests/test_worklist_pool.py +++ b/packages/flow/tests/test_worklist_pool.py @@ -111,7 +111,10 @@ def halting(idx: int) -> ItemRun: halted.set() return ItemRun( state=HALTED, - detail={"halt_reason": "identity check failed", "transaction_outcome": "halted_before_effect"}, + detail={ + "halt_reason": "identity check failed", + "transaction_outcome": "halted_before_effect", + }, ) return ItemRun(state=VERIFIED) @@ -124,7 +127,9 @@ def slow_peer(idx: int) -> ItemRun: w1 = FakeWorker("w1", halting, delay_s=0.01) peers = [FakeWorker(f"w{i}", slow_peer, delay_s=0.03) for i in (2, 3, 4)] - report = run_worklist_pool(_rows(100), [w1, *peers], run_dir=tmp_path, policy=_policy(4)) + report = run_worklist_pool( + _rows(100), [w1, *peers], run_dir=tmp_path, policy=_policy(4) + ) assert not report.success assert report.stop is not None @@ -153,7 +158,10 @@ def behave(idx: int) -> ItemRun: state=HALTED, detail={"transaction_outcome": "halted_before_effect"} ) if idx == 3: - return ItemRun(state=RECONCILIATION_REQUIRED, detail={"halt_reason": "write not confirmed"}) + return ItemRun( + state=RECONCILIATION_REQUIRED, + detail={"halt_reason": "write not confirmed"}, + ) return ItemRun(state=VERIFIED) w = FakeWorker("w1", behave) @@ -171,7 +179,9 @@ def boom(idx: int) -> ItemRun: raise RuntimeError("browser crashed") return ItemRun(state=COMPLETED) - report = run_worklist_pool(_rows(10), [FakeWorker("w1", boom)], run_dir=tmp_path, policy=_policy(1)) + report = run_worklist_pool( + _rows(10), [FakeWorker("w1", boom)], run_dir=tmp_path, policy=_policy(1) + ) states = {item["idx"]: item["state"] for item in report.items} assert states[2] == ERROR assert report.stop is not None and report.stop["idx"] == 2 @@ -181,7 +191,9 @@ def boom(idx: int) -> ItemRun: def test_refuses_more_workers_than_the_app_allows(tmp_path: Path) -> None: workers = [FakeWorker(f"w{i}") for i in range(4)] with pytest.raises(PoolRefusal, match="limit of 2"): - run_worklist_pool(_rows(4), workers, run_dir=tmp_path, policy=_policy(4, max_workers=2)) + run_worklist_pool( + _rows(4), workers, run_dir=tmp_path, policy=_policy(4, max_workers=2) + ) assert not any(w.opened for w in workers) assert not (tmp_path / QUEUE_FILENAME).exists() @@ -196,7 +208,10 @@ def test_refuses_parallel_items_not_declared_independent(tmp_path: Path) -> None ) with pytest.raises(PoolRefusal, match="items_independent"): run_worklist_pool( - _rows(4), [FakeWorker("a")], run_dir=tmp_path, policy=PoolPolicy(workers=1, readback_window=1) + _rows(4), + [FakeWorker("a")], + run_dir=tmp_path, + policy=PoolPolicy(workers=1, readback_window=1), ) @@ -217,7 +232,9 @@ def test_sign_in_failure_runs_nothing(tmp_path: Path) -> None: assert a.ran == [] -def test_reopen_marks_claimed_items_uncertain_and_never_reclaims(tmp_path: Path) -> None: +def test_reopen_marks_claimed_items_uncertain_and_never_reclaims( + tmp_path: Path, +) -> None: rows = _rows(3) fp = rows_fingerprint(rows) q = WorklistQueue.open(tmp_path / QUEUE_FILENAME, rows, fingerprint=fp) @@ -227,12 +244,16 @@ def test_reopen_marks_claimed_items_uncertain_and_never_reclaims(tmp_path: Path) assert q2.items()[0]["state"] == UNCERTAIN assert q2.claim("w2") == (1, rows[1]) with pytest.raises(PoolRefusal, match="different worklist"): - WorklistQueue.open(tmp_path / QUEUE_FILENAME, _rows(4), fingerprint=rows_fingerprint(_rows(4))) + WorklistQueue.open( + tmp_path / QUEUE_FILENAME, _rows(4), fingerprint=rows_fingerprint(_rows(4)) + ) def test_only_the_claiming_worker_records_a_result(tmp_path: Path) -> None: rows = _rows(1) - q = WorklistQueue.open(tmp_path / QUEUE_FILENAME, rows, fingerprint=rows_fingerprint(rows)) + q = WorklistQueue.open( + tmp_path / QUEUE_FILENAME, rows, fingerprint=rows_fingerprint(rows) + ) q.claim("w1") with pytest.raises(RuntimeError, match="not claimed by w2"): q.set_state(0, "w2", VERIFIED, {}) @@ -241,7 +262,9 @@ def test_only_the_claiming_worker_records_a_result(tmp_path: Path) -> None: class PipelinedWorker(FakeWorker): """Read-back runs on a side thread and resolves after ``readback_s``.""" - def __init__(self, name: str, fail_idx: Optional[int], readback_s: float, **kw: Any) -> None: + def __init__( + self, name: str, fail_idx: Optional[int], readback_s: float, **kw: Any + ) -> None: super().__init__(name, **kw) self.fail_idx = fail_idx self.readback_s = readback_s @@ -262,7 +285,9 @@ def readback() -> None: with self._lock: self._outstanding -= 1 if idx == self.fail_idx: - fut.set_result((RECONCILIATION_REQUIRED, {"halt_reason": "saved record differs"})) + fut.set_result( + (RECONCILIATION_REQUIRED, {"halt_reason": "saved record differs"}) + ) else: fut.set_result((VERIFIED, {})) @@ -271,7 +296,9 @@ def readback() -> None: @pytest.mark.parametrize("window", [1, 2]) -def test_pipelined_readback_failure_names_items_written_after(tmp_path: Path, window: int) -> None: +def test_pipelined_readback_failure_names_items_written_after( + tmp_path: Path, window: int +) -> None: w = PipelinedWorker("w1", fail_idx=3, readback_s=0.05, delay_s=0.01) report = run_worklist_pool( _rows(20), [w], run_dir=tmp_path, policy=_policy(1, readback_window=window) @@ -304,9 +331,16 @@ class R: def __init__(self, **kw: Any) -> None: self.__dict__.update(kw) - assert item_state_from_report(R(transaction_outcome="reconciliation_required", execution_outcome="halted")) == RECONCILIATION_REQUIRED + assert ( + item_state_from_report( + R(transaction_outcome="reconciliation_required", execution_outcome="halted") + ) + == RECONCILIATION_REQUIRED + ) assert item_state_from_report(R(execution_outcome="verified")) == VERIFIED - assert item_state_from_report(R(execution_outcome="completed_unverified")) == COMPLETED + assert ( + item_state_from_report(R(execution_outcome="completed_unverified")) == COMPLETED + ) assert item_state_from_report(R(execution_outcome="halted")) == HALTED assert item_state_from_report(R(success=True)) == COMPLETED assert item_state_from_report(R(success=False)) == HALTED From feee79c8713f07b0570f03ba88b0f44ba0fc11a8 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 18 Oct 2026 15:04:46 -0400 Subject: [PATCH 04/18] perf(flow): DOM text check ignores punctuation before falling back to OCR; committed throughput harness Co-Authored-By: Claude Opus 5.5 --- .../benchmark/openemr_throughput/harness.py | 446 ++++++++++++++++++ .../backends/playwright_backend.py | 15 +- .../flow/openadapt_flow/cli_worklist_pool.py | 7 +- packages/flow/tests/test_page_settle.py | 3 + packages/flow/tests/test_worklist_pool.py | 27 +- 5 files changed, 476 insertions(+), 22 deletions(-) create mode 100644 packages/flow/benchmark/openemr_throughput/harness.py diff --git a/packages/flow/benchmark/openemr_throughput/harness.py b/packages/flow/benchmark/openemr_throughput/harness.py new file mode 100644 index 0000000000..73a0382c5f --- /dev/null +++ b/packages/flow/benchmark/openemr_throughput/harness.py @@ -0,0 +1,446 @@ +"""Worklist throughput on a local OpenEMR test copy (synthetic data only). + + python -m benchmark.openemr_throughput.harness run \\ + --bundle BUNDLE --fixture-state STATE --tag TAG --workers N --items M \\ + [--window K] [--halt-at I] [--profile] + +What one run does: + +1. Setup, not timed: reset the fixture database to its verified baseline, + add 8 synthetic clerk accounts (``oa_clerk1`` .. ``oa_clerk8``), seed the + existing chart Rosa Delgado, and generate M distinct made-up referrals. +2. Timed: :func:`openadapt_flow.runtime.worklist_pool.run_worklist_pool` over + the M referrals on N workers. Each worker opens its own browser context + and signs in as its own clerk (counted in the time). For each item it + claims, it closes any open patient chart, opens the blank New Patient + form, and replays the compiled bundle with the engine's own read-back of + the saved chart through the OpenEMR REST API. +3. Judge, not timed: the test setup reads every item back from the database + with SQL (12 of 12 fields, exactly one new chart, none for unrun items). + +``--halt-at I`` replaces item I with a possible duplicate of the seeded +chart; the engine must halt on it and the pool must stop every worker. +``--profile`` (N = 1 only) records an exclusive-time profile per item. + +The fixture is :mod:`benchmark.openemr_local.fixture` (pinned OpenEMR 8.0.0.3 +images) with its own compose project and ports, set by environment: +``THROUGHPUT_UI_PORT`` (HTTP) and ``THROUGHPUT_API_PORT`` (HTTPS). The bundle +is the referral program compiled with DOM identity; it is not in this +repository. +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import os +import shutil +import sys +import threading +import time +from pathlib import Path +from typing import Any + +HERE = Path(__file__).resolve().parent +FLOW = HERE.parents[1] + +#: The existing chart a possible-duplicate referral collides with. +EXISTING_ROSA = dict( + title="Mrs.", fname="Rosa", lname="Delgado", dob="1958-03-27", sex="Female", + street="311 BIRCH COURT", city="EXAMPLETON", state="MA", state_label="Massachusetts", + postal_code="02139", phone_home="555-0119", email="r.delgado.prior@example.invalid", +) +#: Same name, different date of birth: OpenEMR offers the existing chart and +#: the engine must stop (the r6 case of the referral timing benchmark). +POSSIBLE_DUPLICATE = dict( + title="Mrs.", fname="Rosa", lname="Delgado", dob="1958-03-22", sex="Female", + street="7 MILL ROAD", city="EXAMPLETON", state="MA", state_label="Massachusetts", + postal_code="02139", phone_home="555-0188", email="rosa.delgado@example.invalid", +) + +# First names are 4+ letters: the identity check refuses to bind a parameter +# value shorter than 4 characters (see docs/LIMITS.md). +FIRST = { + "Female": ["Adela", "Beatrix", "Clarice", "Dorinda", "Edwina", "Florence", "Gisela", + "Harriet", "Isadora", "Juniper", "Katrina", "Lorena", "Marisol", "Nerissa", + "Ophelia", "Perpetua"], + "Male": ["Ambrose", "Barnaby", "Casimir", "Desmond", "Emrys", "Fitzroy", "Gideon", + "Horatio", "Ignatius", "Jasper", "Leopold", "Lucian", "Montague", "Nathaniel", + "Oswin", "Peregrine"], +} +LAST = ["Quillfern", "Rowanby", "Sedgewick", "Thornvale", "Umberly", "Vantree", "Wexmoor", + "Yarrowby"] +STREETS = ["LANTERN COURT", "SYNTHETIC WAY", "EXAMPLE ROAD", "PLACEHOLDER LANE"] +CLERKS = [f"oa_clerk{i}" for i in range(1, 9)] + + +def synthetic_referrals(n: int) -> list[dict[str, str]]: + """n distinct made-up patients (555-01xx phones, example.invalid emails).""" + out = [] + for i in range(n): + sex = "Female" if i % 2 == 0 else "Male" + fname = FIRST[sex][(i // 2) % 16] + last = LAST[(i // 32) % 8] + ("" if i < 256 else str(i // 256)) + out.append(dict( + title="Ms." if sex == "Female" else "Mr.", fname=fname, + lname=f"{last}{chr(65 + (i % 26))}", + dob=f"19{50 + (i % 45):02d}-{1 + (i % 12):02d}-{1 + (i % 27):02d}", sex=sex, + street=f"{10 + i} {STREETS[i % 4]}", city="EXAMPLETON", state="MA", + state_label="Massachusetts", postal_code="02139", + phone_home=f"555-01{i % 100:02d}", + email=f"{fname.lower()}.{last.lower()}.{i}@example.invalid")) + return out + + +# -- fixture --------------------------------------------------------------- + + +def make_fixture(state: Path) -> Any: + from benchmark.openemr_local import fixture as fx + + ui_port = os.environ.get("THROUGHPUT_UI_PORT", "9911") + api_port = os.environ.get("THROUGHPUT_API_PORT", "9910") + fx.LOCK_PATH = state.parent / "environment.lock.json" + + class ThroughputFixture(fx.OpenEMRFixture): + """Same pinned images, compose file and baseline; own project and ports.""" + + @property + def ui_base_url(self) -> str: + return f"http://127.0.0.1:{ui_port}" + + @property + def api_base_url(self) -> str: + return f"https://127.0.0.1:{api_port}" + + def _validate_runtime_env(self) -> None: + v = self._runtime_values() + assert v.get("OPENEMR_HTTP_PORT") == ui_port + assert v.get("OPENEMR_HTTPS_PORT") == api_port + assert v.get("OPENEMR_IMAGE") == self.lock["services"]["openemr"] + assert v.get("MARIADB_IMAGE") == self.lock["services"]["mariadb"] + self.runtime_env.chmod(0o600) + + return ThroughputFixture(state) + + +def add_clerks(fixture: Any, n: int = 8) -> list[str]: + """Clone the fixture's actor user into n clerks (own username and ACL row).""" + stmts = ["USE openemr"] + for i, name in enumerate(CLERKS[:n], 1): + aro = 100 + i + stmts += [ + "DROP TEMPORARY TABLE IF EXISTS t_u", + "CREATE TEMPORARY TABLE t_u SELECT * FROM users WHERE username='openadapt_actor'", + f"UPDATE t_u SET id=0, uuid=NULL, username='{name}', fname='Clerk', lname='{i}'", + "INSERT INTO users SELECT * FROM t_u", + f"SET @uid=(SELECT id FROM users WHERE username='{name}')", + "INSERT INTO users_secure (id,username,password,last_update_password) " + f"SELECT @uid,'{name}',password,last_update_password FROM users_secure " + "WHERE username='openadapt_actor'", + "INSERT INTO gacl_aro (id,section_value,value,order_value,name,hidden) " + f"VALUES ({aro},'users','{name}',10,'Clerk {i}',0)", + f"INSERT INTO gacl_groups_aro_map (group_id,aro_id) VALUES (11,{aro})", + f"INSERT INTO `groups` (name,user) VALUES ('Default','{name}')", + ] + stmts.append(f"UPDATE gacl_aro_seq SET id={100 + n}") + fixture._db(";\n".join(stmts) + ";") + count = fixture._db( + "SELECT COUNT(*) FROM openemr.users WHERE username LIKE 'oa_clerk%'" + ).decode().strip() + if count != str(n): + raise RuntimeError(f"expected {n} clerks, found {count}") + return CLERKS[:n] + + +SQL_FIELDS = ("id", "title", "fname", "lname", "DOB", "sex", "street", "city", "state", + "postal_code", "phone_home", "email", "country_code") + + +def sql_patients(fixture: Any) -> list[dict[str, str]]: + out = fixture._db( + "SELECT " + ",".join(SQL_FIELDS) + " FROM openemr.patient_data ORDER BY id" + ).decode() + return [ + {k: ("" if v == "NULL" else v) for k, v in zip(SQL_FIELDS, line.split("\t"))} + for line in out.splitlines() + ] + + +def container_started(fixture: Any) -> str: + import subprocess + + project = fixture.lock["project"] + names = [f"{project}-openemr-1", f"{project}-db-1"] + r = subprocess.run( + ["podman", "inspect", "-f", "{{.Name}}={{.State.StartedAt}}/{{.State.Running}}", *names], + capture_output=True, text=True, + ) + return r.stdout.strip().replace("\n", "|") + + +# -- load ------------------------------------------------------------------ + + +def wait_quiet(threshold: float, max_wait_s: float, sustain: int = 4) -> dict[str, Any]: + """Start only once the 1-minute load average stays below ``threshold``.""" + t0 = time.time() + ok = 0 + while True: + one = os.getloadavg()[0] + ok = ok + 1 if one < threshold else 0 + if ok >= sustain or time.time() - t0 > max_wait_s: + return {"threshold": threshold, "waited_s": round(time.time() - t0, 1), + "gave_up": ok < sustain, "load_at_start": os.getloadavg()} + time.sleep(5.0) + + +class LoadSampler: + def __init__(self, every_s: float = 2.0) -> None: + self.samples: list[float] = [] + self._stop = threading.Event() + self._t = threading.Thread(target=self._loop, args=(every_s,), daemon=True) + self._t.start() + + def _loop(self, every_s: float) -> None: + while not self._stop.is_set(): + self.samples.append(os.getloadavg()[0]) + self._stop.wait(every_s) + + def stop(self) -> dict[str, Any]: + self._stop.set() + self._t.join(timeout=5) + s = self.samples + return {"load1_mean": round(sum(s) / len(s), 2) if s else None, + "load1_max": round(max(s), 2) if s else None, "ncpu": os.cpu_count()} + + +# -- the run --------------------------------------------------------------- + + +def _demo_module() -> Any: + spec = importlib.util.spec_from_file_location( + "openemr_local_demo", FLOW / "scripts" / "openemr_local_demo.py" + ) + demo = importlib.util.module_from_spec(spec) + sys.modules["openemr_local_demo"] = demo + spec.loader.exec_module(demo) + return demo + + +def run(a: argparse.Namespace) -> dict[str, Any]: + from openadapt_flow._browser_setup import ensure_chromium_installed + from openadapt_flow.benchmark.openemr_local import ( + OpenEMRPatientOracle, + SyntheticPatientSpec, + ) + from openadapt_flow.cli_worklist_pool import WebPoolWorker + from openadapt_flow.ir import Workflow + from openadapt_flow.runtime import Replayer + from openadapt_flow.runtime.worklist_pool import PoolPolicy, run_worklist_pool + + if a.profile and a.workers != 1: + raise SystemExit("--profile needs --workers 1") + os.environ.setdefault("OPENADAPT_FLOW_BROWSER", "bundled") + demo = _demo_module() + out = Path(a.out) / a.tag + out.mkdir(parents=True, exist_ok=False) + fixture = make_fixture(Path(a.fixture_state)) + t = time.time() + digest = fixture.reset() + clerks = add_clerks(fixture) + seed = fixture.token_session("actor").post( + fixture.api_base_url + "/apis/default/api/patient", + json=dict(SyntheticPatientSpec(**EXISTING_ROSA).fields), + ).status_code + rows = synthetic_referrals(a.items) + if a.halt_at is not None: + rows[a.halt_at] = dict(POSSIBLE_DUPLICATE) + before_ids = {r["id"] for r in sql_patients(fixture)} + bundle = Path(a.bundle) + workflow = Workflow.load(bundle) + engine_sha = os.popen(f"git -C {FLOW} rev-parse HEAD").read().strip() + meta = { + "tag": a.tag, "workers": a.workers, "items": a.items, "window": a.window, + "halt_at": a.halt_at, "profile": a.profile, "engine_sha": engine_sha, + "engine_dirty": bool(os.popen(f"git -C {FLOW} status --porcelain -- openadapt_flow").read().strip()), + "bundle_digest": getattr(workflow.manifest, "content_digest", None), + "baseline": digest[:12], "seed_status": seed, "setup_s": round(time.time() - t, 2), + "started": time.strftime("%Y-%m-%d %H:%M:%S"), + } + ensure_chromium_installed() + importlib.import_module("openadapt_flow.vision.ocr")._get_engine() # load the OCR model before the clock, as a warm worker would + values = fixture._runtime_values() + + class _Clerk: + def __init__(self, user: str) -> None: + self.user = user + self.ui_base_url = fixture.ui_base_url + + def _runtime_values(self) -> dict[str, str]: + return {**values, "OPENEMR_ACTOR_USER": self.user} + + class _PageOnly: + def __init__(self, page: Any) -> None: + self.page = page + + def open_form(page: Any) -> None: + """Start each item where the recording started: no chart open, blank form.""" + if page.evaluate( + "() => !!(window.app_view_model && app_view_model.application_data.patient())" + ): + # The chart's own close button (OpenEMR clearPatient), no finder. + with page.expect_response(lambda r: "unset_session_ajax.php" in r.url, timeout=30_000): + page.evaluate("() => clearPatient(false)") + page.wait_for_function("() => !app_view_model.application_data.patient()", timeout=30_000) + frame = page.frame(name=demo.FORM_FRAME_NAME) + if frame is not None and demo.FORM_PATH in frame.url: + try: + if frame.locator("#form_fname").input_value(timeout=2000) == "": + return # sign-in left the blank form open + except Exception: # noqa: BLE001 - fall through and reload it + pass + page.evaluate( + "({path, name}) => { window.navigateTab(path, name); window.activateTabByName(name, true); }", + {"path": demo.FORM_PATH, "name": demo.FORM_FRAME_NAME}, + ) + frame = demo._form_frame(page) + frame.locator("#form_fname").wait_for(state="visible", timeout=60_000) + if frame.locator("#form_fname").input_value() != "": + raise RuntimeError("the New Patient form was not blank") + + sessions: dict[str, Any] = {} + + def run_item(user: str): + def _run(page: Any, idx: int, row: dict[str, Any], item_dir: Path, pipeline: Any) -> Any: + spec = SyntheticPatientSpec(**row) + if user not in sessions: + sessions[user] = fixture.token_session("oracle") + oracle = OpenEMRPatientOracle(fixture.api_base_url, sessions[user], spec) + backend = demo._BasePlaywrightBackend(page) + prof = None + if a.profile: + from benchmark.openemr_throughput.profiler import RunProfiler + + prof = RunProfiler() + prof.install(backend) + try: + with (prof.span("task") if prof else _null()): + rep = Replayer( + backend, effect_verifier=oracle.verifier, readback_pipeline=pipeline + ).run(workflow, params=spec.params(), bundle_dir=bundle, run_dir=item_dir) + finally: + if prof: + prof.uninstall() + if prof: + (item_dir / "profile.json").write_text(json.dumps( + {"buckets": prof.buckets(), "calls": prof.summary(), + "parents": prof.parents(), "steps": prof.step_buckets()}, indent=1)) + if not a.keep_pngs: + for png in (item_dir / "steps").glob("*.png"): + png.unlink() + return rep + return _run + + workers = [ + WebPoolWorker( + name=user, entry_url=fixture.ui_base_url, run_item_on_page=run_item(user), + sign_in=(lambda page, user=user: demo._preauthenticate_browser(_PageOnly(page), _Clerk(user))), + open_entry=open_form, readback_window=a.window, + ) + for user in clerks[: a.workers] + ] + policy = PoolPolicy(workers=a.workers, readback_window=a.window, + items_independent=True, max_workers=8) + free_gb = shutil.disk_usage(str(out)).free / 1e9 + if free_gb < 8: + raise SystemExit(f"disk below 8 GB ({free_gb:.1f} GB free)") + gate = wait_quiet(a.gate_load, a.gate_max_wait_s) + sampler = LoadSampler() + c0 = container_started(fixture) + t0 = time.time() + report = run_worklist_pool( + rows, workers, run_dir=out / "pool", policy=policy, + on_event=lambda m: print(time.strftime("%H:%M:%S"), m, flush=True), + ) + wall = time.time() - t0 + load = sampler.stop() + c1 = container_started(fixture) + + # Judge (not timed): the test setup's own SQL read-back of every item. + new_rows = [r for r in sql_patients(fixture) if r["id"] not in before_ids] + items = [] + for item in report.items: + spec = SyntheticPatientSpec(**rows[item["idx"]]) + mine = [r for r in new_rows if r["lname"] == spec.lname and r["email"] == spec.email] + fields = dict(spec.fields) + match = sum(1 for k, v in fields.items() if len(mine) == 1 and mine[0].get(k) == v) + duplicate_case = a.halt_at is not None and item["idx"] == a.halt_at + res = item.get("result") or {} + items.append({ + "idx": item["idx"], "state": item["state"], "worker": item["worker"], + "new_charts": len(mine), "readback": f"{match}/{len(fields)}", + "correct": (len(mine) == 0) if duplicate_case else (len(mine) == 1 and match == len(fields)), + "duplicate_case": duplicate_case, "wall_s": res.get("wall_s"), + "readback_mode": res.get("readback"), + "claimed_at": item["claimed_at"], "finished_at": item["finished_at"], + }) + ran = [i for i in items if i["state"] != "pending"] + saved = [i for i in ran if i["correct"] and i["new_charts"] == 1] + summary = { + **meta, "wall_s": round(wall, 3), "pool_wall_s": round(report.wall_s, 3), + "sign_in_s": {w.name: round(w.open_s, 2) for w in report.per_worker}, + "counts": report.counts(), "stop": report.stop, "items_run": len(ran), + "saved_correct": len(saved), + "wrong_or_extra_charts": sum(1 for i in items if not i["correct"] and i["state"] != "pending") + + sum(1 for i in items if i["state"] == "pending" and i["new_charts"]), + "new_charts_total": len(new_rows), + "entries_per_hour": round(len(saved) * 3600 / wall, 1), + "written_after_failed_readback": report.written_after_failed_readback, + "per_worker": [{"name": w.name, "items": len(w.items), "states": w.states, + "busy_s": round(w.busy_s, 1)} for w in report.per_worker], + **load, "gate": gate, "valid": c0 == c1, "ended": time.strftime("%Y-%m-%d %H:%M:%S"), + } + (out / "summary.json").write_text(json.dumps(summary, indent=1, default=str)) + (out / "items.json").write_text(json.dumps(items, indent=1)) + return summary + + +class _null: + def __enter__(self) -> None: + return None + + def __exit__(self, *exc: object) -> None: + return None + + +def main() -> None: + ap = argparse.ArgumentParser(description=__doc__.split("\n\n")[0]) + sub = ap.add_subparsers(dest="cmd", required=True) + r = sub.add_parser("run") + r.add_argument("--bundle", required=True) + r.add_argument("--fixture-state", required=True) + r.add_argument("--out", required=True, help="evidence root; each run gets OUT/TAG") + r.add_argument("--tag", required=True) + r.add_argument("--workers", type=int, required=True) + r.add_argument("--items", type=int, required=True) + r.add_argument("--window", type=int, default=0) + r.add_argument("--halt-at", type=int, default=None) + r.add_argument("--profile", action="store_true") + r.add_argument("--keep-pngs", action="store_true") + r.add_argument("--gate-load", type=float, default=16.0) + r.add_argument("--gate-max-wait-s", type=float, default=7200.0) + r.add_argument("--results", default=str(HERE / "results" / "runs.jsonl")) + a = ap.parse_args() + summary = run(a) + Path(a.results).parent.mkdir(parents=True, exist_ok=True) + with open(a.results, "a") as fh: + fh.write(json.dumps({k: v for k, v in summary.items()}, default=str) + "\n") + print(json.dumps({k: summary[k] for k in ( + "tag", "workers", "window", "wall_s", "items_run", "saved_correct", + "entries_per_hour", "counts", "stop", "load1_mean", "valid")}, default=str)) + + +if __name__ == "__main__": + main() diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 7741310397..f5a04df84c 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -548,7 +548,9 @@ class _FramePoint: # hit-tests every candidate through the real frame chain (occlusion, clipping # by the parent frame) before believing it. _VISIBLE_TEXT_CANDIDATES_JS = r"""(target) => { - const norm = (v) => (v || '').replace(/\s+/g, '').toLowerCase(); + // Letters and digits only: recorded texts come from OCR, which drops + // spaces and misreads punctuation ("October-2026-" for "October 2026"). + const norm = (v) => (v || '').toLowerCase().replace(/[^\p{L}\p{N}]+/gu, ''); const want = norm(target); if (!want || !document.body) return []; const out = []; @@ -591,7 +593,9 @@ class _FramePoint: # At a hit-tested point: is the topmost element (or an ancestor) the one whose # rendered text contains the target? Proves the text is not covered. _TEXT_AT_POINT_JS = r"""([px, py, target]) => { - const norm = (v) => (v || '').replace(/\s+/g, '').toLowerCase(); + // Letters and digits only: recorded texts come from OCR, which drops + // spaces and misreads punctuation ("October-2026-" for "October 2026"). + const norm = (v) => (v || '').toLowerCase().replace(/[^\p{L}\p{N}]+/gu, ''); const want = norm(target); // The topmost element at the point, or one of its ancestors, must be a // DEEPEST holder of the text (no child holds it too). Climbing to a big @@ -2830,9 +2834,10 @@ def visible_text_present(self, text: str) -> Optional[bool]: """Is ``text`` rendered visibly in the viewport, per the DOM? Returns ``True`` only when some element's rendered text (``innerText``) - contains ``text`` as an exact case-insensitive substring once all - whitespace is removed (the OCR tier's squash; recorded OCR texts often - lose spaces), and + contains ``text`` as an exact case-insensitive substring once + everything but letters and digits is removed (recorded OCR texts often + lose spaces and misread punctuation; the OCR tier itself accepts an + 80% contiguous match, so this is still the stricter check), and that element is not hidden or transparent, intersects its document's viewport, AND a hit test through the real frame chain at that point lands on it (so text under a dialog, clipped by its frame, or scrolled diff --git a/packages/flow/openadapt_flow/cli_worklist_pool.py b/packages/flow/openadapt_flow/cli_worklist_pool.py index d132ed61bf..410a53c507 100644 --- a/packages/flow/openadapt_flow/cli_worklist_pool.py +++ b/packages/flow/openadapt_flow/cli_worklist_pool.py @@ -175,6 +175,8 @@ class WebPoolWorker: entry_url: str run_item_on_page: RunItem sign_in: Optional[Callable[[Any], None]] = None + #: How to reach the item's start page; default ``page.goto(entry_url)``. + open_entry: Optional[Callable[[Any], None]] = None cdp_endpoint: Optional[str] = None headed: bool = False readback_window: int = 0 @@ -217,7 +219,10 @@ def session_fingerprint(self) -> Optional[str]: return cookie_fingerprint(self._context.cookies()) def run_item(self, idx: int, row: dict[str, Any], item_dir: Path) -> ItemRun: - self._page.goto(self.entry_url) + if self.open_entry is not None: + self.open_entry(self._page) + else: + self._page.goto(self.entry_url) report = self.run_item_on_page(self._page, idx, row, item_dir, self._pipeline) return _item_run(report, self._pipeline) diff --git a/packages/flow/tests/test_page_settle.py b/packages/flow/tests/test_page_settle.py index 03d0552568..3d4ac8879c 100644 --- a/packages/flow/tests/test_page_settle.py +++ b/packages/flow/tests/test_page_settle.py @@ -206,6 +206,9 @@ def test_dom_text_presence_requires_visible_uncovered_text(browser) -> None: time.sleep(0.3) assert backend.visible_text_present("Visible words here") is True assert backend.visible_text_present("visiblewords") is True # squash + # OCR-recorded texts misread punctuation; letters and digits decide. + assert backend.visible_text_present("Visible-words, here.") is True + assert backend.visible_text_present("Visible words theirs") is None assert backend.visible_text_present("Framed words") is True for absent in ( "Hidden words", diff --git a/packages/flow/tests/test_worklist_pool.py b/packages/flow/tests/test_worklist_pool.py index 52130b1247..76ffa869fe 100644 --- a/packages/flow/tests/test_worklist_pool.py +++ b/packages/flow/tests/test_worklist_pool.py @@ -103,12 +103,13 @@ def test_each_item_is_claimed_exactly_once(tmp_path: Path) -> None: def test_forced_halt_on_one_worker_stops_the_others(tmp_path: Path) -> None: """A halt on worker w1 stops new claims on every worker; in-flight items finish.""" - halted = threading.Event() - in_flight_finished: list[int] = [] + halted_idx: list[int] = [] + w1_items: list[int] = [] def halting(idx: int) -> ItemRun: - if idx == 5: - halted.set() + w1_items.append(idx) + if len(w1_items) == 2: # w1's second item halts, whatever its index + halted_idx.append(idx) return ItemRun( state=HALTED, detail={ @@ -118,36 +119,30 @@ def halting(idx: int) -> ItemRun: ) return ItemRun(state=VERIFIED) - def slow_peer(idx: int) -> ItemRun: - # A peer item that is mid-flight when the halt lands still finishes - # and records its own result. - if halted.is_set(): - in_flight_finished.append(idx) - return ItemRun(state=VERIFIED) - w1 = FakeWorker("w1", halting, delay_s=0.01) - peers = [FakeWorker(f"w{i}", slow_peer, delay_s=0.03) for i in (2, 3, 4)] + peers = [FakeWorker(f"w{i}", delay_s=0.03) for i in (2, 3, 4)] report = run_worklist_pool( _rows(100), [w1, *peers], run_dir=tmp_path, policy=_policy(4) ) assert not report.success assert report.stop is not None - assert report.stop["worker"] == "w1" and report.stop["idx"] == 5 + assert report.stop["worker"] == "w1" and report.stop["idx"] == halted_idx[0] assert "identity check failed" in report.stop["reason"] states = {item["idx"]: item["state"] for item in report.items} - assert states[5] == HALTED + assert states[halted_idx[0]] == HALTED # Far fewer than 100 items ran: claims stopped everywhere. ran = [idx for w in (w1, *peers) for idx in w.ran] assert len(ran) < 40 - # Every item that ran has a final result; nothing is left "claimed". + # Every item that ran, including peers' in-flight ones, has a final + # result; nothing is left "claimed". for idx in ran: assert states[idx] in {VERIFIED, HALTED} assert "pending" in states.values() # No worker claimed anything after the stop flag was set. stop_at = report.stop["at"] for item in report.items: - if item["claimed_at"] is not None and item["idx"] != 5: + if item["claimed_at"] is not None: assert item["claimed_at"] <= stop_at + 0.001 From fa2c6c41b691a4cf6629d1aa120c2126deb43c88 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 18 Oct 2026 15:08:41 -0400 Subject: [PATCH 05/18] feat(flow): worklist.entry_bundle returns each worker to the start page; throughput and limits docs Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/CAPABILITIES.md | 7 + packages/flow/docs/LIMITS.md | 25 ++++ packages/flow/docs/PRODUCTION.md | 2 + packages/flow/docs/THROUGHPUT.md | 138 ++++++++++++++++++ packages/flow/docs/deployment.example.yaml | 19 +++ packages/flow/openadapt_flow/__main__.py | 16 ++ .../flow/openadapt_flow/cli_worklist_pool.py | 37 ++++- packages/flow/openadapt_flow/deployment.py | 4 + packages/flow/tests/test_cli_worklist_pool.py | 61 ++++++++ 9 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 packages/flow/docs/THROUGHPUT.md diff --git a/packages/flow/docs/CAPABILITIES.md b/packages/flow/docs/CAPABILITIES.md index f56380ba0e..c928d684dc 100644 --- a/packages/flow/docs/CAPABILITIES.md +++ b/packages/flow/docs/CAPABILITIES.md @@ -21,6 +21,13 @@ at replaying one. These capabilities layer onto the same $0, model-free runtime. authoring time rather than emitting a bad bundle. This turns a replay of one recorded path into governed execution over a queue: `openadapt-flow for-each bundle --records worklist.csv --out queue-bundle`. +- **Parallel worklists.** `replay` and `run` with `--workers N` enter a + for-each bundle's items on N sessions at once, each signed in as its own + user from the deployment file, with every check still on per item. Each + item is claimed once from a durable queue, a halt on any worker stops new + items on all of them, and `--readback-window K` lets a worker start its + next item while the last save is still being read back. See + [`THROUGHPUT.md`](THROUGHPUT.md). - **Compose separately recorded bundles.** `compose` takes named compiled child bundles and a handoff contract and writes a parent artifact that `certify` and `run` execute. Each child keeps the surface it was recorded diff --git a/packages/flow/docs/LIMITS.md b/packages/flow/docs/LIMITS.md index 49fbd3dea2..6355e1790a 100644 --- a/packages/flow/docs/LIMITS.md +++ b/packages/flow/docs/LIMITS.md @@ -45,6 +45,31 @@ class named with `--allow-halt`. A handoff copies an effect-bound parameter from a confirmed receipt. Missing evidence HALTs. Subflows and worklists stay inside each child, on that child's surface. +## Parallel worklists + +`--workers N` runs a worklist's items on N signed-in sessions at once +([`THROUGHPUT.md`](THROUGHPUT.md)). Every item keeps every check. The run +refuses N above the deployment's `worklist.max_workers`, fewer accounts than +workers, two accounts for the same user, two workers sharing one session, a +program with steps outside its loop, and rows not declared +`items_independent`. It can't tell on its own whether two rows touch the same +record; that declaration is yours. A halt on any worker stops new items on +all of them, and an item claimed before a crash is never run again +automatically: it is marked `uncertain` for a person to check. The pipelined +read-back window applies only to an item's last save and only to read-backs +that don't use the screen; durable runs, which include the Standard and +Regulated profiles, always read back inline. Parallel RDP runs use the +FreeRDP session pool and have unit tests only. Throughput figures come from +one machine and a local OpenEMR test copy, not from a production EMR. + +## Short parameter values + +The right-record check binds a typed value into its expected text only when +the value has at least 4 characters. An item whose bound value is shorter, +such as the first name "Bo", halts at the identity check instead of saving. +This is deliberate: a short value matches too much unrelated text to identify +a record. + ## Save triage `openadapt-flow triage` orders verified saves for review under diff --git a/packages/flow/docs/PRODUCTION.md b/packages/flow/docs/PRODUCTION.md index dc60893fa6..2a8dd0ef76 100644 --- a/packages/flow/docs/PRODUCTION.md +++ b/packages/flow/docs/PRODUCTION.md @@ -99,6 +99,8 @@ map, and it refuses to start while a backend surface is unbound. A program (graph) resume of such a bundle is still refused. Worklists repeat that bundle over input records, and subflows reuse steps inside it. Neither one switches backends. +`--workers N` enters a worklist's items on N signed-in sessions at once, each +item still fully checked ([THROUGHPUT.md](THROUGHPUT.md)). Browser recording follows popups and new tabs in a browser Flow launched and, in attach mode, only pages a followed page opened on the declared origin. macOS and Linux bind one exact app and window, while a governed Windows run binds its application identity. diff --git a/packages/flow/docs/THROUGHPUT.md b/packages/flow/docs/THROUGHPUT.md new file mode 100644 index 0000000000..40fc8920b8 --- /dev/null +++ b/packages/flow/docs/THROUGHPUT.md @@ -0,0 +1,138 @@ +# Throughput: parallel worklists + +A worklist run enters one item after another in one signed-in session. That is +as fast as one careful person. `--workers N` runs the same items on N +sessions at once, each signed in as its own user, with every check still on. +An optional read-back window lets each worker start its next item while the +previous save is still being read back. + +Measured entries per hour on a local OpenEMR test copy are under +[Measured results](#measured-results). + +## What changes and what doesn't + +Each item is the same governed run a serial `--worklist` loop makes for that +row: the program with a one-row worklist, in its own run directory +(`/items/NNNNN/`), with its own authorization bound to that row. +Every check stays on for every item: the right-record check before each +action, the input read-back, the screen postconditions, and the +system-of-record read-back after the save. Only the scheduling changes. + +| Rule | What it means | +| --- | --- | +| One claim per item | Items live in a durable SQLite queue in the run directory (`worklist_queue.sqlite3`). A worker claims the next item in one transaction, so no two workers ever hold the same item. A claimed item is never handed out again, even after a crash: after a restart it is marked `uncertain` for a person to check, because something may have been saved. | +| One session per worker | Each worker opens its own browser context and signs in with its own account from the deployment file. The run refuses to start when two workers would share a session (identical session cookies) or sign in as the same user. Many EMRs lock a record per session or per user. | +| A declared limit | The deployment file states how many sessions the app tolerates at once (`worklist.max_workers`). `--workers` above it is refused. Without a deployment file the limit is 1. | +| One stop for all | A halt, a failure, a worker error, or `RECONCILIATION_REQUIRED` on any worker stops new claims on every worker. Items already in flight finish their own checks and record their results. Unstarted items stay `pending`. | +| Per-worker results | The run writes `worklist_pool.json` and `WORKLIST.md`: per worker, the items it ran and how each ended; per item, the worker, the state and the time. | + +## Run it + +1. Compile the task, and wrap it in a loop over a worklist with `for-each`: + + ```bash + openadapt-flow for-each referral-bundle --records referrals.csv --out referral-queue + ``` + + `--workers` needs this shape: one loop over a worklist and nothing before + or after it. Steps outside the loop would run once per item instead of once + per run, so other program shapes are refused. + +2. Record the sign-in once as its own bundle, with the user name as a + parameter and the password as a secret step. + +3. Declare the accounts and the app's limit in the deployment file: + + ```yaml + backend: + kind: web + url: https://emr.example.test/ + worklist: + max_workers: 4 # sessions this app tolerates at once + items_independent: true # items touch different records, any order + sign_in_bundle: ./sign-in-bundle + accounts: + - name: clerk-1 + params_env: {username: EMR_USER_1} + secrets_env: {password: EMR_PASS_1} + - name: clerk-2 + params_env: {username: EMR_USER_2} + secrets_env: {password: EMR_PASS_2} + # one account per worker + ``` + + The file holds only the names of environment variables, never a + credential. Instead of a sign-in bundle, an account can name + `browser_cdp_endpoint`, a Chromium already signed in as that user; the + worker opens a tab there and never closes the browser. + +4. Run with N workers: + + ```bash + openadapt-flow run referral-queue --config deployment.yaml --workers 4 + ``` + + `replay` takes the same flags. With `run`, each item gets its own + single-use authorization from the same admission gate, bound to that one + row. A managed Cloud dispatch and a qualification case run serially. + +The exit code is 0 only when every item finished and nothing stopped the run. + +## Pipelined read-back + +`--readback-window K` (or `worklist.readback_window`) lets a worker start +item n+1 while the read-back of item n's save is still running, with at most +K read-backs waiting. The default, 0, verifies each save before the next item +starts. + +- Only the last action of an item is deferred, and only when its read-back + doesn't use the screen (a REST, FHIR or database read). If the item runs + any further step, the engine waits for the read-back first and halts on a + failure, as it would inline. +- The item's `report.json` is rewritten with the verdict when the read-back + finishes, and its outcome is classified again from the completed evidence. +- If a read-back fails, the run stops on every worker. The report names the + items, at most K per worker, that the same worker wrote after the failed + one. Each of those still gets its own read-back. +- It needs `items_independent: true`. Durable and idempotency-ledger runs + verify inline, because their terminal state is written once; the Standard + and Regulated profiles are durable, so for them the window has no effect. + +## Remote desktop + +On RDP, `--workers` uses the FreeRDP session pool +(`backends/xfreerdp.py`, Linux): one headless session per account, connected +together, each driven by one worker. Each account names `rdp_username` and +`rdp_password_env`. This path has unit tests only; no parallel run over a real +RDP host has been measured. + +## Limits + +- Items must be independent. The run can't tell whether two rows touch the + same record; you declare it with `items_independent`. If two rows write the + same patient, run them serially. +- The app's capacity sets the ceiling. More workers than the app or its + database can serve make every item slower, and a busy app can trip + screen checks that wait for the page to settle. Those items halt; they don't + save the wrong thing. +- Each worker needs its own account, and the accounts need the same + permissions and screen layout the recording was made with. +- Each item must start where the recording started. On the web the worker + opens `worklist.entry_url` (default `backend.url`) before each item. Apps + that keep state between items, such as an open patient chart, need a way + back to the start page; the OpenEMR benchmark closes the chart first (see + `benchmark/openemr_throughput/harness.py`). +- Throughput was measured on one machine with a local test copy of OpenEMR. + A real deployment adds network latency, a shared database and other users. + +## Where the time goes in one entry + +On the OpenEMR referral (41 steps), one entry spends its time like this, from +the exclusive-time profile in +[`benchmark/openemr_throughput/`](../benchmark/openemr_throughput/README.md): + +PROFILE_TABLE + +## Measured results + +RESULTS_TABLE diff --git a/packages/flow/docs/deployment.example.yaml b/packages/flow/docs/deployment.example.yaml index dd3fe4d143..46d69c87c8 100644 --- a/packages/flow/docs/deployment.example.yaml +++ b/packages/flow/docs/deployment.example.yaml @@ -272,3 +272,22 @@ human_decisions: policy: # A policy YAML path, or a shipped built-in name (permissive, clinical-write). policy: clinical-write + +# -- worklist: parallel worklist runs (docs/THROUGHPUT.md) -------------------- +# Omit this section to run one item at a time. `--workers N` above +# max_workers is refused; each worker signs in with its own account and the +# file holds only the NAMES of environment variables, never a credential. +# worklist: +# max_workers: 4 # sessions this app tolerates at once +# items_independent: true # rows touch different records, in any order +# readback_window: 0 # 0 = read each save back before the next item +# on_halt: stop # or skip_unwritten +# sign_in_bundle: ./sign-in-bundle +# entry_url: https://emr.example.test/patients/new +# accounts: +# - name: clerk-1 +# params_env: {username: EMR_USER_1} +# secrets_env: {password: EMR_PASS_1} +# - name: clerk-2 +# params_env: {username: EMR_USER_2} +# secrets_env: {password: EMR_PASS_2} diff --git a/packages/flow/openadapt_flow/__main__.py b/packages/flow/openadapt_flow/__main__.py index a09356c331..924a5404d4 100644 --- a/packages/flow/openadapt_flow/__main__.py +++ b/packages/flow/openadapt_flow/__main__.py @@ -2436,6 +2436,11 @@ def _replay_worklist_pool( relation = cwp.loop_relation(workflow) rows = cwp.pool_rows(workflow, worklists, relation) accounts = cwp.check_accounts(policy, cfg.worklist, kind) + entry_workflow = ( + cwp.load_entry_bundle(Path(cfg.worklist.entry_bundle)) + if cfg.worklist.entry_bundle and kind == "web" + else None + ) except PoolRefusal as exc: print(f"REFUSED: {exc}. Nothing was executed.") return 2 @@ -2506,12 +2511,23 @@ def run_item(wiring, source, idx, row, item_dir, pipeline, *, page_mode: bool): url=url, run_dir=run_dir / "sign-in" / name, ) + open_entry = None + if entry_workflow is not None: + open_entry = partial( + cwp.entry_with_bundle, + url=entry_url, + workflow=entry_workflow, + bundle=Path(cfg.worklist.entry_bundle), + backend_factory=lambda page: build_backend(backend_cfg, page=page), + run_dir=run_dir / "entry" / name, + ) workers.append( cwp.WebPoolWorker( name=name, entry_url=entry_url, run_item_on_page=partial(run_item, wirings[i], page_mode=True), sign_in=sign_in, + open_entry=open_entry, cdp_endpoint=account.browser_cdp_endpoint if account else None, headed=bool(args.headed or cfg.backend.headed), readback_window=policy.readback_window, diff --git a/packages/flow/openadapt_flow/cli_worklist_pool.py b/packages/flow/openadapt_flow/cli_worklist_pool.py index 410a53c507..9d3b2ed06b 100644 --- a/packages/flow/openadapt_flow/cli_worklist_pool.py +++ b/packages/flow/openadapt_flow/cli_worklist_pool.py @@ -307,7 +307,7 @@ def sign_in_with_bundle( """Replay the recorded sign-in bundle with this account's credentials.""" from openadapt_flow.ir import Workflow - from openadapt_flow.runtime.replayer import Replayer + from openadapt_flow.runtime import Replayer page.goto(url) workflow = Workflow.load(bundle) @@ -320,6 +320,41 @@ def sign_in_with_bundle( raise RuntimeError(f"sign-in bundle did not finish for {account.name}") +def entry_with_bundle( + page: Any, + *, + url: str, + workflow: Any, + bundle: Path, + backend_factory: Callable[[Any], Any], + run_dir: Path, +) -> None: + """Go to the entry URL, then replay the recorded entry navigation.""" + + from openadapt_flow.runtime import Replayer + + from uuid import uuid4 + + page.goto(url) + report = Replayer(backend_factory(page)).run( + workflow, params={}, bundle_dir=bundle, run_dir=run_dir / uuid4().hex[:12] + ) + if not report.success: + raise RuntimeError("the entry bundle did not reach the start page") + + +def load_entry_bundle(path: Path) -> Any: + """Load the entry bundle; refuse one that declares any effect.""" + + from openadapt_flow.ir import Workflow + from openadapt_flow.traversal import iter_workflow_steps + + workflow = Workflow.load(path) + if any(step.effects for step in iter_workflow_steps(workflow)): + raise PoolRefusal("worklist.entry_bundle declares a save; it may only navigate") + return workflow + + def print_pool_report(report: PoolReport, run_dir: Path) -> int: counts = ", ".join(f"{k} {v}" for k, v in sorted(report.counts().items())) print( diff --git a/packages/flow/openadapt_flow/deployment.py b/packages/flow/openadapt_flow/deployment.py index b218454ad1..7ebc757a13 100644 --- a/packages/flow/openadapt_flow/deployment.py +++ b/packages/flow/openadapt_flow/deployment.py @@ -830,6 +830,10 @@ class WorklistSection(BaseModel): sign_in_bundle: Optional[str] = None #: Page each item starts from (default: ``backend.url``). entry_url: Optional[str] = None + #: Recorded navigation replayed after ``entry_url`` before each item, for + #: apps that keep state between items (close the open chart, open a blank + #: form). It must not save anything; the run refuses one with effects. + entry_bundle: Optional[str] = None #: One account per worker. Workers never share a session. accounts: list[WorklistAccount] = Field(default_factory=list) diff --git a/packages/flow/tests/test_cli_worklist_pool.py b/packages/flow/tests/test_cli_worklist_pool.py index 5a4afe14db..dc7d05bfae 100644 --- a/packages/flow/tests/test_cli_worklist_pool.py +++ b/packages/flow/tests/test_cli_worklist_pool.py @@ -138,6 +138,12 @@ def __init__(self, backend, **kwargs): self.kwargs = kwargs def run(self, workflow, **kwargs): + if "worklists" not in kwargs: # the entry navigation, not an item + with lock: + calls.append( + {"entry": workflow.name, "endpoint": self.backend.ctx.endpoint} + ) + return _Report(success=True) note = next(iter(kwargs["worklists"].values()))[0]["note"] with lock: calls.append( @@ -265,3 +271,58 @@ def test_program_with_steps_outside_the_loop_is_refused( assert rc == 2 assert "one loop over a worklist" in capsys.readouterr().out assert calls == [] + + +def _entry_bundle(tmp_path: Path, *, with_save: bool) -> Path: + from openadapt_flow.runtime.effects import Effect, EffectKind + + step = Step(id="new_form", intent="click New", action=ActionKind.CLICK) + if with_save: + step = Step( + id="new_form", + intent="click Save", + action=ActionKind.CLICK, + effects=[Effect(kind=EffectKind.RECORD_WRITTEN, match={"x": "1"})], + ) + wf = Workflow(name="entry", steps=[step]) + path = tmp_path / ("entry-save" if with_save else "entry") + wf.save(path) + return path + + +def test_entry_bundle_runs_before_every_item(tmp_path, monkeypatch) -> None: + bundle, _ = _for_each_bundle(tmp_path, n=4) + cfg = _config(tmp_path) + cfg.write_text( + cfg.read_text().replace( + "worklist:\n", + f"worklist:\n entry_bundle: {_entry_bundle(tmp_path, with_save=False)}\n", + ) + ) + calls: list = [] + _install(monkeypatch, calls) + assert _replay(bundle, cfg, tmp_path / "run", "--workers", "2") == 0 + entries = [c for c in calls if "entry" in c] + items = [c for c in calls if "note" in c] + assert len(items) == 4 and len(entries) == 4 + # Each worker navigated in its own session before each of its items. + for endpoint in {c["endpoint"] for c in items}: + assert sum(c["endpoint"] == endpoint for c in entries) == sum( + c["endpoint"] == endpoint for c in items + ) + + +def test_entry_bundle_with_a_save_is_refused(tmp_path, monkeypatch, capsys) -> None: + bundle, _ = _for_each_bundle(tmp_path, n=4) + cfg = _config(tmp_path) + cfg.write_text( + cfg.read_text().replace( + "worklist:\n", + f"worklist:\n entry_bundle: {_entry_bundle(tmp_path, with_save=True)}\n", + ) + ) + calls: list = [] + _install(monkeypatch, calls) + assert _replay(bundle, cfg, tmp_path / "run", "--workers", "2") == 2 + assert "may only navigate" in capsys.readouterr().out + assert calls == [] From 58a55dc5c84b12ec3f3c40afc46a73e1946a808d Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 18 Oct 2026 15:13:39 -0400 Subject: [PATCH 06/18] fix(flow): a handed-off read-back always settles through the pending path; benchmark README and figure script Co-Authored-By: Claude Opus 5.5 --- .../benchmark/openemr_throughput/README.md | 91 ++++++++ .../openemr_throughput/compute_figures.py | 201 ++++++++++++++++++ .../openadapt_flow/runtime/worklist_pool.py | 4 +- 3 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 packages/flow/benchmark/openemr_throughput/README.md create mode 100644 packages/flow/benchmark/openemr_throughput/compute_figures.py diff --git a/packages/flow/benchmark/openemr_throughput/README.md b/packages/flow/benchmark/openemr_throughput/README.md new file mode 100644 index 0000000000..b1c42ad746 --- /dev/null +++ b/packages/flow/benchmark/openemr_throughput/README.md @@ -0,0 +1,91 @@ +# OpenEMR referral throughput: parallel worklists + +How many synthetic referrals OpenAdapt enters per hour on a local OpenEMR +8.0.0.3 test copy, one worker at a time and with 4 and 8 workers, and where the +time in one entry goes. Every patient, address, phone number (555-01xx) and +email (`example.invalid`) is made up. OpenAdapt isn't affiliated with the +OpenEMR project. + +| File | What it is | +| --- | --- | +| `harness.py` | Runs one worklist on N workers against the fixture and judges every item afterwards | +| `profiler.py` | Exclusive-time profiler for one replay (each wrapped call's own time, no double counting) | +| `compute_figures.py` | `collect` copies raw run data into `results/`; `figures` derives `results/figures.json`; `--check` fails when it is stale | +| `results/runs.jsonl` | One line per run: arm, workers, wall time, counts, load, engine commit | +| `results/items.csv` | One row per item: worker, state, judge's verdict, seconds | +| `results/profiles/*.json` | Per-item time buckets and per-step buckets | +| `results/figures.json` | The figures quoted in docs, derived from the files above | + +## Task and fixture + +The task is the referral program from +[`../openemr_referral_timing/`](../openemr_referral_timing/README.md): 41 steps +that fill and save OpenEMR's New Patient form, compiled with DOM element +identity. The bundle is not in this repository. The engine's own read-back +checks the saved chart through the OpenEMR REST API after every save. + +The fixture is [`../openemr_local/`](../openemr_local/) (pinned images, verified +baseline) under its own compose project and ports. Before each run the harness +resets the database to the baseline, adds 8 clerk accounts cloned from the +fixture's actor user, seeds the existing chart Rosa Delgado, and generates M +distinct synthetic referrals. + +## What one run measures + +- **Wall time** starts before the first worker signs in and ends when the last + worker finishes its last item, including that item's read-back. Each worker + signs in once, as its own clerk, in its own browser context. +- **Per item**: close any open chart (OpenEMR keeps the last patient open after + a save, which moves the form), open a blank New Patient form, replay the + program, and read the save back. Nothing is skipped between items. +- **Judge**, after the clock stops: the test setup reads every item back from + the database with SQL. An item is correct when exactly one new chart exists + for it with 12 of 12 fields matching. Unrun items must have no chart. +- **Entries per hour** = correct items x 3600 / wall time, pooled over rounds. + +The arms ran interleaved in rounds (1 worker on 16 items, then 4 workers on 32, +8 workers on 32, and 4 workers with read-back window 1 on 32), so that load +from other work on the shared machine fell on every arm. Each run waited up to +15 minutes for the 1-minute load average to drop below 24 and records the load +it ran under. + +The serial figure published earlier (29.7 s median per referral) measured a +different thing: a fresh browser context and sign-in for every referral, plus +the judge's read-back inside the clock. A worklist signs in once per worker, +so its per-item time is lower even with one worker. + +## Profiles + +`results/profiles/prof-main.json` profiles current `main` with the published +method (fresh context and sign-in per run). `results/profiles/prof-new.json` +profiles this branch as a one-worker worklist. Each wrapped call's own time is +assigned to one bucket, so the buckets add up to the task time. + +## Rebuild + +```bash +python -m benchmark.openemr_throughput.compute_figures figures --check +``` + +To re-run (needs podman, the fixture's baseline state, and the compiled +bundle): + +```bash +THROUGHPUT_UI_PORT=9911 THROUGHPUT_API_PORT=9910 \ +python -m benchmark.openemr_throughput.harness run --bundle BUNDLE \ + --fixture-state STATE --out OUT --tag n4-r1 --workers 4 --items 32 +``` + +## Results + +RESULTS_SECTION + +## Caveats + +- One machine (12 cores, shared with other work) and a local test copy of + OpenEMR in a 6-CPU podman VM. A production EMR adds network latency, a shared + database and other users; its concurrency limit may be lower. +- One task and one bundle. Items were independent new patients. +- The clerks share the actor's synthetic password and permissions. +- The browser is Playwright's bundled Chromium. The same bundle halted at a + scroll step in an installed Google Chrome; that was not investigated here. diff --git a/packages/flow/benchmark/openemr_throughput/compute_figures.py b/packages/flow/benchmark/openemr_throughput/compute_figures.py new file mode 100644 index 0000000000..56f4c18271 --- /dev/null +++ b/packages/flow/benchmark/openemr_throughput/compute_figures.py @@ -0,0 +1,201 @@ +"""Collect raw run data and derive the throughput figures. + + python -m benchmark.openemr_throughput.compute_figures collect --runs OUT --tags TAG ... + python -m benchmark.openemr_throughput.compute_figures figures [--check] + +``collect`` copies, for each tag, the harness's ``summary.json`` and +``items.json`` (and per-item ``profile.json`` files) from the evidence folder +into ``results/``. ``figures`` derives ``results/figures.json`` from those +files only; ``--check`` fails when ``figures.json`` is stale. + +Entries per hour = items saved and read back correctly x 3600 / wall time. +The wall time of a worklist run starts before the first sign-in and ends when +the last worker finishes; the test setup's own database read-back afterwards +is not in it. Every arm is pooled over its rounds: total correct entries over +total wall time. +""" + +from __future__ import annotations + +import argparse +import csv +import json +import statistics +import sys +from pathlib import Path +from typing import Any + +HERE = Path(__file__).resolve().parent +RESULTS = HERE / "results" + +ARMS = { + "serial": "1 worker", + "n4": "4 workers", + "n8": "8 workers", + "n4w1": "4 workers, read-back window 1", +} + + +def collect(runs: Path, tags: list[str]) -> None: + RESULTS.mkdir(exist_ok=True) + (RESULTS / "profiles").mkdir(exist_ok=True) + summaries = [] + item_rows = [] + for tag in tags: + src = runs / tag + summary = json.loads((src / "summary.json").read_text()) + summary.pop("gate", None) + summaries.append(summary) + for item in json.loads((src / "items.json").read_text()): + item_rows.append({"tag": tag, **item}) + profiles = {} + for prof in sorted((src / "pool" / "items").glob("*/profile.json")): + data = json.loads(prof.read_text()) + profiles[prof.parent.name] = {"buckets": data["buckets"], "steps": data["steps"]} + if profiles: + (RESULTS / "profiles" / f"{tag}.json").write_text(json.dumps(profiles, indent=1)) + with open(RESULTS / "runs.jsonl", "w") as fh: + for s in summaries: + fh.write(json.dumps(s, sort_keys=True) + "\n") + cols = ["tag", "idx", "worker", "state", "correct", "new_charts", "readback", + "duplicate_case", "wall_s", "readback_mode", "claimed_at", "finished_at"] + with open(RESULTS / "items.csv", "w", newline="") as fh: + w = csv.DictWriter(fh, fieldnames=cols, extrasaction="ignore", lineterminator="\n") + w.writeheader() + for row in item_rows: + w.writerow(row) + + +def collect_profile_main(src_root: Path, tag: str) -> None: + """Profiles of current main taken with the published method (fresh + browser context and sign-in per run; see README).""" + out = {} + rows = [] + for result in sorted(src_root.glob(f"{tag}/block-*/*/result.json")): + res = json.loads(result.read_text()) + prof = result.parent / "profile.json" + if not prof.exists(): + continue + data = json.loads(prof.read_text()) + key = f"{result.parent.parent.name}/{result.parent.name}" + out[key] = {"buckets": data["buckets"], "steps": data["steps"]} + rows.append({"run": key, "correct": res.get("correct"), "readback": res.get("readback"), + "outcome": res.get("outcome"), **(res.get("phases") or {}), + "load1_mean": (res.get("load") or {}).get("load1_mean")}) + (RESULTS / "profiles").mkdir(parents=True, exist_ok=True) + (RESULTS / "profiles" / f"{tag}.json").write_text(json.dumps(out, indent=1)) + (RESULTS / f"{tag}.runs.json").write_text(json.dumps(rows, indent=1)) + + +def _q(xs: list[float], p: float) -> float: + xs = sorted(xs) + k = (len(xs) - 1) * p + lo, hi = int(k), min(int(k) + 1, len(xs) - 1) + return xs[lo] + (xs[hi] - xs[lo]) * (k - lo) + + +def _arm(tag: str) -> str | None: + head = tag.split("-")[0] + return head if head in ARMS else None + + +def figures() -> dict[str, Any]: + runs = [json.loads(l) for l in (RESULTS / "runs.jsonl").read_text().splitlines() if l.strip()] + with open(RESULTS / "items.csv") as fh: + items = list(csv.DictReader(fh)) + out: dict[str, Any] = {"arms": {}, "profiles": {}, "halt_test": None} + for arm, label in ARMS.items(): + mine = [r for r in runs if _arm(r["tag"]) == arm] + if not mine: + continue + tags = {r["tag"] for r in mine} + its = [i for i in items if i["tag"] in tags] + times = [float(i["wall_s"]) for i in its if i["wall_s"] and i["correct"] == "True"] + wall = sum(r["wall_s"] for r in mine) + correct = sum(r["saved_correct"] for r in mine) + ran = sum(r["items_run"] for r in mine) + work = [] + for r in mine: + mine_items = [i for i in its if i["tag"] == r["tag"] and i["claimed_at"]] + if mine_items: + work.append(max(float(i["finished_at"]) for i in mine_items) + - min(float(i["claimed_at"]) for i in mine_items)) + out["arms"][arm] = { + "label": label, + "rounds": len(mine), + "items_run": ran, + "saved_correct": correct, + "not_saved": ran - correct, + "wrong_or_extra_charts": sum(r["wrong_or_extra_charts"] for r in mine), + "stopped_runs": sum(1 for r in mine if r["stop"]), + "wall_s_total": round(wall, 1), + "entries_per_hour": round(correct * 3600 / wall, 1) if wall else None, + "entries_per_hour_excluding_sign_in": round(correct * 3600 / sum(work), 1) if work else None, + "item_s": { + "n": len(times), + "median": round(statistics.median(times), 2) if times else None, + "p10": round(_q(times, 0.1), 2) if times else None, + "p90": round(_q(times, 0.9), 2) if times else None, + }, + "sign_in_s_median": round(statistics.median( + [v for r in mine for v in r["sign_in_s"].values()]), 2), + "load1_mean": round(statistics.mean(r["load1_mean"] for r in mine), 1), + "load1_max": max(r["load1_max"] for r in mine), + "engine_sha": sorted({r["engine_sha"][:8] for r in mine}), + "invalid_runs": sum(1 for r in mine if not r["valid"]), + } + for prof in sorted((RESULTS / "profiles").glob("*.json")): + data = json.loads(prof.read_text()) + buckets: dict[str, list[float]] = {} + for item in data.values(): + for k, v in item["buckets"].items(): + buckets.setdefault(k, []).append(v) + totals = [sum(item["buckets"].values()) for item in data.values()] + out["profiles"][prof.stem] = { + "n": len(data), + "task_s_median": round(statistics.median(totals), 2) if totals else None, + "bucket_s_median": {k: round(statistics.median(v), 2) for k, v in + sorted(buckets.items(), key=lambda kv: -statistics.median(kv[1]))}, + } + halt = [r for r in runs if r["tag"].startswith("halt")] + if halt: + r = halt[0] + out["halt_test"] = { + "workers": r["workers"], "items": r["items"], "halt_at": r["halt_at"], + "stop": r["stop"], "counts": r["counts"], "items_run": r["items_run"], + "wrong_or_extra_charts": r["wrong_or_extra_charts"], + } + return out + + +def main() -> int: + ap = argparse.ArgumentParser() + sub = ap.add_subparsers(dest="cmd", required=True) + c = sub.add_parser("collect") + c.add_argument("--runs", required=True, type=Path) + c.add_argument("--tags", nargs="+", required=True) + c.add_argument("--profile-main", default=None, help="tag of the published-method profile") + c.add_argument("--profile-main-root", type=Path, default=None) + f = sub.add_parser("figures") + f.add_argument("--check", action="store_true") + a = ap.parse_args() + if a.cmd == "collect": + collect(a.runs, a.tags) + if a.profile_main: + collect_profile_main(a.profile_main_root or a.runs, a.profile_main) + return 0 + fig = figures() + text = json.dumps(fig, indent=1, sort_keys=True) + "\n" + path = RESULTS / "figures.json" + if a.check: + if not path.exists() or path.read_text() != text: + print("figures.json is stale; run compute_figures figures", file=sys.stderr) + return 1 + return 0 + path.write_text(text) + print(text) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/packages/flow/openadapt_flow/runtime/worklist_pool.py b/packages/flow/openadapt_flow/runtime/worklist_pool.py index 6887100d2b..dc8eb4913c 100644 --- a/packages/flow/openadapt_flow/runtime/worklist_pool.py +++ b/packages/flow/openadapt_flow/runtime/worklist_pool.py @@ -648,7 +648,9 @@ def settle(entry: tuple[int, Future, list[int]]) -> None: wall = time.monotonic() - t_item summary.busy_s += wall detail = {**out.detail, "wall_s": round(wall, 3), "worker": worker.name} - if out.pending_readback is not None and out.state in SUCCESS_STATES: + if out.pending_readback is not None: + # The final state comes from the read-back, whatever the + # report said when the item returned. queue.set_state(idx, worker.name, READBACK_PENDING, detail) fut: Future = Future() From d6067e2bcf9555874a69739be684b852c3264c79 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 18 Oct 2026 15:23:56 -0400 Subject: [PATCH 07/18] feat(flow): per-step timing spans in report.json (settle, screenshot, resolve, identity, ocr, actuate, postcondition, readback, artifact_write) Co-Authored-By: Claude Opus 5.5 --- .../openadapt_flow/backends/linux_backend.py | 2 + .../openadapt_flow/backends/macos_backend.py | 2 + .../backends/playwright_backend.py | 2 + .../openadapt_flow/backends/rdp_backend.py | 2 + .../openadapt_flow/backends/remote_display.py | 2 + .../backends/windows_backend.py | 2 + packages/flow/openadapt_flow/ir.py | 8 ++ .../flow/openadapt_flow/runtime/replayer.py | 35 +++++- .../openadapt_flow/runtime/step_timing.py | 117 ++++++++++++++++++ packages/flow/openadapt_flow/vision/ocr.py | 2 + packages/flow/openadapt_flow/vision/settle.py | 4 + 11 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 packages/flow/openadapt_flow/runtime/step_timing.py diff --git a/packages/flow/openadapt_flow/backends/linux_backend.py b/packages/flow/openadapt_flow/backends/linux_backend.py index bd9eea3f99..fcebd00a72 100644 --- a/packages/flow/openadapt_flow/backends/linux_backend.py +++ b/packages/flow/openadapt_flow/backends/linux_backend.py @@ -46,6 +46,7 @@ StructuralHandle, StructuralLocator, ) +from openadapt_flow.runtime.step_timing import timed_span class LinuxBackendError(RuntimeError): @@ -351,6 +352,7 @@ def viewport(self) -> tuple[int, int]: assert self._viewport is not None return self._viewport + @timed_span("screenshot") def screenshot(self) -> bytes: window = self._resolve_window() if ( diff --git a/packages/flow/openadapt_flow/backends/macos_backend.py b/packages/flow/openadapt_flow/backends/macos_backend.py index 7ff4466f9e..a1f6b67b79 100644 --- a/packages/flow/openadapt_flow/backends/macos_backend.py +++ b/packages/flow/openadapt_flow/backends/macos_backend.py @@ -64,6 +64,7 @@ StructuralHandle, StructuralLocator, ) +from openadapt_flow.runtime.step_timing import timed_span class MacOSBackendError(RemoteDisplayError): @@ -635,6 +636,7 @@ def _window_focus_matches(self, window: WindowInfo) -> bool: and self._mac_client.exact_window_focused_main(window) ) + @timed_span("screenshot") def screenshot(self) -> bytes: if self._require_capture_trust and not self._mac_client.capture_trusted(): raise MacOSBackendError( diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index f5a04df84c..5f0c9bd48c 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -21,6 +21,7 @@ from urllib.parse import urlsplit from openadapt_flow.backends._page_quiescence import PageQuiescence +from openadapt_flow.runtime.step_timing import timed_span if TYPE_CHECKING: # pragma: no cover from playwright.sync_api import Page @@ -2944,6 +2945,7 @@ def _capture_across_navigation(self, options: dict[str, Any]) -> bytes: pass time.sleep(_MASKED_SCREENSHOT_RETRY_SLEEP_S) + @timed_span("screenshot") def screenshot(self) -> bytes: """Return a stable current full-viewport frame as PNG bytes.""" if self._closed_page_frame and self._page_is_closed(): diff --git a/packages/flow/openadapt_flow/backends/rdp_backend.py b/packages/flow/openadapt_flow/backends/rdp_backend.py index 64c9890fad..52f5ffc846 100644 --- a/packages/flow/openadapt_flow/backends/rdp_backend.py +++ b/packages/flow/openadapt_flow/backends/rdp_backend.py @@ -70,6 +70,7 @@ from openadapt_flow.ir import ActionDeliveryReceipt, Point from openadapt_flow.remote_frame_contract import RemoteFrameContract from openadapt_flow.runtime.resolver import visual_resolution_point_fingerprint +from openadapt_flow.runtime.step_timing import timed_span # What a transport may hand back as the current frame: a PIL image, or raw # pixel bytes (RGB or RGBA, row-major) that the backend wraps with the @@ -383,6 +384,7 @@ def viewport(self) -> tuple[int, int]: self._viewport = (int(w), int(h)) return self._viewport + @timed_span("screenshot") def screenshot(self) -> bytes: """Return the current remote frame as PNG bytes. diff --git a/packages/flow/openadapt_flow/backends/remote_display.py b/packages/flow/openadapt_flow/backends/remote_display.py index abdbf6f1dd..abae34e27d 100644 --- a/packages/flow/openadapt_flow/backends/remote_display.py +++ b/packages/flow/openadapt_flow/backends/remote_display.py @@ -76,6 +76,7 @@ from openadapt_flow.ir import ActionDeliveryReceipt from openadapt_flow.remote_frame_contract import RemoteFrameContract from openadapt_flow.runtime.resolver import visual_resolution_point_fingerprint +from openadapt_flow.runtime.step_timing import timed_span _PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n" _LEASE_NONE = 0 @@ -698,6 +699,7 @@ def viewport(self) -> tuple[int, int]: assert self._viewport is not None return self._viewport + @timed_span("screenshot") def screenshot(self) -> bytes: """Return the client window's current pixels as PNG bytes. diff --git a/packages/flow/openadapt_flow/backends/windows_backend.py b/packages/flow/openadapt_flow/backends/windows_backend.py index 9d3aab0c7e..f4a5b97e9b 100644 --- a/packages/flow/openadapt_flow/backends/windows_backend.py +++ b/packages/flow/openadapt_flow/backends/windows_backend.py @@ -51,6 +51,7 @@ StructuralHandle, StructuralLocator, ) +from openadapt_flow.runtime.step_timing import timed_span DEFAULT_SERVER_URL = "http://localhost:5001" @@ -307,6 +308,7 @@ def viewport(self) -> tuple[int, int]: self._viewport = _png_size(self.screenshot()) return self._viewport + @timed_span("screenshot") def screenshot(self) -> bytes: """Return the current frame as PNG bytes (with retries). diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 7ae47ffe36..5eb0572325 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -3639,6 +3639,12 @@ class StepResult(BaseModel): before_png: Optional[str] = None # run-dir-relative paths after_png: Optional[str] = None elapsed_ms: float = 0.0 + #: Where this step's time went, in milliseconds of exclusive time per span + #: (settle_before, screenshot, resolve, identity, ocr, actuate, + #: settle_after, postcondition, readback, artifact_write, other); see + #: ``runtime/step_timing.py``. None when the step was not timed, and then + #: omitted from the serialized result. + timings: Optional[dict[str, float]] = None #: Multi-surface runs only (docs/MULTI_SURFACE.md): the id of the surface #: this step ran on (for a SWITCH_SURFACE step, the surface it made #: active; for a refused switch, the surface the run stayed on). None on @@ -3652,6 +3658,8 @@ def _serialize_compatible(self, handler: Any) -> dict[str, Any]: data: dict[str, Any] = handler(self) if self.surface is None: data.pop("surface", None) + if self.timings is None and isinstance(data, dict): + data.pop("timings", None) if isinstance(data, dict) and not self.effect_field_checks: data.pop("effect_field_checks", None) return data diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 9a526ce95a..5b23bcd821 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -197,6 +197,7 @@ visual_resolution_anchor_contract_sha256, visual_resolution_evaluator_contract_sha256, ) +from openadapt_flow.runtime.step_timing import timed_span from openadapt_flow.runtime.surfaces import SurfaceRouter from openadapt_flow.verification import ( EffectContractMutationError, @@ -6397,6 +6398,7 @@ def _begin_surfaces( return f"{exc}; refusing to run" return None + @timed_span("identity") def _surface_identity_check(self, backend: Any, step: Step) -> Optional[bool]: """Vision-first identity check of one live target for ``step``. @@ -6566,7 +6568,21 @@ def _surface_refusal_result(step: Step, error: str) -> StepResult: error=error, ) - def _run_step( + def _run_step(self, step: Step, **kwargs: Any) -> StepResult: + """Execute one step and record where its time went (``timings``).""" + from openadapt_flow.runtime import step_timing + + timer, previous = step_timing.start_step() + result: Optional[StepResult] = None + try: + result = self._run_step_timed(step, **kwargs) + return result + finally: + step_timing.end_step(previous) + if result is not None: + result.timings = timer.result_ms(result.elapsed_ms or None) + + def _run_step_timed( self, step: Step, *, @@ -8929,6 +8945,7 @@ def _auto_readback_verifier(self) -> Any: self._readback_verifier = verifier return verifier + @timed_span("readback") def _verify_effects( self, step: Step, @@ -9842,6 +9859,7 @@ def _revalidation_frame_capture( ).guarded_keyboard_frame return None + @timed_span("identity") def _revalidate_consequential_actuation( self, step: Step, @@ -10199,6 +10217,7 @@ def _revalidate_consequential_actuation( self._active_delivery_region = fresh_region return fresh_resolution, fresh_region, fresh_png, error + @timed_span("resolve") def _resolve_step( self, step: Step, @@ -10288,6 +10307,7 @@ def _resolve_step( ) return resolution, matched_region, None + @timed_span("resolve") def _resolve_drag_end( self, step: Step, @@ -10323,6 +10343,7 @@ def _resolve_drag_end( ) return resolution, region, error + @timed_span("settle") def _settle_seeded(self) -> bytes: """``vision.wait_settled`` that can extend the last settled run. @@ -11172,6 +11193,7 @@ def _stamp_approval(self, report: RunReport) -> None: needs_person=True, ) + @timed_span("actuate") def _act( self, step: Step, @@ -11232,6 +11254,7 @@ def _arm_dialog(arm: Callable[..., Any], dialog: Any) -> None: else: arm(dialog.response, dialog.message) + @timed_span("postcondition") def _wait_for_dialog(self, take: Callable[..., Any], dialog: Any) -> list[Any]: """Wait briefly for an armed dialog that opens after the input edge.""" @@ -12248,6 +12271,7 @@ def _interstitial_declaration_mutation( ) return None + @timed_span("settle") def _wait_starting_state_settled( self, before_png: bytes ) -> tuple[bytes, Optional[str], Optional[bool]]: @@ -12604,6 +12628,7 @@ def _resettle_after_interstitial( ) return frame, None + @timed_span("postcondition") def _handle_interstitials( self, step: Step, @@ -13544,6 +13569,7 @@ def _compare_direct_signal_text( parameter_names, ) + @timed_span("identity") def _verify_identity( self, step: Step, @@ -14260,6 +14286,7 @@ def _typed_input_landed( ) return landed, changed + @timed_span("postcondition") def _verify_typed_input( self, step: Step, @@ -14470,6 +14497,7 @@ def _verify_typed_input( "a non-input target (focus lost); run aborted" ) + @timed_span("postcondition") def _verify_selected_option( self, step: Step, @@ -14651,6 +14679,7 @@ def _mapped_selection_readback_region( # -- closed-loop scroll ------------------------------------------------------ + @timed_span("resolve") def _implicit_scroll_target_ready( self, step: Step, @@ -14915,6 +14944,7 @@ def readiness_holds(frame_png: bytes) -> bool: f"{target_desc} resolving — target never came into view; run aborted" ) + @timed_span("settle") def _wait_for_scroll_transition_and_settle(self, baseline_png: bytes) -> bytes: """Wait for a delivered scroll to change the frame, then settle it. @@ -15030,6 +15060,7 @@ def _structural_changed( return None return current != start + @timed_span("postcondition") def _check_postconditions( self, step: Step, @@ -15174,6 +15205,7 @@ def _describe_postcondition(pc: Any) -> str: return f"{kind} (vs. the step's start state)" return f"{kind} region={tuple(pc.region) if pc.region else None}" + @timed_span("postcondition") def _poll_postconditions( self, step: Step, @@ -15530,6 +15562,7 @@ def _new_step_evidence_key(run_dir: Path, step_id: str) -> str: return candidate @staticmethod + @timed_span("artifact_write") def _save_step_png(run_dir: Path, step_id: str, suffix: str, png: bytes) -> str: """Save a per-step screenshot; return its run-dir-relative path. diff --git a/packages/flow/openadapt_flow/runtime/step_timing.py b/packages/flow/openadapt_flow/runtime/step_timing.py new file mode 100644 index 0000000000..4dc2a90b0d --- /dev/null +++ b/packages/flow/openadapt_flow/runtime/step_timing.py @@ -0,0 +1,117 @@ +"""Where each step's time goes: per-step spans for ``report.json``. + +Every step result carries ``timings``: milliseconds per span, measured as +exclusive time, so the spans of a step add up to its ``elapsed_ms`` (the rest +is ``other``). The spans are: + +- ``settle_before`` / ``settle_after``: waiting for the screen or page to stop + changing, before and after the action +- ``screenshot``: capturing frames +- ``resolve``: finding the target +- ``identity``: the right-record check +- ``ocr``: text recognition, whichever check asked for it +- ``actuate``: delivering the click, key or text +- ``postcondition``: screen postconditions, input read-back, dialogs +- ``readback``: the system-of-record read-back after a save +- ``artifact_write``: writing evidence files + +Functions opt in with :func:`timed_span`. Timing is per thread, so parallel +worklist workers don't mix their steps, and costs two ``perf_counter`` calls +per span when no step is being timed. +""" + +from __future__ import annotations + +import functools +import threading +import time +from typing import Any, Callable, Optional, TypeVar + +F = TypeVar("F", bound=Callable[..., Any]) + +SPANS = ( + "settle_before", + "settle_after", + "screenshot", + "resolve", + "identity", + "ocr", + "actuate", + "postcondition", + "readback", + "artifact_write", +) + +_local = threading.local() + + +class StepTimer: + """Exclusive time per span for one step on one thread.""" + + def __init__(self) -> None: + self.totals: dict[str, float] = {} + self._stack: list[list[Any]] = [] # [span, start, child_s] + self.acted = False + self._t0 = time.perf_counter() + + def enter(self, span: str) -> None: + if span == "settle": + span = "settle_after" if self.acted else "settle_before" + elif span == "actuate": + self.acted = True + self._stack.append([span, time.perf_counter(), 0.0]) + + def exit(self) -> None: + span, start, child = self._stack.pop() + incl = time.perf_counter() - start + self.totals[span] = self.totals.get(span, 0.0) + (incl - child) + if self._stack: + self._stack[-1][2] += incl + + def result_ms(self, elapsed_ms: Optional[float] = None) -> dict[str, float]: + total_s = ( + elapsed_ms / 1000.0 + if elapsed_ms is not None + else time.perf_counter() - self._t0 + ) + out = {k: round(v * 1000.0, 1) for k, v in self.totals.items() if v > 0} + other = total_s - sum(self.totals.values()) + if other > 0: + out["other"] = round(other * 1000.0, 1) + return out + + +def current() -> Optional[StepTimer]: + return getattr(_local, "timer", None) + + +def start_step() -> tuple[StepTimer, Optional[StepTimer]]: + """Start timing a step; returns the new timer and the one it replaced.""" + previous = getattr(_local, "timer", None) + timer = StepTimer() + _local.timer = timer + return timer, previous + + +def end_step(previous: Optional[StepTimer]) -> None: + _local.timer = previous + + +def timed_span(span: str) -> Callable[[F], F]: + """Count the wrapped call's own time toward ``span`` of the current step.""" + + def deco(fn: F) -> F: + @functools.wraps(fn) + def wrapper(*args: Any, **kwargs: Any) -> Any: + timer = getattr(_local, "timer", None) + if timer is None: + return fn(*args, **kwargs) + timer.enter(span) + try: + return fn(*args, **kwargs) + finally: + timer.exit() + + return wrapper # type: ignore[return-value] + + return deco diff --git a/packages/flow/openadapt_flow/vision/ocr.py b/packages/flow/openadapt_flow/vision/ocr.py index b14934a33f..5a5a3b8bea 100644 --- a/packages/flow/openadapt_flow/vision/ocr.py +++ b/packages/flow/openadapt_flow/vision/ocr.py @@ -20,6 +20,7 @@ from pydantic import BaseModel from openadapt_flow.ir import Region +from openadapt_flow.runtime.step_timing import timed_span from openadapt_flow.vision import imgops from openadapt_flow.vision.match import Match, _clamp_region @@ -101,6 +102,7 @@ def clear_ocr_memo() -> None: _ocr_memo.clear() +@timed_span("ocr") def ocr(screen_png: bytes, *, region: Region | None = None) -> list[OcrLine]: """Run OCR on a screenshot (optionally restricted to a region). diff --git a/packages/flow/openadapt_flow/vision/settle.py b/packages/flow/openadapt_flow/vision/settle.py index 98ec126261..0caab9fdc3 100644 --- a/packages/flow/openadapt_flow/vision/settle.py +++ b/packages/flow/openadapt_flow/vision/settle.py @@ -8,6 +8,7 @@ from typing import Any from openadapt_flow.backend import Backend +from openadapt_flow.runtime.step_timing import timed_span from openadapt_flow.vision.hashing import phash_distance, phash_png logger = logging.getLogger(__name__) @@ -39,6 +40,7 @@ class SettleResult: mode: str = "pixel" +@timed_span("settle") def wait_settled_result( backend: Backend, *, @@ -267,6 +269,7 @@ def _page_settled( time.sleep(PAGE_POLL_S) +@timed_span("settle") def wait_settled_seeded( backend: Backend, seed: SettleSeed | None, @@ -307,6 +310,7 @@ def wait_settled_seeded( return result +@timed_span("settle") def wait_settled( backend: Backend, *, From 477429419c66c910a28e082f32307ccd28052f8f Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 18 Oct 2026 15:38:05 -0400 Subject: [PATCH 08/18] perf(flow): fast path with every check kept: native fill for plain text fields, off-screen scroll probe, one fewer structural round trip, configurable page-quiet window Co-Authored-By: Claude Opus 5.5 --- .../benchmark/openemr_throughput/harness.py | 7 + .../backends/playwright_backend.py | 102 +++++++++++-- packages/flow/openadapt_flow/deployment.py | 10 ++ .../flow/openadapt_flow/runtime/replayer.py | 17 ++- packages/flow/openadapt_flow/vision/settle.py | 12 ++ packages/flow/tests/test_fast_path.py | 143 ++++++++++++++++++ 6 files changed, 275 insertions(+), 16 deletions(-) create mode 100644 packages/flow/tests/test_fast_path.py diff --git a/packages/flow/benchmark/openemr_throughput/harness.py b/packages/flow/benchmark/openemr_throughput/harness.py index 73a0382c5f..a4b5017322 100644 --- a/packages/flow/benchmark/openemr_throughput/harness.py +++ b/packages/flow/benchmark/openemr_throughput/harness.py @@ -242,6 +242,10 @@ def run(a: argparse.Namespace) -> dict[str, Any]: if a.profile and a.workers != 1: raise SystemExit("--profile needs --workers 1") + if a.page_quiet_ms is not None: + from openadapt_flow.vision import settle as settle_mod + + settle_mod.PAGE_QUIET_WINDOW_S = a.page_quiet_ms / 1000.0 os.environ.setdefault("OPENADAPT_FLOW_BROWSER", "bundled") demo = _demo_module() out = Path(a.out) / a.tag @@ -264,6 +268,7 @@ def run(a: argparse.Namespace) -> dict[str, Any]: meta = { "tag": a.tag, "workers": a.workers, "items": a.items, "window": a.window, "halt_at": a.halt_at, "profile": a.profile, "engine_sha": engine_sha, + "page_quiet_ms": a.page_quiet_ms or 100, "engine_dirty": bool(os.popen(f"git -C {FLOW} status --porcelain -- openadapt_flow").read().strip()), "bundle_digest": getattr(workflow.manifest, "content_digest", None), "baseline": digest[:12], "seed_status": seed, "setup_s": round(time.time() - t, 2), @@ -428,6 +433,8 @@ def main() -> None: r.add_argument("--window", type=int, default=0) r.add_argument("--halt-at", type=int, default=None) r.add_argument("--profile", action="store_true") + r.add_argument("--page-quiet-ms", type=int, default=None, + help="page-quiet settle window (runtime.page_quiet_ms); default 100") r.add_argument("--keep-pngs", action="store_true") r.add_argument("--gate-load", type=float, default=16.0) r.add_argument("--gate-max-wait-s", type=float, default=7200.0) diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 5f0c9bd48c..174b1844ed 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -12,6 +12,7 @@ import hmac import json import math +import os import re import time import uuid @@ -570,11 +571,16 @@ class _FramePoint: } return parseFloat(s.fontSize) >= 6; }; + // textContent is the cheap test; rendered text (innerText) also counts, + // because hidden option lists (a date picker's month menu) interleave + // their text into textContent but not into what is shown. + const has = (el) => norm(el.textContent).includes(want) + || norm(el.innerText).includes(want); const visit = (el) => { - if (!norm(el.textContent).includes(want)) return; + if (!has(el)) return; let deeper = false; for (const child of el.children) { - if (norm(child.textContent).includes(want)) { deeper = true; visit(child); } + if (has(child)) { deeper = true; visit(child); } } if (deeper || out.length >= 8) return; if (!norm(el.innerText).includes(want)) return; @@ -593,6 +599,27 @@ class _FramePoint: # At a hit-tested point: is the topmost element (or an ancestor) the one whose # rendered text contains the target? Proves the text is not covered. +# Is the focused element (through same-origin frames) an ordinary text field? +# Ordinary: