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.
8.7 KiB
Sub-project 38 — Orphan-ack housekeeping: Design Spec
Date: 2026-07-07
Status: Draft, awaiting user sign-off
Branch: sp38-orphan-ack-housekeeping
Aesthetic direction: No new UI
1. Scope
Cyclone's acks table currently holds 805 rows of which 804 are
unresolved "orphans" — 999 acks that reference source 837 batches
whose transaction_set_control_number (ST02) does not appear in any
row of the batches table. Investigation on 2026-07-07 surfaced the
root cause: the orphan rows are real production 999s (sender_id =
COMEDASSISTPROG) whose source 837s were submitted to HPE clearinghouse
by a prior state of the codebase (or an upstream system) before the
current cyclone.db snapshot was created. The source 837s themselves
were never re-ingested into the current DB, so the claims table has
no rows that could link against these acks. The orphan count is a
historical-data artifact, not a forward-looking bug; the SP37 canonical
submit-batch flow already captures ST02 going forward, so the count
stays at 804 and does not grow.
This SP-N captures that situation, surfaces it to operators via a
RUNBOOK entry, and adds a one-shot housekeeping helper so operators
can (a) confirm the orphans are stable and (b) optionally seed
synthetic batch rows for the orphan ST02s so that future acks for
the same ST02s can resolve against the batch_envelope_index instead
of remaining forever-orphans.
In scope:
- A RUNBOOK.md entry under "Known historical drift" describing the
804-orphan root cause, the operator's triage path (Inbox >
AckOrphansLane, already working as of SP37 followup commit
893a662), and the choice to accept the drift. - A
cyclone ack-orphans statusCLI subcommand that prints the distinct orphan ST02s + ack count per ST02 + the total orphan count- the count of orphan rows whose ST02 matches a
batchesrow vs those that don't. Deterministic, exit 0 on success / 1 on DB error (matching thecyclone-cliskill convention: 1 = unexpected exception, including DB-side errors).
- the count of orphan rows whose ST02 matches a
- A
cyclone ack-orphans reconcileCLI subcommand that creates syntheticbatchesrows for each distinct orphan ST02 that does NOT already exist inbatches. Synthetic rows are marked withkind = '837p',input_filename = '<synthetic:orphan-reconcile>',totals_json = '{"orphan_reconcile": true, "ack_count": N}', andvalidation_json = '{"orphan_reconcile": true, "note": "synthetic batch row created by sp38 to allow future acks for ST02 X to resolve via batch_envelope_index; the original 837 source data was never ingested into this DB snapshot"}'. Idempotent: re-running on the same DB does not create duplicate synthetic rows. - A pure read-side helper
cyclone.store.find_ack_orphan_st02_summary()that returns the per-ST02 summary the CLI consumes; the helper is the testable surface and the CLI is a thin wrapper. - Tests for both CLI subcommands + the store helper, using the
existing
tmp_path/test.dbconftest pattern.
Out of scope:
- Re-ingesting the original 837 source files. The source 837s are
not in
ingest/,backend/var/sftp/staging/, or any local path — they were transmitted to HPE and never came back. Cyclone is downstream of the clearinghouse and does not retain copies of outbound 837s after SFTP ACK. - Auto-linking the 804 orphan acks to
claims. There are no claim rows for the orphan ST02s (theclaimstable has only 2 rows in the current DB snapshot, both for ST02991102977which is not an orphan). Auto-linking is not possible without source data. - A UI for the
ack-orphans status/reconcilecommands. The Inbox AckOrphansLane (post-893a662) is the operator's view of the orphans; the CLI is for ad-hoc investigation and the one-shot reconciliation. - Removing or archiving the 804 orphan acks. They are valid audit history (real production traffic was acknowledged by HPE) and must remain queryable.
- Any change to the
claim_acksjoin logic or thebatch_envelope_index. The orphan detection behavior is correct as of SP37; this SP only adds housekeeping around the existing behavior.
2. Decisions (locked during brainstorming)
D1: Accept the historical drift, do not backfill.
The 804 orphans reflect a database snapshot that is younger than the traffic that produced the acks. The source 837s are not recoverable. SP37's canonical submit-batch flow captures ST02 going forward, so the count will stay at 804 + future-test-runs rather than grow indefinitely. The right operator posture is to acknowledge the drift in RUNBOOK.md and surface it via the existing Inbox AckOrphansLane.
D2: Synthetic batch rows are kind = '837p' with a distinct
input_filename sentinel.
This makes them trivially distinguishable from real ingest batches
in queries (e.g. WHERE input_filename LIKE '<synthetic:%>'). The
sentinel is <synthetic:orphan-reconcile> so any future contributor
who sees these rows in the DB can grep the codebase for that string
and find this spec.
D3: Synthetic rows get NO claims, NO claim_acks links.
The reconciliation pass creates batch rows but does not synthesize
claim rows for them. The claims table stays accurate to what was
actually ingested. Future acks referencing these synthetic ST02s
will resolve against the batch envelope index (so the operator can
see "this 999 is for a known orphan source") but will not link to
claims.
D4: Reconcile is idempotent, not auto-runnable.
The CLI does not auto-run on boot, in the SFTP polling scheduler, or via cron. Operators run it manually after they confirm the drift is acceptable. Idempotency means a second run is a no-op rather than an error.
D5: Both CLI commands live under cyclone ack-orphans.
This groups them under a shared verb, matching the existing
cyclone submit-batch, cyclone parse-999, cyclone backup
subcommand shape. The CLI file is backend/src/cyclone/cli.py
(where submit-batch already lives); no new top-level CLI module.
D6: The store helper is the testable surface; the CLI is a thin wrapper.
find_ack_orphan_st02_summary() -> list[dict] is what the tests
target. The CLI parses flags, calls the helper, formats output,
sets the exit code. Mirrors the existing cyclone-cli convention.
3. Open questions
None. The operator has confirmed the design via the brainstorming Q&A on 2026-07-07: docs + housekeeping helper + synthetic-batch migration, no UI change, no auto-runnable reconcile.
4. Test impact
Per cyclone-tests (autouse conftest at backend/tests/conftest.py):
backend/tests/test_ack_orphan_summary.py— new, tests the store helper directly. 999-only (matches the helper's scope; 277ca / ta1 orphans are tracked byfind_ack_orphans(kind)but their "ST02" semantics differ — see §1 — so they are excluded from the per-ST02 summary). Asserts the summary shape, asserts idempotency of the reconcile insert, asserts the sentinelinput_filenameis preserved, asserts thekind = '837p'invariant.backend/tests/test_ack_orphans_cli.py— new, tests the CLI subcommands viaclick.testing.CliRunner(matching the SP37-followup #5 pattern). Asserts exit codes (0 / 1), stdout shape, idempotency of reconcile.- No frontend test impact (no UI change).
- No migration test impact (no schema change; the synthetic batch
rows are inserted via SQLAlchemy at runtime, not via the
migrations/directory).
5. Files expected to change
docs/RUNBOOK.md— append "Known historical drift" section under the existing "Operator triage" section.backend/src/cyclone/store/__init__.py— addCycloneStore.find_ack_orphan_st02_summary+ a private_reconcile_orphan_st02helper. Re-export through the facade so callers don't need to import from the subpackage directly.backend/src/cyclone/store/claim_acks.py— extract the orphan ST02 walk fromfind_ack_orphansinto a reusable helper that bothfind_ack_orphansandfind_ack_orphan_st02_summarycan call. Avoids duplicate SQL.backend/src/cyclone/cli.py— add the two subcommands.backend/tests/test_ack_orphan_summary.py— new, store-helper tests.backend/tests/test_ack_orphans_cli.py— new, CLI tests.docs/superpowers/plans/2026-07-07-cyclone-orphan-ack-housekeeping.md— the implementation plan, written after this spec is signed off.
6. Auth boundary
The auth boundary is HTTP (login required, bcrypt + HttpOnly session
cookie); file-system threats remain the local-only threat model
(SQLCipher at rest, macOS Keychain). The two new CLI subcommands
are operator-invoked only and bypass the HTTP auth boundary by design
(matching all existing cyclone CLI subcommands); they read the
DB directly via db.SessionLocal() and require shell access to the
host running Cyclone. No change to the threat model.