tyler/cyclone
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(sp22): backfill plan with 10 as-built deviations

Browse Source 
The implementation shipped with 10 inline bug fixes that weren't reflected
in the original plan document. This commit updates the plan so it
matches the as-built code and adds a summary table near the top so a
future reader of the plan knows what was adjusted and why.

- Bug 1a: AckTimeoutError.__init__ signature widened from int to float
  plus adds self.phase attribute for report remediation hints.
- Bug 1b: wait_for passes timeout_s through without int() round-trip.
- Bug 2: Markdown table assertion updated to match the actual format.
- Bug 3: Added ## Remediation section to write_report_md for soft/hard fails.
- Bug 4: 835 expected-by text now says 'typically the following Monday'.
- Bug 5: Removed bogus HealthSnapshot import in test_pipeline.py.
- Bug 6: All phase tests wrapped in 'async with CyclonePipeline(...) as p:'.
- Bug 7: Playwright + UploadPage imports moved to module level in pipeline.py.
- Bug 8: Added _phase_records_to_results() helper for PhaseRecord→PhaseResult.
- Bug 9: Resume test uses _make_stub factory so stubs append to completed_phases.
- Bug 10: CLI uses context_settings={'allow_interspersed_args': True}.
This commit is contained in:
Tyler
2026-06-21 16:28:07 -06:00
parent 888c99d848
commit ebd2834cc4
1 changed files with 210 additions and 149 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/superpowers/plans/2026-06-21-cyclone-pipeline-agent.md
+98 -37
View File
@@ -12,6 +12,33 @@
--- ---
## As-built deviations
The implementation shipped with 10 deviations from this plan, all fixed
inline during TDD execution. They are catalogued here so a re-run of
the plan (or a reviewer of the implementation) knows what was
adjusted and why.
| # | Bug | Where | Fix |
|---|---|---|---|
| 1a | `AckTimeoutError.__init__(self, ack_kind, timeout_s: int)` — the `int` annotation broke tests asserting `timeout_s == pytest.approx(0.2, abs=0.1)`. | `src/cyclone_pipeline/exceptions.py` | Widened to `timeout_s: float`; added `self.phase = f"wait_{ack_kind}"` so `report.py` can render the right remediation hint. |
| 1b | `wait_for` raised `AckTimeoutError(label, int(timeout_s))` — round-tripping `float → int → float` lost sub-second precision and broke the same `pytest.approx` assertion. | `src/cyclone_pipeline/waiters.py` | Pass `timeout_s` through unchanged. |
| 2 | Markdown table assertion `assert "\| 1. Pre-flight" in text or "\| 1." in text` didn't match the actual table format `\| 1 \| preflight \|`. | `tests/test_report.py::test_markdown_contains_phase_table` | Tightened assertion to `\| 1 \| preflight`. |
| 3 | Markdown report had no "Remediation" section — operators had to read phase detail to figure out next steps when an ACK timed out. | `src/cyclone_pipeline/report.py::write_report_md` | Added a new `## Remediation` section after `expected_835_by` when `report.result` is `soft_fail` or `hard_fail`. Text mentions "in flight" and "Gainwell directly" so it's greppable. |
| 4 | 835 expected-by text didn't say *when* — just "CO Medicaid payment cycle". | `src/cyclone_pipeline/report.py::write_report_md` | Added "typically the following Monday" so operators can plan a check-835 run. |
| 5 | Test imported `HealthSnapshot as _Ignored` from `cyclone_pipeline.exceptions` — it lives in `state.py`. | `tests/test_pipeline.py` | Removed the bogus import. |
| 6 | Plan tests instantiated `CyclonePipeline(cfg, ...)` and called phases directly — but `__aexit__` is required to set `self._client` (and to close the browser on phase 2). | `tests/test_pipeline.py` (all phase tests) | Wrapped every `p = CyclonePipeline(...)` call in `async with CyclonePipeline(...) as p:`. |
| 7 | Plan did `from playwright.async_api import async_playwright` and `from cyclone_pipeline.browser import UploadPage` *inside* `phase2_upload` for "lazy loading" — but tests patch `cyclone_pipeline.pipeline.async_playwright` and `cyclone_pipeline.pipeline.UploadPage` (module-level), so the patches had no effect. | `src/cyclone_pipeline/pipeline.py` | Moved both imports to module level. |
| 8 | Plan had `phases=list(self.run_state.completed_phases)` — `PhaseRecord` (in `state.py`) and `PhaseResult` (in `report.py`) are distinct pydantic v2 models, so passing one where the other is expected fails (no auto-coercion). | `src/cyclone_pipeline/pipeline.py::phase7_scan_and_report` | Added `_phase_records_to_results()` helper that goes through `.model_dump()`; phase 7 now uses it. |
| 9 | Resume test stubbed phases 5/6/7 with `AsyncMock(return_value=PhaseResult(...))` — `resume()` only appends to `completed_phases` via its own logic; the mocks returned a result but didn't mutate state, so `len(report.phases) >= 4` failed (got 3 from the seeded phases 1/2/3). | `tests/test_pipeline.py::test_resume_skips_completed_phases` | Replaced with a `_make_stub(n, name)` factory that appends the result to `p.run_state.completed_phases`. Phase 4 also updated. |
| 10 | CLI `runner.invoke(main, ["status", "--run-dir", ...])` failed with click complaining that `--run-dir` appeared after the subcommand. | `src/cyclone_pipeline/cli.py` | Added `context_settings={"allow_interspersed_args": True}` to the `@click.group()` decorator. |
**Other as-built adjustments (not bugs):**
- Tasks 14–21 were committed as one combined commit `616e3be` instead of 8 separate commits — the plan's per-task commit cadence was dropped in favour of fewer, larger commits once the test-fix-verify cycle accelerated.
- The `uv.lock` file is intentionally untracked. The package is pip-installable (`pip install -e .`) and the lockfile is not required for the dev workflow.
---
## File structure ## File structure
``` ```
@@ -393,9 +420,10 @@ class AckTimeoutError(CyclonePipelineError):
"""An expected inbound ACK did not arrive within the configured """An expected inbound ACK did not arrive within the configured
timeout window. Soft-fail — the submission is in flight.""" timeout window. Soft-fail — the submission is in flight."""
def __init__(self, ack_kind: str, timeout_s: int): def __init__(self, ack_kind: str, timeout_s: float):
self.ack_kind = ack_kind # "ta1" | "999" | "277ca" | "835" self.ack_kind = ack_kind # "ta1" | "999" | "277ca" | "835"
self.timeout_s = timeout_s self.timeout_s = timeout_s
self.phase = f"wait_{ack_kind}"
super().__init__( super().__init__(
f"No {ack_kind} ACK arrived within {timeout_s}s; " f"No {ack_kind} ACK arrived within {timeout_s}s; "
f"submission is in flight — check Gainwell directly." f"submission is in flight — check Gainwell directly."
@@ -1984,7 +2012,7 @@ async def wait_for(
if result is not None: if result is not None:
return result return result
if time.monotonic() >= deadline: if time.monotonic() >= deadline:
raise AckTimeoutError(label, int(timeout_s)) raise AckTimeoutError(label, timeout_s)
await asyncio.sleep(poll_interval_s) await asyncio.sleep(poll_interval_s)
``` ```
@@ -2084,7 +2112,7 @@ def test_markdown_contains_phase_table(sample_report, tmp_path):
assert "# Cyclone pipeline run" in text assert "# Cyclone pipeline run" in text
assert "PASS" in text assert "PASS" in text
assert "Phase summary" in text assert "Phase summary" in text
assert "| 1. Pre-flight" in text or "| 1." in text assert "| 1 | preflight" in text
assert "check-835" in text assert "check-835" in text
assert "Monday" in text assert "Monday" in text
assert "01-preflight.png" in text assert "01-preflight.png" in text
@@ -2186,12 +2214,29 @@ def write_report_md(report: RunReport, path: Path) -> None:
lines.append("") lines.append("")
lines.append( lines.append(
f"- **835 expected by {report.expected_835_by}** " f"- **835 expected by {report.expected_835_by}** "
f"(CO Medicaid payment cycle)." f"(typically the following Monday; CO Medicaid payment cycle)."
) )
cmd = report.next_steps.get("check_835_cmd") cmd = report.next_steps.get("check_835_cmd")
if cmd: if cmd:
lines.append(f" - Re-run: `{cmd}`") lines.append(f" - Re-run: `{cmd}`")
lines.append("") lines.append("")
# Remediation section — only when the run is not a clean pass.
if report.result in ("soft_fail", "hard_fail"):
lines.append("## Remediation")
lines.append("")
timed_out = [p for p in report.phases if p.status == "timeout"]
if timed_out:
names = ", ".join(p.name for p in timed_out)
lines.append(
f"- ACK timeout(s) in: {names}. Submission is in flight "
f"— check Gainwell directly for the inbound 999."
)
if report.result == "hard_fail":
lines.append(
"- Hard failure: review the run logs and any screenshots "
f"in the `screenshots/` directory before retrying."
)
lines.append("")
lines.append("## Artifacts") lines.append("## Artifacts")
lines.append("") lines.append("")
for s in report.screenshots: for s in report.screenshots:
@@ -2433,7 +2478,7 @@ from pathlib import Path
from cyclone_pipeline.pipeline import CyclonePipeline, PipelineConfig from cyclone_pipeline.pipeline import CyclonePipeline, PipelineConfig
from cyclone_pipeline.state import RunState from cyclone_pipeline.state import RunState
from cyclone_pipeline.exceptions import ( from cyclone_pipeline.exceptions import (
BrowserNotReachableError, HealthSnapshot as _Ignored, # noqa BrowserNotReachableError,
) )
@@ -2474,7 +2519,7 @@ async def test_phase1_preflight_passes_when_backend_healthy(tmp_path: Path):
run_dir=tmp_path / "runs", run_dir=tmp_path / "runs",
headless=True, headless=True,
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-001" p._run_id = "test-run-001"
result = await p.phase1_preflight() result = await p.phase1_preflight()
assert result.status == "ok" assert result.status == "ok"
@@ -2511,7 +2556,7 @@ async def test_phase1_preflight_fails_fast_if_browser_down(tmp_path: Path):
run_dir=tmp_path / "runs", run_dir=tmp_path / "runs",
headless=True, headless=True,
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-001" p._run_id = "test-run-001"
with pytest.raises(BrowserNotReachableError): with pytest.raises(BrowserNotReachableError):
await p.phase1_preflight() await p.phase1_preflight()
@@ -2547,12 +2592,14 @@ import uuid
from dataclasses import dataclass, field from dataclasses import dataclass, field
from datetime import datetime, timezone from datetime import datetime, timezone
from pathlib import Path from pathlib import Path
from typing import Any from typing import Any, Sequence
import httpx import httpx
import structlog import structlog
from playwright.async_api import async_playwright
from cyclone_pipeline.api_client import CycloneClient from cyclone_pipeline.api_client import CycloneClient
from cyclone_pipeline.browser import UploadPage
from cyclone_pipeline.exceptions import ( from cyclone_pipeline.exceptions import (
BrowserNotReachableError, CyclonePipelineError, UploadError, BrowserNotReachableError, CyclonePipelineError, UploadError,
ParseError, SubmitError, AckTimeoutError, IdempotencyError, ParseError, SubmitError, AckTimeoutError, IdempotencyError,
@@ -2560,7 +2607,7 @@ from cyclone_pipeline.exceptions import (
from cyclone_pipeline.logging_setup import get_logger from cyclone_pipeline.logging_setup import get_logger
from cyclone_pipeline.report import PhaseResult, RunReport from cyclone_pipeline.report import PhaseResult, RunReport
from cyclone_pipeline.screenshots import ScreenshotWriter from cyclone_pipeline.screenshots import ScreenshotWriter
from cyclone_pipeline.state import RunState from cyclone_pipeline.state import PhaseRecord, RunState
log = get_logger("cyclone_pipeline.pipeline") log = get_logger("cyclone_pipeline.pipeline")
@@ -2706,7 +2753,7 @@ async def test_phase2_upload_retries_on_failure(tmp_path: Path):
run_dir=tmp_path / "runs", run_dir=tmp_path / "runs",
headless=True, headless=True,
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-002" p._run_id = "test-run-002"
fake_page = AsyncMock() fake_page = AsyncMock()
@@ -2763,9 +2810,6 @@ Append to `cyclone_pipeline/pipeline.py`:
last_exc: Exception | None = None last_exc: Exception | None = None
for attempt in range(1, max_attempts + 1): for attempt in range(1, max_attempts + 1):
try: try:
from playwright.async_api import async_playwright
from cyclone_pipeline.browser import UploadPage
async with async_playwright() as pw: async with async_playwright() as pw:
browser = await pw.chromium.launch( browser = await pw.chromium.launch(
headless=self.cfg.headless headless=self.cfg.headless
@@ -2868,7 +2912,7 @@ async def test_phase3_verify_parse_collects_new_claim_ids(tmp_path: Path):
browser_base="http://127.0.0.1:5173", browser_base="http://127.0.0.1:5173",
run_dir=tmp_path / "runs", run_dir=tmp_path / "runs",
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-003" p._run_id = "test-run-003"
result = await p.phase3_verify_parse() result = await p.phase3_verify_parse()
assert result.status == "ok" assert result.status == "ok"
@@ -2887,7 +2931,7 @@ async def test_phase3_verify_parse_fails_on_zero_claims(tmp_path: Path):
browser_base="http://127.0.0.1:5173", browser_base="http://127.0.0.1:5173",
run_dir=tmp_path / "runs", run_dir=tmp_path / "runs",
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-003b" p._run_id = "test-run-003b"
with pytest.raises(ParseError): with pytest.raises(ParseError):
await p.phase3_verify_parse(timeout_s=0.2) await p.phase3_verify_parse(timeout_s=0.2)
@@ -2995,7 +3039,7 @@ async def test_phase4_submit_stores_submission_data(tmp_path: Path):
browser_base="http://127.0.0.1:5173", browser_base="http://127.0.0.1:5173",
run_dir=tmp_path / "runs", run_dir=tmp_path / "runs",
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-004" p._run_id = "test-run-004"
p.run_state.pending_data["claim_ids"] = ["CLM-1", "CLM-2"] p.run_state.pending_data["claim_ids"] = ["CLM-1", "CLM-2"]
result = await p.phase4_submit() result = await p.phase4_submit()
@@ -3019,7 +3063,7 @@ async def test_phase4_submit_propagates_idempotency_error(tmp_path: Path):
browser_base="http://127.0.0.1:5173", browser_base="http://127.0.0.1:5173",
run_dir=tmp_path / "runs", run_dir=tmp_path / "runs",
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-004b" p._run_id = "test-run-004b"
with pytest.raises(IdempotencyError): with pytest.raises(IdempotencyError):
await p.phase4_submit() await p.phase4_submit()
@@ -3128,7 +3172,7 @@ async def test_phase5_wait_ta1_returns_matching_ack(tmp_path: Path):
ta1_timeout_s=2.0, ta1_timeout_s=2.0,
poll_interval_s=0.05, poll_interval_s=0.05,
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-005" p._run_id = "test-run-005"
p.run_state.pending_data["submission_filename"] = ( p.run_state.pending_data["submission_filename"] = (
"11525703-837P-…-1of1.txt" "11525703-837P-…-1of1.txt"
@@ -3152,7 +3196,7 @@ async def test_phase5_wait_ta1_timeout_returns_status_timeout(tmp_path: Path):
ta1_timeout_s=0.2, ta1_timeout_s=0.2,
poll_interval_s=0.05, poll_interval_s=0.05,
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-005b" p._run_id = "test-run-005b"
p.run_state.pending_data["submission_filename"] = "x.txt" p.run_state.pending_data["submission_filename"] = "x.txt"
p.run_state.pending_data["claim_ids"] = ["CLM-1"] p.run_state.pending_data["claim_ids"] = ["CLM-1"]
@@ -3261,7 +3305,7 @@ async def test_phase6_wait_999_returns_matching_ack(tmp_path: Path):
ack_999_timeout_s=2.0, ack_999_timeout_s=2.0,
poll_interval_s=0.05, poll_interval_s=0.05,
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-006" p._run_id = "test-run-006"
p.run_state.pending_data["submission_filename"] = "x.txt" p.run_state.pending_data["submission_filename"] = "x.txt"
p.run_state.pending_data["claim_ids"] = ["CLM-1"] p.run_state.pending_data["claim_ids"] = ["CLM-1"]
@@ -3288,7 +3332,7 @@ async def test_phase6_wait_999_rejected_is_hard_fail(tmp_path: Path):
ack_999_timeout_s=2.0, ack_999_timeout_s=2.0,
poll_interval_s=0.05, poll_interval_s=0.05,
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-006b" p._run_id = "test-run-006b"
p.run_state.pending_data["submission_filename"] = "x.txt" p.run_state.pending_data["submission_filename"] = "x.txt"
p.run_state.pending_data["claim_ids"] = ["CLM-1"] p.run_state.pending_data["claim_ids"] = ["CLM-1"]
@@ -3405,7 +3449,7 @@ async def test_phase7_scan_html_and_write_reports(tmp_path: Path):
browser_base="http://127.0.0.1:5173", browser_base="http://127.0.0.1:5173",
run_dir=tmp_path / "runs", run_dir=tmp_path / "runs",
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "test-run-007" p._run_id = "test-run-007"
# Seed a couple of completed phases to make the report non-empty # Seed a couple of completed phases to make the report non-empty
from cyclone_pipeline.report import PhaseResult from cyclone_pipeline.report import PhaseResult
@@ -3562,7 +3606,9 @@ Append to `cyclone_pipeline/pipeline.py`:
duration_s=sum( duration_s=sum(
p.duration_s for p in self.run_state.completed_phases p.duration_s for p in self.run_state.completed_phases
), ),
phases=list(self.run_state.completed_phases), phases=self._phase_records_to_results(
self.run_state.completed_phases
),
expected_835_by=expected_835, expected_835_by=expected_835,
next_steps={ next_steps={
"check_835_cmd": ( "check_835_cmd": (
@@ -3647,6 +3693,18 @@ Append to `cyclone_pipeline/pipeline.py`:
# --- helpers --------------------------------------------------------------- # --- helpers ---------------------------------------------------------------
def _phase_records_to_results(
records: Sequence[PhaseRecord],
) -> list[PhaseResult]:
"""Convert PhaseRecord (state.py) → PhaseResult (report.py).
Both models have the same fields, but pydantic v2 doesn't
auto-coerce between distinct model classes, so we go via
`model_dump()`. Called when building the final RunReport.
"""
return [PhaseResult(**r.model_dump()) for r in records]
def _next_monday_iso() -> str: def _next_monday_iso() -> str:
"""Return YYYY-MM-DD of the next expected 835 arrival. """Return YYYY-MM-DD of the next expected 835 arrival.
Computed from today in Mountain Time. If today is Monday before Computed from today in Mountain Time. If today is Monday before
@@ -3705,7 +3763,7 @@ async def test_resume_skips_completed_phases(tmp_path: Path):
browser_base="http://127.0.0.1:5173", browser_base="http://127.0.0.1:5173",
run_dir=tmp_path / "runs", run_dir=tmp_path / "runs",
) )
p = CyclonePipeline(cfg, input_path=tmp_path / "x.edi") async with CyclonePipeline(cfg, input_path=tmp_path / "x.edi") as p:
p._run_id = "resume-run-001" p._run_id = "resume-run-001"
run_dir = tmp_path / "runs" / "resume-run-001" run_dir = tmp_path / "runs" / "resume-run-001"
run_dir.mkdir(parents=True, exist_ok=True) run_dir.mkdir(parents=True, exist_ok=True)
@@ -3723,18 +3781,22 @@ async def test_resume_skips_completed_phases(tmp_path: Path):
called = {"phase4": False} called = {"phase4": False}
async def fake_phase4(): async def fake_phase4():
called["phase4"] = True called["phase4"] = True
return PhaseResult(n=4, name="submit", status="ok", duration_s=1.0) result = PhaseResult(n=4, name="submit", status="ok", duration_s=1.0)
p.run_state.completed_phases.append(result)
return result
p.phase4_submit = fake_phase4 # type: ignore[assignment] p.phase4_submit = fake_phase4 # type: ignore[assignment]
# Stub out phases 5, 6, 7 to short-circuit
p.phase5_wait_ta1 = AsyncMock(return_value=PhaseResult( # type: ignore[assignment] def _make_stub(n: int, name: str):
n=5, name="wait_ta1", status="ok", duration_s=0.1 """Stub factory: appends the result to completed_phases so
)) resume() sees them in the final report."""
p.phase6_wait_999 = AsyncMock(return_value=PhaseResult( # type: ignore[assignment] async def stub():
n=6, name="wait_999", status="ok", duration_s=0.1 result = PhaseResult(n=n, name=name, status="ok", duration_s=0.1)
)) p.run_state.completed_phases.append(result)
p.phase7_scan_and_report = AsyncMock(return_value=PhaseResult( # type: ignore[assignment] return result
n=7, name="scan_and_report", status="ok", duration_s=0.1 return stub
)) p.phase5_wait_ta1 = _make_stub(5, "wait_ta1") # type: ignore[assignment]
p.phase6_wait_999 = _make_stub(6, "wait_999") # type: ignore[assignment]
p.phase7_scan_and_report = _make_stub(7, "scan_and_report") # type: ignore[assignment]
# Mock the API responses needed for phases 4-7 # Mock the API responses needed for phases 4-7
respx.post("http://127.0.0.1:8000/api/clearhouse/submit").mock( respx.post("http://127.0.0.1:8000/api/clearhouse/submit").mock(
@@ -3760,7 +3822,6 @@ async def test_resume_skips_completed_phases(tmp_path: Path):
return_value=Response(200, json=[]) return_value=Response(200, json=[])
) )
async with p:
report = await p.resume("resume-run-001") report = await p.resume("resume-run-001")
assert called["phase4"] is True assert called["phase4"] is True
assert len(report.phases) >= 4 assert len(report.phases) >= 4
@@ -4111,7 +4172,7 @@ def _run_dir_default() -> Path:
return Path.cwd() / "runs" return Path.cwd() / "runs"
@click.group() @click.group(context_settings={"allow_interspersed_args": True})
@click.option( @click.option(
"--api-base", default="http://127.0.0.1:8000", "--api-base", default="http://127.0.0.1:8000",
envvar="CYCLONE_API_BASE", envvar="CYCLONE_API_BASE",