sentiment-engine/prod/docs/EXEC_BEST_PRACTICE_AUDIT_20260714.md

# Execution layer — adversarial best-practice audit (the "weird pass")

**From**: Fable, 2026-07-14. **Scope**: `pink_direct.py` + `exec_router.py` +
`SPEC_UNIFIED_EXEC_LAYER_20260714.md` vs industry / best-of-breed execution practice.
**Method**: current-web grounding (cited) + best-of-breed internals (nautilus_trader —
which this repo *vendors* — FIX ClOrdID discipline, professional OMS drop-copy patterns,
Hummingbot in-flight-order tracking). Ranked most-severe first. **Nothing here is a
BLUE/PINK edit order** — it is the port's correctness checklist + escalations for the operator.

---

## HIGH

### H1 — `float` for all money/price/size math (PINK). Against near-universal consensus.
`pink_direct.py` casts everything to `float`: `_slot_to_position_dict` (L55-78), capital,
size, PnL, fees throughout. IEEE-754 cannot represent most decimal money exactly; error
accumulates across many fills — and PnL/fee/capital is precisely where verification is
expensive (doctrine anti-pattern #4). Industry consensus is blunt: *never* use float/double
for monetary math; use arbitrary-precision decimal or integer minor units (rounding every op
*can* be made exact, but a decimal type is the safe default). **The irony**: this system
sits on **nautilus_trader, which ships fixed-precision `Price`/`Quantity`/`Money` value
types** — and `pink_direct.py` casts them straight to raw `float` at the boundary.
- **Mitigation PINK already has**: the K≈E reconcile gate re-anchors capital to exchange
  truth on every balance-bearing `ACCOUNT_UPDATE` (L726-732), so drift is *bounded and
  caught*, not unbounded. That is a backstop, not correctness.
- **Verdict**: real deviation, bounded in practice. **The Unified spec + `contract.py`
  already fix it** — everything is `Decimal` at the boundary (contract §2, done). The port
  must NOT re-inherit float. This finding *validates* the rebuild direction.

## MEDIUM-HIGH

### H2 — crash-recovery state lives in `/tmp`. Against durable-state practice.
`_KERNEL_STATE_PATH = /tmp/.pink_kernel_state.json` (L87) holds crash recovery + fee
calibration + session-to-session continuity. `/tmp` is volatile (cleared on reboot /
`tmpwatch`), world-traversable, and racy. For a subsystem whose entire purpose is surviving
a crash, persisting its recovery tape to the one directory guaranteed to be wiped is
self-defeating. Best practice: a durable app-state dir (`$XDG_STATE_HOME`, `/var/lib/...`),
`0600` perms, **atomic write (tmp-file + `rename`)** so a crash mid-write can't corrupt it.
The append-only-log school goes further: the event log is the source of truth, snapshots
merely accelerate recovery. **Port**: `exec_unified` persists working-order state to a
durable, atomically-written path — never `/tmp`.

### H3 — shared VST account + symbol-membership fill ownership. Architectural foot-gun.
`_fill_is_ours` (L588-611) attributes fills on an account shared by PINK / PRODGREEN / BLUE /
manual, using **symbol membership** because BingX WS does not echo our `clientOrderId`.
Best practice is unambiguous: **isolate strategies by sub-account (or dedicated API key)** —
never multiplex independent trading systems on one account; margin and fills cross-
contaminate. Symbol membership is fragile precisely where it matters: two systems trading
the *same* symbol on the shared account misattribute each other's fills. The reseed-on-
`ACCOUNT_UPDATE` bounds the damage, but the root (shared account) is the deviation.
- **Escalation**: the real bug is the venue not echoing `clientOrderId` on WS. That should
  be pushed to BingX and/or worked around with a per-strategy sub-account. This is the
  correct long-term fix, not a smarter membership heuristic.

### H4 — clientOrderId idempotency on retry (SPEC + my `contract.py` gap).
Industry rule (and FIX ClOrdID law): a **unique key per ATTEMPT**, never reused; a retry
after an INDETERMINATE submit needs a **new** venue id (exchanges reject duplicate
clientOrderId) while preserving linkage to the parent (FIX `OrigClOrdID`). TTL of the dedup
key must exceed the retry window.
- **PINK does this right**: retry builds `new_tid`, `intent_id=new_tid` (L1210-1212) — fresh
  id per retry.
- **My `contract.py` does NOT yet**: `client_order_id_seed` returns the raw `request_id`
  (contract.py). If two attempts derive the same venue id, the retry-after-INDETERMINATE
  collides and is rejected — the exact failure the idempotency is meant to prevent. **Fix**:
  the dialect must mint `u-<request_id>-<attempt_n>` (fresh per attempt, parent-linked) AND
  enforce venue charset/length limits (BingX caps clientOrderId length). Add an `attempt`
  counter to the working-order state; the seed alone is insufficient.

### H5 — WS-primary / REST-failover without explicit sequence-gap detection.
PINK starts the WS account stream as **primary** with poll failover inside it (L386-389).
Best practice: WS is a **delta** channel; the REST snapshot is the reconciliation source of
truth; **seqnum gaps must be detected and force a REST resync**; the append-only event log
is the ultimate truth.
- **PINK partially aligns**: `pump_venue_events` reconciles via `venue.reconcile()` and the
  kernel dedups by `seen_event_ids` / `_last_settled_pnl` (L1236-1237) — that IS event-log-
  style idempotent application. Good.
- **Gap**: no visible WS **sequence-number gap detection**; the SNAPSHOT-burst barrier is
  still a spec TODO (§19). The 5 s own-fill hot-window (L1055, L1323) is a *time-based
  proxy* for "WS may lag REST" — a heuristic where seqnum + REST-snapshot reconciliation is
  the rigorous mechanism. **Port**: add explicit order-stream gap detection → forced REST
  resync; keep the hot-window as belt-and-braces, not the primary guard.

## MEDIUM

### M1 — bare `except Exception: pass` swallowing. The project's own named house-demon.
Multiple silent swallows: venue back-ref (L351), own-fill symbol seed (L370), asset-picker
observe (L1428), and others. Some are *correctly* fail-safe with logging ("Hz read failure
must never affect trading", L881 — that one logs). The **intent is right** (telemetry/Hz
failure must never touch trading), but a bare `pass` in a hot loop hides the bug that
eventually bites — and the repo's TESTING_DOCTRINE explicitly names "silent failure is the
house specialty." Best practice: catch the *specific* exception, always log at ≥debug with
context. **Port**: fail-safe guards keep the guard, lose the silence — log every swallow.

### M2 — dead-man's-stop orphan + flip risk (already spec-flagged; make it a HARD gate).
Attaching STOP_MARKET at ~2× software SL (spec §6/§10) is best practice for silent-death
survival — but two known foot-guns: (a) the attached stop must **auto-cancel on position
close** or it orphans (the proto-PINK hell the spec §6 already flags); (b) it must be
`reduceOnly`/`closePosition` so a fire cannot *open* a reverse position. **Verdict**: the
spec flags (a) as a VST-verify TODO — good, but it must be a **hard pre-live gate with a
mutation-tested assertion**, not a checklist line. `ProtectiveSpec.reduce_only=True` (my
contract) covers (b); keep it enforced.

### M3 — live sign inconsistency `pink_direct.py:673` (`# negative = rebate`).
Contradicts today's Q1 finding (BingX commission negative = **debit/cost**). If
`bingx_user_stream` passes the raw venue sign into `event.fee`, the K-fold books a cost as
income. Bounded by the K≈E gate + `abs()` in calibration (L549), so likely not bleeding —
but it is a real sign inconsistency. **Not fixing** (BLUE/PINK untouchable); the port's §12
friction telemetry must get the sign right at its own boundary. Already logged in the port
inventory §4.2.

## LOW

### L1 — hardcoded cadences / magic numbers (1 s TTL, 5 s window, 15 % dev, `1e-9`).
Best practice + the spec's own NFR: named constants with provenance, cadence injected. PINK
partially violates; the port fixes it (`_constants.py` + injected clock). Minor.

### L2 — "GTX is the ONLY certified fill-improvement technique" — conservative, not wrong.
Industry also uses pegged/midpoint/iceberg/adaptive-limit. The spec is *honestly* scoping to
the only empirically-certified technique for THIS venue/history — disciplined, not a
deviation. The tier ladder (T2+) leaves room for the others. No action.

---

## Where PINK / the spec are AT or ABOVE best practice (balance — do not "fix" these)

- **"Unknown is never flat" / INDETERMINATE triage** (spec §5, L5 doctrine): matches best-
  practice execution-truth handling — never synthesize a REJECT; let the E-feed FILL settle.
  This is *better* than many retail bots, which optimistically assume failure = flat.
- **Idempotent replace = cancel-confirm → place (no blind amend)** (spec §5): FIX-grade.
- **Telemetry off the exec path, lossless side-lane** (spec §12, b46ebd2 lesson): textbook —
  no I/O in the hot loop.
- **Fail-safe requote gate; venue position is the truth** (`_exec_safe_to_requote`, L1049):
  fail-closed on ambiguity is exactly right ("a skipped entry is safe, a doubled one is not").
- **Three price truths never conflated** (mark = geometry, OB-top = execution, last = parity;
  spec §9): matches how professional risk/exec systems separate mark vs trade price.
- **`seen_event_ids` idempotent event application** (L1236): the append-only-log dedup
  discipline the sources call the ultimate source of truth.
- **New `contract.py` uses `Decimal` end-to-end**: directly fixes H1 for the port.

## Net verdict

The **spec is sound**; its deviations (H4 clientOrderId-per-attempt, H5 seqnum gaps, M2
hard-gate the stop) are *additions*, not corrections. The **real deviations are PINK-side**
(H1 float, H2 /tmp, H3 shared account, M1 silent swallows) — and the single most important
one (float money) is **already fixed by the rebuild's Decimal contract**. The audit thus
argues *for* the port, and hands it a concrete checklist.

## Sources
- Idempotency / clientOrderId: [Token Metrics](https://www.tokenmetrics.com/blog/idempotency-keys-order-placement), [Coinbase idempotency](https://docs.cdp.coinbase.com/api-reference/v2/idempotency), [George Tsiokos — idempotent orders](https://george.tsiokos.com/code/2026/idempotent-orders/)
- Float vs decimal money: [Modern Treasury — floats don't work for cents](https://www.moderntreasury.com/journal/floats-dont-work-for-storing-cents), [DZone — never use float for money](https://dzone.com/articles/never-use-float-and-double-for-monetary-calculatio), [evanjones.ca — you *can*, with rounding](https://www.evanjones.ca/floating-point-money.html)
- WS vs REST reconciliation / seqnum: [FIX vs REST vs WebSocket](https://brokeret.com/blog/fix-vs-rest-vs-websocket-trading-api-decision-matrix), [Coinbase Advanced Trade WS](https://docs.cdp.coinbase.com/coinbase-app/advanced-trade-apis/websocket/websocket-overview), [Kraken WS FAQ](https://support.kraken.com/articles/360022326871-kraken-websocket-api-frequently-asked-questions)