fix(autopilot): offload previous-result reads

[leonlaiyc]

Problem / Motivation

_stage_loop builds a fresh context message before every autopilot stage, and _build_stage_context inlines each earlier stage's result by reading it off disk — synchronously, on the gateway event loop:

async def _stage_loop(...)                     # chat_orchestrator.py:141
    context = _build_stage_context(...)        # :246  — sole production caller
        prev_paths = _previous_result_paths()  # :45
            p.exists() / p.stat()
            p.read_bytes()                     # small results
            open(p,"rb") / read / seek / read  # truncated results

_build_stage_context has exactly one production caller and it is inside the async loop, so every one of those syscalls lands on it.

Why it matters

The read count grows with the plan rather than staying flat: stage N re-reads all N-1 earlier result files, so a five-stage run performs ten file reads spread across its boundaries, each up to four syscalls. Every one of them blocks the single gateway loop — chat streaming, WebSocket frames, cron dispatch — for its duration.

Stated precisely: this is a scheduling defect. I have not measured the stall, so the claim is that unrelated loop work is blocked for the duration of the reads, not that the duration is large.

What changed

The reads move into _read_previous_results, handed to asyncio.to_thread. The boundary keeps live state on the loop:

• loop thread — walk range(1, current_idx + 1), pull each recorded path out of tracker._stage_results, build an immutable (stage_num, path) list
• crosses into the worker — that list, nothing else
• worker — exists, stat, read_bytes / open+seek+read, and the compaction

tracker._stage_results is mutated on the loop by record_stage_result as each stage finishes, so passing the tracker into a worker would have made a concurrent record a cross-thread read for no benefit. A first stage has nothing recorded and returns early without a worker hop at all.

_build_stage_context becomes async and its single caller awaits it. chat.py re-exports both symbols under noqa: F401 and calls neither, so no other production code is affected.

Unchanged: the 2000-byte budget and its 30/70 head/tail split, the sensitive-path refusal, except (OSError, ValueError), which stages are injected, and the emitted text.

Tests

New test/test_previous_result_read_off_loop.py, six tests:

Test	Pins
..._read_runs_off_the_loop_thread	the small-file read_bytes lands off-loop
..._stat_runs_off_the_loop_thread	so does the size probe that picks the branch
test_truncated_read_runs_off_the_loop_thread	so does the large-file open/seek/read
test_tracker_state_is_read_on_the_loop_thread	the offload stops at the filesystem
test_context_preserves_content_and_missing_file_semantics	inlined content, and a missing path degrading to path-only
test_sensitive_path_is_not_read	the refusal still short-circuits before any read

Thread assertions are scoped to this stage's own result file so unrelated filesystem traffic cannot decide them, and each carries a non-empty guard against a vacuous pass. The driver tolerates a sync or async _build_stage_context deliberately: a bare await against the old signature would raise "can't be used in 'await' expression", which proves the symbol changed rather than that a read was mis-scheduled.

The truncation test instruments the builtin open, not Path.open, because that is what the production branch calls — patching Path.open records nothing and reports "never opened" instead of the thread.

• Fail-before on 85cf65b22: 3 failed, 3 passed. All three failures are the real thread assertions; the three preservation tests pass on both sides.
• Pass-after: 6 passed.
• Three existing direct callers in test_dashboard_chat.py updated to await the real functions; no assertion weakened.
• Adjacent test_dashboard_chat.py, which drives _stage_loop end to end: 563 passed, 1 skipped.
• flake8, isort, mypy (chat_orchestrator.py, no issues), docs lint (207 files), brand gate, harness-parity gate, git diff --check: all clean.

Screenshots / video

Backend scheduling only. No component, markup, copy, style or catalog string changed.

Related Issues

None. Self-discovered while scoping #3771 (the sibling stage-result write), and deliberately not filed against #1783 — that umbrella tracker does not list previous-result reads as one of its bullets, so claiming it would misattribute the finding. The reproduction above stands on its own.

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 1250db7d066cf1d678398cbb004776c7d703803b via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 1250db7

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Advisory design-level review of 1250db7d066cf1d678398cbb004776c7d703803b via the fork AI-review pipeline — updated in place on each push; does not block merge.

Design-Verdict: PASS

Correct minimal shape for the named defect: snapshot mutable state on the loop, hand only immutable data to the worker, behavior contracts pinned unchanged.

[DESIGN-REVIEWED] 1250db7

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 1250db7d066cf1d678398cbb004776c7d703803b via the fork AI-review pipeline; updated in place on each push.

Review details

The refactor is a clean async conversion. I've verified:

• Both functions became async; the sole production caller (chat_orchestrator.py:246) now awaits.
• Logic equivalence: if not path_str: continue → if path_str: append is the same filter; the early return "" when recorded is empty matches the original "\n\n".join([]) → "".
• The snapshot happens on the loop thread; only the immutable (stage_num, path) list crosses into the worker — tracker mutation stays on-loop.
• asyncio imported at top; Python ≥3.10 has asyncio.to_thread.
• Re-export in chat.py and both caller sites consistent; unrelated context_management.build_stage_context is a different symbol.

No defect grounds out to (a)/(b)/(c). Nothing survives falsification and nothing new is groundable.

No findings.

[OPUS-REVIEWED] 1250db7

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Advisory premise-level review of 1250db7d066cf1d678398cbb004776c7d703803b via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push; does not block merge.

All evidence gathered — contract read, intent and patch read, base file inspected, siblings counted. Final review:

First-Principles-Verdict: CONCERNS

The boundary reads go off-loop, but _stage_loop still does the same sync stage-result I/O on the loop three lines away — this is a point patch.

What this change ships

Intent: stop per-stage rebuilds of the context message from blocking the gateway event loop with disk reads — a FIX.

1. Previous-result reads at each stage boundary now run on a worker thread — justified (AGENTS.md code-style: "async def for all I/O")
2. First stage skips the worker hop when nothing is recorded — rides along, harmless
3. _build_stage_context / _previous_result_paths become async; sole production caller awaits — part of the fix
4. New helper _read_previous_results — one consumer (_previous_result_paths), not generalized
5. Six new tests pinning read-thread placement and preserved semantics — justified
6. Three existing tests made async — mechanical

Watch

• Unfixed siblings, same root cause (sync stage-result file I/O on the loop, inside the very function this PR edits). Grepped _stage_loop for blocking FS calls: 3 found. safe_read_file(result_path) at chat_orchestrator.py:554 reads all N stage results on the loop at plan completion — identical files, identical grows-with-plan shape, undeclared. KiroCrewConfig.load() at :154 (elsewhere wrapped: server.py:234 uses asyncio.to_thread(KiroCrewConfig.load)). _capture_stage_result's write at :517 is declared and deferred to fix(autopilot): offload stage-result writes #3771 — accepted. The description's "the read count grows with the plan" applies verbatim to line 554; either fold it into this fix or state the deferral the way fix(autopilot): offload stage-result writes #3771's was stated.
• The harm is conceded unmeasured ("I have not measured the stall"); each read is ≤2000 bytes of a file this process wrote minutes earlier (page-cache warm). The documented async-I/O invariant carries the change; the latency framing does not.

[FIRST-PRINCIPLES-REVIEWED] 1250db7

[iamwhatever]

Tier 1 auto-approve: perf (3 files). Criteria: no conflict, no requested changes, no security surface, AI reviewers green. Category: performance — moves previous-result file reads off the main autopilot event loop into a thread pool.