fix(sp38): restore per-kind control_number in find_ack_orphans + review cleanups
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.
This commit is contained in:
@@ -127,8 +127,9 @@ assert the row count is unchanged after the second call.
|
||||
- [ ] `status` calls `cycl_store.find_ack_orphan_st02_summary()`,
|
||||
prints a table: `ST02 | ACK COUNT | HAS BATCH`. Total line at
|
||||
the bottom.
|
||||
- [ ] Exit 0 on success, exit 2 on DB error (matching `cyclone-cli`
|
||||
convention).
|
||||
- [ ] Exit 0 on success, exit 1 on DB error (matching the
|
||||
`cyclone-cli` convention: 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
|
||||
@@ -142,7 +143,8 @@ line.
|
||||
- [ ] Calls `cycl_store.reconcile_orphan_st02s(dry_run=dry_run)`,
|
||||
prints `Created: N synthetic batch rows. Skipped: M (already
|
||||
had a batch row).`.
|
||||
- [ ] Exit 0 on success, exit 2 on DB error.
|
||||
- [ ] 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
|
||||
|
||||
@@ -37,7 +37,9 @@ of remaining forever-orphans.
|
||||
- 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 / 2 on DB error.
|
||||
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
|
||||
@@ -138,14 +140,17 @@ migration, no UI change, no auto-runnable reconcile.
|
||||
Per `cyclone-tests` (autouse conftest at `backend/tests/conftest.py`):
|
||||
|
||||
- `backend/tests/test_ack_orphan_summary.py` — new, tests the store
|
||||
helper directly. Parametrised over kind combinations (999 / 277ca
|
||||
/ ta1), asserts the summary shape, asserts idempotency of the
|
||||
reconcile insert, asserts the sentinel `input_filename` is
|
||||
preserved, asserts the `kind = '837p'` invariant.
|
||||
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, stdout shape,
|
||||
idempotency of reconcile.
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user