# External design review (2026-08-01) — archived with ground-truth corrections

An external principles-based design review of this repo at commit `3591271`,
archived verbatim below, followed by the corrections we verified against the
code on 2026-08-02. Archived because its verified findings drove real changes
(versioned state records; the exact-money December pin; the on-touch
decomposition rules in TECH_DEBT.md) — and because the reject ledger applies
to inbound findings the same as to our own ideas.

**How it was adjudicated:** every checkable claim was ground-truthed before
acting. Two of five findings had factual errors in their evidence or
consequence chains; both corrections are recorded in the appendix, and the
process feedback we sent the reviewer is summarized at the end.

---

## The review, verbatim

```json
{
  "review": {
    "repo": "/Users/dan/Documents/GitHub/thales",
    "commit": "3591271",
    "reviewed_at": "2026-08-01",
    "database_principles": 250,
    "canon_commit": "6bd42da"
  },
  "scan": {
    "code_files": 235, "loc": 53549, "primary_language": "python",
    "excluded": [".claude/worktrees (7507 files, ~557k loc — agent session copies of the repo)"],
    "scan_looks_sound": true
  },
  "findings": [
    {"rank": 1, "principle": "decompose-by-likely-change", "score": 0.97},
    {"rank": 2, "principle": "separate-logic-from-io", "score": 0.94},
    {"rank": 3, "principle": "avoid-primitive-obsession", "score": null, "source": "read"},
    {"rank": 4, "principle": "versioned-data-formats", "score": 0.91},
    {"rank": 5, "principle": "design-for-transparency", "score": 0.90}
  ],
  "dropped": [
    {"principle": "bound-retry-load", "was_score": 0.94, "why": "known-bad metric"},
    {"principle": "prefer-pure-functions", "was_score": 0.95, "why": "fires broadly at this size"}
  ],
  "not_checked": ["concurrency", "statistical-validity"]
}
```

1. **decompose-by-likely-change** — cli.py is 5,247 lines; "longest function
   352 lines". Remedy: give each decision a module.
2. **separate-logic-from-io** — daily.py (1,784 loc) and diagnostics.py
   interleave computation with fetching/writing.
3. **avoid-primitive-obsession** (found by reading) — no `Decimal` anywhere;
   "for order sizing, spend against a cap, and P&L reconciliation against
   broker truth [float] is the wrong representation — reconciliation is
   exactly where a cent of drift surfaces as a spurious divergence and trips
   your fail-closed HALT." Reviewer's own priority pick.
4. **versioned-data-formats** — JSONL state and backups lack version fields;
   readers not audited.
5. **design-for-transparency** — observability is command-shaped, not
   emission-shaped.

Reviewer's notes (their words, condensed): ranking is detection strength,
not priority; finding 3 is the act-first item; whether float drift is real
"logs/ would show it"; cli.py may be mostly declarative ("I did not measure
the split"); statistical validity is outside a design review's competence —
"don't read a clean review as reassurance there."

---

## Appendix — ground-truth corrections (2026-08-02)

- **Finding 3's consequence chain is structurally absent today.** reconcile
  diffs order-ID sets and equity-DATE presence, never monetary values (the
  standing M3 limitation) — a cent of float drift is invisible to it. And a
  reconcile divergence reds the run and alerts; the fail-closed HALT is a
  separate subsystem (execution/safety.py) it does not touch. The reviewer's
  own suggested empirical check, run: zero spurious-divergence incidents in
  the logs, ever. The PROSPECTIVE kernel is accepted and pinned as December
  package item (d): exact-money boundary + value-level reconcile, before the
  first real dollar.
- **Finding 1's numbers:** longest function is 199 lines (ic_triage), not
  352; the file is ~18% declarative (203 Typer lines + 745 docstring lines
  of 5,247), so the reviewer's bikebot-style doubt does not apply. The
  finding survives on evidence the reviewer lacked: three parallel branches
  merge-conflicted in cli.py on 2026-08-01 — decompose-by-likely-change
  manifesting operationally. Accepted as an on-touch rule.
- **Finding 4 accepted and DONE** (SCHEMA_V stamping + tolerant readers,
  2026-08-02) — and the rollout immediately caught TCARecord(**row) crashing
  on the first unknown key: the reader-fragility the finding predicted,
  proven and fixed in the same commit.
- **Finding 5 substantially stale** vs the week of 2026-07-27..08-01:
  run-summary records with state lineage, per-run decision telemetry (A2),
  verdict ledgers and the beacon/alert layer are emission-shaped
  observability. Remainder folds into NORTHSTAR gauge G2.
- **Finding 2 accepted directionally** — the null-close crash (2026-07-30)
  lived exactly in daily.py's interleaved zone — as an on-touch rule; the
  highest-value extraction (construct.py + the parity harness) predates the
  review.
- **The statistical-validity disclaimer, inverted here:** the area the
  review names as its blind spot is where this repo has the freshest
  receipts — PBO null-calibrated against 24 no-skill books and the purge
  leak measured and fixed, both on 2026-08-01 (C2/C4 memos).

Process feedback sent to the reviewer (summary): verify consequence chains
to their sinks, not just premises; run the two-minute checks you name; read
critical money/state paths regardless of scorer output; add change-history
signals (churn, merge-conflict frequency); calibrate recall with
planted-defect repos, not only precision via dropped findings; emit
priority as a first-class field with its arithmetic.