# 2026-09-05 — External design review (r2): maintainer-side verification

**Provenance.** An advocate-contested external design review of this repo at
`cd571b2` arrived 2026-09-05 and is committed verbatim as
`research/2026-09-05_external_design_review.md`. It is the second review of
the day at that commit: the first ran without a repo-advocate and was not
kept; the second re-derived every figure against an advocate and says its
predecessor's headline number was wrong by ~10× *in the repository's favour*.
This memo is the maintainer-side verification: every load-bearing claim
re-checked against the code at `cd571b2` (source unchanged through `3779020`,
this PR's base — only two queue rows landed in between), against committed
state, and — where the reviewer explicitly could not look — against broker
truth pulled read-only on 2026-09-05.

**Verdict up front.** Findings 1–4 and 6 are real and reproduce exactly;
findings 5 and 7 are real but smaller than written; the reviewer's stated
residual uncertainty on finding 1 is now *measured*: **0 refused cancels in
544 checked**; one of its two open questions closes (the TCA median is not
biased by the replacement path); and the broker pull surfaced one finding the
review did not have (OSR-1). Nothing ships code from this PR — records only:
four queue rows, one row moved to `built/`, one dated census note on SAC-1,
and the RESEARCH.md banner.

## What the reviewer could not reach, measured

### Finding 1 — the inert cancel guard has been exposed 546 times and has never fired

The review reproduced by execution that `broker.cancel_order` returning
`False` (what every implementation does) still leads to a full-size market
replacement at `daily.py:1252`, counted 546 cancel-replacements in the
meanrev order log, and wrote: *"the residual uncertainty is the broker's
cancel-refusal rate, which I cannot measure without broker logs."* Measured
here with `thales --sleeve meanrev portfolio orders --days 45 --json`
(3,255 broker orders, 07-23 → 09-04) joined to the local log on order id:

| quantity | value |
|---|---|
| local replacements (`market_replace_of` set) | 546 (544 in window; 07-15 and 07-20 predate it) |
| original limit orders — terminal status | **`canceled` 544 / 544**, `filled_qty` 0 on every one |
| cases where original + replacement fills exceed intended qty | **0** |
| observed cancel-refusal rate | **0 / 544** |
| replacement terminal statuses | filled 412 · **rejected 130** · expired 2 · 2 out of window |

So the exposure count is right, the mechanism is right, and the consequence
has never occurred. The review's `p_manifests 0.5` is the probability that a
replacement *happens*; the probability that a refused cancel is *followed*
by one is much lower. The fix is unchanged and trivial (IGD-1). The 130
rejected replacements are a different defect entirely — see "New" below.

### Finding 2 — magnitude reproduced independently

Recomputed from `data/processed/kelly_ledger.json` (5,000 observations; the
production window is the last `rolling_window_months 24 × momentum_sleeve_size
50 = 1200`, `construct.py:129-141` → `kelly.py:91-92`) with the production
`fraction: 0.5`, appending one hypothetical crash-period return per held name
(66 names in the live snapshot):

| pooled sample | half-Kelly f | today's f is |
|---|---|---|
| as-is (crash period lost) | **0.1089** | — |
| +66 × −20% | **0.03796** | 2.9× larger |
| +66 × −30% | **0.01148** | 9.5× larger |
| +66 × −40% | **0.0** — fail-closed zero-deployment branch | unbounded |

Identical to the review's figures through `build_target_weights` and to
`thales inspect kelly` (f 0.109, 90% CI [0.017, 0.191]). Mechanism confirmed
by reading: `daily.py:1421-1423` returns into `_liquidate_to_cash` upstream
of `:1485`, the **only** call site of `_update_kelly_ledger_returns`;
`_liquidate_to_cash` clears the snapshot at `:1130` before `execute_orders`
at `:1133`; the next selection day hits `:929` (`if not prev_prices:
return`). The engine books the period (`engine.py:711`, `:936`). RWG-1 leg 3
already owns the *halted*-liquidation snapshot wipe; the new mechanism — a
**successful** liquidation also never books the crash — is in no row, hence
**KSP-1**. Plain language: the sizer's memory of the crash is erased on the
day of the crash, and the live book would re-enter roughly three to nine
times larger than the validated strategy.

### The review's TCA open question — closed

It asked whether the go-live gate's `median |slippage| 47.3 bps` (against a
binding 50 bps threshold, `go_live.py:309-314`, `settings.yaml:363`) is
inflated by the replacement path. It is not. Decomposed by fill date:

| selection day | fills | median \|slippage\| bps |
|---|---|---|
| 2026-07-01 | 51 | 43.5 |
| 2026-08-03 | 66 | **79.3** |
| 2026-09-01 | 45 | 15.4 |
| pooled since 2026-06-11 | 162 | **47.3** |

Momentum's only 39 replacements were on 2026-06-11, and there are no TCA
records dated 06-11 in the window, so zero replacement fills enter the
median. The 2.7 bps margin is real and is one bad selection day pooled since
inception with no rolling window — recorded for the December reading (the
interpretation memo already says the gate is operational, not alpha proof);
not a defect.

## Per-finding verification

| # | review's claim | verification | disposition |
|---|---|---|---|
| 1 | `cancel_order` returns a bool and never raises, so the `except` at `daily.py:1236` is unreachable and a `False` falls through to a market replacement; same shape at `:1190` (mid-run halt), `cli.py:3641/:3696/:4405` (`send_email` returns `False`, discarded), and `cli.py:1130-1138` exits 0 on failed orders | **Confirmed by reading** all five sites, all three broker implementations (`alpaca_broker.py:421-427`, `simulated.py:195-200`, `broker.py:125-127`), `notifications.py:74-76`, `cli.py:1105-1138`. **Broker truth: 0/544 refused cancels** | **Queued — IGD-1** (with finding 3) |
| 2 | kill-switch day never books the crash period; snapshot cleared before `execute_orders`; engine books it; magnitude 187% / 848% | **Confirmed by reading**; magnitude **reproduced to five significant figures** | **Queued — KSP-1** (with gap 9) |
| 3 | the test pinning the cancel guard gives the mock a `RuntimeError` no broker raises | **Confirmed**: `test_daily.py:569` | folded into IGD-1 |
| 4 | 2,068 skew rows on 2026-06-19, 07-03, 08-29 (Saturday); no reader filters; GHC/NVR 0 rows in 62 days | **Reproduced exactly**: 892 / 283 / 893 against `get_trading_days`; 0 duplicate `(date, symbol)` keys in 50,438; MASI 1, ASGN 18, HOLX 18, PSTG 19 | **Queued — SKW-4** |
| 5 | `skew-snapshot.yml` is the only scheduled state-writer without a `concurrency:` group; launchd 12:00 PT and cron 19:00Z collide under PDT; the loser permanently loses a capture day | **Premise confirmed** (four workflows carry groups; this one does not). **Consequence not**: the two firings on 09-03 and 09-04 were ~2 h apart (19:00Z dispatch, 21:36Z / 21:19Z cron) and both green; the JSONL writer is idempotent per `(date, symbol)`; on a true same-minute collision the winner's data lands and only the loser reds. The real second-firing defect is the panel's **SKW-3** (term rows silently overwritten), already queued | hygiene: add the four-line block; no row |
| 6 | `queue/open/skw-1` still says open; the fix shipped in `29c6f23` | **Confirmed**; RESEARCH.md's 09-01 banner already says "SKW-1 built" — notebook and queue disagreed | **Moved** open → built this PR |
| 7 | the public subscribe route shares the alerting Gmail credential behind a per-instance limiter | **Confirmed by reading** `route.ts:16-23`, `:49-50`; blast radius is a paper account's alerting, as the review itself narrowed | no row; separate credential when convenient |
| — | near-miss: the RUNBOOK's post-halt recovery is a green no-op (`_record_safety_event` writes a `run_summary`, `already_ran_today` honours it) | **Confirmed**: `pipeline.py:274` → `state.log_run` (`type: run_summary`) → `daily.py:1392` skips; `"Already ran today"` is not in `critical_skips` → exit 0 | **dated note on SAC-1** (a census member) |
| — | 1,356 / 1,358 tests pass in a clean copy | not re-run here (records-only PR); the publish-set scrub test was run before commit | — |

## Corrections to the review (it holds up; these sharpen it)

1. **Finding 5's consequence is wrong, and the plist comment is wrong for a
   different reason than the review gives.** The firings do not land in the
   same minute in practice because GitHub's schedule events run hours late
   (the self-audit's M4), so there is no collision to lose; and even a
   collision loses no data, only the loser's push. Meanwhile the plist's
   claim that "the idempotency gate makes the second run a no-op" is true for
   the skew JSONL and **false for the term stream** — SKW-3 found the second
   firing overwriting in-session term rows with post-close quotes. Add the
   concurrency block as hygiene; the defect that matters is SKW-3's.
2. **Finding 1's "up to 2× intended quantity" needs a refused cancel, and the
   observed rate is 0 / 544.** By this shop's ranking the top two swap:
   **KSP-1 first** — its manifestation is certain the first time the kill
   switch fires, which is the scenario the risk layer exists for — then
   IGD-1. Both fixes are cheap; the order only decides what lands first.
3. **"A failed run exits 0" omits a mitigation.** The fleet digest renders
   `!! N FAILED orders` (`digest.py:550-551`, PR #41/#42). Partial — it keys
   on local submit-time status, which is exactly OSR-1's blind spot — but the
   run is not fully silent.
4. The review counts "15 of 33 meanrev trading days"; the order log holds 35
   days with orders. Immaterial.
5. Finding 4's phrasing "in the dataset this project treats as its moat, and
   which it publishes" is accurate, and the STAMP-1 built-row footer already
   records the relabel-vs-quarantine ruling as the owner's open decision —
   SKW-4 routes the 2,068 rows into that ruling rather than inventing a new one.

## New here — outside the review's scope (OSR-1)

Of the 544 in-window replacements, **130 were `rejected` by Alpaca** at the
next session's 07:00 ET pre-market processing: 45 from the 08-28 batch
(broker `submitted_at` 2026-08-31T11:00Z), 85 from the 08-31 batch
(2026-09-01T11:00Z); 90 buys, 40 sells, all fractional market orders
submitted after the close on the displaced-cron sessions. The local order log
records `OrderStatus.ACCEPTED` for all 130 — statuses are write-once at
submit. The digest counted zero failures (`digest.py:114` counts local
`failed` only); reconcile reported no drift (`reconcile.py:135-160` compares
ID sets, never status); FOS-1's streak logic reads the same log; the daily
audit, whose procedure does say to check rejected orders against broker
truth (`ops/DAILY_AUDIT.md:146`), was quota-starved on those days. The 08-31
displaced-cron memo assumed the batches would "fill at the next open". The
positions self-healed because each run recomputes from broker positions, so
the cost to the never-promotable control was a one-day delay — but the
blindness is sleeve-agnostic and would look identical on a refused momentum
exit. This is the truth-divergence-between-layers class the AI-native audit
named as the recurring failure; the cause of the rejections is not exposed by
the orders endpoint and is OSR-1's first data-plan item.

## The family observation, extended

The 08-29 memo's class — *state advanced or keyed without regard to run
completion* — gains two members from this review, both now listed on SAC-1's
census: the kill-switch `active: True` persisted before the liquidation is
attempted (KSP-1 leg 3), and the HALT `run_summary` that arms the same-day
idempotency gate (the RUNBOOK no-op). The review contributes the
complementary class: **a guard that exists and cannot fire.** Its advocate's
observation is worth adopting verbatim — every finding the reviewer drafted
was a *missing* guard; every one of the advocate's best findings was an
*inert* one — and on a codebase this dense with guards a sweep for absence is
structurally blind to the actual defect class. Its two-stage AST sweep (a
function whose `except` returns a value and never raises, called as a bare
statement inside a `try`) returned exactly the five IGD-1 sites with zero
false positives on ~17k lines. The dual — a `MagicMock` given an exception
`side_effect` for a method no concrete implementation can raise — is an hour
and belongs in this repo's test suite, not only in canon's.

## What shipped vs. queued (all owner-directed, recorded as such)

- **Shipped in this PR: nothing in code.** Records only — the review
  verbatim, this memo, four rows (KSP-1, IGD-1, SKW-4, OSR-1), SKW-1 moved
  open → built with its footer, the SAC-1 dated note, the RESEARCH.md banner.
- **Queue arithmetic:** `queue/open/` goes 13 → 16 against the pre-registered
  12-item cap. Monday's triage overflow-dismisses mechanically with reopen
  conditions; each new row names its fold-partner (KSP-1 → SAC-1, never
  RWG-1; IGD-1 → RPL-1; SKW-4 → SKW-2 / the STAMP-1 ruling; OSR-1 → FOS-1).
- **Direct fixes the owner can make without a row:** the `skew-snapshot.yml`
  concurrency block (four lines the other three scheduled writers already
  carry); a separate sending credential for the web form.
- **Routing note:** drafted in an owner-directed interactive session, not via
  `queue/approved/`; the PR contains a rename, which the records fast path
  denies by design, so a human merges — and merging publishes both documents
  to the site on the next daily export (scrub-checked: 0 BLOCK hits, 0
  redactions).

## Verification commands (re-runnable)

```
# finding 1 exposure + broker truth
python -c "import json,collections; r=[json.loads(l) for l in open('data/state/meanrev/order_log.jsonl')]; print(len([x for x in r if x.get('market_replace_of')]))"
thales --sleeve meanrev portfolio orders --days 45 --json > /tmp/mr.json   # then join on id / client_order_id
# finding 2 magnitude
python - <<'EOF'
import json, numpy as np
from thales.portfolio.kelly import kelly_fraction
pool=[float(x) for x in json.load(open('data/processed/kelly_ledger.json'))['returns']]
def f(p):
    a=np.array(p[-1200:]); w=a[a>0]; l=a[a<0]
    return 0.0 if not len(w) or not len(l) else max(0.0, kelly_fraction(len(w)/len(a), w.mean()/abs(l.mean()))*0.5)
print(f(pool), [f(pool+[c]*66) for c in (-0.2,-0.3,-0.4)])
EOF
# finding 4
python -c "import json,collections,pandas as pd; from thales.utils.calendar import get_trading_days; r=[json.loads(l) for l in open('data/options_skew.jsonl')]; c=collections.Counter(x['date'] for x in r); td={str(d)[:10] for d in get_trading_days(pd.Timestamp('2026-01-01'),pd.Timestamp('2026-12-31'))}; print([(d,c[d]) for d in sorted(c) if d not in td])"
# finding 5
gh run list --workflow skew-snapshot.yml -L 8 --json createdAt,event,conclusion
# TCA decomposition
python -c "import json,numpy as np,collections; r=[json.loads(l) for l in open('data/processed/tca_log.jsonl') if l.strip()]; r=[x for x in r if x['date']>='2026-06-11']; by=collections.defaultdict(list); [by[x['date']].append(abs(x['slippage_bps'])) for x in r]; print({d:(len(v),round(float(np.median(v)),1)) for d,v in sorted(by.items())}, round(float(np.median([abs(x['slippage_bps']) for x in r])),1))"
```

## Addendum — built the same night (2026-09-05, owner-directed)

After the records PR (#150) merged, the owner asked to keep going. Built in a
second, human-merge PR (#151), with the negative controls the shop requires:

- **KSP-1 legs 1–3** (`daily.py`): the in-flight holding period is booked
  before the liquidation (`_book_kelly_returns_before_liquidation`, fail-soft,
  marker-idempotent); the trigger-day `active: True` is staged and published —
  together with the snapshot clear — only after `execute_orders` returns
  without a HALT; the snapshot's `kill_switch` flag is intent AND
  `gross_leverage <= 0.05`, with `kill_switch_intent` recorded alongside. Leg
  4 (HWM reset) confirmed at parity by reading (`engine.py:652`). Five new
  tests in `test_kill_switch_booking.py`; the trigger-day contract in
  `test_kill_switch.py` updated.
- **IGD-1 legs 1, 2, 4** (`daily.py`, `cli.py`, tests): both cancel sites
  read the bool and log at ERROR; the three alert-email sites report a False
  send; the pinning test is parametrized over the real failure signal and the
  transport branch, plus two tests against a queryable `SimulatedBroker`
  (confirming cancel → remainder replaced; refusing cancel → nothing).
- **Held:** IGD-1 leg 3 (exit non-zero on failed orders) — meanrev's
  standing AVB failure would red the workflow daily; the owner rules between
  "any failed order", "failed de-risking sell only" (recommended), and "keep
  green". IGD-1 leg 5 (the dual-sweep lint) not built.
- **Verification:** full suite 1,359 passed / 1 skipped; with the two source
  files stashed and the tests kept, the new tests red on exactly the
  predicted assertions (the refusing-fake test buys the double fill; the
  halted-liquidation test finds the snapshot wiped and `active: True`
  published). Engine untouched — no re-baseline.
- Queue: KSP-1 and IGD-1 moved to `built/` with footers (the rename denies
  the records fast path, so a human merges); `open/` returns to 14 rows.

## Addendum 2 — the STAMP-1 ruling executed, SKW-3 built (2026-09-06, owner-directed; PR #152)

- **The ruling.** Asked for a recommendation, the maintainer found the 2,068
  non-session rows were two classes (the capture commit instants decide it):
  holiday re-serves → **quarantine**; STAMP-class post-close firings →
  **re-session per row to the session `market_session_date(captured_at)`
  names** (08-07 → 08-06, 08-28 → 08-27, 08-29 → 08-28; the 08-07 files hold
  both firings, so 82 chain rows / 33 term rows / 5 skew rows stay). The owner
  accepted. Executed by the new `thales capture-resession` — never a hand
  edit — with the per-row ledger appended to CAPTURES.md ("Data-disposition
  rulings"): 239,179 rows relabelled, 85,028 quarantined, 0 dropped.
- **Recorded honestly:** the first `--apply` dropped 892 rows. The JSONL
  step resolved legacy rows' instants by re-reading the chains files AFTER
  the parquet rewrite; for 08-28 — both a source (→ 08-27) and a target
  (← 08-29) — that handed the 08-29 firing's instant to the 08-28 rows,
  which then stayed and collided with the incoming ones. The command's own
  `dropped` count made it visible in the same line; the store was restored
  from git (plus `git clean` of the six new target files the checkout does
  not remove), the plan now carries the instants it resolved, a chained
  source-and-target case is pinned in `tests/test_data/test_resession.py`,
  and the re-apply reported `dropped 0`. Lesson for the next migration
  command: compute every input at plan time, apply from the plan only.
- **Bookkeeping:** `captures.ACKNOWLEDGED_GAPS` now carries the two true
  shortability holes (08-07, 08-28) the writer's file-idempotency caused;
  the deliberately-open 08-06 note it replaced is the ruling's citation.
  `capture-qa` green; the gap detector's only unexplained entries are the
  pre-existing 08-26 pit / 08-27 order_log + portfolio_snapshots holes of the
  no-run day, outside this PR's scope. `pit_snapshots` deliberately untouched.
- **Also in the PR:** SKW-3 (the re-fire collects only the streams a name is
  missing from; workflow hint corrected; commit message stamps the session),
  the `concurrency:` group on `skew-snapshot.yml` with a lint that every
  scheduled state-writer has one, and `load_skew(sessions_only=True)`.
  SKW-3 and SKW-4 moved to `built/` (SKW-4 leg 3, the per-name coverage
  census, re-routed to SKW-2).

## Addendum 3 — IGD-1 leg 3 ruled; FOS-1 built (2026-09-06, owner-directed; PR #154)

- **The ruling.** Neither "red on any failed order" nor "red on any failed
  sell" survived contact with the facts: AVB's standing failure IS a
  de-risking sell (an emergency exit), so both would red meanrev daily. The
  owner accepted the third shape — **escalate NEW de-risking failures, report
  standing streaks** — and it is one build with FOS-1 because both need the
  same per-symbol streak computation (`execution/failed_streaks.py`, sessions =
  the days the sleeve ran).
- **What ships:** `thales run` exits 4 on a fresh sell/close failure (a fresh
  buy leaves the book under-invested, not exposed — not escalated); the digest
  names NEW and STANDING failures distinctly, gives an emergency-exit streak
  its own line, and turns 🔴 only for a new de-risking failure; every failed
  order record carries the broker's rejection text. Diagnosing AVB is now a
  matter of reading tomorrow's record — no order for it has reached the broker
  in ten days, so the failure is at submit, and the text will say why.
- **Verification:** streak/rule unit tests, digest render tests (streak vs
  fresh; fill resets; standing alone is amber), pipeline test for the error
  text, and a source-pinned contract for the run command's exit code (the CLI
  builds a live broker and is not unit-invokable).
- Queue: FOS-1 → `built/`; IGD-1's footer records the ruling; the dual-sweep
  lint (IGD-1 leg 5) stays open as the one unbuilt leg.