Three pr-reviewer followups from the 2026-07-07 review of commit ad14b56:
1. BUG: find_ack_orphans refactor routed 277ca/ta1 through
_ack_control_number which only knew 999 — restored per-kind source
(999 reads raw_json.envelope.control_number, 277ca/ta1 read the ORM
control_number column). Added regression test pinning all three
kinds.
2. DOCSTRING DRIFT: reconcile_orphan_st02s said 'remaining columns
take their defaults' but explicitly passes parsed_at and
transaction_set_control_number — added both to the explicit list
and clarified the rest take schema defaults.
3. WASTED SORT: _iter_orphan_999_st02s yielded sorted() but the
caller re-sorts by (-ack_count, st02) — yielding unsorted now.
Plus three cleanups the reviewer flagged:
* Hoisted 'json' / 'uuid' / 'datetime' imports to module top of
store/__init__.py (replaced in-method imports).
* Added two missing tests: sentinel grep-discoverability via
LIKE '<synthetic:%>' + 999-walk tolerance for non-dict raw_json
(None / list shapes — bytes is unreachable through the ORM).
* Aligned spec/plan exit codes to the cyclone-cli convention
(exit 1 on DB error, not 2 — matches the existing CLI
sys.exit(1) and the cyclone-cli skill documentation).
36/36 SP38 tests pass.
9.3 KiB
Orphan-ack housekeeping Implementation Plan
For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (
- [ ]) syntax for tracking.
Goal: Surface the 804 historical orphan acks to operators via a
RUNBOOK entry + CLI, and provide a one-shot idempotent synthetic-batch
seeder so future acks for those ST02s can resolve against the
batch_envelope_index instead of remaining forever-orphans.
Architecture: Two thin CLI subcommands under cyclone ack-orphans {status,reconcile} backed by a single store helper
find_ack_orphan_st02_summary(). The helper returns a structured
list[dict]; the CLI formats it. Reconcile uses the same SQLAlchemy
session the rest of the store uses, so it inherits the DB-first
invariant (no direct sqlite3 calls, no parallel writers).
Tech Stack: Python 3.11, FastAPI, SQLAlchemy, Click (CLI),
pytest, the existing backend/tests/conftest.py autouse fixtures.
Spec: docs/superpowers/specs/2026-07-07-cyclone-orphan-ack-housekeeping-design.md
File structure
backend/
src/cyclone/
cli.py # + ack-orphans status + reconcile subcommands
store/
__init__.py # + find_ack_orphan_st02_summary() facade method
claim_acks.py # extract _iter_orphan_st02s() helper
tests/
test_ack_orphan_summary.py # NEW: store-helper tests
test_ack_orphans_cli.py # NEW: CLI tests via CliRunner
docs/
RUNBOOK.md # + "Known historical drift" section
Task 1: Extract _iter_orphan_st02s() helper from find_ack_orphans
The current find_ack_orphans(kind) walks the acks table directly
and does its own LEFT JOIN against claim_acks. Both the new store
helper (find_ack_orphan_st02_summary) and the existing
find_ack_orphans need to enumerate the orphan ST02s. Extract the
walk into a private helper in claim_acks.py that returns a flat
list of (st02, ack_count) tuples — the only join key needed for
the summary, and the seed for find_ack_orphans's per-kind filter.
- Read
backend/src/cyclone/store/claim_acks.py:250-330to confirm the current shape offind_ack_orphans. - Add
_iter_orphan_st02s() -> Iterator[tuple[str, int]]toclaim_acks.py. One SELECT againstacksjoiningclaim_ackson(claim_acks.ack_kind = '999' AND claim_acks.ack_id = acks.id), yielding(set_control_number, ack_count)for each group where no claim_acks row exists. For 277ca / ta1 the same shape but with the matching ack table andclaim_acks.ack_kindfilter. - Refactor
find_ack_orphans(kind)to consume the helper. No behavior change; same return shape, same idempotent semantics. All existing tests must still pass.
RED→GREEN test gate: cd backend && .venv/bin/pytest tests/test_api_claim_acks.py tests/test_apply_claim_ack_links.py -v
must remain green.
Task 2: Add find_ack_orphan_st02_summary() to the store facade
- Add
find_ack_orphan_st02_summary(self) -> list[dict]toCycloneStoreinbackend/src/cyclone/store/__init__.py. Returns[{"st02": str, "ack_count": int, "has_batch": bool, "batch_id": str | None}, ...]. has_batchisTrueif a row exists inbatcheswithtransaction_set_control_number = st02.batch_idis that row'sid(orNone).- Sort the result by
ack_count DESCso the heaviest orphans surface first in the CLI output.
RED test first: write
backend/tests/test_ack_orphan_summary.py::test_summary_returns_per_st02_counts
with a seeded DB (two orphan ST02s, one with a batch, one without)
and assert the shape. Watch it fail (no facade method yet), then
implement to GREEN.
Task 3: Test the synthetic-batch seeder
The reconcile CLI inserts synthetic batches rows for any orphan
ST02 that doesn't already have one. This is a write; the store
helper needs a paired write-side method.
- Add
reconcile_orphan_st02s(self, dry_run: bool = False) -> dicttoCycloneStore. Returns{"created": int, "skipped": int, "synthetic_batch_ids": list[str]}. - For each ST02 in the summary where
has_batch is False, insert abatchesrow with:kind = '837p',input_filename = '<synthetic:orphan-reconcile>',totals_json = '{"orphan_reconcile": true, "ack_count": N}',validation_json = '{"orphan_reconcile": true, "note": "sp38 synthetic batch row; source 837 was never ingested into this DB snapshot"}',id = uuid4().hex. The remaining columns take their defaults. - When
dry_run=True, return the plan without writing. - Idempotency: re-running after a previous reconcile returns
created=0, skipped=N. Test this explicitly.
RED test: test_reconcile_is_idempotent — run reconcile twice,
assert the row count is unchanged after the second call.
RED test: test_reconcile_uses_synthetic_input_filename_sentinel
— assert every created row has input_filename = '<synthetic:orphan-reconcile>'.
RED test: test_reconcile_dry_run_does_not_write — pass
dry_run=True, assert created == N in the plan but
batches row count is unchanged.
Task 4: Add cyclone ack-orphans status CLI subcommand
- Open
backend/src/cyclone/cli.py, locate the existingsubmit-batchgroup. - Add an
ack-orphansgroup with astatussubcommand. statuscallscycl_store.find_ack_orphan_st02_summary(), prints a table:ST02 | ACK COUNT | HAS BATCH. Total line at the bottom.- Exit 0 on success, exit 1 on DB error (matching the
cyclone-cliconvention: 1 = unexpected exception, including SQLAlchemy DB errors).
RED test: tests/test_ack_orphans_cli.py::test_status_prints_table
using CliRunner.invoke(["ack-orphans", "status"]). Assert exit
code 0, assert the table contains the seeded ST02s and the total
line.
Task 5: Add cyclone ack-orphans reconcile CLI subcommand
- Same
ack-orphansgroup,reconcilesubcommand. - Accepts
--dry-runflag (default False). - Calls
cycl_store.reconcile_orphan_st02s(dry_run=dry_run), printsCreated: N synthetic batch rows. Skipped: M (already had a batch row).. - Exit 0 on success, exit 1 on DB error (same convention as
status).
RED test: tests/test_ack_orphans_cli.py::test_reconcile_creates_synthetic_batches
— seed two orphan ST02s (one with batch, one without), invoke
reconcile, assert created=1, assert a row with
input_filename='<synthetic:orphan-reconcile>' exists.
RED test: tests/test_ack_orphans_cli.py::test_reconcile_dry_run_flag
— invoke reconcile --dry-run, assert created=1 in plan but
no DB row inserted.
RED test: tests/test_ack_orphans_cli.py::test_reconcile_is_idempotent
— invoke reconcile twice, assert the second invocation reports
created=0.
Task 6: Add RUNBOOK.md entry
- Open
docs/RUNBOOK.md, find the existing "Operator triage" section. - Add a
## Known historical driftsubsection under it. - Content (3-4 paragraphs):
- What the 804 orphans are (real production 999s whose source 837s pre-date the current DB snapshot).
- Why they cannot be auto-linked (the source 837s were not preserved).
- How to triage them via the Inbox > AckOrphansLane.
- Optional:
cyclone ack-orphans statusto see the ST02 breakdown,cyclone ack-orphans reconcileto seed synthetic batch rows for them.
Task 7: Live verification + autoreview + commit
- Run the full backend test suite:
cd backend && .venv/bin/pytest. Confirm pre-existing failure count (20) is unchanged. - Live-run
cyclone ack-orphans statusagainst the current~/.local/share/cyclone/cyclone.db. Confirm output matches the SQLite investigation: 5 distinct ST02s, 805 total orphan rows. - Run autoreview (subagent) on the diff.
- Commit with the SP-N commit prefixes. Per the SP38 spec, the
work is: store-helper extraction (Tasks 1-3), CLI (Tasks 4-5),
docs (Task 6). Three commits, all prefixed
feat(sp38): …. Plus amerge: SP38 orphan-ack housekeeping into maincommit at the end of the review cycle.
Anti-patterns
- Don't add a UI for the CLI commands. Spec is explicit: no UI change.
- Don't auto-run reconcile on boot. Operators invoke it manually after they accept the drift.
- Don't try to backfill claim rows. Source data is gone; no way to fabricate claim rows that would be meaningful.
- Don't put the synthetic sentinel in a constant somewhere. The
string
<synthetic:orphan-reconcile>appears in the spec, the plan, the store helper, the CLI output, and the seeded batches table. Keep it inline so a future grep finds it.
Related skills to load while implementing
cyclone-spec— already loaded (this plan is its output).cyclone-cli— for theclick.testing.CliRunnerpattern and the exit-code convention.cyclone-store— for the facade re-export pattern instore/__init__.py.cyclone-tests— for thetmp_path/test.dbconftest pattern and the BACKFILL_SQL-loads-from-file pattern from SP37 followup #1.