40 lines
2.0 KiB
Markdown
40 lines
2.0 KiB
Markdown
|
# Code Review — risk-engine-and-risk-posture-m20-halt (Slice 18: Pure Core)
|
||
|
|
|
||
|
|
**Reviewer:** qwopus35b | **Implementer:** ornith-35 | **Date:** 2026-07-01
|
||
|
|
**Phase:** code_review
|
||
|
|
|
||
|
|
## VERDICT: REQUEST CHANGES
|
||
|
|
|
||
|
|
### Defect
|
||
|
|
|
||
|
|
**ADR-0007 footer not appended to recommendations (HIGH)**
|
||
|
|
|
||
|
|
The `ADR_0007_FOOTER` constant is defined in `RiskEngine.ts` but never used. The spec requires "Every recommended action has a 'trade-off to think through' frame + ADR-0007 footer." Recommendations include the "trade-off to think through" frame but lack the footer text:
|
||
|
|
|
||
|
|
> "Educational analysis, not investment advice. Verify the underlying data; you are responsible for your own decisions."
|
||
|
|
|
||
|
|
The test named `includes ADR-0007 footer in all recommendations` is misleading — it only checks field existence (`tradeOff`, `explanation`, `severity`), not actual footer content.
|
||
|
|
|
||
|
|
**Fix:** Append `ADR_0007_FOOTER` to each recommendation's `explanation` field in `generateRecommendations()`. Update the test to verify footer text presence.
|
||
|
|
|
||
|
|
### Verified ✓
|
||
|
|
|
||
|
|
- RiskEngine pure (no I/O) — zero network calls
|
||
|
|
- Asymmetry < 1 → `consider_reducing_position` warning
|
||
|
|
- Cluster breach: beginner = hard cap (`warning`), intermediate = warn (`info`)
|
||
|
|
- Actions REWORDED — no imperative trade verbs (`buy/sell/cut/trim`)
|
||
|
|
- "trade-off to think through" frame present on all four recommendation paths
|
||
|
|
- haltCircuitBreaker: 24h cooldown, isHalted true within 24h / false after
|
||
|
|
- Existing positions unaffected by halt
|
||
|
|
- HaltedError clearable via `clearHalt()`
|
||
|
|
- HaltedError is a regular class (NOT TS parameter property) — works with `--experimental-strip-types`
|
||
|
|
- `halt_state` table in schema.sql: `user_id`, `halted_until`, `triggered_by`, `ts`
|
||
|
|
- Pure/cache-deterministic — NO network in tests
|
||
|
|
- 35/35 tests pass under `node --test --experimental-strip-types`
|
||
|
|
- Zero TS errors in risk module (pre-existing errors only in adapters)
|
||
|
|
|
||
|
|
### Deferred Items (Acceptable for Pure Core Review)
|
||
|
|
|
||
|
|
- tRPC `risk.posture` + `risk.haltStatus` endpoints
|
||
|
|
- Server-side `HaltedError` gate wiring into `journal.trade.create`
|