07ea7ca1d6
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.
187 lines
8.7 KiB
Markdown
187 lines
8.7 KiB
Markdown
# 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 status` CLI subcommand that prints the
|
|
distinct orphan ST02s + ack count per ST02 + the total orphan count
|
|
+ the count of orphan rows whose ST02 matches a `batches` row vs
|
|
those that don't. Deterministic, exit 0 on success / 1 on DB
|
|
error (matching the `cyclone-cli` skill convention: 1 =
|
|
unexpected exception, including DB-side errors).
|
|
- A `cyclone ack-orphans reconcile` CLI subcommand that creates
|
|
synthetic `batches` rows for each distinct orphan ST02 that does
|
|
NOT already exist in `batches`. Synthetic rows are marked with
|
|
`kind = '837p'`, `input_filename = '<synthetic:orphan-reconcile>'`,
|
|
`totals_json = '{"orphan_reconcile": true, "ack_count": N}'`, and
|
|
`validation_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.db` conftest 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 (the `claims` table has only 2 rows in
|
|
the current DB snapshot, both for ST02 `991102977` which is *not*
|
|
an orphan). Auto-linking is not possible without source data.
|
|
- A UI for the `ack-orphans status` / `reconcile` commands. 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_acks` join logic or the
|
|
`batch_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 by `find_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 sentinel
|
|
`input_filename` is preserved, asserts the `kind = '837p'`
|
|
invariant.
|
|
- `backend/tests/test_ack_orphans_cli.py` — new, tests the CLI
|
|
subcommands via `click.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` — add
|
|
`CycloneStore.find_ack_orphan_st02_summary` + a private
|
|
`_reconcile_orphan_st02` helper. 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 from `find_ack_orphans` into a reusable helper that
|
|
both `find_ack_orphans` and `find_ack_orphan_st02_summary` can
|
|
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. |