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

docs(reviews): capture 2026-06-23 docset review + CSS reduction experiment

Browse Source 
Commit the day's review artifacts so they aren't lost in the working tree:

* 2026-06-23-cyclone-docset-review-A.md — full docset review (Reviewer A:
  requirements + architecture lens; flags the 8-state vs 7-state claim
  lifecycle divergence, FR-20 vs ARCHITECTURE §5.4 tail-endpoint conflict,
  api_routers/ package split status, etc.)
* 2026-06-23-cyclone-docset-review-B.md — code-first counterpart (Reviewer B)
* 2026-06-23-cyclone-groundtruth-audit.md — live-data readiness audit
* 2026-06-23-css-reduction/ — iterative CSS reduction experiment
  (baseline 60,695 B raw / 11,786 B gzip / 9,875 B brotli; 18 prioritized
  candidates; per-iter reports and progress log)

Binary artifacts (PNGs, dist build output) are intentionally not committed —
they're working files for the experiment, not review records.
This commit is contained in:
cyclone
2026-06-23 16:28:15 -06:00
parent 2194d35ea1
commit 616d467c65
13 changed files with 5623 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/reviews/2026-06-23-css-reduction/baseline-report.json
+1322
View File
File diff suppressed because it is too large Load Diff
docs/reviews/2026-06-23-css-reduction/baseline.md
+103
View File
@@ -0,0 +1,103 @@
# CSS Reduction — Baseline (2026-06-23)
## Goal
Reduce `dist/assets/index-*.css` bytes without changing any tested screen.
One declaration/rule at a time, verified by pixel-identical screenshots
and project checks. This file is the baseline the iterative loop is
compared against.
## Build (the size we are trying to shrink)
| metric | bytes |
| ------------ | ------ |
| raw | 60,695 |
| gzip | 11,786 |
| brotli | 9,875 |
`dist/assets/index-BznqBut5.css`, built with `npx vite build` (Vite 5.4.21,
8.9s).
## Test matrix (the screens we will hold pixel-identical)
- **Theme**: dark only (the app is `<html class="dark">` + `theme="dark"`
for the Toaster; no light path exists in the design system).
- **Sizes**: 3 (desktop 1440×900, tablet 768×1024, mobile 375×812).
- **Routes**: 13 (`/`, `/upload`, `/inbox`, `/claims`, `/claims?status=denied`,
`/remittances`, `/providers`, `/reconciliation`, `/acks`, `/batches`,
`/batch-diff`, `/activity`, `/does-not-exist`).
- **Interactive states**: 6 (claim drawer open, remit drawer open, inbox
row focused, search focused, keyboard cheatsheet open, upload dropzone
focused) × 3 sizes = 18 captures.
- **Total**: 13 × 3 + 6 × 3 = **57 screenshots** per build.
Capture source: `audit-uiux-extended.mjs` (new, superset of
`audit-uiux.mjs`). Pixel-diff harness: `tools/css-screenshots-diff.mjs`
using `pixelmatch` (threshold 0).
## Project checks (the gates we hold green)
| check | baseline | used as gate? | reason |
| --------------------------- | -------- | ------------- | --------------------------------------- |
| `npx vite build` | passes | **yes** | produces the CSS bundle under reduction |
| `npx vitest run` | 498 / 501 | **yes** | pre-existing 3 failures, see "Known failures" below |
| `tsc -b` (in `npm run build`)| **fails**| no | 15 type errors in test files + `Upload.tsx`, all pre-existing |
| `npm run typecheck` | fails | no | same as above |
| `npm run lint` | fails | no | `sh: eslint: command not found` (eslint not installed) |
### Known pre-existing failures (not caused by this work)
`vitest run` — 3 failing tests:
- `src/pages/Inbox.test.tsx > Inbox page > SP14: payer-rejected row count rolls up into the need-eyes header`
- `src/store/tail-store.test.ts > useTailStore > test_fifo_cap_evicts_oldest_when_over_10000`
- `src/components/inbox/InboxHeader.test.tsx > InboxHeader > renders the date and counts`
None touch CSS. Each candidate run will compare the new `vitest` exit
status and failure set to the baseline — a candidate is rejected if it
adds a new failure, even if the count stays at 3.
`tsc -b` / `npm run typecheck` — 15 errors in test files and
`src/pages/Upload.tsx`. Not exercised by the reduction loop.
`npm run lint` — `eslint` is not installed in `devDependencies`; the
script `npm run lint` exits 127. Not exercised.
## Interactive-state coverage
Out of 18 interactive captures, 15 succeed and 3 fail (the same 3 every
run):
- `remittances-drawer` × all sizes — the `/remittances` page renders with
no table rows in the current backend state, so the row-click step finds
nothing. The screenshot falls back to the static `/remittances` page
with no drawer; pixel-identical comparison still works for that
fallback, but the remittance-drawer CSS itself is not exercised. This
is captured here as an **untested state** for the final report.
The other 15 interactions (`claims-drawer`, `inbox-row-focused`,
`search-focused`, `cheatsheet-open`, `upload-dropzone-focused` × 3 sizes)
all succeed. They exercise the CSS for the open claim drawer, the
keyboard cheatsheet overlay, the focused search button, the focused
upload dropzone, and the focus ring on inbox rows.
## Pre-existing noise (not regressions, not caused by this work)
From the baseline run (`baseline-report.json`):
- 6/57 flows have ≥1 console error. All from the `inbox` route and the
`inbox-row-focused` state. The 4 interactive ones are the same
source.
- 24/57 flows have ≥1 failed request. Likely the dashboard summary
endpoint (`/api/dashboard/summary`) and a few analytics / SSE probes
in the inbox.
- 0 page errors.
A candidate that does not increase these counts is fine; an increase
is informational, not a gate.
## Self-diff sanity check
`tools/css-screenshots-diff.mjs compare` with `BASE == CAND` returns
`PIXEL-IDENTICAL — 57 compared, 0 differing`. The harness works.
## What is in scope for the iterative loop
- Source: `src/index.css` (custom CSS + Tailwind base/components/utilities)
- Config: `tailwind.config.js`
- Component `.tsx` files (only if removing a Tailwind utility from a
`className` produces a smaller bundle)
## What is out of scope
- `index.html` (Google Fonts preconnect, hard to prove pixel-identical
after font source change)
- The JS bundle (separate goal)
- The backend (`backend/`)
- Any non-CSS build artifact
docs/reviews/2026-06-23-css-reduction/candidates-scan.md
+110
View File
@@ -0,0 +1,110 @@
# CSS Candidates — unused-or-low-value
Generated from `tools/css-candidate-scan.mjs`. 'used' = referenced in any source file. Not used does NOT mean safe to remove — verify with screenshots before keeping a change.
| type | name | used | location |
| ---- | ---- | ---- | -------- |
| css-var | `--background` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--card-foreground` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--popover` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--popover-foreground` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--muted` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--secondary` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--secondary-foreground` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--accent-foreground` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--primary` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--primary-foreground` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--destructive-foreground` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--success-foreground` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--input` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--ring` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--sidebar` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--sidebar-foreground` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--sidebar-accent` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--sidebar-border` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--surface-line` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--surface-line-soft` | **no** | src/index.css :root |
| css-var | `--m-error-bg` | **no** | src/index.css :root |
| css-var | `--radius` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--font-sans` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--font-mono` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--font-display` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--background` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--card-foreground` | **no** | src/index.css :root (referenced inside css/config) |
| css-var | `--muted` | **no** | src/index.css :root (referenced inside css/config) |
| class | `.text-balance` | **no** | src/index.css (components/utilities layer) |
| class | `.text-pretty` | **no** | src/index.css (components/utilities layer) |
| class | `.ring-inset-hairline` | **no** | src/index.css (components/utilities layer) |
| animation | `animate-accordion-down` | **no** | tailwind.config.js animation |
| animation | `animate-accordion-up` | **no** | tailwind.config.js animation |
| animation | `animate-fade-in-soft` | **no** | tailwind.config.js animation |
| animation | `animate-slide-in-right` | **no** | tailwind.config.js animation |
| animation | `animate-ticker-blink` | **no** | tailwind.config.js animation |
| css-var | `--foreground` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--card` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--muted-foreground` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--accent` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--signal` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--destructive` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--success` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--warning` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--warning-foreground` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--border` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--surface` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--surface-ink` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--surface-ink-2` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--surface-ink-3` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--tt-bg` | yes | src/index.css :root |
| css-var | `--tt-bg-elev` | yes | src/index.css :root |
| css-var | `--tt-ink` | yes | src/index.css :root |
| css-var | `--tt-ink-dim` | yes | src/index.css :root |
| css-var | `--tt-amber` | yes | src/index.css :root |
| css-var | `--tt-oxblood` | yes | src/index.css :root |
| css-var | `--tt-ink-blue` | yes | src/index.css :root |
| css-var | `--tt-muted` | yes | src/index.css :root |
| css-var | `--m-surface` | yes | src/index.css :root |
| css-var | `--m-ink-primary` | yes | src/index.css :root |
| css-var | `--m-ink-secondary` | yes | src/index.css :root |
| css-var | `--m-ink-tertiary` | yes | src/index.css :root |
| css-var | `--m-border-heavy` | yes | src/index.css :root |
| css-var | `--m-font-mono` | yes | src/index.css :root |
| css-var | `--m-error` | yes | src/index.css :root |
| css-var | `--m-success` | yes | src/index.css :root |
| css-var | `--m-warning` | yes | src/index.css :root |
| css-var | `--m-accent` | yes | src/index.css :root |
| css-var | `--foreground` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--card` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--muted-foreground` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--border` | yes | src/index.css :root (referenced inside css/config) |
| css-var | `--accent` | yes | src/index.css :root (referenced inside css/config) |
| class | `.num` | yes | src/index.css (components/utilities layer) |
| class | `.display` | yes | src/index.css (components/utilities layer) |
| class | `.mono` | yes | src/index.css (components/utilities layer) |
| class | `.surface` | yes | src/index.css (components/utilities layer) |
| class | `.surface-2` | yes | src/index.css (components/utilities layer) |
| class | `.hairline` | yes | src/index.css (components/utilities layer) |
| class | `.nav-active` | yes | src/index.css (components/utilities layer) |
| class | `.skip-link` | yes | src/index.css (components/utilities layer) |
| class | `.kbd` | yes | src/index.css (components/utilities layer) |
| class | `.eyebrow` | yes | src/index.css (components/utilities layer) |
| class | `.editorial` | yes | src/index.css (components/utilities layer) |
| class | `.row-hover` | yes | src/index.css (components/utilities layer) |
| class | `.animate-scan` | yes | src/index.css (components/utilities layer) |
| class | `.drillable` | yes | src/index.css (components/utilities layer) |
| keyframes | `accordion-down` | yes | tailwind.config.js keyframes |
| keyframes | `accordion-up` | yes | tailwind.config.js keyframes |
| keyframes | `fade-in` | yes | tailwind.config.js keyframes |
| keyframes | `fade-in-soft` | yes | tailwind.config.js keyframes |
| keyframes | `fade-in-up` | yes | tailwind.config.js keyframes |
| keyframes | `slide-in-right` | yes | tailwind.config.js keyframes |
| keyframes | `row-flash` | yes | tailwind.config.js keyframes |
| keyframes | `pulse-dot` | yes | tailwind.config.js keyframes |
| keyframes | `ticker-blink` | yes | tailwind.config.js keyframes |
| animation | `animate-fade-in` | yes | tailwind.config.js animation |
| animation | `animate-fade-in-up` | yes | tailwind.config.js animation |
| animation | `animate-row-flash` | yes | tailwind.config.js animation |
| animation | `animate-pulse-dot` | yes | tailwind.config.js animation |
## Summary
- 36 unused / 100 total
- 64 used / 100 total
docs/reviews/2026-06-23-css-reduction/candidates.jsonl
+1
View File
@@ -0,0 +1 @@
{"id":"01-var-surface-line-soft","priority":"H","label":"drop --surface-line-soft declaration","file":"src/index.css","before":" --surface-line-soft: 30 14% 14% / 0.12;\n","after":""}
docs/reviews/2026-06-23-css-reduction/candidates.md
+42
View File
@@ -0,0 +1,42 @@
# CSS Reduction — Candidate Queue
Atomic removals to try, in priority order. Each is one revertable unit
(edit one file, revert if pixel-identical or project checks regress).
Priority legend: **H** (high confidence, no visible effect expected),
**M** (medium, may affect pixels in edge cases), **L** (low, will likely
revert because it changes visible output).
| # | priority | id | what | where | expected | notes |
|---|----------|----|------|-------|----------|-------|
| 1 | H | `var-surface-line-soft` | drop the `--surface-line-soft` declaration | `src/index.css` line 77 | ~30 B | Only mentioned in a comment in `BarChart.tsx`; no `var(--surface-line-soft)` ref anywhere |
| 2 | H | `var-m-error-bg` | drop the `--m-error-bg` declaration | `src/index.css` line 102 | ~30 B | No `var(--m-error-bg)` ref anywhere in src |
| 3 | H | `kf-fade-in-soft` | drop `fade-in-soft` keyframe + `animate-fade-in-soft` util | `tailwind.config.js` | ~150 B | `animate-fade-in-soft` is not used in any source file |
| 4 | H | `kf-slide-in-right` | drop `slide-in-right` keyframe + animation | `tailwind.config.js` | ~180 B | `animate-slide-in-right` not used |
| 5 | H | `kf-ticker-blink` | drop `ticker-blink` keyframe + animation | `tailwind.config.js` | ~200 B | `animate-ticker-blink` not used |
| 6 | H | `kf-accordion-down` | drop `accordion-down` keyframe + animation | `tailwind.config.js` | ~150 B | `animate-accordion-down` not used; no accordion in app |
| 7 | H | `kf-accordion-up` | drop `accordion-up` keyframe + animation | `tailwind.config.js` | ~150 B | same as above |
| 8 | H | `media-print` | drop the entire `@media print { ... }` block | `src/index.css` lines 402–446 | ~1 KB | Print not in test matrix; the `display: none` rules apply to aside/header — they don't appear in screenshots anyway. **Risk**: a printed detail drawer would no longer be styled for paper, but no print preview is captured. |
| 9 | M | `body-before` | drop the `body::before` radial gradients (the upper-right accent + top-center softbox) | `src/index.css` lines 147–156 | ~400 B | Subtle background. Likely visible on all screenshots. **Will likely revert.** |
| 10 | M | `body-after` | drop the `body::after` hairline grid | `src/index.css` lines 163–176 | ~450 B | Subtle background grid. Likely visible. **Will likely revert.** |
| 11 | M | `scrollbar` | drop `::-webkit-scrollbar*` rules | `src/index.css` lines 179–195 | ~330 B | Scrollbars only visible when content overflows. Most screenshots are above-the-fold; may not trigger. |
| 12 | M | `nav-active-before` | drop the `::before` accent strip on `.nav-active` | `src/index.css` lines 290–300 | ~180 B | Visible on the active sidebar item. |
| 13 | M | `surface-2` | drop the `.surface-2` class entirely | `src/index.css` lines 266–277 | ~250 B | Used in some components — likely a revert. |
| 14 | M | `focus-visible` | drop the global `:focus-visible` ring | `src/index.css` lines 200–204 | ~90 B | All focusable elements would lose the global ring (but Tailwind focus-visible: utilities still apply). May or may not change pixels. |
| 15 | L | `body-bg-image` | drop the body background-image gradients | `src/index.css` lines 136–139 | ~250 B | **Will revert** — visible. |
| 16 | L | `skip-link` | drop the `.skip-link` class | `src/index.css` lines 303–338 | ~700 B | Component exists (`ui/skip-link.tsx`). Removing the class breaks it visually. **Will revert.** |
| 17 | L | `drillable` | drop `.drillable` and its hover | `src/index.css` lines 449–461 | ~220 B | Used in `DrillableCell.tsx`. **Will revert.** |
| 18 | L | `row-hover` | drop the `.row-hover` hover state | `src/index.css` lines 379–384 | ~100 B | Used in tables. Hover state not captured (no mouse). Could be safe. |
## Out of scope (intentionally not tried)
- Tailwind preflight (`*, ::before, ::after`, `::backdrop` with `--tw-*` vars). These power the utility system; removing would break all utilities.
- `@media (prefers-reduced-motion)`. Safety block, removing would change behavior in non-default OS settings.
- Specific custom class definitions that are heavily used (`.num`, `.display`, `.mono`, `.surface`, `.eyebrow`, `.editorial`).
- Any Tailwind utility removal in component `className` props (small wins, lots of churn — would be done in a follow-up pass).
- The `index.html` Google Fonts `<link>` (it's HTML, not CSS, and removing it would change the rendered output).
## Stopping conditions
- Queue exhausted.
- 5 consecutive candidates fail the **size** check (CSS not smaller after the change).
- 2 hours elapsed in Phase 3.
- User requests a stop.
docs/reviews/2026-06-23-css-reduction/iter/01-var-surface-line-soft/_cand/_report.json
+1327
View File
File diff suppressed because it is too large Load Diff
docs/reviews/2026-06-23-css-reduction/iter/01-var-surface-line-soft/_diff/summary.json
+468
View File
@@ -0,0 +1,468 @@
{
"baseDir": "docs/reviews/2026-06-23-css-reduction/iter/01-var-surface-line-soft/_ref",
"candDir": "docs/reviews/2026-06-23-css-reduction/iter/01-var-surface-line-soft/_cand",
"diffOutDir": "docs/reviews/2026-06-23-css-reduction/iter/01-var-surface-line-soft/_diff",
"threshold": 0,
"compared": 57,
"diffCount": 15,
"missingCount": 0,
"extraCount": 0,
"results": [
{
"file": "404--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "404--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "404--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "acks--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "acks--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "acks--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "activity--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "activity--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "activity--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "batch-diff--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "batch-diff--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "batch-diff--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "batches--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "batches--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "batches--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "cheatsheet-open--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 2523,
"totalPixels": 1296000
},
{
"file": "cheatsheet-open--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "cheatsheet-open--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "claims--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "claims--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 1,
"totalPixels": 304500
},
{
"file": "claims--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "claims-denied--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "claims-denied--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "claims-denied--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "claims-drawer--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "claims-drawer--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "claims-drawer--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "dashboard--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 2523,
"totalPixels": 1296000
},
{
"file": "dashboard--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 3,
"totalPixels": 304500
},
{
"file": "dashboard--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "inbox--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 530,
"totalPixels": 1296000
},
{
"file": "inbox--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 142,
"totalPixels": 304500
},
{
"file": "inbox--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 271,
"totalPixels": 786432
},
{
"file": "inbox-row-focused--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 529,
"totalPixels": 1296000
},
{
"file": "inbox-row-focused--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 150,
"totalPixels": 304500
},
{
"file": "inbox-row-focused--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 150,
"totalPixels": 786432
},
{
"file": "providers--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "providers--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "providers--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "reconciliation--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "reconciliation--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "reconciliation--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "remittances--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "remittances--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "remittances--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "remittances-drawer--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "remittances-drawer--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "remittances-drawer--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 372,
"totalPixels": 786432
},
{
"file": "search-focused--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 9684,
"totalPixels": 1296000
},
{
"file": "search-focused--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 47,
"totalPixels": 304500
},
{
"file": "search-focused--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "upload--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 1,
"totalPixels": 1296000
},
{
"file": "upload--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 1,
"totalPixels": 304500
},
{
"file": "upload--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
},
{
"file": "upload-dropzone-focused--desktop.png",
"size": "ok",
"width": 1440,
"height": 900,
"diffPixels": 0,
"totalPixels": 1296000
},
{
"file": "upload-dropzone-focused--mobile.png",
"size": "ok",
"width": 375,
"height": 812,
"diffPixels": 0,
"totalPixels": 304500
},
{
"file": "upload-dropzone-focused--tablet.png",
"size": "ok",
"width": 768,
"height": 1024,
"diffPixels": 0,
"totalPixels": 786432
}
]
}
docs/reviews/2026-06-23-css-reduction/iter/01-var-surface-line-soft/_ref/_report.json
+1322
View File
File diff suppressed because it is too large Load Diff
docs/reviews/2026-06-23-css-reduction/iter/_summary.json
+11
View File
@@ -0,0 +1,11 @@
[
{
"id": "01-var-surface-line-soft",
"priority": "H",
"label": "drop --surface-line-soft declaration",
"file": "src/index.css",
"kept": false,
"diffCount": 15,
"diffMax": 9684
}
]
docs/reviews/2026-06-23-css-reduction/progress.md
+10
View File
@@ -0,0 +1,10 @@
# CSS Reduction — Progress Log
## Baseline
- raw: 60,695 B · gzip: 11,786 B · brotli: 9,875 B
- 57 screenshots captured
- vitest: 498/501 (3 pre-existing failures)
- tsc: 15 pre-existing failures (gate skipped)
- lint: `eslint` not installed (gate skipped)
## Candidates
docs/reviews/2026-06-23-cyclone-docset-review-A.md
+307
View File
@@ -0,0 +1,307 @@
# Cyclone Documentation Review — Reviewer A
## 1. Reviewer identity
**Reviewer A** (Reviewer B is run in parallel; this review covers the full doc set + codebase ground-truth verification).
---
## 2. Executive summary
Cyclone's documentation is unusually thorough for a single-operator tool: a top-level `REQUIREMENTS.md` (38 FRs + 18 NFRs), a top-level `ARCHITECTURE.md`, 27 design specs (20 dated 2026-06-19 to 2026-06-22 + 7 backfilled 2026-06-23 for SP10–SP16), and 27 implementation plans (15 original + 12 backfilled for SP9–SP20). The doc set has the structure of a well-run engineering project, and the prose quality is high — every spec re-asserts the local-only/single-operator/single-payer design contract, every plan has TDD discipline, and the layering (requirements → architecture → spec → plan → tests) is internally consistent at a high level. **However, the doc set has accumulated at least 25 specific contradictions, ambiguous forks, untested artifacts, and stale numbers** between the two top-level docs alone — and the contradictions are concentrated in the most consequential areas: the live-tail HTTP surface, the claim-state machine, and the LOC/version counters used in the verification matrix. The 7-state claim lifecycle is wrong in 5 places; the codebase has an 8-state enum (`backend/src/cyclone/db.py:180-188`). The tail endpoint URL is `/api/{resource}/stream` per REQUIREMENTS FR-20 + the live-tail spec, but `/api/tail?since=…` per `ARCHITECTURE.md` §5.4 — two competent engineers reading only the docs could ship two incompatible APIs. Below, the 4 reviewer questions and 4 flag sections detail every issue with file:line citations.
---
## 3. Q1: Components
### Description
Cyclone is composed of six concentric layers: (1) the SQLite/SQLCipher store at the bottom (`backend/src/cyclone/db.py`, `backend/src/cyclone/db_crypto.py`, migrations 0001–0012), (2) ORM models and pure-function business logic (`db.py`, `reconcile.py`, `scoring.py`, `npi.py`, `cas_codes.py`, `inbox_lanes.py`, `inbox_state.py`, `inbox_state_277ca.py`, `audit_log.py`), (3) the per-parser modules under `parsers/` (16 files including the 270/271/277CA/835/999/TA1/837P parsers and serializers), (4) the persistence facade `store.py` (CycloneStore) plus per-feature handlers (`scheduler.py`, `backup_service.py`, `backup_scheduler.py`, `logging_config.py`, `security.py`, `pubsub.py`, `clearhouse/__init__.py`, `providers.py`, `payers.py`), (5) the FastAPI surface — a monolithic `api.py` (~3,548 LOC per ARCHITECTURE §3.1 / 3,145 per REQUIREMENTS) plus a partial `api_routers/` package (`acks.py`, `admin.py`, `health.py`, `ta1_acks.py`) that owns the new admin/health endpoints, and (6) the React + Vite + TypeScript frontend (`src/`) with TanStack Query v5, Zustand, Radix UI primitives, and shadcn-style components.
The top-level docs (`REQUIREMENTS.md` §2.1, `ARCHITECTURE.md` §3) describe these layers at the same level of granularity and agree on the boundaries. The cross-SP doc set reinforces them: each spec names the modules it extends (e.g. SP13 `clearhouse/__init__.py:319`, SP14 `inbox_lanes.compute_lanes`, SP15 `db_crypto + db + api`, SP16 `scheduler.py:720`). The ASCII topology diagram in `ARCHITECTURE.md` §1.2 is the canonical picture.
### Consistency verdict
**Mostly consistent, with three meaningful gaps:**
1. The `api_routers/` package is partially implemented (`acks.py`, `admin.py`, `health.py`, `ta1_acks.py`) but documented only as "planned" in `ARCHITECTURE.md` §3.2 and not mentioned at all in `REQUIREMENTS.md`; the specs still reference monolithic `cyclone.api` (e.g. SP15 §3.3 says `cyclone/api.py:2797-2928` for the rotation endpoint, but the actual code lives at `api_routers/admin.py`). Engineers building "what does Cyclone's API look like" from the spec set will get the monolithic picture; engineers reading the codebase will see a partially split package.
2. `backup.py` (file upload/retention module) and `backup_service.py` vs `backup_scheduler.py` (periodic scheduler) are described as a single "SP17" subsystem across the docs, but the spec file (`2026-06-21-cyclone-encrypted-backup-design.md`) is the *original* SP17 spec while the implementation plan is dated `2026-06-23-cyclone-sp17-encrypted-backup.md`. The two documents describe the same work but the round-3 doc-prep added the plan but did NOT add a backfilled spec for SP17. So the SP17 docs are pre/back mixed, not a clean backfill like SP10–SP16 (which got both spec and plan).
3. The `workflow/` subdirectory visible under `backend/src/cyclone/` has only `__pycache__/` entries on disk (`workflow/__pycache__/__init__.cpython-313.pyc`, `workflow/__pycache__/batch_diff.cpython-313.pyc`, `workflow/__pycache__/inbox_lanes.cpython-313.pyc`, `workflow/__pycache__/inbox_state.cpython-313.pyc`, `workflow/__pycache__/reconcile.cpython-313.pyc`, `workflow/__pycache__/scoring.cpython-313.pyc`) — but no `.py` source files. This indicates `workflow/` was a refactoring target (likely SP21 store-split) whose sources were removed/moved but whose compiled artifacts were not cleaned. **None of the docs mention `workflow/` as a deprecated stub**; engineers reading the docs will not understand why the directory exists in compiled form only.
**Verdict: medium confidence in component consistency.** The high-level decomposition is consistent across the top-level docs and the spec set. The drift is in the partial-API-router split, the SP17 plan/spec asymmetry, and the unexplained `workflow/` ghost directory.
---
## 4. Q2: Data model
### Description
The data model is a SQLAlchemy 2.0 ORM (`backend/src/cyclone/db.py`) backed by SQLite, optionally encrypted via SQLCipher (`backend/src/cyclone/db_crypto.py` — SP12). Migration runner is `db_migrate.py` reading `PRAGMA user_version` — **12 migrations on disk**: `0001_initial` through `0012_backups` (listed at `backend/src/cyclone/migrations/`). The core tables are: `batches`, `claims`, `remittances`, `cas_adjustments`, `matches`, `activity_events`, `service_lines`, `service_line_payments`, `line_reconciliations`, `acks`, `ta1_acks`, `providers`, `payers`, `clearhouse`, `payer_rejected` columns on `claims` (SP10, migration 0008), `payer_rejected_acknowledged_at`/`_actor` (SP14, migration 0010), `audit_log` (SP11, migration 0009), `processed_inbound_files` (SP16, migration 0011), and `backups` (SP17, migration 0012).
The key entity relationships are: `Batch 1—N Claim`, `Batch 1—N Remittance`, `Remittance N—1 Claim` (nullable FK on `claims.matched_remittance_id`), `Remittance 1—N CasAdjustment`, `Claim 1—1 Match` (unique), `Claim 1—N ServiceLine`, `Remittance 1—N ServiceLinePayment`, `Claim 1—N LineReconciliation`, `Remittance 1—N LineReconciliation`, and `AuditLog` as a chain on its own (no FKs; the chain links by `prev_hash`). `ServiceLinePayment` is the SP7 bridge between the 837 and 835 line worlds; it carries the `procedure_code`/`service_date`/`charge`/`payment`/`units` plus `modifiers_json` as a serialized list. The `audit_log` table (SP11) is a separate hash-chained append-only log keyed by SHA-256 of `(event_type, entity_type, entity_id, actor, created_at, prev_hash)`; the chain is verifiable via `cyclone.audit_log.verify_chain(session)` (verified at `backend/src/cyclone/audit_log.py:190`) which recomputes every hash and walks the `prev_hash` links (`audit_log.py:228-242`).
The state machine has multiple representations in the docs:
- **`REQUIREMENTS.md` §3.1 + §6.3 (traceability)** says: "7-state claim lifecycle: submitted, received, paid, partial, denied, reconciled, reversed."
- **`backend/src/cyclone/db.py:180-188` (code)** says: `ClaimState` enum with **8 values**: `SUBMITTED, RECEIVED, REJECTED, PAID, PARTIAL, DENIED, RECONCILED, REVERSED`. The 8th state, `REJECTED`, was added by the workflow-automation/SP6 plan (`docs/superpowers/plans/2026-06-20-cyclone-workflow-automation.md` Task 1) when 999 AK9 R/E rejection handling was introduced.
- **`ARCHITECTURE.md` §6.3** says: "states: submitted, accepted, paid, denied, appealed, rejected, payer_rejected" — 7 states, including `accepted` (which is NOT in the code) and `payer_rejected` (which is NOT a state in the code but a set of columns on `claims` filtered on by `inbox_lanes.compute_lanes`).
- **`docs/superpowers/specs/2026-06-19-cyclone-db-reconciliation-design.md` §6.7** (pre-SP6) says: "submitted, received, paid, partial, denied, reconciled, reversed" — 7 states, matches the pre-SP6 plan.
The `payer_rejected` status is *not* a `ClaimState` value. It is stored as four columns on `claims`: `payer_rejected_at`, `payer_rejected_reason`, `payer_rejected_status_code`, `payer_rejected_by_277ca_id` (verified at `backend/src/cyclone/db.py:270-280`; SP10 spec §2.1). The 5th inbox lane (`payer_rejected`) is computed by `inbox_lanes.compute_lanes` filtering on `payer_rejected_at IS NOT NULL AND payer_rejected_acknowledged_at IS NULL` (SP14 spec §3.2). So `payer_rejected` is a *view*, not a *state* — yet `ARCHITECTURE.md` lists it as a state.
The `Match` model carries an `is_reversal` flag (defined in the SP2 plan) and the spec calls for a `state_before_reversal` column on `claims` to preserve reversals (`db_reconciliation-design.md` §6.6). The 835 reverse-direction (CLP02 ∈ {21, 22}) is handled by `apply_reversal()` in `reconcile.py` which restores the prior state. **However, the SP10 277CA path does NOT extend the state machine** — claims with a payer rejection stay in their prior state (`paid`, `denied`, `partial`, etc.) and gain the `payer_rejected_at` timestamp; the 5-lane inbox surfaces them but `claim.state` does not transition to a new value. This is consistent with the SP10 spec §2.3 ("we explicitly do not clear `payer_rejected_at` on acknowledge") and SP14 spec §2.3, but contradicts the more casual phrasing in `ARCHITECTURE.md` §6.3.
### Consistency verdict
**Low confidence in data-model consistency across the doc set.** The `ClaimState` enum has 8 values (verified at `backend/src/cyclone/db.py:180-188`) but the docs claim 7 in 4 places (`REQUIREMENTS.md` §3.1 and §6.3, `ARCHITECTURE.md` §6.3, the db-reconciliation spec §6.7, and the workflow-automation plan Task 1 implicitly). The 5th `payer_rejected` "state" in `ARCHITECTURE.md` §6.3 is not a state in the code — it is a column-and-filter view. Engineers building the data model from the docs alone will produce either an 8-state enum without a `payer_rejected` value (correct) or a 9-state enum with both `rejected` and `payer_rejected` (wrong, not in the code).
The second meaningful gap is the dual audit mechanism. `activity_events` (db.py original migration 0001) is a regular table for *transition events*; `audit_log` (SP11, migration 0009) is a separate hash-chained table for *HIPAA-relevant events*. The docs are inconsistent on which events go where: the SP14 spec §3.3 says `claim.payer_rejected_acknowledged` is appended to `audit_log`, but the SP6 workflow plan implies `claim.rejected` (the 999 R/E event) lands in `activity_events`. The two tables overlap in name space but have different writers — `audit_log.py:99-126`'s `append_event` takes an `AuditEvent` payload but `activity_events` is written from a different code path (`store.py`). No doc explicitly enumerates which `kind` strings go to which table.
---
## 5. Q3: Dependencies
### Description
There are four overlapping dependency graphs in this doc set: (a) spec-level dependencies ("depends on SP X"), (b) plan-level task dependencies (TDD phases 1 → 2 → 3 with `git worktree` rebases), (c) module/import dependencies in the codebase, and (d) data-model migration ordering. The graphs mostly agree; the divergences are concentrated in SP21/SP22 (in-flight) and SP23 (candidate).
**(a) Spec dependencies.** The 7 backfilled specs (SP10–SP16) each declare their upstreams explicitly:
- SP10 (277CA) `2026-06-23-cyclone-sp10-277ca-payer-rejected-design.md` §"Depends on:" → SP3 (999 parser) + SP6 (workflow/inbox).
- SP11 (audit log) → SP6 + SP10 + SP12.
- SP12 (SQLCipher) → standalone.
- SP13 (SFTP) → SP9 (SftpClient stub + secrets).
- SP14 (5-lane inbox) → SP6 + SP10 + SP11.
- SP15 (key rotation) → SP11 + SP12 (cannot ship without SP12; `2026-06-23-cyclone-sp15-key-rotation-design.md` line 11: "Cannot ship without SP12").
- SP16 (MFT polling) → SP9 + SP11 + SP13 + SP15 (implicitly, since SFTP auth uses the Keychain) + the per-parser modules.
These declarations are consistent with the implementation order in the git log (SP10 → SP11 → SP12 → SP13 → SP14 → SP15 → SP16) and with the migration versions (0008 through 0011).
**(b) Plan-level task dependencies.** The plans use a `[ ] Step N → commit` pattern with explicit prerequisites (e.g. `live-tail.md` Task 0 requires SP5 to be merged first; `claims-unique-constraint-and-409-ux.md` worktree setup requires `pip install -e backend`). The plan-level dependencies are clean within each plan; the cross-plan dependencies are weak. For example, the SP14 plan does not declare it depends on SP10's migration `0008_payer_rejected_columns.sql` — it just references "the SP10 `payer_rejected_*` columns" without naming the migration.
**(c) Module dependencies.** The import graph in `backend/src/cyclone/` shows the expected layering: `parsers/` modules import `cyclone.exceptions` and `cyclone.segments` only (clean isolation); `db.py` imports nothing from `cyclone.*` (forward references via string IDs); `reconcile.py`, `scoring.py`, `inbox_lanes.py` import `cyclone.db`; `store.py` imports everything (`db`, `reconcile`, `scoring`, `inbox_state`, `inbox_state_277ca`, `pubsub`); `api.py` imports `store`, `db`, `audit_log`, `scheduler`, `db_crypto`, `security`, etc. The `scheduler.py` module *intentionally duplicates* small helpers (`_ack_count_summary`, `_ack_synthetic_source_batch_id`, `_277ca_synthetic_source_batch_id`) to break what would otherwise be a circular dependency with `cyclone.api` — this is documented in SP16 spec §3.5. The `audit_log.py:99-126` `append_event` function takes a session and an `AuditEvent`, called by SP14 ack endpoint, SP15 rotation endpoint, and SP16 scheduler handlers. The store-scheduler boundary is clean.
**(d) Migration ordering.** 12 migrations on disk (`0001_initial` through `0012_backups`) are forward-only via `PRAGMA user_version`. The runner (`db_migrate.py`) reads `-- version: N` headers, sorts by filename, applies any with `version > current` (`db_migrate.py:438-466`). The version assertions in `test_db_migrate.py` confirm monotonic ordering. The `claims-unique-constraint-and-409-ux.md` plan adds migration `0013_drop_claims_unique_constraint.sql` on a worktree (`claims-unique-fix`); this migration does NOT exist on `main` yet, which is consistent with the plan being in-flight. **However, the `2026-06-23-cyclone-sp11-hash-chained-audit-design.md` spec refers to migration `0009_audit_log.sql` and the docs say SP11 ships migration 0009 — that migration IS on disk and `audit_log.py` exists, confirming SP11 shipped.**
**(e) SP21 / SP22 / SP23 status.** SP21 (store split) is in-flight on `refactor/store-split` branch (`docs/superpowers/specs/2026-06-21-cyclone-store-split-design.md` Status: "Draft / not merged"); SP22 (parse-decide) lives on `claims-unique-fix` worktree (`docs/superpowers/specs/2026-06-21-cyclone-parse-decide-workflow-design.md` + the unique-constraint plan). SP23 (Ubuntu Docker + RBAC) is a candidate only (`docs/superpowers/specs/2026-06-22-cyclone-ubuntu-docker-deployment-design.md` Status: "Draft / candidate"). These are called out in the spec titles, but the top-level docs (`REQUIREMENTS.md` §1.4, `ARCHITECTURE.md` §0.1) do NOT explicitly mark SP21/SP22/SP23 as in-flight. An engineer reading only the top-level docs would assume all 23 SPs are at the same ship-readiness level.
### Consistency verdict
**High confidence within the shipped SPs (SP1–SP20)**, where the spec→plan→code→migration→test graph is internally consistent. **Low confidence for SP21/SP22/SP23**, which are in-flight or candidate and not marked as such in the top-level docs. The spec-level "Depends on" declarations are accurate and actionable.
---
## 6. Q4: Definition of done
### Description
`REQUIREMENTS.md` §8 (lines 643–681) defines "Definition of Done" in three sub-sections: §8.1 *Functional DoD* (every FR covered by ≥1 spec + ≥1 plan + ≥1 integration test), §8.2 *Non-Functional DoD* (every NFR has a verification command), §8.3 *Operational DoD* (deployment posture, backup readiness, encryption posture). Each FR has a row in the §6.1 traceability matrix pointing at the covering spec/plan and a test file. `ARCHITECTURE.md` §10 ("Operational Story") describes the deploy / backup / runbook posture in narrative form.
The §8.1 Functional DoD says every FR is "Met" if it has a covering spec, plan, and integration test. The §6.1 traceability matrix marks FR-1 through FR-38 with a single test file each (e.g. FR-20 → `test_api_stream_live.py` + `TailStatusPill.test.tsx`). The matrix does NOT explicitly require happy-path AND failure-path test coverage — but `REQUIREMENTS.md` §4 NFR-2 ("Test discipline: every route has a happy-path test and at least one failure-path test") adds this requirement at the NFR layer. So the union of FR + NFR DoD is: each route has a happy + failure path test.
The DoD is *partially testable*: the test-file pointers in the traceability matrix are verifiable by `ls backend/tests/` and `ls src/**/*.test.ts*`; the happy/failure-path test count is verifiable by reading each test file. The encryption-at-rest posture (NFR-5 / SP12) is verifiable by checking that `cyclone.db_crypto.is_encryption_enabled()` returns `True` when `sqlcipher3` is installed and the Keychain entry exists (per the SP12 spec §1). The single-process posture (NFR-14) is verifiable by reading the scheduler module's lack of `threading.Thread` / `multiprocessing.Process` callers. The audit-chain integrity (NFR-4 / SP11) is verifiable by `GET /api/admin/audit-log/verify` returning `ok: true, checked: N+1`.
### Testability verdict
**Medium confidence.** Four issues break testability:
1. The `REQUIREMENTS.md` §8.3 Operational DoD mentions "Vitest ^4.1.9" and flags it in §5.3 / R-25 as "unusually new — verify the version pin is intentional before the next npm install; fallback to `^2.x` if accidental." The `ARCHITECTURE.md` §3.2 (tooling table) lists `vitest 4.1.9` without flagging it. **The flagging is inconsistent — one top-level doc treats it as a problem to fix, the other treats it as normal.** A CI gate that asserts "vitest@^2.x" would fail on the codebase.
2. The §8.3 DoD claims `vitest@^4.1.9` is "unusually new (current stable line is 1.x/2.x)" — but Vitest 4.x *does exist* (released after Vitest 3.x in 2025); the claim is also inaccurate relative to the doc's own date. The §6.3 acceptance row for SP5 says the live-tail test count target was "~144 frontend pass" with "~437 backend" — but `backend/tests/` contains 94 test files (verified by directory listing). The number is in the right order of magnitude but unverified.
3. The §10 Operational Story in `ARCHITECTURE.md` describes "5-second heartbeat" on the live-tail stream — but `REQUIREMENTS.md` §4 NFR-10 (line 163) says "Heartbeat every 15s default (`CYCLONE_TAIL_HEARTBEAT_S`)". The actual `backend/src/cyclone/api.py:1679-1680` comment says `CYCLONE_TAIL_HEARTBEAT_S` (consistent with REQUIREMENTS, not ARCHITECTURE). **The 5s heartbeat claim is wrong against both the codebase AND REQUIREMENTS.md.** The operational story in §10 cannot be the basis for a deploy runbook until this is corrected.
4. The §10 Operational Story does not describe the SP16 scheduler (`CYCLONE_SCHEDULER_AUTOSTART` opt-in, `CYCLONE_SCHEDULER_POLL_SECONDS` default 60) at all — only the manual operator path. The §8.3 DoD also does not mention "scheduler running without `CYCLONE_SCHEDULER_AUTOSTART=true`" as a verified-state precondition. The "auto-poll" deploy posture introduced by SP16 is invisible to the top-level DoD.
**Verdict:** The DoD is *partially* testable. The functional DoD (each FR → test file) is verifiable by file enumeration. The non-functional DoD (each NFR → verification command) is mostly verifiable but the Vitest version flag, the heartbeat interval, and the scheduler autostart posture create 3 places where a competent engineer reading only the top-level docs would either install the wrong version or deploy the wrong polling config.
---
## 7. Contradictions
The following are direct contradictions in the doc set. Each cites file:line for both sides.
1. **`api.py` LOC count.** `REQUIREMENTS.md` §3.2 / R-23 says `api.py` is **3,145 LOC**. `ARCHITECTURE.md` §3.1 says `cyclone.api:app` is **3,548 LOC** with `cyclone.store` at **2,423 LOC**. `REQUIREMENTS.md` R-24 says `store.py` is **2,172 LOC**. Three different numbers for two files.
2. **`store.py` LOC count.** Same as above: 2,172 (REQUIREMENTS R-24) vs 2,423 (ARCHITECTURE §3.1).
3. **Live-tail heartbeat interval.** `REQUIREMENTS.md` §4 NFR-10 (line 163): "Heartbeat every 15s default (`CYCLONE_TAIL_HEARTBEAT_S`)". `ARCHITECTURE.md` §5.4: "5s heartbeat" (in the description of the live-tail stream). Code (`backend/src/cyclone/api.py:1679-1680`): "every `CYCLONE_TAIL_HEARTBEAT_S` seconds when idle". The `live-tail` plan (§7 / Phase 3 Task 11) says "the 15s heartbeat via `asyncio.wait_for` loop (overridable for tests)". **The 5s claim is wrong; the code and NFR-10 agree on 15s.**
4. **Live-tail endpoint URL.** `REQUIREMENTS.md` §3.1 FR-20 (line 128) says `/api/{claims,remittances,activity}/stream`. `ARCHITECTURE.md` §5.4 says `GET /api/tail?since=…`. The `live-tail` spec (`2026-06-20-cyclone-live-tail-design.md`) and plan (`2026-06-20-cyclone-live-tail.md`) use the per-resource URL pattern. The code (`api.py:1679-1680`) matches the per-resource pattern. **The ARCHITECTURE §5.4 single-endpoint claim is wrong.**
5. **Claim lifecycle state count.** `REQUIREMENTS.md` §3.1 + §6.3 traceability: "7-state claim lifecycle". `ARCHITECTURE.md` §6.3: "7 states: submitted, accepted, paid, denied, appealed, rejected, payer_rejected". Code (`backend/src/cyclone/db.py:180-188`): **8 states** (SUBMITTED, RECEIVED, REJECTED, PAID, PARTIAL, DENIED, RECONCILED, REVERSED). The docs disagree with themselves and with the code.
6. **Claim lifecycle state names.** `ARCHITECTURE.md` §6.3 lists `accepted` and `appealed` — neither exists in the code or any spec. The code uses `RECEIVED` (not `accepted`) and has no `appealed` state. The lifecycle spec says reversal preserves the prior state via `state_before_reversal`; there is no appeal flow.
7. **`payer_rejected` as a state.** `ARCHITECTURE.md` §6.3 lists it as a state. The SP10 spec §2.3 and SP14 spec §2.3 explicitly state `payer_rejected_at` is a timestamp column on `claims`, NOT a `ClaimState` value. The lane filter (SP14 spec §3.2) is `payer_rejected_at IS NOT NULL AND payer_rejected_acknowledged_at IS NULL`. **A lane is not a state.**
8. **Vitest version flagging.** `REQUIREMENTS.md` §5.3 R-25 (line 652) flags `vitest@^4.1.9` as "unusually new" and a fallback to `^2.x` is suggested. `ARCHITECTURE.md` §3.2 lists `vitest 4.1.9` in the tooling table without any flagging. Engineers reading only the architecture doc will not realize the version is under review.
9. **Plan ID numbering.** The user's plan inventory says "15 original plans + 12 backfilled plans = 27 plans". The on-disk count matches (15 + 12). But the original plans use a SP-numbering scheme ("Sub-project 4", "Sub-project 6", "Sub-project 7") while the backfilled plans use a `sp9`–`sp20` naming scheme. There is no `sp1`–`sp8` backfill because those SPs were the originals. **Engineers searching for "SP7 plan" find `2026-06-20-cyclone-line-reconciliation.md` (Sub-project 7) — works — but searching for "SP8 plan" finds `2026-06-20-cyclone-serialize-837.md` whose own title says "Sub-project 7" (the file was re-purposed from SP7 to SP8).** Both top-level docs note this self-acknowledged mis-labeling (`REQUIREMENTS.md` §1.4 + R-22 + `ARCHITECTURE.md` §0.1 mention it) but do not fix it on disk.
10. **SP number ↔ filename mapping.** SP9 = `multi-payer-npi-sftp` (spec is "SP9 — Multi-payer NPI + SFTP"). But the user's plan inventory says "12 backfilled plans (dated 2026-06-23-cyclone-sp{9..20}-*.md)" — SP9 is the *first* backfilled. The original spec for SP9 lives at `2026-06-20-cyclone-multi-payer-npi-sftp-design.md` (not in the backfill range). So SP9 has an original spec + a backfilled plan + a backfilled spec for SP13-SFTP. The mapping is "SP9 = NPI + SFTP" but the "SFTP" name also gets re-used for SP13's `SftpClient` paramiko wire-up. **Two SPs share the SFTP concept; SP9 = config/secrets, SP13 = transport. The docs do not always disambiguate.**
11. **SP17 spec/plan asymmetry.** SP17 (encrypted backup) has a spec file dated `2026-06-21` (the original) but the plan is dated `2026-06-23-cyclone-sp17-encrypted-backup.md` (backfilled). The other 6 backfilled SPs (SP10–SP16) have both spec and plan backfilled; SP17 has only the plan backfilled. The user said "12 backfilled plans for SP9-SP20" — so SP17 falls in that range — but the spec is original. This is an asymmetry in the doc-prep pass.
12. **`audit_log` event-type vocabulary.** The SP14 spec §3.3 enumerates `claim.payer_rejected_acknowledged` as the audit event. The SP15 spec §3.4 enumerates `db.key_rotated`. The SP16 spec §3.5 enumerates `claim.rejected` (from 999 `_handle_999`) and `claim.payer_rejected` (from 277CA `_handle_277ca`). The completeness review (2026-06-20 §3.1 item 2) calls out `ActivityEvent` as "not tamper-evident" — but the SP11 audit log adds tamper-evidence only for events that *opt in* to `append_event`. The completeness review and the round-3 docs do not reconcile which `kind` strings go to `activity_events` (mutable) vs `audit_log` (hash-chained).
13. **Live-tail reconnect backoff cap.** `REQUIREMENTS.md` §4 NFR-10 and FR-21: "backoff ladder `1s → 2s → 4s → 8s → 16s → 30s` capped". The `live-tail` spec §3.4 says the same ladder. The plan (`live-tail.md` Task 19) repeats it. **No contradiction here, but** the spec/plan/test all say "stalled at 30s silence" while the plan also says "the hook should NOT auto-reconnect on stall (user confirms)" — the FR-21 backoff ladder applies only to `reconnecting`, not `stalled`. A naive reader of FR-21 could think the stalled state auto-reconnects.
14. **`store.add(rec, event_bus=…)` parameter.** The `production-readiness.md` plan Task 4 wires `store.add()` with `event_bus=request.app.state.event_bus`. The `claims-unique-constraint-and-409-ux.md` plan Task 1.3 calls `store.add(rec, event_bus=request.app.state.event_bus)` from the 837/835 endpoints. **But** the original `store.py:226` `InMemoryStore.add()` and the `CycloneStore.add()` impl do not have an `event_bus=` parameter — the call sites pass `event_bus=` to a function that does not accept it. This is *historical drift* (the InMemoryStore predates the EventBus in SP5) but the docs do not call out the refactor.
15. **Migrations total count.** `REQUIREMENTS.md` §5.2 lists migrations `0001` through `0012` (12 total) but the §6.3 acceptance rows reference `0013_drop_claims_unique_constraint` (which is on the `claims-unique-fix` worktree, NOT on `main`). Engineers running `ls backend/src/cyclone/migrations/` on `main` see 12 files; engineers on the worktree see 13. The doc text is ambiguous about which branch it describes.
16. **Test count target.** The historical `2026-06-20-completeness-review.md` §1 says **574 backend tests**. The `production-readiness.md` plan §"Test count targets" (Phase 2 final target) says **437 backend**. The `live-tail.md` plan §"Test count targets" says **437 backend + 144 frontend**. The user's question-statement says **964 tests**. **Four different numbers for "how many tests does Cyclone have".** The discrepancy is partly due to date (574 was measured in 2026-06-20; 437 was a target for SP5; 964 is current).
17. **API router location for the rotation endpoint.** `2026-06-23-cyclone-sp15-key-rotation-design.md` §3.3 says the endpoint lives at `cyclone/api.py:2797-2928`. The actual endpoint lives at `backend/src/cyclone/api_routers/admin.py` (the `api_routers/` split happened after SP15 was written). The line range is stale.
18. **Encrypted-at-rest posture.** `REQUIREMENTS.md` §4 NFR-5: "SQLite is encrypted via SQLCipher AES-256 when the macOS Keychain entry exists and `sqlcipher3` is installed; plain SQLite otherwise." `ARCHITECTURE.md` §10 Operational Story: "the SQLite file at `~/.local/share/cyclone/cyclone.db` is plaintext by default; SQLCipher optional." The two phrasings agree on substance but the operational story makes the "plaintext by default" stance more prominent than the requirements doc.
19. **API endpoint for the MFT scheduler status.** The SP16 spec §4.1 says `GET /api/admin/scheduler/status` returns a `SchedulerStatus` JSON. The SP16 spec §3.3 says the `GET /api/health` endpoint also reports `snap.scheduler.running`. **Two endpoints expose scheduler state** with different shapes. The DoD does not say which is canonical.
20. **Audit-event `actor` semantics.** SP14 spec §3.3 says `actor` defaults to `"operator"` and is operator-supplied via request body. SP15 spec §3.4 says the same. The SP23 candidate (`ubuntu-docker-deployment-design.md`) introduces RBAC and `actor` becomes a real identity from a JWT claim. The two phrasings agree on default behavior but differ on whether `actor` is free-form text (today) vs identity-validated (SP23 future). Engineers building an audit-evidence tool today must treat `actor` as untrusted free-form text.
21. **`/api/clearhouse/submit` parameter shape.** SP13 spec §4 says no new endpoints, but references the SP9 endpoint `POST /api/clearhouse/submit`. The SP9 spec is the original spec (`2026-06-20-cyclone-multi-payer-npi-sftp-design.md`). The two docs disagree on the request shape: SP9 spec §4 has `body: { sftp_block_name, payload }`; SP13 spec §4 says SP9 endpoint body is "unchanged" but adds an `auth` dict to `SftpBlock` model. The exact request schema is documented in SP9 only; SP13 trusts it. Two readers building SP13 from spec-only could ship two different request bodies.
22. **`processed_inbound_files` table ↔ scheduler hand-off.** The SP16 spec §3.5 says `_handle_999` and `_handle_277ca` write SP11 audit events. The plan for SP14 says `claim.payer_rejected_acknowledged` is an audit event. **Both audit events end up in the same `audit_log` table** but no doc names a "row kind namespace" — an operator querying `SELECT event_type, COUNT(*) FROM audit_log GROUP BY event_type` would need to enumerate every spec's event types to build a vocabulary.
23. **Reconciliation failure semantics.** `REQUIREMENTS.md` §4 NFR-2 says reconciliation must be "fail-soft — a 999 parser crash does not lose the 835". The db-reconciliation spec §6.6 says the same. The completeness review §4 item 4 says "ActivityEvent is mutable". **Both can be true**, but the reconciliation "fail-soft" event goes to `activity_events` (mutable) and the SP16 "scheduler per-file try/except" event goes to `processed_inbound_files.error_message` (also not in `audit_log`). An auditor searching the hash-chained log for a reconciliation failure will find nothing for SP5-era crashes.
24. **Store ownership of `parse_inbound_filename`.** The SP16 spec §3.5 says `_handle_999` etc. use `cyclone.edi.filenames.parse_inbound_filename` (a separate module under `cyclone/edi/filenames.py`). The completeness review (pre-SP16) doesn't mention this module. The SP16 spec was backfilled; the upstream SP9 spec for `SftpClient` does not mention `edi/filenames.py` either. The module exists in the codebase (`backend/src/cyclone/edi/filenames.py`) and is covered by `test_filenames.py` — but no spec describes *why* it lives in `edi/` instead of `parsers/`.
25. **`claim.rejected` audit event idempotency.** SP16 spec §3.5 says `_handle_999` writes one `claim.rejected` audit event per matched claim. The SP6 workflow plan (`workflow-automation.md` Task 3) says `apply_999_rejections` is idempotent on already-rejected claims. **The interaction is: a 999 file applied twice (e.g. a retry) writes 0 new audit events on the second call (because `claim.state == REJECTED` short-circuits in `apply_999_rejections`).** The SP16 spec does not say "scheduler retries are idempotent on the audit chain" — only on the parser. A future scheduler retry-aware design might emit a `claim.rejected_reapplied` audit event, but no doc says so.
---
## 8. Forks (highest-priority first)
These are the items where two competent engineers reading only the docs would plausibly build different systems.
**F1 (P0). Tail endpoint URL.** FR-20 says `/api/{resource}/stream`; ARCHITECTURE §5.4 says `/api/tail?since=…`. Two URLs, two shapes, two different filter mechanisms. The codebase matches FR-20. **An engineer reading only ARCHITECTURE.md would build a single-endpoint `/api/tail?since=…` that does not exist.**
**F2 (P0). Claim lifecycle state names.** REQUIREMENTS says 7 states (no `REJECTED`, has `received`); ARCHITECTURE says 7 states (with `accepted`, `appealed`, `rejected`, `payer_rejected`); code has 8 states (`SUBMITTED, RECEIVED, REJECTED, PAID, PARTIAL, DENIED, RECONCILED, REVERSED`); `ARCHITECTURE` adds `payer_rejected` as a state but the code does not. **An engineer building from REQUIREMENTS would ship a 7-state enum without `REJECTED` and break the SP6 workflow-automation `apply_999_rejections` path; an engineer building from ARCHITECTURE would ship a 7-state enum with `appealed` and `payer_rejected` and break the codebase.**
**F3 (P0). Heartbeat interval.** 5s vs 15s. The code is 15s. An engineer deploying based on ARCHITECTURE §5.4 would set `CYCLONE_TAIL_HEARTBEAT_S=5` (or not set it and expect 5s) and double the bandwidth on the live-tail stream. An operator monitoring for "stalled" would flip state 5× as often.
**F4 (P0). 409 collision response shape.** The SP22 `parse-decide-workflow-design.md` describes the 409 body for a CLM01 collision as `{parse_result, collisions: [{colliding_claim_ids, existing_batch_id}]}` — a richer shape that lets the UI surface "you have N existing batches that already hold this CLM01". The `claims-unique-constraint-and-409-ux.md` plan + spec describe the 409 body as `{error, detail, batch_id, existing_batch_id}` — a simpler shape that only carries the first matching batch. **The two specs describe different 409 contracts.** A frontend engineer building from SP22 will render a richer panel; one building from the claims-unique-constraint plan will render a simpler panel.
**F5 (P1). `?ack=true` response shape on parse-837.** REQUIREMENTS §3.1 FR-9 says `?ack=true` "auto-generates a 999 acknowledgement". The `edi-features.md` plan Task 15 says "persist `Ack` row, attach to response body" — but the response shape is described as "the 999 is attached inline". The live-tail spec does not re-acknowledge. **Three different response shapes are plausible: (a) `{claims, summary, ack: {ack_id, body}}` with the 999 inline, (b) `{claims, summary, ack_id}` requiring a follow-up GET, (c) `{claims, summary, ack_999_x12: "ISA*..."}` with raw text. The plan's test (`test_api_parse_persists.py` ack=true happy path) checks the response, but no doc publishes the canonical shape.**
**F6 (P1). `payer_rejected` lane ↔ `rejected` lane ordering.** SP14 spec §1 says the 5 lanes are ordered "envelope problems → payer problems → recon opportunities → claims in flight → done today". SP14 spec §3.7 code shows the order `rejected, payer_rejected, candidates, unmatched, done_today`. The SPEC.md says the same. The summary tile grid is `grid-cols-2 sm:grid-cols-3 lg:grid-cols-5`. **No conflict in the docs, but** if a future engineer "tidies" the lane order to alphabetical, the "Need eyes" counter (which sums `rejected + payer_rejected + candidates + unmatched` per SP14 spec §3.8) would still work; the visual hierarchy would change.
**F7 (P1). `ActivityEvent` vs `audit_log` event namespace.** The codebase has two parallel event tables. The SP11 spec says `audit_log` is for HIPAA-relevant events (`claim.rejected`, `claim.payer_rejected_acknowledged`, `db.key_rotated`). The pre-SP11 design (still in `store.py`) writes `activity_events` for state transitions (`claim_submitted`, `manual_match`, `manual_unmatch`, `reconcile`, etc.). **No doc publishes a complete list of which event types go to which table.** A future engineer adding a new event (e.g. `claim.appealed`) would have to choose which table to use with no explicit guidance. *Note: SP23 candidate mentions "appeals workflow" without specifying the table.*
**F8 (P1). `store.add(rec, event_bus=…)` signature.** The live-tail plan (SP5) and the unique-constraint plan (SP22) both call `store.add(rec, event_bus=request.app.state.event_bus)`. The pre-SP5 `InMemoryStore.add()` does not accept `event_bus=`; the post-SP5 `CycloneStore.add()` signature is unclear from the docs. **A new engineer refactoring `store.py` (which SP21 plans to do) could break both call sites.** The plan-level dependency on SP5's `event_bus` is implicit in 7+ places; the SP21 spec does not list this as a constraint.
**F9 (P1). `api.py` → `api_routers/` split.** The codebase has `api_routers/{acks,admin,health,ta1_acks}.py`. The specs all reference `cyclone.api:app`. **An engineer building from the docs would add new endpoints to `api.py`; an engineer reading the codebase would add to `api_routers/`.** The split is undocumented in the top-level docs.
**F10 (P1). Live-tail stream filter.** FR-20 says the stream is per-resource. The `live-tail.md` plan Task 11 shows the endpoint signature accepting `status, provider_npi, payer, date_from, date_to, sort, order, limit` — i.e. the stream is a *filterable* snapshot+subscription, not a firehose. **Two engineers reading the docs would build: (a) firehose stream that the frontend filters (matches the FR-20 phrasing); (b) filtered stream matching `listClaims` parameters (matches the plan Task 11 signature).** The actual code matches (b).
**F11 (P2). `processed_inbound_files` row identity.** SP16 spec §2.2 says the unique index is `(sftp_block_name, name)`. The plan test `test_persist_835_creates_one_service_line_payment_per_svc_composite` builds a `SftpBlock` and a `ClaimPayment` with one `name`. **If a single inbound file has two `999` envelopes concatenated (the HCPF pattern is one file per inbound type), the scheduler classifies via `parse_inbound_filename` and stores one `processed_inbound_files` row per inbound type — but the spec does not say whether the same inbound MFT path could have two acks for two different control numbers.** Edge case; probably out of scope; but undocumented.
**F12 (P2). `audit_log` clock source.** SP11 spec §3 says `created_at` is "the wall-clock at the time of `append_event`". The SP16 spec says the scheduler "stagger by 1s on startup so a multi-operator restart doesn't hammer the MFT server". If a 999 arrives at wall-clock `T`, the `claim.rejected` audit event has `created_at = T`. If the operator's wall-clock drifts 2s from the server's, the chain still works (it's hash-chained by SHA-256, not timestamp-ordered). **No doc says what "created_at" means — wall-clock at the operator host, or monotonic at the scheduler.** This matters for HIPAA §164.312(b) audit retention (6 years) where operators may need to prove the audit chain was written *before* the corresponding DB write.
**F13 (P2). `claim.id` source.** SP10 spec §2 says `claim.id` is the X12 CLM01. SP14 spec §2 says the lane filter is on `payer_rejected_at` (not `claim.id`). The uniqueness-constraint spec (SP22) says `claim.id = CLM01` AND that 837P allows many `CLM*` segments per 2000B subscriber loop. **If a 837P file has two CLM01 collisions (e.g. two CLM01="CLM-A" segments in different 2000B loops), the `PK on claims.id` raises IntegrityError on the second insert.** SP22 migration 0013 drops the inline UNIQUE on `(batch_id, patient_control_number)` to allow this, but the PK remains on `claims.id`. **Two CLM01="CLM-A" segments still trip the PK.** The 409 handler returns `existing_batch_id` only if the prior batch holds the colliding CLM01 — but if the second collision is *within the same file*, the second insert raises before any prior batch is consulted. The 409 body in this case is `{error, detail, batch_id}` *without* `existing_batch_id`. SP22 spec does not document this edge case.
**F14 (P2). SFTP host-key policy.** SP13 spec §3.2 uses `AutoAddPolicy()` (trust-on-first-use). The plan does not change this. The spec §7 "Open questions" lists `SftpBlock.strict_host_key_check: bool` as a future hardening. **An engineer hardening security to `RejectPolicy` today would break first-connect for any new operator.** No doc says "do not change this without a migration path".
**F15 (P2). `processed_inbound_files` status `pending`.** SP16 spec §2.3 says `pending` is "reserved for the retry-on-next-tick semantics" but is "currently unused in shipping code". The plan does not add a test for the `pending` status. **A future engineer implementing retry semantics would need to know the schema already supports `pending`**; the spec calls this out but the plan does not.
**F16 (P2). Scheduler first-tick stagger.** SP16 spec §3.1.2 says `_run` staggers the first tick by 1s on startup. The plan test does not assert the stagger (only that the scheduler "starts"). **A future engineer removing the `asyncio.sleep(1)` for "simplicity" would re-introduce the thundering-herd problem without test coverage catching it.**
**F17 (P3). `_handle_277ca` vs `_handle_999` line numbers.** SP16 spec §3.1.1 says both handlers emit `claim.rejected` / `claim.payer_rejected` audit events. The `_handle_277ca` path is also called for filename `277` (not just `277CA`) — same parser, different filename pattern. **A future engineer renaming the handler would have to update both the spec and the plan.**
**F18 (P3). `serialize_837_for_resubmit` parameter shape.** SP8 plan Task 5 says `serialize_837_for_resubmit(claim, interchange_index=42)` assigns `f"{interchange_index:09d}"`. The spec says the same. But the spec §3.1 originally proposed "hybrid" approach; the plan Task 0 amends to "Approach A full rebuild". **An engineer reading only the spec would write a hybrid serializer; one reading the plan would write a full rebuild.**
**F19 (P3). SP22 parse-decide spec vs plan.** The SP22 parse-decide spec §3.2 says the 409 endpoint returns `parse_result` and `collisions` arrays. The SP22 plan does not exist (only the claims-unique-constraint plan exists for SP22). **Engineers building from the spec alone have no test plan.** The spec mentions a "TBD: 409 panel UX" in §6.
**F20 (P3). Inbox lane ordering by `state_changed_at`.** SP6 workflow-automation plan Task 6 `compute_lanes` uses `Claim.state_changed_at >= cutoff` for the `done_today` lane (cutoff = 24h ago). The migration `0004_rejections_and_state_history.sql` adds `state_changed_at` and the index `ix_claims_state_changed_at`. **The migration adds the column WITHOUT a default value** — so existing claims (from migrations 0001–0003) have `state_changed_at = NULL`. **They never appear in `done_today`.** An operator who upgrades from pre-SP6 to SP6 would see their historical claims never land in `done_today`. SP6 spec does not say "backfill `state_changed_at` on existing claims"; the plan does not test for this.
**F21 (P3). The SP10 277CA path does not extend the state machine.** A claim with a payer rejection stays in its prior state (`paid`, `denied`, `partial`, etc.) and gains the `payer_rejected_at` timestamp. This is consistent with SP10 spec §2.3 ("we explicitly do not clear `payer_rejected_at` on acknowledge") and SP14 spec §2.3, but the architecture doc's casual phrasing ("states: ... payer_rejected") suggests otherwise. **An engineer building the data model from the architecture doc will add a `payer_rejected` ClaimState value that breaks the lane query.**
---
## 9. Untested artifacts
The following are claims in the docs that are not (verifiably) backed by tests in `backend/tests/` or `src/**/*.test.ts*`. I name the test file that *should* cover each.
**U1.** `REQUIREMENTS.md` §4 NFR-2: "every route has a happy-path test and at least one failure-path test". **This NFR is a meta-claim about test discipline.** No meta-test asserts NFR-2 (e.g., a `pytest --collect-only | jq '.[] | select(.failure_path == false)'` would fail). The convention is followed in some places (e.g. `test_api_rotate_key.py` has happy + 409 + 503 paths; `test_payer_rejected_acknowledge.py` has 6 tests covering happy / idempotent / missing / not-rejected / empty / audit) but not enforced.
**U2.** `REQUIREMENTS.md` §4 NFR-10: "backoff ladder 1s → 2s → 4s → 8s → 16s → 30s capped". The `live-tail.md` plan Task 19 enumerates 6 status states (`connecting / live / reconnecting / stalled / error / closed`) but the test plan (`test_useTailStream.test.ts`) covers only 4 (connecting→live, error, abort/closed, reconnecting→connecting→live). **The backoff ladder cap (30s) is not asserted.** A future engineer could ship backoff that climbs past 30s without test failure.
**U3.** `REQUIREMENTS.md` §4 NFR-9: "thread-affinity under FastAPI: every SQLCipher connection opens on the calling thread via `poolclass=NullPool`". `db_crypto.py` is the only module that wires `NullPool`. **No test in `test_db_crypto.py` asserts `poolclass == NullPool` is set on the SQLCipher branch.** An engineer refactoring `_make_engine` to switch to `QueuePool` (SQLAlchemy default) would break thread-affinity without test coverage catching it.
**U4.** SP15 spec §3.3 says the rotation endpoint has status codes `200, 400, 409, 503`. `test_api_rotate_key.py` covers happy + 409 + 503. **The 400 ("encryption not enabled") path is not in the test file.** An engineer removing the `is_encryption_enabled()` check would not break tests.
**U5.** SP14 spec §3.3 says `POST /api/inbox/payer-rejected/acknowledge` returns 400 when `claim_ids` is "missing, empty, or contains non-string elements". `test_payer_rejected_acknowledge.py` (per SP14 spec §8) has 6 tests: happy path; idempotent re-ack; skip non-payer-rejected; missing-id count; **400 on empty list**; audit event written + chain intact. **The 400-on-non-string-elements path is not tested.**
**U6.** SP16 spec §3.5 says `_handle_ta1` "writes no audit event — TA1s are envelope-level interchange acks and are not HIPAA-relevant". **No test asserts that `_handle_ta1` writes zero audit rows.** A future engineer adding a "ta1.received" audit event would not break tests.
**U7.** SP16 spec §7.4 says "list_inbound() runs on a thread via asyncio.to_thread so the event loop stays responsive". **No test asserts that `SftpClient.list_inbound()` does not block the event loop** (e.g. a `test_list_inbound_runs_off_loop` would need to schedule a CPU-bound task on the loop and verify it ran concurrently with `list_inbound`).
**U8.** SP12 spec §7 says "Plain-SQLite fallback remains silent. `is_encryption_enabled()` returns False when `sqlcipher3` isn't installed or the Keychain entry is missing." `test_db_crypto.py` covers the case where encryption IS enabled. **The "sqlcipher3 not installed" branch is not tested** (the test environment always has `sqlcipher3` because the dev deps install it).
**U9.** SP13 spec §3.2 says `_connect` raises `RuntimeError("SftpBlock.auth must contain either 'password_keychain_account' or 'key_file'")` when both keys are absent. **No test in `test_sftp_paramiko.py` covers this exact error message** (the plan Task 19 enumerates 5 tests: connect happy, key-file happy, missing-password RuntimeError, network error, AutoAddPolicy — but the "both auth keys absent" case is not explicit).
**U10.** SP11 spec §3.4 says `append_event` re-raises if the hash computation fails (which is impossible in practice because SHA-256 doesn't fail, but the doc claims the function is "fail-fast"). **No test asserts the fail-fast behavior under any failure mode.**
**U11.** SP14 spec §3.8 says "The row payload already carries `payer_rejected_acknowledged_at` / `_actor` (always null on the current lane)". `test_lane_filter_acknowledged.py` covers "row payload carries the ack fields (forward-compat)". **The test name says "forward-compat" but the test does not assert the fields are null on the lane — only that they exist on the row.**
**U12.** SP18 spec (`structured-logging`) is not yet a backfilled spec; the implementation lives at `backend/src/cyclone/logging_config.py`. `test_logging_formatter.py`, `test_logging_scrubber.py`, `test_logging_setup.py` exist. **No spec describes the `PiiScrubber` regex list** (which PHI patterns it redacts); the spec is a backfill gap.
**U13.** `REQUIREMENTS.md` §6.1 traceability FR-35 says "5-lane Inbox" was satisfied by SP14; the §6.3 acceptance row for SP14 says "All 5 lanes render in `/inbox`; bulk acknowledge action drops payer-rejected claims without erasing the original rejection". **`test_lane_filter_acknowledged.py` covers the lane filter and `test_payer_rejected_acknowledge.py` covers the ack endpoint. There is no integration test that renders all 5 lanes simultaneously** (each lane's UI rendering is in `BulkBar.test.tsx` per the SP14 spec §8 table, but the page-level test for `Inbox.tsx` with 5 populated lanes is not enumerated).
**U14.** SP16 spec §3.5 says `_handle_ta1` calls `cyclone.store.store.add_ta1_ack`. **The `test_api_ta1.py` exists and the `test_scheduler.py` has a TA1 routing test, but no test asserts that `_handle_ta1` correctly persists a TA1 ack row end-to-end through the scheduler (vs through the manual `/api/parse-ta1` endpoint).**
**U15.** `REQUIREMENTS.md` §6.1 FR-15 says "Per-service-line adjustment audit: 837 SV1 ↔ 835 SVC strict match, line reconciliation tab". `test_line_reconciliation.py` covers the pure-function `match_service_lines` (per SP7 plan Task 7). **`test_reconcile_line_level.py` covers the integration with `reconcile.run()`. But there is no frontend test for the "Line Reconciliation tab" UI** (the spec is for backend data only; the tab is described but the SP7 spec is silent on the frontend tab layout; the frontend is presumably covered by `Reconciliation.tsx.test.tsx` but SP7 doesn't enumerate it).
**U16.** `REQUIREMENTS.md` §6.1 FR-33 says "Rich health payload on `GET /api/health` with last-batch-timestamp, parser state, pubsub health, scheduler state". `test_security.py` exists for the `get_health_snapshot()` function. **No test asserts that `last_batch_timestamp` is populated when a batch has been parsed** (the field is added by SP19; the spec expansion is not enumerated in any plan's test plan).
**U17.** `REQUIREMENTS.md` §6.1 FR-26 says "paramiko-backed SFTP submit, credentials via `keyring`". `test_sftp_paramiko.py` covers the connect / write / list / read happy paths. **The test does not assert that an empty password raises `RuntimeError("SFTP: Keychain entry ... missing or stub")`** — it only asserts the runtime error type, not the message. An engineer changing the error message would not break the test.
**U18.** SP11 spec §7 says the audit chain verifier returns `ok: true, checked: N+1` where N is the number of rows. **`test_audit_log.py` exists but the test for `verify_chain` with a tampered row is not enumerated in any plan's test inventory.** A future engineer changing `_hash_row` to use a different canonicalization (e.g. including the row id) would silently break the chain with no test catching it.
---
## 10. Escalations (`ESCALATE:` prefix)
These are non-obvious product decisions in the docs that an engineer would benefit from confirming with the user.
**ESCALATE: SP23 — Ubuntu Docker + RBAC.** `docs/superpowers/specs/2026-06-22-cyclone-ubuntu-docker-deployment-design.md` is marked "Draft / candidate". It introduces (a) a Docker image (`Dockerfile` + `docker-compose.yml`), (b) 3-role RBAC (admin / operator / viewer) backed by `cyclone.auth`, (c) per-role scoping on every endpoint, (d) JWT-based session tokens. **This is a fundamental posture change** — Cyclone today binds `127.0.0.1` with no auth. The spec does not say whether the existing single-operator flow is preserved as a "no-auth" mode under Docker (with RBAC opt-in), or whether RBAC is mandatory. The completeness review (2026-06-20) and the round-3 docs both treat SP23 as optional, but the spec title (`ubuntu-docker-deployment`) suggests it is the canonical deployment posture. **Confirm: is SP23 in scope for the next release, or is it parked? If parked, what is the deployment story?**
**ESCALATE: SFTP `AutoAddPolicy` vs `RejectPolicy`.** SP13 spec §3.2 says `AutoAddPolicy()` is the v1 trade-off; §7 "Open questions" lists `strict_host_key_check: bool` as future hardening. **In a HIPAA-regulated workflow, trust-on-first-use is a known MITM-attack vector.** The spec acknowledges this and proposes a `known_hosts` switch as future work. **Confirm: is `AutoAddPolicy` acceptable for the v1 release, or should SP13 ship with `RejectPolicy` + a bundled `known_hosts` file? The Gainwell MFT host key would need to be obtained and pinned.**
**ESCALATE: `audit_log` retention is "≥ 6 years" (HIPAA §164.316(b)(2)) but the docs do not say how.** SP11 spec §7 says "pinned audit-log retention for rotation events: `db.key_rotated` rows live in the SP11 chain; SP11 already commits to long-term retention per HIPAA §164.316(b)(2). No additional retention config is needed." But the migration `0009_audit_log.sql` does NOT add a retention config (no `created_at + INDEX`, no `prune_event` cron). The 6-year retention is implicitly guaranteed by (a) the operator never pruning the table, and (b) the encrypted backup SP17 covering the `audit_log` rows. **Confirm: is "operator never prunes" the retention policy, or should the spec describe an automated retention policy (e.g. a `prune_audit_log` cron at 6 years + 1 day)?**
**ESCALATE: The 5-lane Inbox vs 4-lane state machine.** The SP10 spec says `payer_rejected_at` is a column on `claims` (not a state). The SP14 spec says the 5th lane is computed by filtering. **The operator's mental model is: "I have a `rejected` claim or a `payer_rejected` claim." But the code says: a claim is in one state (`paid`, `denied`, `partial`, etc.) AND has a `payer_rejected_at` timestamp (or doesn't).** The 5-lane UI surfaces 5 working categories, but the underlying state machine has 8 states + 4 timestamp columns. **Confirm: should the operator see `state` AND `payer_rejected_at` as separate concerns, or should the data model be unified into a 9-state enum where `payer_rejected` is a terminal state (like `reversed`)?**
**ESCALATE: SP21 store split is in-flight on `refactor/store-split`.** The spec is dated 2026-06-21; the current `backend/src/cyclone/store.py` is the monolithic version. The DR plan ("3 reviewers, 5-day merge window") is in the spec §10 but no plan exists for SP21. **Confirm: is the store split a release-blocker, or can it ship after the audit chain + MFT scheduler are stable? An engineer building SP22+ on top of the monolithic store would have to migrate to the split store later.**
**ESCALATE: SP22 parse-decide spec describes a 409 shape that contradicts the unique-constraint spec.** The 409 body in SP22 is `{parse_result, collisions: [{colliding_claim_ids, existing_batch_id}]}`; in the unique-constraint spec it is `{error, detail, batch_id, existing_batch_id}`. **Confirm: which is canonical? If the richer shape is canonical, the claims-unique-constraint spec needs a follow-up to update the 409 body and the frontend `Upload.tsx` error panel.**
**ESCALATE: Reconciliation failure events go to `activity_events` (mutable), not `audit_log` (hash-chained).** The completeness review (2026-06-20 §3.1 item 2) flagged `activity_events` as not tamper-evident. The round-3 docs (SP11) added `audit_log` but did not migrate the reconciliation failure events. **Confirm: should `activity_events` rows for `kind='reconcile_failure'` be migrated to `audit_log`? Or is the operator's "reconciliation is fail-soft" claim enough, and the audit-evidence requirement only applies to HIPAA-relevant events (parse, reject, payer-rejected, key rotation)?**
**ESCALATE: `?ack=true` on parse-837 returns the 999 inline vs in a separate field.** REQUIREMENTS FR-9 says the 999 is auto-generated; the edi-features plan Task 15 says the ack is "attached to the response body"; the live-tail spec does not re-acknowledge. **Confirm: should the 999 be returned inline as `ack.x12_text` (raw X12 string), as `ack.body` (parsed object), or as a `ack_id` requiring a follow-up `GET /api/acks/{id}`? The `serialize_999` module exists in `parsers/`, so inline raw text is straightforward.**
**ESCALATE: 5-lane Inbox claim ownership.** The lane filter for `payer_rejected` is on the `claims` table. The 999-rejected lane (`rejected`) is also on the `claims` table. The `unmatched` lane is on `claims` with no matched remit. **But what about a claim that is both `rejected` (by 999) and `payer_rejected` (by 277CA)?** The SP14 spec §3.2 SQL filter puts it in the `rejected` lane (because `state == REJECTED` takes precedence over the `payer_rejected_at` check). The spec does not say this explicitly. **Confirm: should a 999-rejected + 277CA-payer-rejected claim appear in both lanes (UI splits), or in the `rejected` lane only (operator workflow assumes rejection order)?**
**ESCALATE: SP15 key rotation endpoint has actor="operator" hard-coded default.** The actor is request-body-supplied; the SP15 spec §7 says "for v1 single-operator, `'operator'` is honest". The SP23 candidate introduces RBAC where actor becomes a real identity. **Confirm: should the v1 single-operator posture be honored with `actor="operator"`, or should the request require an actor parameter (rejecting "operator" as ambiguous)?**
**ESCALATE: `processed_inbound_files` is operational metadata, not in the HIPAA audit chain (SP16 spec §2.4).** The scheduler's per-file errors are recorded in `processed_inbound_files.error_message` (mutable, not in `audit_log`). **Confirm: should scheduler errors (e.g. `parse_999` raising `Exception` on a malformed inbound) ALSO be logged in `audit_log` as `scheduler.parse_error`? An auditor investigating a 999 parse failure today would find nothing in the hash-chained log.**
**ESCALATE: Backend test count is variously reported as 574 (completeness review 2026-06-20), 437 (SP5 plan target), and 964 (current — per user).** The codebase has 94 test files in `backend/tests/`; individual test functions are not enumerated in this review (running pytest was not permitted). **The traceability matrix in REQUIREMENTS.md §6.1 cites one test file per FR — but a single FR may have 5+ tests in that file.** Confirm: is the FR-coverage criterion "≥1 test file" or "≥1 test function"?
---
## 11. Confidence
**Overall confidence in the doc set: medium-low.**
The doc set has high production quality: 27 specs + 27 plans + 2 top-level docs, all version-controlled and consistent in style. The TDD discipline in the plans (write failing test, run, implement, run, commit) is exemplary. The layering (requirements → architecture → spec → plan → tests) is the right structure. The completeness review (2026-06-20) and the round-3 doc-prep pass added the missing SP10–SP16 specs/plans, which closes a major gap. **But the doc set has accumulated at least 25 specific contradictions, ambiguous forks, and stale numbers** between the two top-level docs and across the spec/plan set. The contradictions are concentrated in the most consequential areas: the live-tail HTTP surface, the claim-state machine, and the LOC/version counters used in the verification matrix. The 7-state claim lifecycle is wrong in 5 places; the codebase has an 8-state enum (verified at `backend/src/cyclone/db.py:180-188`). The tail endpoint URL is `/api/{resource}/stream` per FR-20 + the live-tail spec, but `/api/tail?since=…` per ARCHITECTURE.md §5.4 — two competent engineers reading only the docs could ship two incompatible APIs. The 5 s heartbeat in ARCHITECTURE is contradicted by REQUIREMENTS and the code (which both use 15s via `CYCLONE_TAIL_HEARTBEAT_S`). The 409 collision response shape has two different shapes in two specs (SP22 vs unique-constraint). The `api_routers/` partial split is invisible to the top-level docs. SP21/SP22/SP23 are in-flight or candidate but not marked as such in the top-level docs. The audit_log vs activity_events event-namespace ambiguity is not documented. **The doc set is shippable as a historical record of work done, but is NOT safe as the *only* source of truth for a future engineer building on top of it.** For each of the 21 forks in §8, two competent engineers could ship different systems. For each of the 18 untested artifacts in §9, an engineer refactoring could silently break documented behavior without test coverage catching it. For each of the 12 escalations in §10, a product decision needs user input before the next release.
**Recommendation**: do not ship the doc set as-is. Fix the contradictions in §7 #1–7 (the LOC counts, the heartbeat interval, the tail endpoint URL, the 7-state lifecycle, the payer_rejected-as-state) before the next reviewer pass. Resolve the 409 shape fork in §8 #F4 (SP22 vs unique-constraint) before any 837P file with internal CLM01 collisions can ship. Document the audit_log vs activity_events split (§8 #F7) before the SP21 store split lands. Add tests for the 18 untested artifacts in §9. Confirm the 12 escalations in §10 with the user before the next release cycle.
---
*End of Reviewer A.*
docs/reviews/2026-06-23-cyclone-docset-review-B.md
+372
View File
@@ -0,0 +1,372 @@
# Cyclone doc-prep review — Reviewer B (code-first lens)
## 1. Reviewer identity + lens
I am **Reviewer B**. I used a **code-first lens** for the Cyclone doc-prep pass:
the bulk of my exploration was in the source tree — `backend/src/cyclone/`
(`api.py`, `api_helpers.py`, `api_routers/`, `db.py`, `store.py`, `audit_log.py`,
`security.py`, `db_crypto.py`, `backup_service.py`, `reconcile.py`,
`inbox_lanes.py`, `scoring.py`, `inbox_state.py`, `inbox_state_277ca.py`,
`scheduler.py`, `parsers/`, `migrations/0001-0012`) and
`src/{routes,store,lib,hooks,components}/` (notably the live-tail client
`src/lib/tail-stream.ts` and the React hook `src/hooks/useTailStream.ts`).
I then read the top-of-funnel docs in the intended order:
`docs/README.md` → `docs/REQUIREMENTS.md` → `docs/ARCHITECTURE.md`. From
there I cross-referenced the 20 spec files and 27 plan files in
`docs/superpowers/specs/` and `docs/superpowers/plans/`. **I did not read
Reviewer A's `docs/reviews/2026-06-23-cyclone-docset-review-A.md` before
writing this review** so my findings are independent of any
docs-first anchoring. I have flagged every claim with a `file:line`
citation, and where the code disagrees with the doc, the code wins —
the doc needs fixing.
## 2. Executive summary
The Cyclone doc set is in **moderate health with a small number of
high-severity contradictions** that would mislead any engineer reading
docs first. The implementation is unusually self-consistent and matches
the SP-spec lineage, but the two top-level docs (`REQUIREMENTS.md`,
`ARCHITECTURE.md`) and several inline code docstrings have drifted from
the SP-spec + code truth. The top three issues are:
1. **The audit-chain hash recipe is wrong in three places** (the
`AuditLog` docstring in `db.py:649-650`, the `audit_log.py:6-7`
module docstring, and `ARCHITECTURE.md` §4.6) and only the SP11 spec
(`docs/superpowers/specs/2026-06-23-cyclone-sp11-hash-chained-audit-design.md:104-114`)
plus the actual `_hash_row` function at
`backend/src/cyclone/audit_log.py:80-89` agree. This is a C-1
severity finding because the verifier and any future auditor will
compute different hashes depending on which doc they read first.
2. **`ARCHITECTURE.md` §6.3 lists the wrong claim state set** (eight
values, but the wrong eight — it has `accepted`, `appealed`, and
`payer_rejected`, none of which exist in code). The actual `ClaimState`
enum (`db.py:180-188`) is `submitted, received, rejected, paid, partial,
denied, reconciled, reversed`. The `RECONCILED` state is defined and
is treated as terminal by `apply_payment` (`reconcile.py:163-178`) but
is never actually set by any code path, which is a separate
fork-shaped finding (F-1).
3. **The API surface in `ARCHITECTURE.md` §8.1 is materially wrong** for
live tail, the Inbox 5-lane, and the backup restore flow. Live tail
is documented as `GET /api/tail?since=…` (ARCH §5.4 / §8.2) and as
`GET /api/{claims,remittances,activity}/stream` in REQUIREMENTS FR-20
— the code is the latter (`api.py` 3,548 LOC; verified at multiple
routes). Inbox is listed as `/api/inbox` in ARCH §8.1 but the real
surface is `/api/inbox/lanes`, `/api/inbox/payer-rejected/acknowledge`,
etc. Backup restore is a one-step URL in ARCH §8.1 but the
`backup_service.py` is a two-step `restore_initiate` / `restore_confirm`
(matches FR-30). The two-step shape is what the code ships.
Below the top three, the doc set has many smaller drifts — the live-tail
heartbeat cadence is documented as 5s in ARCH §5.4 (code is 15s default;
NFR-10 is silent on default but the SP5 spec is 15s), the `clearhouse`
ORM is a singleton (singular table) but ARCH §6.2 draws the ERD as
`clearhouses` (plural), the migration 0004 is misdescribed in ARCH §6.1
as adding a new table when it actually adds four columns to `claims`,
and the spec-vs-code disagreement over the `Match.claim_id` UNIQUE
constraint (SP2 §6.5 says UNIQUE; code explicitly doesn't enforce it so
reversals can add a second row).
There are also **two product forks that are blocking-or-near-blocking**
that the docs do not decide: the SP23 Ubuntu/Docker/auth/RBAC/LAN-bind
direction (spec at `2026-06-22-cyclone-ubuntu-docker-deployment-design.md`
exists, no plan exists; REQUIREMENTS R-1, R-3, R-24 raise but do not
close this), and the `RECONCILED` state being declared terminal but
never assigned. Neither has a test, plan, or design decision recorded.
## 3. Reviewer questions
### Q1: Are the components described consistently across REQUIREMENTS, ARCHITECTURE, specs, and plans?
**Verdict: NO — multiple component-name and component-shape mismatches.**
Reasoning:
- The **`Live tail` component** is described three different ways:
REQUIREMENTS FR-20 says `/api/{claims,remittances,activity}/stream`
(NDJSON); ARCH §5.4 and §8.2 say `GET /api/tail?since=<last_event_id>`;
SP5 spec says per-resource `…/stream` and the NDJSON line shape is
`{"kind": "item"|"snapshot_end"|"heartbeat"|"item_dropped"|"error", …}`
(matches the React client's `src/lib/tail-stream.ts` `TailEvent`
type). The code is the SP5 spec; REQUIREMENTS matches the code; ARCH
does not.
- The **`Inbox` component** is described as `/api/inbox` in ARCH §8.1
but as a 5-lane surface in REQUIREMENTS FR-12 / FR-13 / FR-14 and in
the SP14 spec, and the code (`api.py`; `inbox_lanes.py` at lines
1-280) implements `/api/inbox/lanes` plus
`/api/inbox/payer-rejected/acknowledge`, plus `GET /api/inbox/999` and
`GET /api/inbox/277ca` style endpoints. ARCH §8.1 is missing the 5-lane
detail.
- The **`Backup restore` component** is described as a single URL in
ARCH §8.1 (`/api/admin/backup/{id}/restore?confirm=true`) but as a
two-step `restore_initiate` / `restore_confirm` in REQUIREMENTS FR-30
and in the `backup_service.py:851` LOC file. The code is the
two-step shape.
- The **`Database key rotation` component** is described in ARCH §8.1
only by the endpoint `/api/admin/db/rotate-key`, but the SP15 spec
defines a four-step ordering (DB-first → Keychain-second →
engine-rebuild → audit-last) and a specific `db.key_rotated` audit
event payload `{old_fingerprint, new_fingerprint, table_count, reason}`.
Neither the ordering nor the payload shape is in ARCH.
### Q2: Is the data model consistent (schema, enums, lifecycle, audit chain)?
**Verdict: NO — five material data-model disagreements.**
Reasoning:
- **Claim state enum is wrong in ARCH §6.3 and partially wrong in
REQUIREMENTS FR-6.** REQUIREMENTS FR-6 says "7-state" but lists 8
(in different versions of the doc over time). ARCH §6.3 lists
`submitted, accepted, paid, reversed, denied, appealed, rejected,
payer_rejected`. The code (`db.py:180-188`) is `submitted, received,
rejected, paid, partial, denied, reconciled, reversed`. Only five
values match across all four (`submitted, paid, partial, denied,
reversed`; note: ARCH does not have `partial` either, which is the
most common auto-reconcile outcome). SP2 spec lists 7
(`submitted, received, paid, partial, denied, reconciled, reversed`).
The `rejected` state is set by 999 AK9 set-level R/E (per
`inbox_state.py` lines 1-76 and migration 0004), not by any spec
other than SP6. The `reconciled` state is dead code — see F-1.
- **Audit chain hash recipe disagrees in 3 places** (see C-1 below).
- **The `clearhouse` table is a singleton** in code (`db.py:824-838`
`__tablename__ = "clearhouse"`, single row, with a `SingletonError`
exception for inserts past row 1) but the ERD in ARCH §6.2 shows
`clearhouses` plural. Migration 0011 (`0001-0012` index) also writes
to `clearhouse` (singular).
- **The `Claim(batch_id, patient_control_number)` UNIQUE constraint
is missing in code** but is asserted in SP2 spec §6.2 line 166 and
was the subject of migrations 0003 and 0013+0014 (the in-flight
`claims-unique-fix` worktree). The `claims` table in migration 0001
has no such UNIQUE; the `Claim` ORM (`db.py:310-319`) explicitly
notes the absence. The SP2 spec is out of sync with the SP-13/14
reality.
- **The `Match.claim_id` UNIQUE constraint is missing in code** but is
asserted in SP2 spec §6.5 line 207. Migration 0001 line 65-66
(comment) and `db.py:505` (ORM comment) say reversals add a 2nd row
to `matches`, so UNIQUE on `claim_id` would be wrong. The spec is
wrong; the code is correct.
### Q3: Are the dependencies and external integrations (SQLCipher, SFTP, MFT, key rotation, audit) consistent and traceable?
**Verdict: MOSTLY YES — SQLCipher + key rotation are well-traced, MFT file routing has one ambiguity.**
Reasoning:
- **SQLCipher + key rotation** is end-to-end traceable:
`db_crypto.py:389` LOC defines `KEYCHAIN_ACCOUNT = "cyclone.db.key"`
and `KEYCHAIN_ACCOUNT_PREVIOUS = "cyclone.db.key.previous"`, uses
`NullPool` for thread affinity, exposes `RotateKeyResult` with
`old_fingerprint` and `new_fingerprint` (matches SP15 spec),
and `api.py:2797` exposes `POST /api/admin/db/rotate-key` (matches
REQUIREMENTS FR-32 and the SP15 spec endpoint name). Migration 0012
is the SQLCipher-key-on-disk path. The audit event `db.key_rotated`
has the spec-mandated payload shape `{old_fingerprint, new_fingerprint,
table_count, reason}` in `db_crypto.py`. **Consistent.**
- **MFT file routing** has a small ambiguity: `scheduler.py:316-321`
routes `ROUTED_FILE_TYPES = {"999", "835", "277", "277CA", "TA1"}` to
the corresponding parsers. SP10 spec says HCPF sends a bare `277` (not
`277CA`) but the scheduler accepts both. This is intentional (defense
in depth) but the doc should say so.
- **Audit chain** has a write/read asymmetry: the chain is computed on
append by `append_event()` and verified by `verify_chain()`, but the
*docstring recipe* is wrong (see C-1). Code wins.
- **SFTP and MFT** are documented only by reference to `paramiko` (SFTP
transport) in `pyproject.toml` and by the `MFT_SCHEDULE` config in
`config/payers.yaml`. There's no spec or plan I could find that says
what the MFT pickup cadence should be, what the retry policy is, or
what the failure alert looks like. This is a coverage gap, not a
contradiction.
- **SSE / live tail transport** is described as "NDJSON pubsub" in
REQUIREMENTS NFR-10 and "NDJSON via `EventBus`" in ARCH §5.4. The code
is NDJSON via `tail_events()` in `api_helpers.py:225` reading from
`EventBus`. Consistent.
### Q4: Is "definition of done" testable for every artifact?
**Verdict: NO — at least 6 artifacts are undertested, and the test surface is uneven.**
Reasoning:
- **Audit chain verifier** has tests in `test_audit_log.py` (per
the `verify_chain()` reference in REQUIREMENTS NFR-4) but the
*docstring recipe* in `audit_log.py:6-7` and `db.py:649-650` does
not match the implementation. The tests verify the implementation
but the docstring is the wrong contract.
- **Score breakdown** (40/25/20/15) has tests and is documented in
REQUIREMENTS FR-15, `scoring.py:42-46`, and the SP13 spec.
Testable.
- **Backup passphrase-missing degraded mode** (backup service falls
back to deriving from SQLCipher key with a WARNING when no
Keychain passphrase is set) is implemented in `backup_service.py`
but has no test asserting the WARNING is emitted; REQUIREMENTS
NFR-6 only asserts that the passphrase is in Keychain. The
degraded-mode code path is untested.
- **5-lane Inbox** has tests but the lane-tally API contract is not
documented anywhere; the code returns a JSON with `lane_counts`
but no spec defines the exact shape.
- **Live tail stall detection** (30s stall timeout in
`src/lib/tail-stream.ts` and `src/hooks/useTailStream.ts`) is
not asserted in any backend test — only the client side knows
about the stall; the server keeps streaming. Not testable from
the spec alone.
- **277CA monotonic stamping** (`inbox_state_277ca.py:1-108`) is
documented in SP10 spec §"Monotonic rule" but has no test asserting
the monotonic invariant is preserved across replays.
- **Migration 0013/0014 in-flight on `claims-unique-fix`** worktree are
not described in REQUIREMENTS NFR-15 (which says "12 migrations
shipped") and not in ARCH §6.1 (which lists 0001-0012). The
"definition of done" for the in-flight fix is "merged into main" —
there is no acceptance criterion.
## 4. Contradictions
Each row is `C-N | code says X | doc says Y | code wins because …`.
| ID | Code (file:line) | Doc (file:line) | Resolution |
|----|------------------|-----------------|------------|
| C-1 | `_hash_row` at `backend/src/cyclone/audit_log.py:80-89` joins fields in this order: `row_id`, `event_type`, `entity_type`, `entity_id`, `actor`, `created_at_iso`, `payload`, `prev_hash`. | (a) `backend/src/cyclone/audit_log.py:6-7` module docstring says `(id, event_type, entity_type, entity_id, actor, payload_json, created_at, prev_hash)` — payload BEFORE created_at. (b) `backend/src/cyclone/db.py:649-650` AuditLog class docstring says the same wrong order. (c) `docs/ARCHITECTURE.md` §4.6 says `SHA-256(prev_hash || kind || actor || payload_json || ts)` — a third, completely different 5-field recipe. (d) `docs/superpowers/specs/2026-06-23-cyclone-sp11-hash-chained-audit-design.md:104-114` matches the code (8 fields, payload AFTER created_at). | **Code wins.** The implementation + SP11 spec agree. The module docstring + `AuditLog` class docstring + ARCH §4.6 are all wrong. The two inline docstrings need to be rewritten to match `_hash_row`; ARCH §4.6 needs the recipe and field list corrected. This is the highest-severity finding because an external auditor reading `audit_log.py` first will compute a different hash than `verify_chain()`. |
| C-2 | `ClaimState` enum at `backend/src/cyclone/db.py:180-188` has 8 values: `SUBMITTED, RECEIVED, REJECTED, PAID, PARTIAL, DENIED, RECONCILED, REVERSED`. | (a) `docs/REQUIREMENTS.md` FR-6 says "7-state" (off-by-one). (b) `docs/ARCHITECTURE.md` §6.3 lists 8 values: `submitted, accepted, paid, reversed, denied, appealed, rejected, payer_rejected` — three of these (`accepted`, `appealed`, `payer_rejected`) do not exist in code. (c) `docs/superpowers/specs/2026-06-19-cyclone-db-reconciliation-design.md` §6 lists 7: `submitted, received, paid, partial, denied, reconciled, reversed` — missing `rejected`. (d) `docs/superpowers/specs/2026-06-23-cyclone-sp10-277ca-payer-rejected-design.md` treats `payer_rejected` as a *column* (`payer_rejected_at`), not a state. | **Code wins.** Drop `accepted`, `appealed`, `payer_rejected` from ARCH §6.3; add `received`, `rejected`, `reconciled` everywhere. FR-6 "7-state" is wrong (the actual count is 8). SP2 spec is missing the `rejected` state (set by 999 AK9 R/E per `inbox_state.py:1-76` and migration 0004). |
| C-3 | Live tail endpoints are `/api/claims/stream`, `/api/remittances/stream`, `/api/activity/stream` (NDJSON). | (a) `docs/REQUIREMENTS.md` FR-20 says `GET /api/{claims,remittances,activity}/stream` (matches code). (b) `docs/ARCHITECTURE.md` §5.4 and §8.2 say `GET /api/tail?since=<last_event_id>` (does not exist in code). | **Code wins.** ARCH §5.4 and §8.2 need to be rewritten to match the per-resource `/api/{...}/stream` shape. |
| C-4 | Heartbeat cadence is 15s default, configurable via `CYCLONE_TAIL_HEARTBEAT_S`, code at `backend/src/cyclone/api_helpers.py:209-222`. | (a) `docs/ARCHITECTURE.md` §5.4 says "every 5s when no events". (b) `docs/REQUIREMENTS.md` NFR-10 says "≤15s" (so 15s is within budget). (c) `docs/superpowers/specs/2026-06-20-cyclone-live-tail-design.md` says 15s (matches code). | **Code wins.** ARCH §5.4 is wrong; the 15s default is also the SP5 spec default. NFR-10's "≤15s" is loose enough that 15s is compliant. |
| C-5 | Migration `0004_rejections_and_state_history.sql` adds 4 columns to `claims` (`rejection_reason`, `rejected_at`, `resubmit_count`, `state_changed_at`) and one index (`ix_claims_state_changed_at`). | `docs/ARCHITECTURE.md` §6.1 says migration 0004 is "rejections table + claim_state_history" — implying a *new table*. | **Code wins.** ARCH §6.1 is wrong; migration 0004 alters only the `claims` table. No new table is created. The reviewer suspects the author confused this with migration 0008 (`0008_*.sql`, the `claim_state_history` table) or migration 0007 (the `rejections` audit-trail table; verify by listing). |
| C-6 | Backup restore is a two-step flow: `POST /api/admin/backup/{backup_id}/restore/initiate` returns a 64-char token with 300s TTL; `POST /api/admin/backup/{backup_id}/restore/confirm` consumes the token. Code at `backend/src/cyclone/backup_service.py:851` LOC, plus endpoints wired in `backend/src/cyclone/api.py`. | (a) `docs/ARCHITECTURE.md` §8.1 lists `/api/admin/backup/{id}/restore?confirm=true` as a single URL. (b) `docs/REQUIREMENTS.md` FR-30 documents the two-step shape. | **Code + REQUIREMENTS win.** ARCH §8.1 should be corrected to two URLs. |
| C-7 | Inbox 5-lane surface is `GET /api/inbox/lanes` plus `POST /api/inbox/payer-rejected/acknowledge`, `GET /api/inbox/999`, `GET /api/inbox/277ca`, plus per-lane claim listing endpoints. Code in `backend/src/cyclone/inbox_lanes.py:1-280` + `backend/src/cyclone/api.py`. | `docs/ARCHITECTURE.md` §8.1 lists the inbox as a single `/api/inbox` endpoint. | **Code wins.** ARCH §8.1 is missing the 5-lane detail. |
| C-8 | `ClearhouseORM` is a **singleton** at `backend/src/cyclone/db.py:824-838` with `__tablename__ = "clearhouse"` (singular), with a guard that raises on insert past row 1. | `docs/ARCHITECTURE.md` §6.2 ERD shows the table as `clearhouses` (plural) and the SP9 spec describes the seed as "the single clearhouse config row". | **Code wins.** ARCH §6.2 table name is wrong; the table is `clearhouse` (singular). |
| C-9 | `Claim` table has no UNIQUE constraint on `(batch_id, patient_control_number)`. Migration 0001 (initial) has no such UNIQUE; migration 0003 explicitly drops UNIQUE constraints; the `Claim` ORM at `backend/src/cyclone/db.py:310-319` notes the absence. Migrations 0013/0014 (in-flight on `claims-unique-fix` worktree) re-add it. | `docs/superpowers/specs/2026-06-19-cyclone-db-reconciliation-design.md` §6.2 line 166 asserts the UNIQUE is present. | **Code wins (today).** The in-flight 0013+0014 will re-add the UNIQUE; SP2 spec should be updated to reflect "as of migration 0013/0014". |
| C-10 | `matches` table has no UNIQUE on `claim_id` — reversals intentionally add a 2nd row. Migration 0001 lines 65-66 (comment) and the `Match` ORM at `backend/src/cyclone/db.py:505` (comment) both note this is "non-unique: reversals add a 2nd row". | `docs/superpowers/specs/2026-06-19-cyclone-db-reconciliation-design.md` §6.5 line 207 says `claim_id` is UNIQUE — "one current match per Claim". | **Code wins.** SP2 spec §6.5 is wrong; the SP2 spec author was thinking of a 1:1 "current match" view but the schema models the full history. |
| C-11 | `apply_payment` at `backend/src/cyclone/reconcile.py:163-178` returns `DENIED`, `RECEIVED`, `PAID`, or `PARTIAL`. The function never returns `RECONCILED`. | SP2 spec transition table (line 250-253) shows `apply_payment` paths all ending in `→ reconciled` (terminal). | **Code wins** — but the spec was likely trying to describe a *separate* finalize step. The current code declares `RECONCILED` as a terminal state (per `reconcile.py:163-164` "already in terminal state" check at line 156-157) but never assigns it. See F-1. |
| C-12 | API ingestion routes are `/api/parse-837`, `/api/parse-835`, `/api/parse-999`, `/api/parse-ta1`, `/api/parse-277ca`, plus `/api/batches/{batch_id}/export-837`. | `docs/ARCHITECTURE.md` §8.1 lists `/api/upload` as a single ingestion endpoint. | **Code wins.** There is no `/api/upload` route. ARCH §8.1 is wrong. |
| C-13 | `Claim` has a `payer_rejected_at` column (`db.py:270-289`) but no state transition to `payer_rejected`. The claim's state remains `SUBMITTED` even after a 277CA STC A4/A6/A7 rejection (per `inbox_state_277ca.py:1-108` and SP10 spec). | `docs/ARCHITECTURE.md` §6.3 transition diagram shows `rejected → payer_rejected`. | **Code wins.** ARCH §6.3 is wrong. The 277CA payer_rejected state is captured as a *column* on the claim, not a `ClaimState` value. |
| C-14 | `Match.is_reversal` exists (`db.py:480-510`); `apply_reversal` at `backend/src/cyclone/reconcile.py:181-189` flips paid/partial → REVERSED and inserts a 2nd row with `is_reversal=True`. | `docs/superpowers/specs/2026-06-19-cyclone-db-reconciliation-design.md` §6.5 says "Match is unique per Claim" — same as C-10. | **Code wins.** See C-10. The reversal second-row is the design; the spec is wrong. |
| C-15 | `manual_unmatch` at `backend/src/cyclone/store.py:2122` resets state to `ClaimState.SUBMITTED` (per REQUIREMENTS FR-6). | `docs/ARCHITECTURE.md` §6.3 doesn't show a `submitted ← <anything>` transition. | **Code wins.** ARCH §6.3 is missing the manual-unmatch reset edge. |
| C-16 | `frontend/src/lib/tail-stream.ts` `TailEvent` union has 5 kinds: `item`, `snapshot_end`, `heartbeat`, `item_dropped`, `error`. | `docs/ARCHITECTURE.md` §5.4 lists 3 kinds: `item`, `heartbeat`, `error`. | **Code wins.** ARCH §5.4 is missing `snapshot_end` (sent at start of subscription to send the current tail buffer) and `item_dropped` (sent when the server-side buffer overflows and starts dropping events). The client-side hook `src/hooks/useTailStream.ts` handles both. |
| C-17 | `frontend/src/hooks/useTailStream.ts` `TailStatus` union has 6 states: `connecting`, `live`, `reconnecting`, `closed`, `stalled`, `error`. | `docs/ARCHITECTURE.md` §5.4 doesn't document the client-side status states at all. | **Code wins.** ARCH §5.4 should list the client status states; the `stalled` state (30s no events, no heartbeat) is a real product decision that ARCH currently hides. |
| C-18 | Backoff schedule in `frontend/src/lib/tail-stream.ts` and `src/hooks/useTailStream.ts` is 1s, 2s, 4s, 8s, 16s, capped at 30s. | `docs/ARCHITECTURE.md` §5.4 doesn't specify the reconnect backoff. | **Code wins.** ARCH §5.4 is silent. The schedule is not in any spec either. |
| C-19 | `EventBus` is the in-process pubsub that `tail_events()` subscribes to (`backend/src/cyclone/api_helpers.py:225`). | `docs/ARCHITECTURE.md` §5.4 calls it "NDJSON pubsub" and lists 3 event types (`item`, `heartbeat`, `error`). | **Code wins** on the protocol (NDJSON via per-resource `/api/.../stream`); ARCH §5.4 is wrong on the kinds per C-16. |
| C-20 | SPEC traceability in `docs/REQUIREMENTS.md` lists 22 SPs (SP1–SP22) and a future SP23 (Ubuntu/Docker). | `docs/superpowers/specs/` directory contains specs for SP1–SP22 only (22 files, plus 1 older `2026-06-19-cyclone-db-reconciliation-design.md` and 1 older `2026-06-20-cyclone-live-tail-design.md`, plus 1 in-flight `2026-06-22-cyclone-ubuntu-docker-deployment-design.md` which is the SP23 spec). | **Code/specs win.** REQUIREMENTS R-1 should explicitly call out that SP23 has a spec but no plan. |
## 5. Forks (priority order)
| ID | Priority | What is ambiguous | What decision is needed |
|----|----------|-------------------|--------------------------|
| F-1 | **P0** | The `RECONCILED` state in `ClaimState` (`db.py:187`) is declared terminal in `apply_payment` (`reconcile.py:163-164`) but is never actually set by any code path. The SP2 spec transition table shows it as the terminal step of every `apply_payment` path. | Decide one of: (a) keep `RECONCILED`, add a `set_reconciled()` step after `apply_payment` succeeds, write a test for it; (b) remove `RECONCILED` from the enum and from the terminal check, update SP2 spec; (c) keep `RECONCILED` as a "synthetic" terminal that some future finalize flow will set, and explicitly defer the implementation. |
| F-2 | **P0** | The audit-chain hash recipe is wrong in `audit_log.py:6-7`, `db.py:649-650`, and `ARCHITECTURE.md` §4.6. Three of these must be corrected to match the code at `audit_log.py:80-89`. The order of fields in the canonical string is `row_id, event_type, entity_type, entity_id, actor, created_at_iso, payload, prev_hash`. | Pick the source of truth (the code + SP11 spec already agree) and rewrite the two docstrings + ARCH §4.6 to match. Add a `test_audit_log.py` test that asserts the recipe against a hand-computed expected hash for a single-row chain, so the next refactor cannot silently change the recipe. |
| F-3 | **P0** | The SP23 product fork (Ubuntu/Docker deployment, multi-user auth, RBAC, LAN-bind) is documented as a spec at `docs/superpowers/specs/2026-06-22-cyclone-ubuntu-docker-deployment-design.md` but has **no plan**. REQUIREMENTS R-1, R-3, R-24 raise the question but do not close it. The product is currently local-only, no-auth, single-user. | The user must decide: is SP23 in-scope for the next release, or deferred? If in-scope, write a plan file under `docs/superpowers/plans/2026-06-23-cyclone-sp23-*.md`. If deferred, move the spec to `docs/superpowers/deferred/` and remove R-1/R-3/R-24 from REQUIREMENTS. |
| F-4 | **P1** | The backup service (`backup_service.py:851` LOC) has a degraded mode when the encryption passphrase is missing: it derives a key from the SQLCipher DB key with a WARNING logged. REQUIREMENTS NFR-6 promises the passphrase is in Keychain. | Decide: is the degraded mode acceptable, or must the backup service fail-hard when no passphrase is set? The current behavior is a foot-gun (the warning can be missed in logs). If degraded mode stays, add a test asserting the WARNING is emitted. |
| F-5 | **P1** | `vitest@^4.1.9` is pinned in `package.json` per REQUIREMENTS R-25, which flags it as "unusually new" (current stable is 1.x/2.x). The frontend test framework choice affects every PR. | The user must confirm: (a) the `^4.1.9` pin is intentional and there is a known-working toolchain, or (b) revert to `^2.x` (or whatever the latest stable is at the time). If (a), add a CI job to verify the install. If (b), update REQUIREMENTS R-25 to read "reverted to `^2.x`". |
| F-6 | **P1** | The Inbox 5-lane `lane_counts` JSON contract is implemented in `inbox_lanes.py:1-280` but the shape is not specified in any spec. The frontend (`src/components/...`) presumably renders lane badges from this count. | Decide: write a SP14-addendum spec for the lane-counts JSON contract, OR reference the Pydantic model from `api.py` as the contract and inline-link it from the spec. Either way, the contract should be testable from the spec. |
| F-7 | **P2** | Score weights are hardcoded at `backend/src/cyclone/scoring.py:42-46` as `40/25/20/15` (patient/date/amount/provider). REQUIREMENTS R-10 says these are configurable via `config/payers.yaml`. The `config/payers.yaml` file does not currently expose them. | Decide: (a) move the weights into `config/payers.yaml` with the existing score-weight keys, write a test that reads them from config; (b) keep the weights hardcoded and update R-10 to say "constants". |
| F-8 | **P2** | MFT file routing accepts `277` and `277CA` interchangeably (`scheduler.py:316-321`). SP10 spec says HCPF sends bare `277`. The scheduler's acceptance of both is defense-in-depth. | Decide: (a) keep both, document the defense-in-depth in the spec; (b) restrict to `277CA` only and require HCPF to send `277CA`. (a) is the path of least disruption. |
| F-9 | **P2** | The `WORKTREE` stale-artifact at `backend/src/cyclone/workflow/__pycache__/` is flagged in REQUIREMENTS R-27 as needing deletion, but still exists. | Decide: just delete it. This is mechanical. Add a `.gitignore` rule to prevent recurrence. |
| F-10 | **P2** | PHI fixtures in `docs/prodfiles/` are not flagged as PHI (REQUIREMENTS R-16). The fixtures are local-only but a future contributor who adds them to a public repo would be in trouble. | Decide: (a) add a `README.md` in `docs/prodfiles/` warning the files are synthetic but look like PHI, and add a `.gitattributes` rule; (b) replace the fixtures with obviously-fake data. |
| F-11 | **P2** | The `co_medicaid()` factory in `backend/src/cyclone/parsers/payer.py:57-58` exists as a fallback (REQUIREMENTS R-4, NFR-11). It is unclear whether it is part of the test fixture taxonomy or technical debt. | Decide: (a) keep as a test fixture only and move to a `tests/fixtures/` location; (b) remove it and rely on the seed data; (c) document it as a fallback for unconfigured payers in the SP9 spec. |
| F-12 | **P2** | Migration manifest / checksum file (REQUIREMENTS R-13) is listed as needed but never built. There is no `manifest.json` or `manifest.yaml` next to the migrations directory. | Decide: (a) build the manifest, add a CI check that the manifest is in sync with the migrations; (b) drop R-13 and rely on the in-tree `git log` for migration history. |
| F-13 | **P3** | 277CA monotonic stamping (`inbox_state_277ca.py:1-108`) has the rule "do not regress a claim from rejected → not-rejected" but the rule is documented in prose only. | Decide: write a test that replays a 277CA batch twice and asserts the second pass is a noop for the rejection columns. |
| F-14 | **P3** | The live-tail `item_dropped` kind (sent when the server-side buffer overflows) is implemented in `api_helpers.py:225` and handled by the client, but no spec defines the buffer size threshold or the alert policy when `item_dropped` is received. | Decide: (a) spec the buffer size and the alert policy; (b) decide that `item_dropped` is fire-and-forget for now and the alert comes from the next full reconciliation. |
| F-15 | **P3** | `docs/README.md` lists the read-order as `REQUIREMENTS → ARCHITECTURE → spec → plan` but does not say what to do when ARCH disagrees with the spec. The convention is "spec wins for design decisions, ARCH wins for cross-component structure" but this is not written. | Decide: write a one-paragraph convention note in `docs/README.md` covering precedence when docs disagree. |
| F-16 | **P3** | The audit-log API surface (`GET /api/admin/audit-log/verify` at `api.py:2757`) is in code but the response shape (a list of broken-chain indices) is not in any spec. | Decide: write a SP11-addendum or inline-link the Pydantic model from the spec. |
## 6. Untested artifacts
| ID | Artifact | Where it is documented | Where the test should be | Why it is untested |
|----|----------|------------------------|-------------------------|--------------------|
| U-1 | Audit-chain hash recipe (the canonical string format at `audit_log.py:80-89`) is documented in the SP11 spec and in two wrong docstrings, but the spec's recipe is only verified by tests of `verify_chain()` against rows the test itself wrote. There is no test that pins a hand-computed SHA-256 hex for a single known row. | `docs/superpowers/specs/2026-06-23-cyclone-sp11-hash-chained-audit-design.md:104-114`. | `backend/tests/test_audit_log.py` — add a test `test_hash_recipe_is_stable_against_hand_computed_value`. | Without this test, a future refactor of `_hash_row` could silently change the recipe and `verify_chain()` would still pass (because it recomputes). |
| U-2 | The `vitest@^4.1.9` pin per REQUIREMENTS R-25 is flagged as "unusually new" but no test asserts that `npm install` produces a working test runner. | `docs/REQUIREMENTS.md` R-25. | A CI workflow or a `package.json` `engines` field; there is no CI workflow file. | The pin could break on a future `npm install` if vitest 4.x is unpublished. No fallback plan. |
| U-3 | The "964 tests collected" claim in REQUIREMENTS NFR-15 is not verified by any artifact. The reviewer did not count `backend/tests/test_*.py` files (this is a research gap, not a contradiction). | `docs/REQUIREMENTS.md` NFR-15. | A test-counting step in CI. | The number may be out of date by the time you read this. |
| U-4 | The `Matches.is_reversal` second-row insertion is described in migration 0001 comment lines 65-66 and the SP2 spec but no explicit test asserts the second row is inserted with `is_reversal=True` and the same `claim_id` after `apply_reversal` is called on a paid claim. | Migration 0001, SP2 spec. | `backend/tests/test_reconcile.py` (assumed). | The reviewer did not verify the test exists. Add a test that calls `apply_reversal` on a paid claim and asserts `len(matches_for(claim_id)) == 2` and the new row has `is_reversal=True`. |
| U-5 | The "operator UI for the audit log" is listed as out-of-scope in SP11 spec §"Future work" but REQUIREMENTS NFR-4 says `verify_chain()` detects breaks. There is no test that asserts a manual `verify_chain()` call (e.g., from a CLI command) reports a break when one row is tampered. | `docs/REQUIREMENTS.md` NFR-4, `docs/superpowers/specs/2026-06-23-cyclone-sp11-hash-chained-audit-design.md`. | `backend/tests/test_audit_log.py` — add a test that tampers with a row in a fresh chain and asserts `verify_chain()` returns the tampered index. | The reviewer assumes such a test exists in `test_audit_log.py` but did not verify. |
| U-6 | The backup-passphrase-missing degraded mode in `backup_service.py` falls back to deriving from SQLCipher key with a WARNING. REQUIREMENTS NFR-6 only asserts the Keychain-with-passphrase path. | `docs/REQUIREMENTS.md` NFR-6, `backend/src/cyclone/backup_service.py`. | `backend/tests/test_backup_service.py` — assert the WARNING is logged and the backup is still produced. | The degraded mode is a code path that runs in production but has no test. |
| U-7 | The 5-lane Inbox `lane_counts` JSON shape is not specified in any spec. The frontend presumably depends on it. | `docs/superpowers/specs/2026-06-23-cyclone-sp14-inbox-5lane-design.md`. | A contract test (e.g., `tests/test_inbox_lanes.py`) that asserts the response shape. | The shape is a contract between backend and frontend; no test pins it. |
| U-8 | The live-tail `stalled` state (30s no events) is a real client-side detection in `src/hooks/useTailStream.ts`. There is no backend test that asserts the server keeps streaming heartbeats correctly under stall conditions. | `docs/superpowers/specs/2026-06-20-cyclone-live-tail-design.md`. | `backend/tests/test_api_helpers.py` — assert heartbeats continue at the configured cadence under no-event load. | The stall detection is on the client; the server-side heartbeat is implicit. |
| U-9 | The 277CA monotonic stamping (`inbox_state_277ca.py`) is documented in SP10 spec but has no test asserting the monotonic invariant is preserved across replays. | `docs/superpowers/specs/2026-06-23-cyclone-sp10-277ca-payer-rejected-design.md`. | `backend/tests/test_inbox_state_277ca.py` — replay a 277CA batch twice and assert the rejection columns are not regressed. | The invariant is in prose only. |
| U-10 | The `rejected` claim state (set by 999 AK9 R/E at `inbox_state.py:1-76`) is in code and in migration 0004 but is not in SP2 spec or REQUIREMENTS FR-6's list. There is no test that asserts a 999 envelope with `AK9*R*` moves the claim from `SUBMITTED` to `REJECTED`. | Migration 0004, `backend/src/cyclone/inbox_state.py`. | `backend/tests/test_inbox_state.py` — assert 999 R/E transitions `SUBMITTED` → `REJECTED`. | The transition is in code but not in the spec, and may not be in the tests. |
| U-11 | The migration 0013/0014 `claims-unique-fix` worktree is in flight but has no acceptance criterion in REQUIREMENTS, ARCH, or any plan. The "definition of done" is "merged into main" — not testable. | None. | The `claims-unique-fix` branch's PR description (not visible from this review). | The fork is being made without a published acceptance criterion. |
| U-12 | The `co_medicaid()` factory in `backend/src/cyclone/parsers/payer.py:57-58` is not exercised by any test (assumed; not verified). | `docs/REQUIREMENTS.md` R-4, NFR-11. | `backend/tests/test_parsers.py` — add a test that the factory returns a valid PayerConfig. | The factory is in code but is not in the test plan. |
## 7. Escalations
- **ESCALATE-1 — SP23 product fork (Ubuntu/Docker/auth/RBAC/LAN-bind).**
The spec at `docs/superpowers/specs/2026-06-22-cyclone-ubuntu-docker-deployment-design.md`
exists but no plan exists. REQUIREMENTS R-1, R-3, R-24 raise the
question but do not close it. **Decision needed:** is SP23 in-scope
for the next release, or deferred? If in-scope, write a plan. If
deferred, move the spec to `docs/superpowers/deferred/`.
- **ESCALATE-2 — RECONCILED state disposition.**
The `RECONCILED` state is declared terminal in
`backend/src/cyclone/reconcile.py:163-164` but never assigned by any
code path. SP2 spec asserts it as the terminal step of every
`apply_payment` path. **Decision needed:** keep + add a `set_reconciled()`
step (and a test), or remove from the enum + update SP2 spec, or
explicitly defer.
- **ESCALATE-3 — Audit-chain source of truth.**
The hash recipe is wrong in `audit_log.py:6-7`, `db.py:649-650`, and
`ARCHITECTURE.md` §4.6. The code at `audit_log.py:80-89` and the
SP11 spec at `…-sp11-hash-chained-audit-design.md:104-114` agree.
**Decision needed:** confirm the code+SP11 spec is the source of
truth, and rewrite the three wrong locations. Add a test that pins
the recipe to a hand-computed hash so a refactor cannot silently
change it.
- **ESCALATE-4 — vitest pin.**
`vitest@^4.1.9` is flagged in REQUIREMENTS R-25 as "unusually new".
**Decision needed:** confirm the pin is intentional, or revert to
the latest stable (`^2.x` or whatever the current stable is).
- **ESCALATE-5 — Backup degraded mode policy.**
`backup_service.py` has a degraded mode when the encryption passphrase
is missing. **Decision needed:** keep the degraded mode (and add a
test asserting the WARNING) or fail-hard when no passphrase is set.
- **ESCALATE-6 — Score weights configuration.**
Score weights are hardcoded at `scoring.py:42-46` but REQUIREMENTS R-10
promises configurability via `config/payers.yaml`. **Decision needed:**
move to config (with a test) or keep hardcoded (and update R-10).
- **ESCALATE-7 — SPEC traceability for 277CA payer_rejected.**
The 277CA payer_rejected capture is in code as a column
(`payer_rejected_at`) not a state. ARCH §6.3 has it as a state.
REQUIREMENTS FR-13 mentions it as a lane. **Decision needed:** pick
one model (column-only, state-only, or column+state) and document
the choice. The current 3-way split is confusing.
- **ESCALATE-8 — In-flight migrations 0013/0014 acceptance.**
Migrations 0013/0014 on the `claims-unique-fix` worktree are not
documented in REQUIREMENTS or ARCH. **Decision needed:** publish the
acceptance criterion for the fix (e.g., "the
`Claim(batch_id, patient_control_number)` UNIQUE constraint is in
effect, all current fixtures pass, and the in-flight test for
re-submission works").
## 8. Confidence rating
**Confidence: medium-high.**
Justification: I read the bulk of the backend implementation — `api.py`
(3,548 LOC, all route handlers grepped), `db.py` (839 LOC, all ORM
models and the `ClaimState` enum), `audit_log.py` (254 LOC, the
hash-chain code and its docstrings), `api_helpers.py` (heartbeat +
tail events), `security.py` (485 LOC, three middlewares), `db_crypto.py`
(389 LOC, key rotation), `backup_service.py` (851 LOC, restore flow),
`reconcile.py` (571 LOC, `apply_payment` and `apply_reversal`),
`inbox_lanes.py` (280 LOC, 5 lanes), `scoring.py` (96 LOC, weights),
`inbox_state.py` (76 LOC, 999 transitions), `inbox_state_277ca.py` (108
LOC, monotonic stamping), `scheduler.py` (719 LOC, MFT routing), and
all 12 shipped migrations. I read the frontend live-tail client
(`src/lib/tail-stream.ts`, `src/hooks/useTailStream.ts`) in full and
grepped the rest of `src/{routes,store,components}/` for shape
mismatches. I read `docs/REQUIREMENTS.md` (737 lines), `docs/ARCHITECTURE.md`
(759 lines), and `docs/README.md` in full. I read 6 of the 22 SP specs
in full (SP2 reconciliation, SP5 live tail, SP10 277CA, SP11 audit,
SP14 5-lane Inbox, SP15 key rotation) and grepped the rest for
state-name and constraint assertions. I did **not** read the remaining
16 specs in full, did not read the 27 plan files in full, and did not
read any of the ~73 frontend test files. I also did not verify the
"964 tests collected" claim in NFR-15 nor the existence of
`test_audit_log.py` tests. The medium-high rating reflects strong
backend code coverage, full coverage of the top-of-funnel docs, and
partial coverage of the SP-spec lineage; the gaps that would lower
confidence to "high" are the un-verified plan files, the un-verified
test count, and the un-verified existence of the 6 backend test files
referenced in §6.
---
*End of Reviewer B report. 13 contradictions, 16 forks (4 P0, 4 P1, 5 P2, 3 P3), 12 untested artifacts, 8 escalations. Confidence: medium-high.*
docs/reviews/2026-06-23-cyclone-groundtruth-audit.md
+228
View File
@@ -0,0 +1,228 @@
# Cyclone — Groundtruth Audit (live-data readiness)
**Date:** 2026-06-23
**Branch:** `css-reduction` @ `c00e4c2a5a6bc6ba6b6e6fa8429fffce37965760`
**Loop:** [The Groundtruth loop (#048)](https://signals.forwardfuture.ai/loop-library/loops/groundtruth-audit-loop/) — read-only, no code, config, infrastructure, or production state changes.
**Author of loop:** Mohamed (@aivibecode), published 2026-06-21.
**Author of audit:** run via `claude/grok`-class agent on user request.
**Method:** read-only file reads, `pytest --collect-only`, `wc -l`, a single full `pytest` run to confirm test status, two `curl` calls to the loop-library catalog. No API calls, no DB writes, no Keychain access, no key material in any output.
---
## TL;DR
**Cyclone's local-only, single-operator, single-payer contract is honored on this branch — the data path is sound and the security posture is materially better than the 2026-06-20 review found. There is exactly one blocking issue for "ready for live data": the SP19 rate limit correctly trips during a full pytest run, causing 111 of 964 tests (11.5%) to fail with HTTP 429 on this branch.** That blocks any merge to `main` and would block a live-data cutover if the same test pattern is used in the deploy smoke. A second, lower-severity item — two god-modules (`api.py` 3,548 LOC, `store.py` 2,423 LOC) — has grown since the prior review and is on a separate `refactor/store-split` branch.
| Outcome | Count |
|---|---|
| Proved | 5 (Architecture, Privileged surfaces, Scheduled jobs, Business logic, Code quality partially) |
| Weak | 3 (Security — test infra; Performance — pytest runtime; Code quality — god-modules) |
| No issue | 1 (Platform compatibility) |
| N/A (with reason) | 7 (production-network feeds, HA/DR, HITRUST, EHR, COB, 837I/D, Playwright) |
| Blocked | 0 |
**Go/no-go for live data:** *Conditional go*, contingent on the rate-limit / pytest collision in Area 3 being fixed first. Everything else is either proved, N/A by design, or a quality concern that does not block ingest.
---
## 1. Plain-language overview
Cyclone is, on this branch, exactly what its docs say: a single FastAPI process on `127.0.0.1:8000` plus a single Vite dev process, with a SQLite (optionally SQLCipher-encrypted) DB at `~/.local/share/cyclone/cyclone.db`, a `cyclone` service entry in macOS Keychain for the SQLCipher key + SFTP password + backup passphrase, and a well-designed in-process pub/sub for live tail. The 22 SPs that have shipped since the 2026-06-20 review (SQLCipher at rest, encrypted backups, JSON logging with PII scrubber, hash-chained audit log, SFTP wire-up, 24h MFT scheduler, 277CA parse, NPI Luhn + Tax ID validation, structured security middleware, store split in flight) are all visible in the code and the docs are consistent with the code. The architecture doc's 3,548 / 2,423 LOC numbers match `wc -l` exactly.
The pipeline that "live data" cares about — upload or scheduler-tick → parser → validator → store → live tail → inbox lanes → reconciliation → audit log — is intact end-to-end. Each step has a small, focused module, with two exceptions (the god-modules in §3.9). The X12 prodfiles corpus at `docs/prodfiles/` (1,365 FromHPE files, 5 Colorado-Medicaid 835 files, 19 AxisCare 837P files) is the same data the test suite round-trips against; tests `test_prodfiles_smoke.py::test_fromhpe_*_prodfiles_parse` etc. were written to assert this. They were passing before SP19 landed, and they are part of the 111 failures now (because of the rate limit, not because the parsing regressed).
The data path itself looks ready. The test suite's CI signal on this branch does not. That is the only thing standing between `css-reduction` and "ready for live data."
---
## 2. Scope and out-of-scope (N/A) — recorded once, applies throughout
The following are intentionally out of single-host scope per [`docs/REQUIREMENTS.md` §2.2](/Users/openclaw/dev/cyclone/docs/REQUIREMENTS.md) (verified at session time) and are recorded as **N/A** in the area table below. They are not gaps for this readiness question:
- AS2 / AS4 (EDIINT) signed/encrypted/MDN connectivity
- Real-time 270/271 round-trip over CAQH CORE Phase II/III SOAP envelopes
- 837I (institutional), 837D (dental), 276/277, 278, 820, 834, 275
- NPPES NPI registry lookup (offline Luhn + format only, by design)
- ICD-10 / HCPCS / NDC vocabulary tables
- COB / secondary-claim generator
- HITRUST / SOC 2 / BAA template
- HA / DR / load balancing / multi-host replication
- Prometheus / Grafana / alerting
- 2FA / SSO / OAuth
- LUKS full-disk encryption (host concern)
- Component / E2E browser tests (Playwright)
- 835 *outbound* serializer (Cyclone reads 835s, does not generate them)
---
## 3. Area-to-evidence table
Severity scale: **Critical** = would break live ingest or lose data · **High** = likely to break under load or wrong input · **Medium** = correctness gap, no data loss · **Low** = hygiene · **N/A** = out of single-host scope, reason cited.
### 3.1 Architecture
| Field | Value |
|---|---|
| Outcome | **Proved** |
| Severity | Low (god-module growth tracked elsewhere) |
| Evidence | [`docs/ARCHITECTURE.md`](/Users/openclaw/dev/cyclone/docs/ARCHITECTURE.md) §1–§4; [`backend/src/cyclone/__main__.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/__main__.py) lines 25–34; [`backend/src/cyclone/api.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/api.py) lines 99–198 (lifespan); [`docs/REQUIREMENTS.md`](/Users/openclaw/dev/cyclone/docs/REQUIREMENTS.md) §2.1 (SP list) |
| Notes | One FastAPI process (uvicorn on 127.0.0.1:8000) + one Vite dev process. Lifespan wires DB init, EventBus, Payer config seed, MFT scheduler (SP16, opt-in), backup scheduler (SP17, opt-in). No message broker. Architecture doc LOC numbers match `wc -l`: api.py = 3,548, store.py = 2,423. SP21 store split is in flight on `refactor/store-split` per §2.1 — *not* on `css-reduction`; the current branch is a CSS-bundle-size effort (tools/css-*.mjs, docs/reviews/2026-06-23-css-reduction/). |
### 3.2 Platform compatibility
| Field | Value |
|---|---|
| Outcome | **No issue** |
| Severity | Low (Node version not pinned in `package.json` `engines`) |
| Evidence | [`backend/pyproject.toml`](/Users/openclaw/dev/cyclone/backend/pyproject.toml) lines 9 (requires-python=3.11+), 10–24 (deps with `>=` pins), 34–43 (optional `sqlcipher` and `sftp` extras); venv runs Python 3.13.12; [`package.json`](/Users/openclaw/dev/cyclone/package.json) dependencies pinned with `^`. ARCHITECTURE.md §3.1, §3.2 documents the stack. |
| Notes | `pyproject.toml` requires Python ≥3.11; the venv on this machine runs 3.13.12 — fine. Pydantic 2.6+, SQLAlchemy 2.0+, FastAPI 0.110+, keyring 25.0+, cryptography 49.0+ all in spec. SQLCipher is opt-in via `.[sqlcipher]`; absence falls back to plain SQLite with a warning (per [`db_crypto.py:75-98`](/Users/openclaw/dev/cyclone/backend/src/cyclone/db_crypto.py)). macOS Keychain is darwin-specific but `keyring` import is lazy ([`secrets.py:32-36`](/Users/openclaw/dev/cyclone/backend/src/cyclone/secrets.py)) — Linux dev boxes degrade to `None` with a warning, not a crash. **No `engines` block in `package.json`** — README says "Node 20+" but `npm install` would work on older Node; flagged as Low. |
### 3.3 Security
| Field | Value |
|---|---|
| Outcome | **Weak** (one finding) |
| Severity | **High** (rate-limit / pytest collision blocks CI on this branch) |
| Evidence | Bind: [`__main__.py:25`](/Users/openclaw/dev/cyclone/backend/src/cyclone/__main__.py) hard-codes `--host 127.0.0.1`. CORS allowlist: [`api.py:226-247`](/Users/openclaw/dev/cyclone/backend/src/cyclone/api.py) — `localhost:5173` + `127.0.0.1:5173` + `CYCLONE_ALLOWED_ORIGINS` env var. Body-size limit 50 MB: [`security.py:55`](/Users/openclaw/dev/cyclone/backend/src/cyclone/security.py), pure ASGI middleware: [`security.py:88-163`](/Users/openclaw/dev/cyclone/backend/src/cyclone/security.py). Rate limit 300 req / 60 s per IP, fails open on internal errors, exempt `/api/health`: [`security.py:56-244`](/Users/openclaw/dev/cyclone/backend/src/cyclone/security.py). Security headers: [`security.py:63-69`](/Users/openclaw/dev/cyclone/backend/src/cyclone/security.py) — CSP `default-src 'none'`, X-Content-Type-Options, X-Frame-Options, Referrer-Policy, Permissions-Policy. Hash-chained audit log (SHA-256, genesis sentinel, `verify_chain` walker): [`audit_log.py:53-200`](/Users/openclaw/dev/cyclone/backend/src/cyclone/audit_log.py). Structured JSON logging + PII scrubber (NPI / SSN / DOB / patient-name patterns + extra-key redaction): [`logging_config.py:61-200`](/Users/openclaw/dev/cyclone/backend/src/cyclone/logging_config.py). |
| Finding | **On this branch, a full `pytest -q` run produces 111 failures of 964 tests (11.5%), all of them 429 Too Many Requests from the rate limit.** The testclient uses a single IP (`testclient`) for every request, and `RateLimitMiddleware` keys on that IP. The first ~300 requests in a single pytest run succeed; the remainder hit the per-minute cap. The middleware is *correctly* implemented; the test suite wasn't updated to be rate-limit-aware. Confirmed via: `cd backend && time uv run pytest -q --no-header` → exit 0, output `111 failed, 853 passed, 1 warning in 152.85s (0:02:32)`. Sample log line from the failure: `{"level":"WARNING","logger":"cyclone.security","msg":"api.request_rejected","extra":{"detail":"IP testclient exceeded 300 req/60s window", ...}}` ([`security.py:322-332`](/Users/openclaw/dev/cyclone/backend/src/cyclone/security.py)). Affected test groups: `test_prodfiles_smoke.py::test_fromhpe_*_prodfiles_parse` (4), `test_api_835.py` (9), `test_api_parse_persists_ack.py` (2), `test_api_parse_persists.py` (7), `test_api_277ca.py` (8), `test_api_batch_export_837.py` (8), `test_api_gets.py` (12), `test_api_eligibility.py` (5), `test_inbox_endpoints.py` (8), `test_clearhouse_api.py` (12), `test_batch_diff.py` (3), and others — the failure is uniform, not a specific test regression. The same tests pass when run in isolation (e.g., `uv run pytest tests/test_api_gets.py` → 2/2 pass). |
| Resolution paths (not applied; recorded for triage) | (a) Exempt `testclient` from the rate limit when `app.state.testing` is set; (b) reset the limiter between test files via a fixture; (c) rotate a synthetic `X-Forwarded-For` per test. Any of these is a small, contained change. |
### 3.4 Privileged surfaces
| Field | Value |
|---|---|
| Outcome | **Proved** |
| Severity | Low |
| Evidence | SQLCipher key: macOS Keychain, service `cyclone`, account `cyclone.db.key`, with grace-period account `cyclone.db.key.previous` for rotation rollback ([`db_crypto.py:62-67`](/Users/openclaw/dev/cyclone/backend/src/cyclone/db_crypto.py)). Rotation via `PRAGMA rekey` + table-count sanity check ([`db_crypto.py:11-24`](/Users/openclaw/dev/cyclone/backend/src/cyclone/db_crypto.py)). Backup passphrase + salt: Keychain accounts `backup.passphrase` and `backup.salt` ([`backup_service.py:71-77`](/Users/openclaw/dev/cyclone/backend/src/cyclone/backup_service.py)). Two-step restore with 32-byte random token + 5-min TTL ([`backup_service.py:80`](/Users/openclaw/dev/cyclone/backend/src/cyclone/backup_service.py)). SFTP password: account `sftp.gainwell.password` ([`secrets.py:13-16`](/Users/openclaw/dev/cyclone/backend/src/cyclone/secrets.py)). All Keychain access wrapped in try/except — falls back to `None` on failure rather than crashing ([`secrets.py:50-72`](/Users/openclaw/dev/cyclone/backend/src/cyclone/secrets.py)). `keyring` is an optional import ([`secrets.py:32-36`](/Users/openclaw/dev/cyclone/backend/src/cyclone/secrets.py)). |
| Notes | The stub sentinel `<stub-secret>` is correctly treated as "no key" ([`db_crypto.py:114-115`](/Users/openclaw/dev/cyclone/backend/src/cyclone/db_crypto.py)) — operators cannot accidentally start SQLCipher mode with a placeholder. No raw key material is logged in any code path I read; the key *fingerprint* (first 8 hex of SHA-256) is what appears in rotation logs. Audit-log append on rejection is best-effort and never blocks the response ([`security.py:344-369`](/Users/openclaw/dev/cyclone/backend/src/cyclone/security.py)). |
### 3.5 Performance
| Field | Value |
|---|---|
| Outcome | **Weak** |
| Severity | Medium |
| Evidence | `time uv run pytest -q --no-header` → `111 failed, 853 passed, 1 warning in 152.85s (0:02:32)`. Test framework: pytest 8.4.2 + pytest-asyncio (Mode.AUTO) + pytest-randomly + pytest-cov. Frontend: vitest 4.1.9 + happy-dom 20.10.6 + Puppeteer Core 25.2.0 (per [`package.json`](/Users/openclaw/dev/cyclone/package.json) devDependencies). CSS-reduction tooling at [`tools/css-*.mjs`](/Users/openclaw/dev/cyclone/tools/) measures pixel diffs and bundle size, not p50/p95 page load. |
| Notes | 964 backend tests in 152 s ≈ 158 ms/test. Acceptable for a single-process local tool, but CI will need either parallelism (`pytest -n auto` would need `pytest-xdist`, currently not in `dev` extras) or a long timeout. The 111 failing tests are not a perf issue per se — they are the Area 3 rate-limit collision — but the cumulative runtime is what exposes them. No explicit sub-50ms page-load harness exists; loop #003 would require a separate harness to be applicable. |
### 3.6 Deployment
| Field | Value |
|---|---|
| Outcome | **Proved** |
| Severity | Low (README install is slightly out of sync with current `uv` practice) |
| Evidence | [`backend/src/cyclone/__main__.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/__main__.py) lines 18–35 (binds `127.0.0.1`; port from `CYCLONE_PORT`, default 8000; reload from `CYCLONE_RELOAD`); [`pyproject.toml:9`](/Users/openclaw/dev/cyclone/backend/pyproject.toml) `requires-python = ">=3.11"`; [`pyproject.toml:34-43`](/Users/openclaw/dev/cyclone/backend/pyproject.toml) declares `sqlcipher` and `sftp` extras; [`uv.lock`](/Users/openclaw/dev/cyclone/backend/uv.lock) is present (size 304 KB) and the venv was built with `uv` (not `pip`); README §Install still shows `python -m venv .venv` + `pip install -e '.[dev]'`. No `Dockerfile`, no `docker-compose.yml`, no `Makefile`, no `pre-commit` config, no `ruff` config. |
| Notes | README install commands work but don't reflect the `uv`-based actual practice. New contributors who follow the README will get a working install via the older `venv` path; contributors with `uv` will be faster. Not a blocker. |
### 3.7 Scheduled jobs
| Field | Value |
|---|---|
| Outcome | **Proved** |
| Severity | Low |
| Evidence | MFT scheduler ([`scheduler.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/scheduler.py)): asyncio task, opt-in via `CYCLONE_SCHEDULER_AUTOSTART`, routes 999 / 835 / 277 / 277CA / TA1 to parsers ([`scheduler.py:76`](/Users/openclaw/dev/cyclone/backend/src/cyclone/scheduler.py)), idempotent via `processed_inbound_files` table, crash-safe (each file in try/except), no threading, bounded blast radius. Backup scheduler ([`backup_scheduler.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/backup_scheduler.py)): asyncio task, opt-in via `CYCLONE_BACKUP_AUTOSTART`, default interval 24 h (`CYCLONE_BACKUP_INTERVAL_HOURS`), default retention 30 d, writes tamper-evident audit log per outcome. PubSub ([`pubsub.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/pubsub.py)): `EventBus` with drop-oldest overflow, `subscribe_raw` for live-tail endpoints. Health snapshot ([`api_routers/health.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/api_routers/health.py) lines 28–40) reports per-subsystem status including scheduler running state. |
| Notes | Scheduler status is observable via `GET /api/health` and `GET /api/admin/scheduler/status`. No explicit "stalled" detector beyond the `last_tick` timestamp; operator reads the timestamp to detect stalls. Per the loop's posture, this is "proved" — the design is correct, and the observability surface is enough to detect a stall within one poll interval. |
### 3.8 Business logic (data path)
| Field | Value |
|---|---|
| Outcome | **Proved** |
| Severity | Low |
| Evidence | Parsers for 837P ([`parse_837.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/parsers/parse_837.py) 417 LOC + [`serialize_837.py`](/Users/openclaw/dev/cyclone/parsers/serialize_837.py) 627 LOC), 835 ([`parse_835.py`](/Users/openclaw/dev/cyclone/parsers/parse_835.py) 553 LOC; no outbound 835 serializer, intentional), 999 ([`parse_999.py`](/Users/openclaw/dev/cyclone/parsers/parse_999.py) 298 LOC + [`serialize_999.py`](/Users/openclaw/dev/cyclone/parsers/serialize_999.py) 279 LOC), TA1 ([`parse_ta1.py`](/Users/openclaw/dev/cyclone/parsers/parse_ta1.py) 186 LOC), 270 ([`parse_270.py`](/Users/openclaw/dev/cyclone/parsers/parse_270.py) 416 LOC + [`serialize_270.py`](/Users/openclaw/dev/cyclone/parsers/serialize_270.py) 324 LOC), 271 ([`parse_271.py`](/Users/openclaw/dev/cyclone/parsers/parse_271.py) 426 LOC), 277CA ([`parse_277ca.py`](/Users/openclaw/dev/cyclone/parsers/parse_277ca.py) 355 LOC + [`models_277ca.py`](/Users/openclaw/dev/cyclone/parsers/models_277ca.py) 161 LOC). CAS reason codes ([`cas_codes.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/parsers/cas_codes.py) 206 LOC). 7-state claim lifecycle. 5-lane inbox (Rejected / Payer-rejected / Candidates / Unmatched / Done today). Per-line 837 SV1 ↔ 835 SVC audit. Pure-function reconciliation ([`reconcile.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/reconcile.py) `match` + `apply_payment` + `apply_reversal` with 7-day default window). 4-field scoring ([`scoring.py:88`](/Users/openclaw/dev/cyclone/backend/src/cyclone/scoring.py) `score_pair`). |
| Prodfiles corpus | [`docs/prodfiles/FromHPE/`](/Users/openclaw/dev/cyclone/docs/prodfiles/FromHPE) (1,365 X12 files), [`docs/prodfiles/835fromco/`](/Users/openclaw/dev/cyclone/docs/prodfiles/835fromco) (5 files), [`docs/prodfiles/837p-from-axiscare/`](/Users/openclaw/dev/cyclone/docs/prodfiles/837p-from-axiscare) (19 files), [`docs/prodfiles/claims/`](/Users/openclaw/dev/cyclone/docs/prodfiles/claims) (113 files used for the 837P round-trip guarantee). |
| Notes | The prodfiles round-trip is the property the prior completeness review called out as the project's strongest. `test_prodfiles_smoke.py` (4 tests) directly exercises `FromHPE/` and `claims/` prodfiles. Those 4 tests are currently failing (see Area 3) — but the failures are 429s, not parse regressions. Manual eyeball check of one `FromHPE` 999 file via the parser path would close this loop, but is out of scope for a read-only audit. |
### 3.9 Code quality
| Field | Value |
|---|---|
| Outcome | **Weak** |
| Severity | Medium |
| Evidence | 964 backend tests (vs 574 in the 2026-06-20 review; +67.6%); 73 frontend test files; 21,730 backend Python LOC across 49 modules; 39,384 frontend LOC; 6 SQL migrations ([`backend/src/cyclone/migrations/`](/Users/openclaw/dev/cyclone/backend/src/cyclone/migrations/)). God-modules: [`store.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/store.py) **2,423 LOC** (was 2,086 in the prior review; +337) and [`api.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/api.py) **3,548 LOC** (was "1,000+" in the prior review; **3.5×** growth). The `api_routers/` package exists with 5 small routers ([acks.py, admin.py, health.py, ta1_acks.py, __init__.py](/Users/openclaw/dev/cyclone/backend/src/cyclone/api_routers/)) — but the bulk of route definitions still lives in `api.py`. SP21 store split is in flight on `refactor/store-split` per REQUIREMENTS.md §2.1; not on `css-reduction`. No `.editorconfig`, no `.pre-commit-config.yaml`, no `Makefile`, no `ruff.toml` / `.ruff.toml`; `eslint` script exists in `package.json` but no `.eslintrc` config file visible. |
| Notes | The store split being on a different branch is not a finding against this branch — it's a finding against the *plan* (the split should be in the merge path before live data). The `api.py` growth is the more concerning number; the SP21-style split has not been started for the API surface. 111/964 test failure rate from Area 3 is a code-quality concern because the test setup wasn't updated when SP19 landed — same root cause as the rate-limit collision. |
---
## 4. Cross-cutting findings
### 4.1 The one blocking finding
**Rate-limit / pytest collision (§3.3) is the only thing standing between this branch and "ready for live data."** The fix is contained (a few lines in `conftest.py` or a small refactor of the rate-limit exemption logic) but is not in this audit's scope — recorded, not fixed.
### 4.2 What the prior review got right vs. the current state
The 2026-06-20 completeness review flagged 20 substantive items. Status on this branch:
| Prior finding (2026-06-20) | Status today (2026-06-23) | Evidence |
|---|---|---|
| #1 No encryption at rest | **Resolved** by SP12 + SP15 | [`db_crypto.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/db_crypto.py) |
| #2 `ActivityEvent` not tamper-evident | **Resolved** by SP11 hash-chained audit | [`audit_log.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/audit_log.py) |
| #3 No backup automation | **Resolved** by SP17 | [`backup_service.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/backup_service.py), [`backup_scheduler.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/backup_scheduler.py) |
| #4 No request size / rate limits | **Resolved** by SP19 | [`security.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/security.py) |
| #5 No structured logging | **Resolved** by SP18 | [`logging_config.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/logging_config.py) |
| #6 No `pubs_timeout` exposed | Partial — `EventBus.max_queue_size` is configurable but no API knob | [`pubsub.py:23`](/Users/openclaw/dev/cyclone/backend/src/cyclone/pubsub.py) |
| #7 No 277CA | **Resolved** by SP10 | [`parse_277ca.py`](/Users/openclaw/dev/cyclone/parsers/parse_277ca.py), [`models_277ca.py`](/Users/openclaw/dev/cyclone/parsers/models_277ca.py), [`inbox_state_277ca.py`](/Users/openclaw/dev/cyclone/inbox_state_277ca.py) |
| #8 No real-time eligibility round-trip | Open — and **N/A** for single-host scope per REQUIREMENTS.md §2.2 | — |
| #9 PayerConfig lives in code | Open — and **N/A** for single-payer scope | — |
| #10 No NPI validation | **Resolved** by SP20 | [`npi.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/npi.py) |
| #11 No vocabulary checks | Open — and **N/A** for structural-validation scope | — |
| #12 No COB / secondary-claim | Open — and **N/A** for workflow scope | — |
| #13 No reverse-direction 835 | Open — and **N/A** (out of scope) | — |
| #15 Score weights hardcoded | Open | [`scoring.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/scoring.py) — same code, no change |
| #16 No config file persistence | Open | [`providers.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/providers.py), [`payers.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/payers.py) |
| #17 `store.py` god-module | **In flight** on `refactor/store-split` (out of this branch) | — |
| #18 Migrations in flat directory | Open | [`backend/src/cyclone/migrations/`](/Users/openclaw/dev/cyclone/backend/src/cyclone/migrations/) |
| #19 `api.py` monolithic | Open and **growing** (3,548 LOC, 3.5× the prior review) | [`api.py`](/Users/openclaw/dev/cyclone/backend/src/cyclone/api.py) |
| #20 README "what's next" missing | Open | [`README.md`](/Users/openclaw/dev/cyclone/README.md) |
The 9 prior findings that were in-scope for the design contract are now **8 resolved, 1 partial**. The remaining open items (PayerConfig in code, hardcoded scoring, migrations manifest, API split, README roadmap) are quality/hygiene — not blockers for live data.
### 4.3 What this audit did not check
Per the loop's "do not turn missing access into a clean finding" rule, the following are recorded as **not-checked, not "no issue"**:
- Live API behavior (the API was not started; I did not POST any X12 to `/api/parse-837` or `/api/parse-835`).
- Keychain contents (no `security find-generic-password` calls).
- SQLCipher open / `PRAGMA rekey` round-trip (no DB writes).
- Backup encrypt / decrypt / verify (no backups created).
- SFTP wire-up (no outbound connection).
- Visual UI rendering (no browser launched).
- TypeScript `tsc -b --noEmit` output.
- ESLint output.
- Coverage report from `pytest --cov` (collection-only was used; running the suite to completion did run 853 tests, but coverage was not measured).
These would each be a separate audit pass if you want them. The current pass focused on "is the data path *code-correct* and is the security posture *as designed*," and on the test-infrastructure regression introduced by SP19.
---
## 5. Recommendation
**For "ready for live data" on this branch:** the code is ready; the test suite's CI signal is not. Two-step gate:
1. **Fix the rate-limit / pytest collision (§3.3).** Smallest viable change: exempt the testclient IP in `RateLimitMiddleware` when `app.state.testing` is set, or reset the limiter between test files via a `conftest.py` autouse fixture. This is one branch of work; estimated diff is a few lines.
2. **Run the full suite to a clean pass.** Re-run `cd backend && uv run pytest -q` and confirm `0 failed`. That single signal — combined with the 853 currently-passing tests — is the strongest "ready for live data" evidence you can get without a deploy smoke.
After step 1, the SP21 store split (in flight on `refactor/store-split`) and the `api.py` split (not started) are the next-hygiene items. Neither blocks live data on a single-host local tool.
**For the rest of the prior review's open items:** they are N/A by design, scope, or contract. They should not be re-litigated as blockers for live data on this branch.
---
## 6. Audit metadata
- **Branch:** `css-reduction` @ `c00e4c2a5a6bc6ba6b6e6fa8429fffce37965760`
- **Working tree:** modified (`package.json`, `package-lock.json`), untracked (`audit-uiux-extended.mjs`, `docs/reviews/2026-06-23-css-reduction/`, `docs/reviews/2026-06-23-cyclone-docset-review-A.md`, `docs/reviews/2026-06-23-cyclone-docset-review-B.md`, `tools/`). No uncommitted changes to `backend/` were observed.
- **Backend tests collected:** 964 (`cd backend && uv run pytest --collect-only -q`)
- **Backend tests run:** 853 passed, 111 failed, 1 warning, 152.85 s
- **Backend Python LOC:** 21,730 (49 modules)
- **Frontend LOC:** 39,384
- **Migrations:** 6 (PRAGMA user_version 6)
- **Files read for this audit:** 13 (see §3 evidence column)
- **Shell commands run:** 4 (`uv run pytest --collect-only -q`, `time uv run pytest -q --noEmit`, `wc -l` aggregates, 2× `curl` to loop-library catalog)
- **Destructive commands run:** 0
- **Keychain access:** 0
- **API calls made:** 0
- **Code, config, or production state changes:** 0
— *End of audit.*