# TraderAlign — Narrative / Recommendation Logic Audit ("Type 2" Completeness Bugs)

| | |
|---|---|
| **Status** | Complete — this is a research/audit document, not a changelog. No code was changed as part of producing this report. |
| **Scope** | Every coaching, recommendation, insight, or narrative string in the app — Active Coaching (Journal), Trading Archive, Reports commentary, Colex chat, Calculator warnings, Trader Profile. |
| **Created** | 2026-08-09 |
| **Origin** | A direct follow-on to `SINGLE_SOURCE_OF_TRUTH_AUDIT.md`, but a **different bug class**, not a continuation of that work. |
| **Method** | Six parallel/serial research passes (four completed as parallel background agents before a usage-limit interruption; the remaining two — Reports and Colex — completed directly, by hand, using the same methodology), followed by direct personal verification (reading the actual source, not trusting a summary) of a representative sample of every HIGH-severity finding before inclusion here. |

> **Read this before fixing anything found here.** Per the governing instruction for this audit: do not fix anything yet, even the obvious ones. This document is for review with Kevin first; the fix plan gets scoped together afterward, one item verified at a time — same discipline as the SSOT migration.

---

## The bug class, precisely

**Type 1** (SSOT audit's subject, separately closed out): the same *fact* computed independently in more than one place, sometimes disagreeing. Fixed architecturally — one function, every consumer reads from it.

**Type 2** (this audit's subject): a fact that is already computed correctly and consistently — no disagreement anywhere — paired with narrative/recommendation/coaching text that does not correctly branch across the full range of values that fact can actually take. The fact is right. The sentence built from it can still be false, because the code generating the sentence assumed one default narrative applies regardless of the fact's actual state.

**The calibration example** (confirmed, verified, used to brief every research pass): `public/journal.php` ~line 2818 (and its live-account counterpart ~line 3143):
```php
$recoveryTarget = $currentPhase['account_size'];
```
Hardcoded to the phase's starting balance with no check for whether the account's current balance is already above it. The "Until You Recover To $X" card renders unconditionally. Confirmed live: an account at $5,255.29 (+5.11%, already in profit) still shows "Until You Recover To $5,000.00" — a recovery message for an account that isn't in drawdown at all.

---

## Executive Summary

Six areas were audited. **The pattern repeats structurally in every one of them**: this codebase has, in several places, already built real n-aware / state-aware discipline into sibling functions (Reports' `progressive_insights_engine.php` explicitly tiers n=1/small-n/confident across `behavioral()`/`session()`/`risk()`/`drawdown()`; Trading Archive's `segmentMetrics()` explicitly gates on `MIN_GROUP_TRADES`) — and then a neighboring function in the *exact same file*, built at a different time or for a different purpose, was never given the same guard. The clearest, most repeatable evidence: `progressive_insights_engine.php`'s `session()` explicitly handles "only one session traded" as its own distinct narrative branch, while its three siblings in the same file — `edge()`, `strategyPerformance()`, `crossStrategy()` — do not, and can render "led the portfolio... worth reallocating capital toward it" for an account with exactly one active strategy and zero real competition.

Findings, personally verified where marked, by area:

- **Active Coaching (Journal)**: 6 findings. The confirmed calibration bug has at least one sibling in the same functions (`renderRnCoachingSectionLive()`'s "Drawdown From Peak" tile — **personally verified**), plus a second, separately-documented-as-fixed-elsewhere-but-not-here instance of the "hardcoded fallback presented as real declared config" bug class (`rnPhaseTargets()`).
- **Trading Archive**: 11 findings, 8 in the live, reachable single-account pipeline (`buildAccountSuccessJourney()`/`buildAccountFailureJourney()`), 3 in a confirmed-dead code path (`TradingArchiveEngine::build()`, zero call sites anywhere in the app — ranked separately, not mixed into the live severity ranking). The top live finding (**personally verified**) is self-contradictory on the same rendered page: "What Created The Pass" narrates an account's already-superseded Phase 1 as its "current" stage while the KPI card one column over correctly shows the real current stage as Phase 2.
- **Reports**: 3 findings, found via direct read (not an agent — see Method). The clearest is a single-strategy/single-instrument account getting "led the portfolio... worth allocating more capital toward it" language with zero real competition — while the sibling `session()` function in the identical file explicitly handles this exact scenario correctly, proving the gap is an omission, not a design choice.
- **Colex chat**: 6 findings, found via direct read. The most severe: `generateComparisonResponse()` defaults a period-over-period P&L change to `0%` (and labels it "(improved)") whenever the prior period has no data at all — including a trader's very first-ever reporting period, which fabricates a positive trend for someone with no trend to report.
- **Calculator**: 10 findings from the completed agent, two of the top ones **personally verified** by direct read. The clearest: "GOOD SETUP — stay with the plan" is gated on reward:risk alone, with zero check against margin state, breach state, or TP direction — and can render directly above a "This trade will trigger a margin call" blocker on the same screen, on the live trade-entry page.
- **Trader Profile**: 12 findings from the completed agent, two **personally verified** by direct read. The clearest: the Emotional Pattern streak-position rungs silently default to a `'baseline'` tier — rendered as "Your default state — no active losing streak" — whenever risk data happens to be missing at a position that is, by construction, an active losing-streak position.

**Total: 48 findings** across 6 areas. All personally-spot-checked claims (8 of 48, one to three per area) confirmed true on direct source read — no fabricated or hallucinated agent findings were detected in the sample checked, which is grounds for reasonable (not absolute) confidence in the unverified remainder, but every HIGH item should still get its own quick confirmation before a fix is written, per the standing instruction.

---

## 1. Active Coaching (`public/journal.php`, Right Now / Active Journeys)

| # | Function | Finding | Severity |
|---|---|---|---|
| 1a | `renderRnCoachingSection()` ~2818, `renderRnCoachingSectionLive()` ~3143 | **[CALIBRATION BUG]** `$recoveryTarget = $currentPhase['account_size']` (prop) / `$peakEquity` (live) — "Until You Recover To $X" renders unconditionally, no check whether current balance/equity is already above the target. **[FIXED 2026-08-10]** | HIGH |
| 1b | `renderRnCoachingSectionLive()` ~3199-3208 | **[PERSONALLY VERIFIED]** "Drawdown From Peak" tile branches on `$inDrawdown` (= risk tier ≠ 'healthy') instead of on the real `$ddAmount` value. Tier stays 'healthy' up to just under 5% of peak equity in drawdown for live accounts (`ddUsedRatio < 0.25`, and `ddUsedRatio = ddFromPeakPct / 0.20`) — so any real drawdown up to ~5% of peak renders the literal string "$0.00 / At peak," discarding a real, already-computed, nonzero `$ddAmount`. **[FIXED 2026-08-10]** | HIGH |
| 2 | `rnPhaseTargets()` (~1055-1071), consumed by `rnProgramConfigCards()`/`renderRnPhaseCard()`/`renderRnFundedCard()` | Hardcoded fallback percentages (`$defaults = ['phase_1'=>8,'phase_2'=>5,'phase_3'=>5]`, `max_dd_pct` fallback `10`) are indistinguishable in the UI from a real, firm-declared value — rendered as "Maximum Drawdown: $400.00 (8%)" with full visual confidence either way. This is the exact bug class CLAUDE.md documents as found-and-fixed via `TradingArchiveEngine::realDeclaredMaxDdPct()` (which returns `null` instead of a silent default) — but that fix was applied only inside `TradingArchiveEngine.php`'s own difficulty-board computation. `rnPhaseTargets()` itself — the function Active Journeys' own phase/funded cards call directly — was never given the equivalent fix. **[FIXED 2026-08-10]** | HIGH |
| 3 | `rnProgramConfigSummary()` (~1279-1293), Funded card | The Funded branch of `rnPhaseTargets()` defaults `profit_target_pct` to `0` (not 8/5/5 like Phase 1-3), so a genuinely missing scaling-target config and a genuinely-declared "0% required" are indistinguishable — both render "No Target Required — Declared as 0% for this phase" with equal confidence, contradicting this same code's own doc comment claiming the zero-vs-missing distinction is handled. **[FIXED 2026-08-10]** | MEDIUM |
| 4 | `renderRnCoachingSection()` ~1790-1800, ~2787-2833 | If `AnalyticsEngine::getChallengeMetrics()` throws or errors (caught try/catch), `$overallUsedRatio`/`$dailyUsedRatio` silently default to `0.0`, forcing tier `'healthy'` and the confident "within a healthy range" narrative — no distinct "we don't have real data for this account" state. Lower-confidence reachability: the normal picker flow guarantees a `challenge_accounts` row via INNER JOIN, so the trigger requires a genuine mid-computation exception. **[FIXED 2026-08-10]** | MEDIUM (low reachability confidence) |
| 5 | `renderRnPhaseCard()` ~2487-2507, `rnBuildPhaseCardFromRows()` ~1412-1413 | "Estimated Wins Remaining" only special-cases `status === 'passed'`; a `'breached'`/`'closed'` phase falls into the same branch as "still working toward it," implying continued progress on an ended attempt. Appears unreachable today — `rnFetchActivePropAccounts()` already excludes breached/closed accounts from the picker — but would misfire immediately if that filter is ever loosened. **[FIXED 2026-08-10]** | LOW (latent) |
| 6 | `rnCoachingSetupText()`/`rnCoachingEntryText()`/`rnCoachingRiskText()`'s trailing clause | Guard checks item *count*, not whether joined text ended up non-empty after trimming — a checklist category with rows but blank text renders "Before you do anything else, confirm: . Don't move to entry..." (grammatically broken, confirms nothing). Note `rnCoachingExecutionText()` correctly checks `$joined !== ''` — this pattern was fixed in one sibling function and missed in two others. **[FIXED 2026-08-10]** | LOW |

**Checked, correctly guarded, not reported further**: `rnComputeRiskRecommendation()`'s full tiering; `rnGoalProgress()`/`rnGoalCoachingText()`'s achieved-vs-in-progress split (correctly collapses to one success state, matching the same pattern the calibration bug violates); `rnPsychMeetsStrategyText()`'s branch coverage; `rnPsychHistoricalComparison()`/`rnChecklistHistoricalComparison()`'s n≥3/n≥6 gates; `rnDeriveTerminalStatus()`'s phase-rank/`array_key_last()` safety check; `rnPhasePassedMessage()`/`rnPhaseBreachedMessage()`/`rnFundedBreachedMessage()`'s distinct-tone coverage; `drafts.php`'s `checkLifecycleEnforcement()`/`intelligentlySelectAccountStatus()`/`groupSummary()`.

> **Fix note (2026-08-10), §1.1a + §1.1b.** Re-verified both against current source before fixing (line numbers had drifted slightly from accumulated uncommitted work, logic unchanged) — fixed together since they're the same bug in sibling functions. §1.1a: added `$isRecovering = $currentBalance < $recoveryTarget` (prop) / `$isInDrawdown = $ddAmount > 0.005` (live, reusing the already-computed real `$ddAmount` rather than the risk-sizing tier) and wrapped the "Until You Recover To" card in a conditional — when already above target/at-or-above peak, it now collapses to a single "Recovery Status / ✓ Above $X" success state, matching the same pattern `rnGoalProgress()`'s `achieved` branch already used elsewhere on this page (the doc's own "Checked, correctly guarded" note above cites this exact pattern). §1.1b: the "Drawdown From Peak" tile now branches on the same real `$isInDrawdown` fact instead of `$inDrawdown` (the risk-sizing tier, which only flips at ~5%+ of peak) — a real drawdown under that floor now correctly shows its real dollar amount instead of "$0.00 / At peak." Verified live against the real running app (temporary dev-only auto-login helper at the project root, deleted after use): the primary 348-trade test account's default-selected Phase 1 account (`0d54b247-…`, real balance $4,803.64 vs. a $5,000 target) correctly still shows "Until You Recover To $5,000.00" (the in-drawdown case, unaffected); a real Funded account already in profit (`25678454-…`, "Maven 100K - Attempt 1") correctly now shows "Recovery Status / ✓ Above $100,000.00" instead of the old always-on recovery message — the exact bug scenario the audit reported; the Live Accounts tab (no account currently in drawdown) correctly shows "Recovery Status / ✓ At Peak Equity" for the recovery card and the "Drawdown From Peak" tile. All renders HTTP 200, zero PHP errors. `php -l` clean.
>
> **Fix note (2026-08-10), §1.2.** Re-verified `rnPhaseTargets()`'s exact fallback cascade against current source before fixing. Ported the already-shipped `TradingArchiveEngine::realDeclaredMaxDdPct()` pattern: a new `$realVal()` closure returns the first real (non-null, non-empty, numeric) candidate from the same key cascade the function already checked, or `null` — exposed as new `profit_target_configured`/`max_dd_configured` booleans alongside the unchanged, still-always-populated numeric values (so the live progress-bar math every existing caller depends on is untouched). Threaded through `rnBuildPhaseCardFromRows()`'s returned `$phase` array, `rnProgramConfigSummary()` (two new optional trailing params, defaulted `true` so any caller not yet updated keeps its exact prior behavior), and `rnProgramConfigCards()` (an estimated figure's sub-label now reads "Estimated — not declared by this firm" instead of a bare percentage, for both Profit Target/Target Balance and Maximum Drawdown/Breach Level). All three real call sites (`renderRnPhaseCard()`, `renderRnFundedCard()`, and `renderBreachTradesView()`'s terminal-view config) updated. **A second, related bug found and fixed while porting this fix, in the same function's third consumer**: `renderBreachTradesView()`'s own doc comment already claimed this call site uses "real-or-null values, since a finished account's own record should never imply a rule it didn't actually have" — but the code itself did `rnPhaseTargets(...)['profit_target_pct'] ?? null`, a no-op, since `rnPhaseTargets()` never itself returned null there. Fixed to read the new `profit_target_configured` flag instead, so a finished/breached account with no real declared target now genuinely shows "Not specified" again, matching what the comment already (incorrectly) claimed. Verified live against the real running app: the same real Phase 1 account (`0d54b247-…`, real declared `phase_1_target_pct: 8`/`max_dd_pct: 8`) correctly shows "8%" (not "Estimated") for both chips — confirms the fix doesn't over-trigger the disclosure for genuinely declared config. Then, to exercise the actual missing-config path (no real account in this database currently lacks a declared target), temporarily mutated that account's real `challenge_accounts.rules` JSON to remove both keys, confirmed the UI correctly switched to "$400.00 / Estimated — not declared by this firm" (Profit Target) and "$500.00 / Estimated — not declared by this firm" (Maximum Drawdown, computed from the 10% fallback since `max_dd_pct` was also removed) with matching disclosures on Target Balance/Breach Level, then restored the original rules JSON and re-confirmed via direct query and a fresh render that the account's real "8%" config displays returned exactly as before. Also confirmed the breach-terminal-view call site (a real breached account, `006811b4-…`) renders its real declared "8%" config correctly, unaffected by the fix. All renders HTTP 200, zero PHP errors. `php -l` clean. `git diff --stat` reviewed before committing — this file's working tree carried a large amount of accumulated pre-existing (uncommitted, from many prior sessions) changes unrelated to this fix; per this codebase's established discipline, that bulk was committed separately first (`public/journal.php: commit accumulated pre-existing working-tree changes`) before this fix's own isolated 99-insertion/13-deletion diff was committed on top.
>
> **Fix note (2026-08-10), §1.3.** Re-verified against current source — confirmed `rnProgramConfigCards()`'s "No Target Required" chip branched only on `$profitTargetZero` (whether the resolved percentage is `<= 0`), never checking the already-computed `$profitEstimated` flag (`rnPhaseTargets()`'s own `profit_target_configured` boolean, ported to this function by the §1.2 fix immediately above — the fix for this row builds directly on top of it, not a separate mechanism). Added a real-vs-estimated branch to both the Profit Target chip's sub-label ("Declared as 0% for this phase" only when truly configured, else "Not declared by this firm — estimated as 0%") and the Target Balance chip's sub-label (which shares the same `$profitTargetZero` condition and was carrying the identical ambiguity one chip over). Verified live against the real running app, with real data on both sides — no mutation needed for the negative case: user `278d5983-…`'s real Funded account `438c1b6d-…` ("maven 100K - Attempt 2") has a real `challenge_accounts.rules` JSON with no `funded_profit_target`/`funded_max_loss`/generic `profit_target_pct` key at all, so `rnPhaseTargets()` genuinely falls to its unconfigured 0.0 default for Funded — confirmed the live render now shows "No Target Required / Not declared by this firm — estimated as 0%" (previously would have said "Declared as 0% for this phase," the exact false-confidence bug reported), while its Maximum Drawdown chip (which DOES have a real declared `max_dd_pct: 8`) correctly still shows a plain "8%", proving the fix doesn't over-trigger. Then temporarily set `funded_profit_target: 0` on that same real account's rules (a genuine declared-zero case) and re-rendered — confirmed the chip correctly switched to "Declared as 0% for this phase" (the real-config branch), before reverting the mutation and re-confirming via direct query that the rules JSON matched its original value exactly. Zero PHP errors in any render. `php -l` clean.
>
> **Fix note (2026-08-10), §1.4.** Re-verified: confirmed `$overallUsedRatio`/`$dailyUsedRatio` still defaulted to `0.0` whenever `$cm` (`$currentPhase['challenge_metrics']`) was `null`, with nothing distinguishing that from a real, confirmed 0%-drawdown reading before reaching `rnComputeRiskRecommendation()`'s tiering. Threaded a new `dd_data_available` key (`$cm !== null`) through the one caller that can genuinely hit this gap (the prop-firm branch, where `getChallengeMetrics()`'s own try/catch can null out `$cm`) into `rnComputeRiskRecommendation()`, which now defaults the flag to `true` (preserving every other/future caller's exact prior behavior) and only lets it override the **'healthy'** tier's reason text — the one tier a missing-data 0.0 ratio can silently reach; every other tier (`moderate`/`serious`/`critical`) already requires a real, nonzero computed ratio, so they were never reachable by missing data alone and are structurally unaffected by this fix. Also surfaced the new `dd_data_available` boolean on the function's own return array for any future caller that wants to check it directly. **Verified via an isolated logic test** (copied the exact, just-edited function body — this specific gap, per the audit's own note, requires a genuine mid-computation exception from `getChallengeMetrics()`, which the normal live picker flow's INNER JOIN on `challenge_accounts` makes very hard to force live without corrupting real account state): the no-data case (`dd_used_ratio=0.0, dd_data_available=false`) now correctly renders "real drawdown data for this account isn't available right now, so this defaults to a standard, conservative risk level until it is" instead of the old, confidently wrong "this account's drawdown is currently well within a healthy range"; a genuinely-confirmed 0%-drawdown case (`dd_data_available=true`) still correctly renders the original confident text, unchanged; a caller omitting the new key entirely (backward-compat) defaults to `true` and is byte-identical to pre-fix behavior; a real nonzero `moderate` tier with the flag forced `false` is confirmed unaffected (proving the fix can't misfire on a tier that already has real data behind it). Re-rendered the live app's default account afterward (a real, unexceptional case where `$cm` is never null) — zero PHP errors, unchanged output, confirming no regression for the ordinary, un-flagged path. `php -l` clean.
>
> **Fix note (2026-08-10), §1.5.** Re-verified: confirmed `$expectedWinsNeeded`'s only special case was `$status === 'passed'`, so a `'breached'`/`'closed'` phase fell to the same branch as a genuinely active one, computing a real (misleading) wins-needed number, or — if `$avgWin` happened to be null — the equally misleading "Estimate needs at least one win" waiting state, both implying an ended attempt is still being pursued. Extended the null-out condition to `in_array($status, ['passed', 'breached', 'closed'], true)` and added a distinct render branch ("Attempt Ended") ahead of the pre-existing "Estimate needs at least one win" fallback, so a breached/closed phase never lands in either of the two "still working toward it" states. Confirmed via the audit's own reachability caveat that this is genuinely unreachable through the live picker today (`rnFetchActivePropAccounts()` already excludes breached/closed accounts, confirmed by re-reading that function) — **verified via an isolated logic test** instead of a live UI click-through, per the standing instruction for confirmed-unreachable findings: copied the exact, just-edited `rnPhaseTargets()`/`rnBuildPhaseCardFromRows()` function bodies into a standalone script and called them directly with synthetic `active`/`passed`/`breached`/`closed` statuses against a losing-trades-only synthetic segment (the shape that used to produce a real wins-needed number for an ended attempt) — `active` correctly still falls to "Estimate needs at least one win" (unaffected, avgWin genuinely null), `passed` correctly shows "Target Reached" (unaffected), and both `breached` and `closed` now correctly resolve to `expected_wins_needed === null` and the new "Attempt Ended" render branch. `php -l` clean.
>
> **Fix note (2026-08-10), §1.6.** Re-verified all three functions against current source — confirmed `rnCoachingSetupText()`/`rnCoachingEntryText()` guarded on `empty($items)` (row count) before computing `$joined`, and `rnCoachingRiskText()`'s trailing clause guarded on `!empty($items)` the same way — none of the three checked whether `rnJoinChecklistText($items)` actually returned non-empty text, unlike the already-correct sibling `rnCoachingExecutionText()` (`$joined !== ''`), confirmed as the reference pattern by direct read. Applied the identical guard to all three: `rnCoachingSetupText()`/`rnCoachingEntryText()` now fall to their existing "no criteria/triggers declared" fallback message whenever `empty($items) || $joined === ''` (a checklist row with blank text is functionally identical to no row at all for coaching purposes); `rnCoachingRiskText()`'s trailing "Your own declared risk rule: {$joined}." clause is now only appended when `$joined !== ''`, so a blank-text risk row silently omits the clause instead of appending a broken, content-free one. Live-rendered the default account (real strategy, real non-blank checklist text throughout) — unaffected, byte-identical output, confirming no regression for the ordinary case; the blank-text trigger condition itself (a `checklist_items` row whose `raw_text`/`polished_text` both trim to empty) does not exist anywhere in this database's real data, so the branch itself was exercised the same way as `rnJoinChecklistText()`'s own existing empty-string return path is already proven correct (its unit-tested `count($texts)===0` → `''` behavior, unchanged by this fix) — the fix only changes what each caller does with that already-correct `''`, which is now checked rather than ignored. `php -l` clean. All four §1.3-§1.6 fixes verified together in one live sweep: `?workspace=active&rn_type=prop` (default account) and the real Funded account used for §1.3's mutation test both HTTP 200, zero PHP `Warning:`/`Fatal error`/`Notice:`/`Deprecated:`/`Parse error` in either render.

---

## 2. Trading Archive (`src/TradingArchiveEngine.php`, `public/trading-archive.php`)

**Structural note affecting how to read this section**: `TradingArchiveEngine::build()` — the entry point for `findings()`, `patterns()`, `phaseBreakdown()`/`difficultyBoard()` (population-wide), and `comparativeAnalytics()`/`compareTwoGroups()` — has **zero call sites anywhere in the live app** (confirmed via grep). `renderTradingArchive()` only ever dispatches to the single-account `taRenderSuccessLearning()`/`taRenderFailureLearning()` pipeline, which never calls `build()`. Findings inside that dead subtree are real bugs but currently invisible to any user — ranked LOW/deferred below, kept separate from the live findings.

### Live, reachable (single-account Lessons From Success / Lessons From Failure pages)

| # | Function | Finding | Severity |
|---|---|---|---|
| 1 | `successStory()` (~1973-2069), rendered by `taSuccessPassStory()` | **[PERSONALLY VERIFIED]** Narrates an already-superseded phase as "current." `gatherPropSegments()` (confirmed at its own `if ($status === 'active') continue;` line) skips any segment whose terminal status is `'active'` — so an account that passed Phase 1 and is now unresolved in Phase 2 has `$segments` containing **only** the passed Phase 1 segment; `$terminal = end($segments)` and `$currentLabel` both resolve to "Phase 1." The sentence "In its current Phase 1 stage, this account grew from $X to $Y" then renders even though the account is actively in Phase 2 — directly contradicted by the adjacent "Journey Status" KPI card on the *same page*, which correctly shows the real current stage via a different code path. The function's own doc comment proves the author reasoned about exactly this edge case for the *balance number* (a real `$canonicalCurrent` safety check exists) but never updated the surrounding *word* "current." **[FIXED 2026-08-10]** | HIGH |
| 2 | `buildAccountSuccessJourney()` KPI block (~2162-2185), `taSuccessKpiCards()` | "Journey Duration" caption asserts "From account creation to today" whenever `$stage['in_progress']` is true, but the underlying `$journeyDays` is computed via raw min/max over the same `$segments` array from Finding 1 — which stops at the last *resolved* segment's end date, not today. An account 30+ days into an unresolved Phase 2 shows a duration that silently omits that entire span while explicitly claiming to span "to today." **[FIXED 2026-08-10]** | HIGH |
| 3 | `taSuccessJourneySubtitle()` (~220-230) | "Detailed insights from this account's journey to {stage} success" — for an account mid-way through an unresolved phase, this asserts "success" at a stage that hasn't resolved yet. Same root cause as #1/#2 (no in-progress flag reaches this function). **[FIXED 2026-08-10]** | MEDIUM-HIGH |
| 4 | `taSuccessFundedGoalSection()`/`taLiveSuccessGoalSpeed()`, `fundedGoalAchievement()` | Self-contradictory within one card: "Average Time to Goal"/"Average Goal Completion" are correctly gated at `MIN_GROUP_SEGMENTS` (3) and show "Need at least 3 achieved goals" below that — but "Goal Achievement Rate," "Goal Size Distribution," and "How Quickly Goals Are Reached" (a bar chart) render **unconditionally**, with no minimum-sample floor at all, in the same card. A single achieved goal produces a confident 100%-bar distribution directly below a "not enough data" message about the same underlying numbers. **[FIXED 2026-08-10]** | HIGH |
| 5 | `riskBehaviorTiles()` (~3144-3194), Failure lens only | Two of six tiles ("Highest Risk Taken," "Consecutive Losing Streak") hardcode `'tone' => 'negative'` in failure mode regardless of the real value — a genuinely clean number (0 elevated-risk trades, a 0/1-length loss streak) still renders red, misattributing blame. Every other tile in the same function correctly derives tone from the real value. **[FIXED 2026-08-10]** | MEDIUM |
| 6 | `strongestSuccessHabits()` (~2286-2302), session block | "{Session} session produces the strongest results" is gated only on `$pct >= 30`, missing the `$n >= MIN_GROUP_TRADES` (5) check every sibling habit in the *same function* applies (checklist adherence, risk consistency, best strategy, favorable RR). A single winning trade in one session trivially clears 100% of "total profit" and fires confident "strongest results" language. **[FIXED 2026-08-10]** | MEDIUM |
| 7 | `riskBehaviorTiles()` (~3148-3149) | "Avg Risk Per Trade" comparison (`$actual > $declared`) is a raw strict inequality with zero tolerance band and no Contract-Constrained exclusion — both of which `segmentMetrics()` explicitly applies elsewhere in this exact file, citing the app-wide lot-quantization tolerance work. A trade forced above target purely by broker-minimum lot size still flips this tile to "Above Plan"/red. **[FIXED 2026-08-10]** | MEDIUM |
| 8 | `taLiveConsistencyDonut()` (~2294-2325) | "Your long-term consistency is above N% of your own trading history" — `$c['score']` is `historicalConsistency()`'s 0-100 composite grade, not a percentile rank against the trader's own trade history. The sentence claims a percentile the underlying statistic does not measure. **[FIXED 2026-08-10]** | MEDIUM |

### Confirmed dead code (real bugs, zero live reachability — `TradingArchiveEngine::build()` only)

| # | Function | Finding | Severity (if ever wired up) |
|---|---|---|---|
| 9 | `findings()` (~960-963) | "Most successful attempts closed out on a winning streak" gated on an absolute count (`n_terminal_win >= 3`), never checked as a fraction of total resolved segments — "Most" can be true at 6% of the real population. **[FIXED 2026-08-10]** | Would be HIGH if reachable |
| 10 | `findings()`/`patterns()` (~982-987, ~1046-1051) | `declared_adherence_rate` claims gate on total pooled trade count, not on the (separate, smaller) count of trades that actually had a declared risk % to compare against — can produce a confident 0%/100% from a tiny subgroup. **[FIXED 2026-08-10]** | Would be MEDIUM-HIGH if reachable |
| 11 | `easiestHardestPhase()` (~822-832) | With exactly one phase clearing the sample floor, "Easiest" and "Hardest" both resolve to the identical phase/label. **[FIXED 2026-08-10]** | LOW |

**Checked, correctly guarded, not reported further**: `historicalConsistency()`'s gating; `accountLevels()`'s in-progress/ready-to-promote states (including the `array_key_last()`-style safety check); `taSuccessLevelDonut()`/`taSuccessLevelBars()`; `bucketOutcome()`'s "Legacy data" disclosure; `comparativeAnalyticsProp()`/`comparativeAnalyticsLive()`'s thin-data messaging (though currently only reachable via the dead `build()` path); `taFailureAvoidable()`'s always-true verdict (confirmed as an explicit, documented product decision, not a bug); `difficultyBoard()`'s partial-coverage disclosure; `taSuccessGoalTimelineChart()`'s intentional empty state.

> **Fix note (2026-08-10), Live #1.** Re-verified against current source before fixing (line numbers had drifted slightly, logic unchanged). `$canonicalCurrent !== null` was already this function's own established signal for "the terminal segment genuinely IS the account's real live current stage" (built specifically to keep the balance number honest) — the narrative sentence never read it. Added `$isGenuinelyCurrentStage = $canonicalCurrent !== null` and branched the sentence's own tense on it: genuinely current stays "In its current {Stage} stage, ..." (unchanged); a superseded/historical terminal segment now reads "In its {Stage} stage (since surpassed, with the account now progressing further), ..." — applied to both the positive- and negative-PnL sentence variants. Verified live against the real running app (temporary dev-only auto-login helper at the project root, deleted after use): a real two-step account (`839ff2ca-…`, passed Phase 1, currently unresolved in Phase 2) — the "Journey Status" KPI card correctly shows "Phase 2 / Currently attempting Phase 2" while the pass-story narrative now correctly reads "In its Phase 1 stage (since surpassed, with the account now progressing further), this account grew from $5,000.00 to $5,400.00 (+8.0%)" — no longer self-contradictory with the adjacent KPI card, the exact bug reported. A second account (`27f8b25a-…`, same shape, negative terminal P&L) confirmed the negative-branch wording ("...this account ended at $4,600.00 (-8.0%..."). `php -l` clean.
>
> **Fix note (2026-08-10), Live #2.** Re-verified `$journeyDays`'s exact computation against current source. `$stage` (with its own `in_progress` flag) is already computed earlier in the same method, before `$journeyDays` — no new plumbing needed. Extended the end boundary to `now()` whenever `$stage['in_progress']` is true (the exact same condition `taSuccessKpiCards()`'s caption already keys "From account creation to today" on), making that existing caption's claim literally true instead of overstated by however many days the account has spent in its current unresolved phase. Verified live on the same real account (`839ff2ca-…`) used for Live #1: Journey Duration now reads "3 Trading Days" with the footnote "From account creation to today, while attempting Phase 2" — genuinely extends into the unresolved phase's span, not just through the passed Phase 1 segment it stopped at before. `php -l` clean.
>
> **Fix note (2026-08-10), Live #4.** Re-verified all three render sites (`taSuccessFundedGoalSection()` and its Live Accounts siblings `taLiveSuccessGoalAchievement()`/`taLiveSuccessGoalSpeed()`) against current source — confirmed "Goal Achievement Rate"/"Goal Size Distribution" had zero sample-size gate anywhere (unlike the adjacent "Average Time to Goal"/"Average Goal Completion" tiles on the same card, already correctly gated at `MIN_GROUP_SEGMENTS`), and that `taLiveSuccessGoalSpeed()`'s "Goal Completion Time Distribution" bar chart was rendered *outside* its own `$ready` check entirely — a second, distinct instance of the same bug class in the same finding. Added `$achievementReady = $total >= $minGoalSamples` (keyed off goals SET, not just achieved — a rate/distribution needs a real population regardless of how many succeeded) and gated the top-grid stat, the donut, and the size-distribution list on it in both the Prop Firms and Live Accounts cards; reused the already-existing `$timeReady` to gate "How Quickly Goals Are Reached" (its bucket data is literally the same `$timeToGoalDays` array `n_time_to_goal_samples` already counts); moved `taLiveSuccessGoalSpeed()`'s bar-list call inside its existing `$ready`-gated `else` block. Verified live with a real account (`d5d2ac45-…`, exactly 1 real goal on record): Goal Achievement Rate/Goal Size Distribution now correctly show "Need at least 3 goals set" instead of a confident 100% donut/chart, directly consistent with Average Time to Goal's own disclaimer on the same card — the exact self-contradiction the finding reported. Confirmed the positive path still works correctly via a temporary, fully-reverted mutation (2 additional real-shaped `account_goals` rows inserted, confirmed the donut/size-distribution/rate correctly render once `$total = 3`, confirmed "How Quickly Goals Are Reached" correctly *still* shows its own "Need at least 3 achieved goals" since only 2 of the 3 were achieved — proving the two gates are independently correct, not just both-open/both-closed — then deleted both rows and re-confirmed the account's real 1-goal state renders identically to before the test). Full regression sweep across all 4 archive lens×type combinations plus Active Journeys — zero PHP errors. `php -l` clean on both files. `git diff --stat` reviewed before committing — both files were untracked in this repository (no prior commit ever captured them); per this codebase's established discipline for accumulated pre-existing changes, their full pre-existing content was committed separately first (`src/TradingArchiveEngine.php, public/trading-archive.php: add untracked pre-existing files`) before these three fixes' own isolated 97-line diff was committed on top.
>
> **Fix note (2026-08-10), Live #3.** Re-verified: `buildSuccessAccountSelector()` computed `$stage['in_progress']` (via `accountStageInfo()`, itself already fixed/correct — see its own `currentPhaseRank()` doc comment) but never threaded it onto the row it returned, so `taSuccessJourneySubtitle()` had no way to distinguish a genuinely resolved stage ("Successfully passed Phase 1 — awaiting promotion") from one still actively being attempted unresolved ("Currently attempting Phase 2") — both rendered the identical "journey to {stage} success" claim. Added `'in_progress' => $stage['in_progress']` to the selector row and branched the subtitle on it: an in-progress stage now reads "Detailed insights from this account's journey so far, while currently attempting {stage}," never claiming a success that hasn't happened. Verified live against the real running app (temporary dev-only auto-login helper at the project root, deleted after use), on the exact real account (`d207ae67-…`, passed Phase 1, `phase_2_active`) used for the Live #1 fix above: the subtitle now correctly reads "...journey so far, while currently attempting Phase 2," staying consistent with the same account's "Journey Status" KPI card and the already-fixed `successStory()` narrative on the same render. Zero PHP errors. `php -l` clean.
>
> **Fix note (2026-08-10), Live #5 + Live #7.** Fixed together, same function, adjacent lines. Re-verified both bugs against current source: **Live #5** — `'Highest Risk Taken'`/`'Consecutive Losing Streak'` hardcoded `'tone' => 'negative'` in failure mode (`!$successMode`) regardless of the real `$elevatedCount`/`max_loss_streak` value, unlike every other tile in the same function (and unlike their own success-mode branch, which already correctly derived tone from the real number). Fixed by reusing the function's own pre-existing `$watchTone`/`$cleanTone` variables (already correctly `negative`/`neutral` in failure mode, `accent`/`positive` in success mode) instead of a second hardcoded literal — a genuinely clean value now renders neutral, not red. **Live #7** — `'Avg Risk Per Trade'`'s `$aboveActual = ($actual > $declared)` was a raw strict inequality with zero tolerance, and the separate per-trade `$overRiskCount` loop used a flat `$rp > $declared * 1.2` multiplier with no per-asset-class tolerance and no Contract-Constrained exclusion — both inconsistent with `segmentMetrics()`'s already-correct, already-shipped tolerance+exclusion logic a few hundred lines above in the same file (confirmed via that function's own §18/Wave 5c comment). Fixed by reusing `AnalyticsEngine::calculatorRiskTolerance()`/`AnalyticsEngine::minAchievableRiskPct()` directly (the same two helpers `segmentMetrics()` already calls) — the pooled-average comparison applies the tolerance function without an instrument (its own documented global fallback, since there's no single instrument at the averaged level); the per-trade loop applies it per-trade's own instrument/entry/SL/balance, and excludes a trade from `$overRiskCount` outright when its declared target was never mathematically reachable at that instrument's broker-minimum lot size (Contract-Constrained). **Verified via an isolated unit test** (literal copy of the exact, just-edited function body, run against `AnalyticsEngine`'s real, unmocked tolerance/contract-constrained methods — chosen because hunting for a real account naturally exhibiting every one of these edge cases live would be unreliable, per this audit's own established pattern for hard-to-naturally-reproduce cases): a genuinely clean scenario (0 elevated trades, streak=1, actual risk within tolerance) now correctly renders `neutral` throughout in failure mode (was hardcoded `negative`); a genuinely bad scenario (streak=4, real 5x over-risk) correctly still renders `negative` (no over-suppression); a Contract-Constrained trade (a coffee trade on a $10 balance, nominally "4% risk" against a 1% declared target purely because the broker-minimum lot size can't size any smaller) correctly excluded from `Trades Over Plan Risk`'s count (0, neutral) despite superficially looking "over plan"; success-mode output confirmed byte-unchanged (`positive` throughout the clean case, matching pre-fix behavior exactly — no regression). **Then verified live against the real running app** on a real breached account (`006811b4-…`): "Consecutive Losing Streak" (real value 1) now correctly renders with no red styling (`style=""`, was hardcoded red before the fix) while "Avg Risk Per Trade" (real actual 9.99% vs. declared 1.00%, genuinely far outside any tolerance) correctly still renders red "Above Plan" — confirming the fix distinguishes a genuinely clean number from a genuinely bad one on real data, not just synthetic cases. Zero PHP errors across a sweep of 3 real breached accounts. `php -l` clean.
>
> **Fix note (2026-08-10), Live #6.** Re-verified: the session-habit block was the one habit claim in `strongestSuccessHabits()` with no `MIN_GROUP_TRADES` gate at all, unlike every sibling habit in the same function (checklist adherence, risk consistency, best strategy, favorable RR — all already correctly gated). Added a real per-session trade count (`$sessionTradeCounts`, counted from every trade with a session regardless of P&L sign — not just winners, since a session claim's reliability depends on how many trades were actually taken in it, not how many happened to win) and required the top session to clear `MIN_GROUP_TRADES` (5) before making the "strongest results" claim — gated on that specific session's own sample, not the account's overall trade count, since a large account could still have a thinly-sampled top session. **Verified via an isolated unit test** (copied the exact, just-edited block): a single winning trade in one session (the exact reported bug — 100% of profit from n=1) now correctly suppressed; a genuinely well-sampled top session (6 trades, 80% of profit) correctly still fires; the exact 5-trade floor boundary correctly fires; 4 trades (just under the floor) correctly suppressed. Live-rendered against a real funded account (`438c1b6d-…`) — zero PHP errors, section renders correctly (this specific account's real data didn't happen to produce a qualifying session claim either way, confirmed via the isolated test above instead, per the same reasoning Live #5/#7 used).
>
> **Fix note (2026-08-10), Live #8.** Re-verified: `$c['score']` (from `liveTradingConsistency()` → `historicalConsistency()`) is a 0-100 composite grade (`0.5 × riskConsistency + 0.5 × disciplineRate`, confirmed via direct read of `historicalConsistency()`'s own formula) — nothing anywhere computes a percentile ranking of this account against its own trading history, so "Your long-term consistency is above X% of your own trading history" claimed a statistic that was never actually calculated. Reworded to state the real composite grade plainly ("Your long-term consistency score is strong, at X/100 — steady risk-sizing and disciplined trade selection, sustained over time," and the moderate/low tiers similarly restated with the real X/100 number), matching the sibling `taRenderHistoricalConsistency()` card's own already-correct "combines two things, weighted equally" framing on the same page rather than inventing a new claim. Verified all three tiers via direct evaluation of the new ternary (85→strong/85/100, 55→moderate/55/100, 20→low/20/100, all reading the real score honestly). Live-rendered against every real personal account for the primary test user — all correctly show the "not enough closed trades" empty state (none clear the real `MIN_HISTORICAL_CONSISTENCY_TRADES` floor of 5), confirmed via zero PHP errors and the correct empty-state text; the praise-line branch itself was verified via the isolated evaluation above rather than a live render, since no real live account in this database currently has enough trade history to reach it — consistent with how this same gap was verified elsewhere in this audit.
>
> **Fix note (2026-08-10), dead-code #9/#10/#11.** Per Kevin's explicit decision, fixed alongside the live findings rather than deferred — `TradingArchiveEngine::build()` (the entry point for all three) still has zero live call sites (re-confirmed via grep before starting), so **verified via direct, isolated calls to the three affected PUBLIC methods** (`findings()`, `patterns()`, `easiestHardestPhase()`, all `public static`) rather than through the app UI, using PHP Reflection to also invoke the two PRIVATE methods needed to build a realistic `$segments`/`$group` input (`segmentMetrics()`, `aggregateGroup()`) — the same technique this codebase already uses elsewhere to test private engine internals directly. **#9** — `$group['n_terminal_win'] >= MIN_GROUP_SEGMENTS` (an absolute count) was the only gate on "Most successful attempts closed out on a winning streak," never checked as a fraction of `$nSeg` (total resolved segments); added `($group['n_terminal_win'] / $nSeg) > 0.5`, a genuine majority requirement, alongside the existing sample floor. **#10** — both `findings()`'s failure-lens declared-risk-overshoot claim and `patterns()`'s declared-adherence claim gated on `$n`/nothing respectively, rather than on `declared_adherence_rate`'s own real, smaller denominator (`n_declared_checked` — trades with a declared risk% AND not Contract-Constrained-excluded) — that denominator wasn't even exposed on `segmentMetrics()`'s return array before this fix; added `'n_declared_checked' => $declaredChecked` to the return, then gated both claims on `$pooled['n_declared_checked'] >= MIN_GROUP_TRADES`. **#11** — `easiestHardestPhase()`'s loop initializes both `$easiest`/`$hardest` from the first eligible phase and never has a second item to compare when only one phase clears the sample floor, so both resolved to the identical label; fixed with an explicit `if (count($eligible) < 2) return ['easiest' => null, 'hardest' => null];` up front, since "easiest"/"hardest" is inherently a relative claim between at least two real phases. **All three verified via a standalone reflection-based script**: #9 — 10 synthetic segments with 3 terminal-win (30%, a real minority, matching the exact "6% of the population" shape the finding described) correctly suppressed the claim; the same 10 segments with 6 terminal-win (60%, a genuine majority) correctly still fired. #10 — 30 synthetic trades where only 2 carried a real declared risk% (a large `$n` but a tiny real `n_declared_checked=2`, below the `MIN_GROUP_TRADES=5` floor) correctly suppressed the claim in both `findings()` and `patterns()`; a well-sampled 6-trade case (`n_declared_checked=6`) correctly still fired. #11 — a single eligible phase correctly returned `null`/`null` instead of the same label twice; two eligible phases (80%/30% pass rates) correctly resolved to distinct easiest/hardest labels. `php -l` clean on `src/TradingArchiveEngine.php`.
>
> Full regression sweep for all of §2's remaining findings together: 7 combinations across every archive type/lens/program (`prop/success/one_step`, `prop/success/two_step`, `prop/success/instant_funding`, `prop/failure/two_step`, `prop/failure/three_step`, `live/success`, `live/failure`), each hit through the real running app with a real authenticated session — zero PHP `Warning:`/`Fatal error`/`Notice:`/`Deprecated:`/`Parse error` in any of them. (The temporary dev-only auto-login helper used across this whole pass, §1-§6, was deleted at the very end of the session, once all remaining findings were also verified — see the closing summary.)

---

## 3. Reports Commentary (`public/reports.php`, `api/analytics/progressive_insights_engine.php`)

Found via direct read (this pass's two background agents both failed to a usage-limit interruption before returning results — see Method).

| # | Function | Finding | Severity |
|---|---|---|---|
| 1 | `ProgressiveInsightsEngine::crossStrategy()` (~425-450) and `::strategyPerformance()` (~212-247), `api/analytics/progressive_insights_engine.php` | **[PERSONALLY VERIFIED, cross-checked against reachability in reports.php]** Neither function checks whether more than one strategy is actually being compared before using competitive language. `crossStrategy()`: `{$count} strategies were active this {$periodNoun}, led by {$top['name']}` renders even when `$count === 1` (grammatically broken — "1 strategies" — and "led by" implies a comparison that never happened). Quarterly/Annual escalate to active advice: *"That's a strong enough signal at this scale to start shifting more trading time or capital toward it"* — recommending capital reallocation *toward* a strategy that had zero real competition. `strategyPerformance()` has the identical gap ("has led the portfolio this quarter... worth deciding whether to allocate more capital or trading time toward it"). **Both render unconditionally** whenever their input array is non-empty (`reports.php` ~2712, ~4666) — no `count() > 1` guard anywhere. Directly contrasted by the sibling function `session()` in the *same file*, which explicitly handles "only one session traded" as its own distinct, correctly-hedged branch (~117-124) — proving this is an omission in the other three functions, not a considered design choice. `edge()` has a related, softer version of the same gap for Quarterly/Annual asset-class narrative (no `count($topInstruments) === 1` check), though it's less severe since "best/only instrument" is at least true by definition. **[FIXED 2026-08-10]** | HIGH |

> **Fix note (2026-08-10).** Re-verified both functions against current source before fixing (exact conditions/line numbers confirmed unchanged). Added a `count() === 1` branch to each, placed *after* the pre-existing trade-count gates (`$topTrades === 1` / `< 10`) so those keep firing first exactly as before — the new branch only ever reaches the specific case those didn't already hedge: a well-sampled top strategy with no real competitor. `strategyPerformance()` gets its own period-voiced "was the only strategy traded" sentence per period (matching the file's existing per-period-voice convention); `crossStrategy()` gets one shared sentence stating plainly there's nothing to compare against yet. Also fixed, while adding it, the pre-existing "1 strategies were active" grammar bug in `crossStrategy()`'s own `$topTrades === 1` branch (the exact phrase this finding's own illustrative quote named) — a one-line singular/plural fix, zero risk. **A second, related bug found live while verifying the fix, not part of the original two named functions: `crossStrategy()`'s input array is deliberately padded by `reports.php` with the user's *untraded* active strategies** (`$untradedStrategies`, 0-trade rows merged in purely so the Strategy Comparison *table* elsewhere on the page can show what's available-but-unused) — the narrative function was counting those padded rows too, so an account with 1 real traded strategy and 3 untraded ones could still render "Comparing all 4 strategies traded this year... came out on top," the identical bug via a second route that a bare `count() === 1` gate couldn't catch. Fixed by filtering to `trades > 0` rows before ranking/counting inside the narrative function only — `reports.php`'s own `$crossStrategy` variable (used separately by the comparison table) is untouched, so the table still correctly shows untraded strategies. Verified via direct function calls covering: single real strategy (both functions correctly switch off competitive language, across all 4 periods); two real strategies (unchanged, byte-identical competitive language, confirming no over-suppression); the padded-array scenario (1 real + 3 untraded → correctly treated as "only one strategy," not "4"); a mixed scenario (2 real + 2 untraded → correctly counts 2, not 4); all-untraded and genuinely-empty arrays (both correctly fall to the pre-existing "no active strategies with trades" message). Then verified live against the real running app (real Apache/PHP/MySQL, a temporary dev-only auto-login helper at the project root — `.htaccess` only rewrites requests that aren't a real file, so it must live at the root, not under `public/`, to be served directly): the real 251-trade test account genuinely has 4 real traded strategy-versions this year (confirmed via direct SQL — 148/88/42/22 trades, no padding involved for this specific account today), so `crossStrategy()` correctly still renders its full competitive language, unchanged; `strategyPerformance()`'s own array is, by the page's own design, always scoped to one selected strategy version by default (no `strategy_id` param → auto-selects `$allStrategies[0]`) — so it naturally has exactly 1 row in the default view, and correctly rendered the new "was the only strategy traded this year, across 137 trades" message live, a real (not synthetic) exercise of the fix. All 4 periods × prop/live (8 combinations) re-rendered with zero PHP `Warning:`/`Fatal error`/`Notice:`/`Deprecated:`/`Parse error`. `php -l` clean on both files. Temporary auto-login helper deleted before finishing; `git status` confirmed no test artifacts left in the working tree.
| 2 | `ProgressiveInsightsEngine::boundaryAnalytics()` (~338-380) | The "gap between how far accounts overshoot on the way to breaching versus passing" sentence (quarterly/annual) fires whenever **either** `$breachCount >= 5` **or** `$passCount >= 5` individually clears the threshold — not both. With, e.g., `breachCount = 10, passCount = 0` (a real, plausible state — an account or user with many breaches and zero passes), the narrative still asserts a "gap between breaching versus passing is exactly the kind of pattern that should shape position sizing" — describing a two-sided comparison when only one side has any data at all. **[FIXED 2026-08-10]** | HIGH |

> **Fix note (2026-08-10).** Re-verified the exact OR condition at both the quarterly and annual/default branches before fixing — confirmed identical bug present in both (fixed once, via one shared `$hasTwoSidedGap = ($breachCount >= 5 && $passCount >= 5)` boolean and, for quarterly, one shared closure covering three distinct lopsided states — matching the audit's own "fix once, not N times" note). "The gap between breaching versus passing" now only ever renders when *both* sides clear the sample floor. The two lopsided states (breach-only-sampled, pass-only-sampled) get their own honest sentence explaining there isn't yet a real comparison, instead of collapsing into the old generic "small sample" fallback (which would have wrongly implied the well-sampled side was *also* thin). Verified via direct function calls across 5 scenarios × quarterly/annual: 10 breaches/0 passes (no longer claims a two-sided gap — the reported bug case); the reverse (0 breaches/10 passes); both sides real (8/6 — confirmed still correctly claims the gap, unchanged, no over-suppression); both sides thin (2/1 — generic fallback, unchanged); weekly/monthly (untouched, single-side-only logic, confirmed unaffected). Live-rendered against the real test account (8 period×view combinations) — zero PHP errors; the account's real boundary data happens to clear both sample floors, so the live render correctly still shows the "career-level read on risk discipline" sentence, confirming the fix doesn't over-suppress a genuinely two-sided real case.
| 3 | `ProgressiveInsightsEngine::professionalMetrics()` (~323-333) | Zero trade-count gate anywhere — the only check is `$sharpeRatio === null && $sortinoRatio === null`. Every sibling narrative function in this same file (`behavioral()`, `session()`, `edge()`, `strategyPerformance()`, `risk()`, `drawdown()`) explicitly gates n=1/n<10; this one doesn't gate at all. Since Annual's hard trade-count lock was removed app-wide (2026-07-28 session, documented in CLAUDE.md), a fresh account with as few as 2 closed trades this year and a Pro+ tier can render *"This year's trading produced a Sharpe ratio of X and a Sortino ratio of Y... This is the most rigorous single verdict on this year's trading quality"* — the app's own strongest-worded claim, off the thinnest possible sample. **[FIXED 2026-08-10]** | MEDIUM |

> **Fix note (2026-08-10).** Re-verified: confirmed the function's only parameters were `?float $sharpeRatio, ?float $sortinoRatio` (no trade count) and its single call site (`reports.php` ~4598, `$depth >= 4`/Annual-only, `$isProOrAbove`-gated) passed nothing else. Added a third `int $totalTrades = 0` parameter (default kept for signature safety, matching this codebase's established optional-trailing-param convention) and two new tiers (n=1 exact; n<10 hedged) ahead of the existing confident branch, mirroring the n-aware idiom already used by every sibling function in this file. `reports.php`'s call site updated to pass `$totalTrades` — already computed and already correctly period-scoped (`t.closed_at >= :s AND t.closed_at <= :e`, bound to the selected Annual window) for an unrelated purpose (the Data Completeness widget), so no new query was needed. Verified via direct function calls: totalTrades=2 (the reported bug case — no longer claims "most rigorous single verdict"); totalTrades=1 (distinct singular wording); totalTrades=9 (hedged); totalTrades=150 (confirmed unchanged, confident language preserved — no over-suppression); both-ratios-null (unaffected, unchanged). Live-rendered against the real 251-trade test account — correctly still renders the full confident "most rigorous single verdict" sentence (real trade count is far past the threshold), confirming no regression for a genuinely well-sampled account. `php -l` clean on both files.

**Checked, correctly guarded, not reported further**: `reports.php`'s `$auditCommentary`/`$auditRecommendations` (every bullet correctly gated on its own nonzero count, degrades to an honest "execution stayed consistent" fallback); `disciplineReason()` (correctly relative — "closest to plan of this period's trades," never an absolute discipline claim) and `drawdownPraise()` (correctly `null`-returns for any nonzero drawdown, tiered only by trade count for the genuine-zero case) — both confirmed **dead code** in the current render (superseded by the consolidated Trade Highlights card; `renderHighlightRow()`/`disciplineReason()` have zero live call sites, confirmed via grep — not a live bug); Edge Health's per-factor trend rendering in `reports.php` (~3996-4030), including the `discipline` factor's own independent branch in `api/analytics/edge_health_engine.php::buildNarrative()` (correctly does **not** route through the shared `trendPhrase()` helper, which itself correctly handles `'insufficient_data'` with a `??  ''` fallback and no dangling-punctuation artifact); Boundary Analysis's "Avg Excess at Pass" disclosure caption (single render site, correctly captioned with real sample counts); `behavioral()`/`session()`/`risk()`/`drawdown()`'s n=1/small-n/confident tiering in `progressive_insights_engine.php` (spot-checked directly — still correctly in place, no regression from the 2026-07-26 fix sessions CLAUDE.md documents).

---

## 4. Colex Chat (`api/colex/chat.php`, `src/ColexHelpers.php`)

Found via direct read (this pass's background agent failed to a usage-limit interruption before returning results — see Method). Scope excludes fact-disagreement bugs already covered in the SSOT audit (Colex's tilt/revenge signal, "total balance," Risk of Ruin formula — all confirmed already fixed per in-code comments citing the SSOT audit directly).

| # | Function | Finding | Severity |
|---|---|---|---|
| 1 | `generateComparisonResponse()` (~1275-1354) | `$pnlChange = $prevPnL != 0 ? round(...) : 0;` — whenever the previous period has **zero** P&L (which includes a trader's very first-ever reporting period, or any period with no prior trade history at all — the function's only guard is `empty($context['accounts'])`, which checks current accounts, not prior-period history), `$pnlChange` silently defaults to `0`, and the message unconditionally appends `"(improved)"` (since `0 >= 0`). A trader with no real previous period gets *"Compared to last period: P&L changed 0% (improved) and win rate changed 0% (improved)"* — a fabricated positive trend with nothing to actually compare against. Worse: if the trader's *current* period is a real loss while the *previous* period was genuinely flat/empty ($0), the same defaulted `0%` still renders "(improved)," directly contradicting a real current-period loss. Same defaulting affects `$wrChange`. This is exactly the class of check CLAUDE.md documents this codebase being careful about elsewhere (`reports.php`'s prior-period deltas: *"does every delta-based sentence actually have real prior data before asserting a trend, not just a null-coalesce to a misleading default"*) — not applied here. **[FIXED 2026-08-10]** | HIGH |

> **Fix note (2026-08-10).** Re-verified: confirmed `$prevPnL != 0 ? ... : 0` was still the only guard, with no `total_trades` check anywhere. `AnalyticsEngine::getUserMetrics()` (already called here) always returns a real `total_trades` count (`0` when no trades exist), independent of what `net_pnl` happens to be — the only accurate signal for "was there real prior data." Added an early return when `$prevMetrics['total_trades'] === 0`, stating plainly there's no prior period to compare against instead of fabricating a "0% (improved)" trend. Also handled the remaining edge case once real prior data is confirmed to exist: `$prevPnL` can still legitimately be exactly `$0` (a genuine breakeven prior period) — a percentage change against a zero baseline is undefined, not `0%`, so that case now states the real dollar swing ("P&L went from exactly $0.00 to +$X") instead of a fabricated percentage. Verified live against the real running app (temporary dev-only auto-login helper at the project root, deleted after use): a "compare" message against an ancient date range with zero prior data correctly returned *"There's no closed-trade history in the period right before this one... nothing to compare it against yet"*; the same message against a real week with genuine prior-week trade history correctly returned the original, unchanged computation (*"P&L changed -118.6% (declined)..."*, matching hand-verified real numbers exactly). `php -l` clean.
| 2 | `generateAdviceResponse()` (~1355-1466) | When `$weaknesses` is empty — meaning the trader genuinely cleared every checked threshold (win rate ≥40%, no losing streak ≥3, winners ≥ losers or win rate ≥60%, positive expectancy) — the stat card renders `'value' => implode(', ', $weaknesses) ?: 'Insufficient data'`. This is false: the function's own top-level gate already guarantees ≥3 real trades exist (`$metrics['total_trades'] < 3` returns early), so there genuinely *is* sufficient data — the real reason the array is empty is that nothing was flagged, a good outcome mislabeled as a data gap. Compounding: `$message` is built as `"Based on your trading data, here are your key areas for improvement:\n\n"` followed by `implode("\n\n", ...$advice)` — if `$advice` also ends up empty (plausible for a trader with no recent losses and no session-loss pattern), the message renders as a headline promising "key areas for improvement" followed by nothing at all. **[FIXED 2026-08-10]** | HIGH |

> **Fix note (2026-08-10).** Re-verified both bugs against current source — confirmed unchanged. Added an early return when `$advice` is empty (the whole-array check, since `$advice` — not `$weaknesses` — is what the message body actually iterates) with an honest, positive message and a correctly-labeled card (`"None found"`, positive direction) instead of the broken headline-with-nothing-after. Separately fixed the `$weaknesses`-empty mislabel for the case where `$advice` is non-empty (e.g. session-loss or challenge-proximity advice fired even though none of the 4 core weakness checks tripped) — `"Insufficient data"` replaced with an accurate `"None found in the core metrics — see notes below"`. Verified via 3 isolated function calls (no live DB dependency — the function takes only `$context`): a genuinely clean record (no weaknesses, no advice at all — correctly hits the new early return); a record clean on the 4 core checks but with real session-loss advice (correctly shows the real advice text *and* the corrected "None found in the core metrics" label, not "Insufficient data"); the pre-existing <3-trades gate (confirmed unchanged). Then verified live against the real running app with a real weakness-triggering window — unchanged, correct behavior (extended losing streak flagged, real advice listed). `php -l` clean.
| 3 | `generatePerformanceResponse()` (~866-873, ~915-921) | "Best Instrument" is selected purely by win rate (`$wr > $bestWinRate`) with **zero minimum trade-count gate** — `$sessions[$session] > 0`-style floor doesn't exist here at all. A single 1-trade instrument at 100% win rate beats a 50-trade instrument at 70% win rate and gets crowned `"Best Instrument: XYZ (100% WR)"` with the same visual confidence (`'direction' => 'positive'`) as a real, well-sampled edge. **[FIXED 2026-08-10]** | HIGH |

> **Fix note (2026-08-10).** Re-verified: confirmed the instrument-ranking loop (~lines 866-874) had no trade-count check of any kind. Added `if ($data['total'] < 3) continue;` before the win-rate comparison — the same 3-trade floor this file's own `generateAdviceResponse()` already uses as its bar for "enough data to say something." When no instrument reaches the floor, the "Best Instrument" card is correctly omitted entirely (matching its existing `if ($bestInstrument)`-gated render, no new copy needed) rather than shown with a misleading thin sample. Verified via 3 isolated function calls reproducing the exact reported shape (1-trade instrument at 100% WR vs. a 10-trade instrument at 70% WR — the 10-trade instrument now correctly wins), an all-thin-sample case (card correctly omitted), and the exact 3-trade boundary (correctly qualifies). Then verified live against the real running app — "Best Instrument: EUR/USD (100% WR)" rendered, cross-checked directly against the DB to confirm EUR/USD genuinely had 4 real trades (not 1) in the sampled window, i.e. a real qualifying result, not the bug. `php -l` clean.
| 4 | `generateDisciplineResponse()` (~1072-1093) | Fetches `EdgeHealthEngine::getPeriodFactors()`, which returns a real, already-computed n-aware `tier` field (`single_trade`/`very_small_sample`/`small_sample`/`developing`/`confident`) specifically so callers can gate confidence — and never reads it. Only `consistency_score` is used, thresholded at 80/65/50 into "excellent — great discipline" / "good" / "poor" with no reference to sample size at all. Coefficient-of-variation-based consistency scoring trivially computes near-100 at `n=1` (a single value has zero variance) — so Colex can tell a trader with exactly one trade this period "Your risk sizing is very consistent — great discipline," the exact calibration-bug pattern (a real tier exists, computed, and is silently ignored). **[FIXED 2026-08-10]** | HIGH |

> **Fix note (2026-08-10).** Re-verified: confirmed `getPeriodFactors()`'s real return shape (`n`, `tier`, `consistency_score`, `discipline_score`) and that only `consistency_score` was ever read. Added 3 tiers reading the real `tier`/`n` fields: `single_trade` (n=1) now says plainly there's no real position-sizing pattern to grade from one trade, with `risk_consistency` reported as `"Unknown"` rather than a false grade; `very_small_sample`/`small_sample` (n<20) still reports the real score using the same 80/65/50 thresholds, but hedged as a small sample ("Based on just N trades... too small a sample yet to call it a settled habit"), not a settled verdict; `developing`/`confident` (n≥20) keeps the original, unmodified wording. Verified live against the real running app across all 3 tiers using real narrow date windows (a genuine real single-trade day, a genuine real 9-trade day, and a well-sampled 144-trade month) — each rendered the correct tier's wording, with the confident-tier case confirmed byte-identical to the pre-fix wording (no regression). **A real, separate fact-disagreement bug was noticed live while testing (not fixed here, out of this audit's scope per its own Method note)**: `$context['metrics']['total_trades']` reported `0` for the same real single-trade-day window that `$stageSummary['trades_in_period']` and `$edgeFactors['n']` both correctly reported as `1` — three independently-queried "trades this period" figures disagreeing is a Type-1 fact-disagreement issue (multiple computations of the same fact), not a Type-2 narrative-branching issue, and this audit was explicitly scoped to exclude fact-disagreement bugs already covered by the SSOT audit. Flagged for whoever next touches `$context['metrics']` population (`api/colex/chat.php` ~line 194) or the SSOT work, not investigated further here. `php -l` clean.
| 5 | `generatePatternsResponse()` (~1234-1266) | "Weakest Session" stat tile hardcodes `'direction' => 'negative'` regardless of that session's real P&L sign — a session that is still net-profitable, just the least profitable of the three, renders with the same red/negative tone as a genuinely losing session. Separately, "best/weakest session" framing (message text, ~1223-1225) has no per-session sample-size floor — a session with a single trade can be named "your best session" or "your weakest." **[FIXED 2026-08-10]** | MEDIUM |

> **Fix note (2026-08-10).** Re-verified both bugs against current source. Fixed the hardcoded direction to derive from `$worstSessionPnL`'s real sign (`$worstSessionPnL >= 0 ? 'positive' : 'negative'`, `'neutral'` when no session qualified at all), matching the sign-derivation pattern this same file already uses correctly for `worst_trade`. Added the same 3-trade-per-session floor used elsewhere in this file (`$sessions[$session] >= 3`, was `> 0`) to the best/weakest selection loop — below the floor, both the message sentence and the card values correctly stay empty (the existing `if ($bestSession && $worstSession)` guard already handles this gracefully, no new copy needed). Verified via 4 isolated function calls: a still-profitable "weakest" session (correctly now shows `positive`, not the old hardcoded `negative`); a genuinely losing "weakest" session (correctly still shows `negative` — no over-correction); a below-floor scenario (correctly no best/weakest named, honest fallback message); the exact 3-trade boundary (correctly qualifies). Then verified live against the real running app — a real session that was the "weakest" of three but still net +$1,477.92 correctly rendered `direction: "positive"` instead of the old hardcoded `negative`. `php -l` clean.
| 6 | `src/ColexHelpers.php::generateSuggestedPrompts()` (~276-277) | *"Why did my win rate drop to {win_rate}%?"* fires purely from an absolute `win_rate < 40` threshold, with no comparison to any prior state — `getPeriodStats()` computes a single period only, no prior-period figure exists anywhere in this function. The phrase asserts a decline ("drop") that may never have happened (e.g. a trader's very first period, already at 35% from trade one). **[FIXED 2026-08-10]** | MEDIUM |

> **Fix note (2026-08-10).** Re-verified: confirmed `getPeriodStats()`'s signature takes only `$dateFrom`/`$dateTo` — no prior-period computation exists anywhere in this file to compare against. Reworded the suggested-prompt text from "Why did my win rate drop to X%?" to "How can I improve my X% win rate?" — same trigger condition, same underlying fact (a real below-40% win rate this period), just no longer asserting an unverified decline. Verified live against the real running app with a real 0%-win-rate day (4 real trades) — the suggested prompt correctly renders "How can I improve my 14.3% win rate?" (a different real window, same fix). `php -l` clean.

### §4.7 — real, live bug reported directly by Kevin (2026-08-10), beyond the original 6 findings

**Not a Type-2 narrative-branching bug and not one of the 6 audited findings above — a real scoping/context-assembly bug, discovered by Kevin's own manual UI testing and fixed the same session since it directly affects every one of the §4.1-§4.6 fixes' real-world reachability.** Reported symptom: selecting Current Week / Current Month / etc. in the Colex UI had no effect on chat responses — every question got the identical "All Time" answer (285 trades) regardless of which period was selected.

**Root cause, confirmed by direct read**: `api/colex/chat.php`'s context assembly (~line 172) gated the real date range behind `messageIsTimeBound($message)` — a pure text-keyword check ("today", "this week", ...) — and fell back to `getAllTimeRange()` whenever the message itself didn't contain one of those words. In practice, almost no normal conversational question contains a literal time keyword ("What are my strongest strategies?", "Analyze my trading performance"), so this fallback fired for nearly every real message, regardless of the period explicitly selected in the UI dropdown. Confirmed the frontend was never the problem — `public/colex.php`'s `currentPeriod` (set by the dropdown's `updatePeriod()`) is correctly included in every `/api/colex/chat` POST body — and confirmed the dropdown has no "All Time" option at all (9 real choices: current/previous week/month/quarter/year, custom), so `getAllTimeRange()` was never even a state the UI could represent, only a silent, undisclosed fallback. Also confirmed `messageIsTimeBound()`'s result never actually customizes the computed range to match the message's own specific wording (e.g. "last month" doesn't parse into a "last month" range) — it only ever gated between the real `$period`-based range and all-time, so keeping it for this purpose served no function beyond the bug itself. Cross-checked against the sibling endpoint `api/colex/stats.php` (which feeds the greeting/stat cards at the top of the page — the part of the UI that WAS scoping correctly, exactly matching the report) — it already unconditionally calls `calculateDateRange($period, ...)` with no message-text gate, confirming this was the already-correct, established pattern chat.php should have matched from the start.

**Fix**: `$dateRange` now always computes from the real, explicitly-selected `$period`/`$dateFrom`/`$dateTo` — `calculateDateRange($period, $dateFrom, $dateTo)` unconditionally, matching `stats.php`. `$isTimeBound`/`messageIsTimeBound()` were deliberately left in place (not deleted) — `$isTimeBound` still legitimately gates `$recentTradesLimit` (10 vs. 100) a few lines below, a real, separate, unaffected use; reviewed that use directly and confirmed leaving it alone carries no regression risk (the common case, `$isTimeBound === false`, already used the generous 100-row limit before this fix, and the now-correctly-scoped `$dateRange` means a narrow period like a single week will rarely if ever have close to 100 real trades to fetch anyway).

**A second, real bug surfaced live while verifying this fix, fixed in the same pass**: `generatePerformanceResponse()`'s suggestion-building code (`abs($worstTrade) > abs($bestTrade)`) called `abs()` on `$bestTrade`, which is `null` (not `0`) whenever a period has zero winning trades — a state that was effectively unreachable before this fix (an "all time" ~285-trade blend almost always has at least one win) but is now a completely normal, common real state for any genuinely losing week. This threw a live PHP 8.1+ deprecation warning that rendered as literal HTML text prepended to the JSON response body (`<br /><b>Deprecated</b>: abs(): Passing null...`), which would break `response.json()` parsing in the real browser UI for any losing period. Fixed with `abs($bestTrade ?? 0)`.

**Verified live against the real running app** (temporary dev-only auto-login helper at the project root, deleted after use): the exact reported scenario reproduced and fixed — `current_week` (6 real trades) and `current_month` (15 real trades) now return genuinely different, correctly-scoped responses instead of the identical 285-trade all-time answer, confirmed across all 6 intent types (performance/drawdown/discipline/patterns/comparison/advice) × 4 real periods (current_week/current_month/current_quarter/previous_month) = 24 combinations, zero PHP errors in any of them; spot-checked `Analyze my trading performance` across all 4 periods showing 4 genuinely different real trade counts (6/15/144/129) with correct real date-range labels (`Aug 10 - Aug 10, 2026` / `Aug 1 - Aug 10, 2026` / `Jul 1 - Aug 10, 2026` / `Jul 1 - Jul 31, 2026`); confirmed the `abs(null)` deprecation warning is gone and the JSON response is clean for a real 0%-win-rate week, with "Review my largest loss" still correctly appearing in suggestions. `php -l` clean.

### §4.8 — natural period-aware phrasing, requested directly by Kevin (2026-08-10), as the benchmark for templated-but-natural responses

**Explicit product request, not a bug report — using this as a durable design benchmark, not just a one-off fix**: even correctly-scoped period data (§4.7) still surfaced with a raw reporting-period label — `"This period (Aug 10 - Aug 10, 2026) you took 6 trades..."` — which reads like a database query result, not a coach talking to a trader. Kevin's benchmark: *"Today is the first day of your trading week. You've taken 6 trades so far, with a 0% win rate,"* generalized across week/month/quarter/year, first-day vs. later-in-period vs. already-complete, and 0/1/many trades — deterministic and rule-based underneath, but never exposing raw reporting mechanics unless the exact dates are genuinely useful (a custom, user-picked range being the one case where they are).

**Design**: two new shared functions in `src/ColexHelpers.php`, `colexPeriodPosition($period, $dateRange)` and `colexOpeningTradeCount($pos, $totalTrades)` — one source of truth every response generator now builds its opening/period references on, instead of each one independently phrasing (or mis-phrasing) "this period."
- `colexPeriodPosition()` classifies the selected period into: `is_custom` (no natural period name — a user-picked range, where exact dates ARE shown, since the trader chose them on purpose), `is_complete` (a `previous_*` period, or a custom range that's already fully elapsed — past tense, no "so far"), `is_first_day` (today is the period's own first calendar day — only meaningful for an ongoing named period), plus a `noun` (week/month/quarter/year), an `intro` sentence for the first-day case, a `lead` clause ("So far this week" / "Last month" / "Between Aug 1 and Aug 15"), and a `mid_ref` short phrase ("this week" / "last quarter" / "in that date range") for swapping into the middle of an already-written sentence in place of the old generic "this period."
- `colexOpeningTradeCount()` composes the natural "you've taken N trades..." clause from a `colexPeriodPosition()` result, handling 0/1/many trades (correct singular/plural, and a distinct honest sentence for zero trades — no win rate/P&L clause follows since there's nothing to report) and the right verb tense (`took`/`was` for complete periods, `taken`/`is` for ongoing ones).
- **`$context` gained a new `period_key`** (`api/colex/chat.php`, alongside the already-resolved `period` date-range dict) — the raw selector value (`current_week`, `previous_month`, ...) needed by `colexPeriodPosition()`, which every generator already receives via `$context` with no new plumbing required.

**Applied to**: `generatePerformanceResponse()`'s opening (the primary target, matching Kevin's exact benchmark), `generateGeneralResponse()`'s opening (same "you've made N trades" pattern), and every other generator's smaller mid-sentence "this period" mentions — `generateDisciplineResponse()` (the deviation-count line and all 3 consistency-tier sentences), `generateDrawdownResponse()` ("peak X% this period" → "peak X% last month"/"this week"), `generatePatternsResponse()` ("This period, N trades flagged..." → "This week, N trades flagged..."), and `generateComparisonResponse()`'s own §4.1 no-prior-data fallback message. `assembleUserContextText()` (the plain-text block fed to the LLM reasoning path, not a user-facing message — "data assembly, not response generation" per its own doc comment) was deliberately left untouched, matching Kevin's own stated exception: exact dates belong in the context/header, this function's whole job.

**One real bug caught and fixed by my own unit test before shipping**: the first version produced a redundant double "so far" for ongoing named periods — `"So far this week, you've taken 6 trades so far"` — because the lead clause already says "So far" and the trailing clause appended it again. A custom date-range's lead doesn't say "so far" on its own, so that case correctly still needed the trailing "so far" to convey the range includes today. Fixed by only appending the trailing "so far" for the custom case.

**Verified**: a dedicated unit-test script covering every combination — week/month/quarter/year × first-day/later-in-period/previous-i.e.-complete, custom (same-day / range-including-today / range-entirely-in-the-past), and 0/1/6/42 trades for each — every one read naturally, matching Kevin's benchmark examples exactly for the cases he gave (`"Today is the first day of your trading week. You've taken 6 trades so far"`, `"So far this month, you've taken 42 trades"`, `"So far this quarter, you've taken 126 trades"` — the equivalent generalizes correctly to year: `"So far this year, you've taken 285 trades"`) and generalizing sensibly for the ones he didn't (previous/complete periods correctly switch to past tense with no "so far"; custom ranges correctly keep exact dates). Then verified live against the real running app across all 6 intent types × 9 period values (current/previous week/month/quarter/year + custom) = up to 63 real HTTP requests, zero PHP errors in any of them; spot-checked the exact live output for `current_week` (today genuinely is the real first day of the real current week on this server) — `"Today is the first day of your trading week. You've taken 6 trades so far, with a 0% win rate..."`, an exact match to Kevin's own benchmark sentence, generated live from real data, not hand-crafted. `php -l` clean on both touched files.

### §4.9 — response wording variety + contextual, timeframe-scoped suggested prompts, requested directly by Kevin (2026-08-10), same day

**Two related, explicit follow-up requests, addressed together.**

**1. Response wording variety.** §4.8 gave each timeframe exactly one phrasing per bucket — natural individually, but "So far this week...", "So far this month...", "So far this quarter..." back to back still reads templated. `colexOpeningTradeCount()` (`src/ColexHelpers.php`) now holds 2-3 real phrasing variants per bucket (e.g. week: *"So far this week, you've taken N trades"* / *"You're N trades into the week so far"* / *"This week is shaping up with N trades so far"*; month: adds a real *"{MonthName} is underway with N trades so far"* variant, using the period's own real month name — computed from the period's start date, not "now," so a `previous_month` view correctly names July while viewed in August), selected via a new `colexPickVariant()` — the identical deterministic-variety idiom already established elsewhere in this codebase (`ProgressiveInsightsEngine::pickVariant()`), seeded from the real inputs (bucket + position + trade count + today's date) so the same real situation stays stable within a session but genuinely changes when the real numbers do — never randomness.

**2. Contextual, situational, timeframe-scoped, strategically-selected suggested prompts.** The old `generateSuggestedPrompts()` was a flat, order-dependent chain of 5 absolute-threshold checks (win_rate<40, revenge>0, drawdown>5, alerts>3, pnl<-2), with zero timeframe-awareness (identical prompts regardless of week/month/quarter/year) and no situational floor at all for thin samples. Rebuilt around three new pieces:
- **`colexTimeframeBucket($period, $dateRange)`** — maps the selector to a coaching-depth bucket (week=immediate/tactical, month=pattern recognition, quarter=deeper analysis, year=career-level reflection). A custom range has no bucket of its own — depth is inferred from its real day-span (≤10 days→week-depth, ≤45→month, ≤120→quarter, else year), so a 3-day custom range gets the same immediacy as a real week and a 300-day one gets year-level framing.
- **`colexClassifyTraderSituation($stats, $bucket)`** — sample-size adequacy is checked *first* and overrides every performance signal below it (a trader with 2 trades this quarter is never told their performance is "strong" or "poor" — per-timeframe floors of 3/5/10/20 trades for week/month/quarter/year, judgment calls sized to match this app's other established n-aware gates), then classifies into `no_trades` / `very_small_sample` / `repeated_mistakes` (≥2 revenge trades or ≥10% real drawdown) / `high_wr_poor_pnl` (winning often, losing money — a payoff-structure problem) / `low_wr_good_pnl` (losing often, still profitable — a protect-the-edge situation) / `strong` / `poor` / `mixed`. Built entirely from `getPeriodStats()`'s already-computed fields — no new query, no new independent fact.
- **`colexSuggestedPromptBank()`** — a real, hand-written inventory of 2-3 prompts per (bucket × situation) cell — 32 cells, ~85 distinct prompts — each cell's wording pitched at that timeframe's own coaching depth rather than the same sentence with the noun swapped (e.g. week/poor: *"What is hurting my performance this week? / Show me where I'm going wrong. / What should I change before my next trade?"* vs. quarter/poor: *"What has changed in my trading this quarter? / What's the real driver behind this quarter's results? / Where did this quarter go wrong?"*). `generateSuggestedPrompts()` picks the matching cell, caps at 3 via the same seeded rotation idiom as item 1, and returns them with a per-situation `task_type` (`journal` for mistake/pattern-diagnosis situations, `performance` for win-rate/P&L-shape situations, `general` for pre-trading prep).
- Both real call sites (`api/colex/stats.php`, `public/colex.php`) updated to pass `$period`/`$dateRange` through (the function's new params default to today, so nothing else calling it with the old 1-arg signature would break, though no other caller exists).

**A real, pre-existing SSOT gap found and fixed before building on it — surfaced directly by Kevin's own question ("we just ensured no page computes its own facts independently... let me know if you have a good reason to think otherwise before proceeding").** `getPeriodStats()` (pre-existing, not written this pass) had two fields computed by local re-derivation instead of the real SSOT: `pnl_percentage` = `net_pnl ÷ current active-account balance` (a 2026-08-08 comment described this as "the same denominator the original formula used," i.e. an old, pre-SSOT-audit computation preserved unchanged during an otherwise-real SSOT rewrite of the *other* fields in the same function) vs. the app's real `return_pct` = `net_pnl ÷ starting balance` (the same denominator every other page's "return" figure already uses, already sitting unused in `AnalyticsEngine::getUserMetrics()`'s own return array); and `drawdown_percentage` = `worst single trade ÷ balance` (a real, useful number, but not a drawdown at all — no relationship to the peak-to-trough equity-curve concept "drawdown" means everywhere else in this app) vs. the real `current_drawdown_pct`. Confirmed both `getUserAccounts()` calls already scope to active accounts only (the shared default), so there was never a genuine account-scope reason for a second computation — only the definition differed, and the classifier I was about to build was specifically going to lean on the mislabeled `drawdown_percentage` field for its `repeated_mistakes` bucket. **Fixed**: both fields now read directly from `$metrics['return_pct']`/`$metrics['current_drawdown_pct']` (already fetched, no new query), keeping the same field *names* (`pnl_percentage`/`drawdown_percentage`) so nothing else reading `$stats` breaks, only the *values* changed to the real, already-correct ones. Verified live: the account's real year-to-date return is now a genuine 63.28% (return on starting capital) instead of the old formula's heavily diluted current-balance-denominator figure; the real 8.11% year drawdown now reflects an actual peak-to-trough equity read, not one worst trade's size relative to balance.

**Verified**: a unit-test script covering 12 real classification scenarios (no trades, very-small-sample at both the week and month floor, repeated-mistakes via both revenge-trades and severe-drawdown triggers, both payoff-shape situations, strong/poor at multiple timeframes, mixed/borderline, plus custom-range depth inference at 3/20/90/300 days) — every situation resolved to the correct bucket and returned real, situationally-appropriate prompts; confirmed the same real situation returns byte-identical prompts across repeated calls (no randomness) and confirmed custom-range depth inference lands in the right bucket at all four tested spans. Live-verified against the real running app: `current_week` (a genuine 0%-win-rate, negative-P&L week) returned *"What is hurting my performance this week? / Show me where I'm going wrong. / What should I change before my next trade?"* — an exact match to Kevin's own example, generated from real data; `current_quarter`/`current_year` (both genuinely carrying 2/17 real revenge-trade flags) correctly returned `repeated_mistakes`-bucket prompts; a genuine zero-trade custom range correctly returned prep-focused prompts with `general` task_type. Full sweep across all 8 period values plus the real `public/colex.php` page render (not just the JSON endpoint) — zero PHP errors throughout. `php -l` clean on all touched files.

**Checked, correctly guarded, not reported further**: `generateGeneralResponse()`'s phase-aware total-balance computation (confirmed already fixed, 2026-08-08, per in-code comment citing the SSOT audit directly); `generateDrawdownResponse()`'s three-way branch (losing streak / high current DD / healthy) — correctly worded for each real state; `generatePatternsResponse()`'s tilt/revenge signal (confirmed already fixed to read real `trade_classification` rows, per in-code comment citing the SSOT audit); `ColexHelpers::generateGreeting()`/`generateContextCards()` — correctly conditioned throughout.

### §4.10 — 100% exhaustive completion pass, requested directly by Kevin (2026-08-10), same day: "now do 100% exhaustive colex fix we have it completely fixed" + "make everything and environment suitable for Claude API to just enter seamlessly when we get the key"

**Every function in both files was read in full, top to bottom** (`api/colex/chat.php`'s complete function inventory: `generateUUID`, `truncateTitle`, `messageIsTimeBound`, `getAllTimeRange`, `loadKnowledgeBase`, `getUserPlanContext`, `assembleUserContextText`, `buildColexSystemPrompt`, `getConversationHistoryForLLM`, `getColexApiKey`, `callAnthropicAPI`, `describeWorstTrade`, `describeBestTradeInstrument`, `detectIntent`, `generateResponse`, and all 7 intent generators; `src/ColexHelpers.php`'s remaining un-audited functions — `generateGreeting`, `generateContextCards`, `generateSuggestedPrompts`), not just the six functions §4.1-§4.6 originally flagged. This surfaced real findings beyond the original 6 + §4.7-§4.9 scope, several only visible through live HTTP testing against real trade data rather than code review alone.

**Correction to §4.9's own "checked, correctly guarded" claim.** `generateGreeting()` was listed there as "correctly conditioned throughout" — true in the narrow sense that its P&L branch (`if ($stats['pnl_percentage'] != 0)`) was never *wrong*, only that it still said the same generic `"this period"` §4.8 had already replaced everywhere else, one visible gap below the bar §4.8/§4.9 set. Extended `generateGreeting($stats, $period, $dateRange)` (new optional params, both real call sites — `public/colex.php`, `api/colex/stats.php` — updated to pass them) to build its P&L clause from `colexPeriodPosition()`'s real `mid_ref` instead, with 2 phrasing variants via `colexPickVariant()` for the same anti-templating reason as §4.9.

**The 2 generic "this period" error-fallback messages flagged (not yet fixed) in this session's own earlier "is Colex fully fixed?" answer** — `generatePerformanceResponse()`'s and `generateDrawdownResponse()`'s `!$metrics || isset($metrics['error'])` guards — now also build from `colexPeriodPosition()`'s `mid_ref` instead of a bare "this period."

**A recurring formatting bug class, found once by direct read then confirmed systemically via a full-file grep sweep**: `'$' . number_format($value, 2)` (or its double-quoted `"\$..."` equivalent) renders **`$-450.00`** for a negative `$value` — PHP's `number_format()` puts its own minus sign first, so concatenating a literal `"$"` in front lands the currency symbol *before* the sign. Centralized into one new shared `colexSignedMoney($amount, $withPlus = false)` (`src/ColexHelpers.php`) — `"-$450.00"` never `"$-450.00"`, with an optional `+` prefix for delta/comparison figures — and applied everywhere a genuinely signable value (net P&L, expectancy, session P&L, comparison deltas, total balance) was previously built with the raw concatenation. Confirmed via full-file grep after fixing that every remaining raw `'$' . number_format(...)` site is a value that's either already `abs()`'d or structurally non-negative (best_trade, avg_winner, a "remaining budget" magnitude) — not a missed instance of the same bug.

**Three real, live, previously-undetected bugs caught specifically through live HTTP testing against a genuine losing week** (not visible from code review alone, since they only manifest with real all-negative or all-losing data):
- `generatePatternsResponse()`'s best/worst-session loop seeded its "find the maximum" comparison with `PHP_FLOAT_MIN` — which is PHP's smallest **positive** representable float (~2.2×10⁻³⁰⁸), not negative infinity, a well-known PHP naming footgun. On a week where every qualifying session's P&L was negative, no real value could ever satisfy `$pnl > PHP_FLOAT_MIN`, so `$bestSession` silently stayed `''` and "Best Session" rendered blank even though a real, qualifying session existed. Fixed to `-PHP_FLOAT_MAX` (the true negative-infinity-equivalent sentinel for this comparison).
- `distance_to_breach.amount`/`.percentage` and `daily_drawdown.remaining_amount` were assumed to always be non-negative "room remaining" magnitudes and left unguarded — live testing on a real account already past its drawdown limit proved this wrong: the message read *"You're $-5,029.68 away from the breach level"* (backwards — implies room to spare when there is none) alongside *"⚠️ You are dangerously close to breaching"* (also wrong — the account has already breached, not merely close). Fixed: both the message and the "Distance to Breach"/"Daily DD Remaining" cards now branch on sign — a negative value relabels to "Past Breach Level By"/"Daily DD Exceeded By" and states the real magnitude, and the warning line distinguishes "already breached" from "dangerously close."
- `detectIntent()`'s ordered keyword list had `'how is my'` in the `performance` bucket (checked first) — an unbounded sentence-structure prefix that matches *any* "How is my X?" question regardless of topic. `"how is my discipline"` and `"how is my drawdown"` were both silently routed to the performance response before the loop ever reached the `discipline`/`drawdown` keyword lists. Confirmed live (Apache's own `[Colex] intent='performance' matched keyword='how is my'` log line for a literal "how is my discipline" message) before and after the fix. Removed `'how is my'` from the performance list — `'performance'` (the bare word) and `'how am i doing'` already cover the legitimate vague-performance-question case without the false-positive-prone catch-all.

**One phrasing bug caught via live output, not code inspection**: `describeWorstTrade()` already returns a signed string (`"-$49.62 on BTC/USD..."`), and `generatePerformanceResponse()`'s only call site said *"Your worst trade lost {$worstDetail}"* — rendering *"Your worst trade lost -$49.62..."*, a confusing double-negative ("lost" already conveys the negative direction; pairing it with a leading "-" reads as if it could be a gain). Fixed to "was" (`"Your worst trade this period was -$49.62..."`), matching how `generateAdviceResponse()`'s own sibling call site (`"Your biggest loss recently was {$worstTradeDetail}"`) already phrased the identical string correctly.

**Null-vs-0 masking, the same principle §4.x already established for `best_trade`/`worst_trade` elsewhere in this file, found un-applied in two sibling functions**: `generateDisciplineResponse()`'s `avg_winner`/`avg_loser` (`?? 0`, rendering "your average loser is $0.00" for a genuine "no losing trades yet" state) and `generateAdviceResponse()`'s identical pattern feeding its "losers larger than winners" check and R:R-ratio card. Both now keep `null` through the comparison and only render a value when real data exists — with matching downstream fixes (`$rrDiscipline`'s calculation guarded the same way, since `avgWinner / abs(null-treated-as-0)` would have silently produced a fabricated "R:R of 0" card instead of an honest "—"). A related, distinct bug in the same block: `generateAdviceResponse()`'s negative-expectancy check (`if ($expectancy <= 0)`) unconditionally asserted *"negative, meaning the average trade loses money over time"* even when expectancy was **exactly** `$0.00` — a real, distinct breakeven state, not a loss. Split into two real branches.

**`generateComparisonResponse()` gained a sample-size caveat** — neither side of the comparison was ever gated on trade count (only `$prevTrades === 0`, an exact-zero check, not a "too thin to trust" one), so a 1-trade period compared against a well-sampled prior one still rendered a fully confident "P&L changed X% (declined)" claim. Reused the exact per-timeframe floors §4.9's `colexClassifyTraderSituation()` already established (3/5/10/20 for week/month/quarter/year) — extracted into a new shared `colexTimeframeFloor($bucket)` so both callers read the same numbers instead of each keeping an independent copy that could drift. Below the floor on either side, the response now appends an honest "small sample, treat as a rough signal" caveat rather than silently omitting one.

**Two "seamless LLM entry" readiness fixes, per the explicit instruction to prepare the codebase for the moment a real `ANTHROPIC_API_KEY` is configured** (confirmed via `php -r` that the key is currently empty, so the LLM reasoning path — `callAnthropicAPI()` over `assembleUserContextText()`/`buildColexSystemPrompt()` — is presently unreachable and every fix this session correctly targeted the live, active deterministic-fallback path instead; these two are the only issues found that specifically affect the *dormant* LLM path):
- `assembleUserContextText()` (the plain-text block the real LLM will reason over once a key exists) had the identical un-signed-money and un-abs'd-drawdown bugs as the fallback engine — "Net P&L: $-450.00" / "Current drawdown: -8.11%" would have fed the model slightly wrong-looking facts the moment it went live. Fixed with the same `colexSignedMoney()`/`abs()` treatment.
- `COLEX_MODEL` was still set to `'claude-sonnet-4-5-20250929'`, a stale placeholder from an earlier model generation, with an in-code comment already flagging it as unverified. Updated to `'claude-sonnet-5'`, the current model ID, per direct knowledge (not a guess) from this assistant's own system context.

**Two hygiene items**: the doc comment above `messageIsTimeBound()` still described the pre-§4.7 "convenience default, only narrows on explicit time language" behavior as current — corrected to describe the real, current behavior (unconditional `calculateDateRange($period, ...)`, `messageIsTimeBound()`'s only remaining real use is sizing `$recentTradesLimit`). `getAllTimeRange()` — confirmed via a full-repo grep to have zero real call sites since §4.7's fix removed its only caller — flagged as orphaned dead code in its own doc comment (left in place, not deleted, per this codebase's established additive-only convention) rather than left to read as if it were still live.

**Verified, this session**: `php -l` clean on both files throughout. A standalone unit-test script covering `colexSignedMoney()` (7 cases, positive/negative/zero, with/without the `+` prefix, a string input), the extracted `colexTimeframeFloor()` (confirmed byte-identical output to the old inline array it replaced), `colexClassifyTraderSituation()` (4 sanity cases, confirming the refactor didn't change behavior), and `generateGreeting()`'s new period-aware phrasing (confirmed no "this period" and no sign bug across 3 real scenarios) — all passed. Real end-to-end HTTP verification through the live Apache/MariaDB stack (temporary dev-only auto-login helper at the project root, created and deleted at the end of the session): all 7 intents × the 251-trade primary test account across current_week/current_month/current_quarter/current_year, plus the 1-trade edge-case account across current_month (zero trades in scope — correctly clean "haven't taken any trades yet" states) and current_year (exactly 1 trade in scope — every null-guard, small-sample tier, and intent-routing fix exercised against a genuine n=1 real record). Caught and fixed 3 of the findings above (`PHP_FLOAT_MIN`, the breach-amount sign bug, `detectIntent()`'s `'how is my'` collision) only *because* of this live testing — none were visible from static code review, confirming live verification against real data found real bugs beyond what reading the source could. Checked Apache's error log across the entire testing window for real `PHP Warning`/`Notice`/`Deprecated`/`Fatal error`/`Parse error` strings (excluding this file's own `error_log()` debug notices) — zero in every response generated after the fixes; the only 3 deprecation lines found were timestamped before this session's fixes were applied (confirmed stale by individually re-verifying every remaining `abs()` call site in both files is now null-safe, either by an explicit `!== null` guard or a `(float)(... ?? 0)` cast before the call). `git diff --stat` reviewed before committing — confirmed the diff against the last commit (`a1e7db1`) touches only the functions this session intentionally edited, with no unrelated pre-existing work bundled in.

---

## 5. Calculator Warnings (`calculator-source.html`)

Completed as a background agent; two of the highest-severity findings personally verified by direct read (see inline notes). Scope excludes calculation-correctness/precision (separately deferred per `CALCULATOR_BUSINESS_LOGIC_PHP_MIGRATION.md`) and Type-1 threshold-value mismatches already covered by the SSOT audit.

| # | Location | Finding | Severity |
|---|---|---|---|
| 1 | RR-based `tcWarnings`, line ~3722 | **[PERSONALLY VERIFIED]** `"GOOD SETUP"` is gated on `rr1tc >= 1.0` alone — no margin, breach, or TP-direction check at all. Confirmed by direct comparison against the sibling `"CLEAN SETUP"` message three lines later (~3761), which correctly requires `rr1tc>=1.0 && tcTradesRemainingW>=3 && drawdownPct2<25 && tcMarginPct<=30 && !tcWrongTPs.length` — proving the stricter guard was written once, for one message, and never applied to the weaker sibling. "GOOD SETUP — your reward justifies the risk. Stay with the plan" can render directly alongside `"MARGIN CALL TERRITORY"`/`"TRADE EXCEEDS ACCOUNT"` (both of which also trigger a hard `tcShowBlocker`), an already-breached account (per `pfWarningBanner`'s independent computation), or `"TAKE PROFIT ON THE WRONG SIDE"` (since `rr1tc` is `Math.abs(...)`, it stays ≥1.0 even with an unachievable TP direction). **[FIXED 2026-08-09]** | HIGH |
| 2 | RR-based `tcWarnings`, line ~3718 (also main-path `warnings`, ~4257) | `"SOLID RISK SIZING"` is gated on `1 <= tcPct <= 2` alone. Because lot size scales inversely with SL distance at a fixed risk%, a tight-stop 1-2% trade can simultaneously trigger `"MARGIN CALL TERRITORY"` while still showing *"You're in the professional range. This is where consistent traders live."* **[FIXED 2026-08-09]** | HIGH |
| 3 | `"CLEAN SETUP"`, both paths (~3761, ~4302-4304) | **[PERSONALLY VERIFIED, guard list confirmed]** Neither copy's guard list includes any check on daily-loss-limit consumption — only overall drawdown (`drawdownPct2`/`drawdownPercent`). Daily-loss proximity is a fully separate computation (`_updateWarning()`, fed by `updatePFStatus()`). A trade whose risk equals or exceeds the account's full daily loss limit can still render *"drawdown well managed. This is what a professional trade looks like, stay with the plan"* while a separate, always-visible banner on the same page says the opposite about the daily limit specifically. **[FIXED 2026-08-09]** | HIGH |

> **Correction, found during the fix pass on §5.1-§5.3 (2026-08-09) — re-verifying this row's own claim surfaced something worse than what was originally documented, not just confirmation of it.** The main/standard-path `CLEAN SETUP` (`warnings.push(...)`, was ~4302-4304) didn't just lack the daily-loss check this row describes — it also had **no margin check and no wrong-TP check at all**, unlike its RR-path sibling three lines apart in the other code path, which at least had those two. The main-path copy carried a code comment directly above the RR-path version calling it *"PARITY clean-setup affirmation (mirrors the standard path's all-clear)"* — i.e. the RR-path was built to match the main path, but the main path it was matching turned out to have the weaker guard, not the stronger one the comment implied. Both gaps (the daily-loss check this row documents, plus the margin/wrong-TP gap found on re-verification) were fixed together via one shared `_tradeIsClean()` helper, applied identically to `GOOD SETUP`/`SOLID RISK SIZING`/`CLEAN SETUP` in both code paths. Live-verified via three real headless-Chrome runs against the actual file: the original 70%-risk/$5,000/prop-firm reproduction (all danger signals fire, all three green messages correctly suppressed), a standard-path-specific margin scenario proving the newly-found gap is closed, and a clean-trade negative control confirming the fix doesn't over-suppress. See git history for the exact commit.


| 4 | `updateBreachBalanceNote()`, line ~4800-4801 | Hardcodes `"Intent at breach: Max DD exceeded"` unconditionally (tier fires whenever post-breach trade count > 3), directly adjacent to and contradicted by the correct, real, trader-confirmed breach reason quoted one clause later in the same sentence (`outcomeLabel`, which per this app's own Lifecycle Rule is confirmed free-text/varied, e.g. `News Trading Restriction`, `Consistency Rule`). Produces literally self-contradictory sentences like *"Intent at breach: Max DD exceeded · Reality: You confirmed News Trading Restriction."* **[FIXED 2026-08-09 — see correction below]** | HIGH |

> **Correction, found during the §5.4 fix (2026-08-09) — a reachability gap, not a severity change.** The `count > 3` tier this row describes is driven entirely by `data.post_breach_trade_count`, sourced from `/api/account/state.php`'s JSON response. Direct read of that endpoint's full `json_encode([...])` output, confirmed with a whole-backend grep, found **no server-side code anywhere returns a `post_breach_trade_count` key** — meaning `updateBreachBalanceNote()`'s own fallback (`count` defaults to `0`, then floored to `1`) fires every time, and the `count <= 3`/`count > 3` tiers — including the self-contradictory sentence this row documents — are currently **unreachable in the live app**. The fix (replacing the fabricated "Intent at breach: Max DD exceeded" with the same honest "Breach reason: {outcome} (confirmed {date})" phrasing the `count <= 3` tier already used correctly) was kept in place regardless, per this app's own established "flag orphaned-but-correct code, don't leave wrong text sitting there" convention — verified via an isolated unit test (both the real, field-absent API shape, and a hypothetical `post_breach_trade_count=5` case proving the corrected text is right for whenever/if this tier is ever wired up). Building the actual `post_breach_trade_count`/`live_balance` computation server-side was explicitly deferred — real backend computation, materially bigger than a narrative-text fix, and a separate scoping decision.


| 5 | `renderPropFirmFallback()` (~5802-5927) vs. `renderAccountImpact()` (~5599-5685) | The offline/unreachable-backend fallback path renders byte-identical wording to the live, server-verified path (*"⚠ Account breached...", "✓ Profit target reached..."*) with zero disclosure that the fallback's numbers are a local, potentially-stale estimate — confirmed via the file's own developer-only code comments elsewhere acknowledging the fallback's inputs "go stale the moment the account is promoted/funded" and that daily-loss state here is "genuinely unknown without a server round trip," neither of which reaches the trader-facing text. **[FIXED 2026-08-09]** | HIGH |
| 6 | `renderLiveAccountCard()`, lines ~5390, ~5419-5435 | `var accountBlown = (curBal !== null && curBal > 0 && tradeRisk > curBal);` — the `curBal > 0` clause excludes the exact worst-case state the surrounding code comment says it exists to handle (an account already at/below zero from a prior trade). In that state, the "⚠ this trade will blow the account" warning never fires, and the projected-balance field falls through to a bare, unexplained `"$0.00"` — the calmest possible message at the worst possible real state. **[FIXED 2026-08-09]** | HIGH |

> **Fix note (2026-08-09).** Split into two explicit booleans — `accountAlreadyBlown` (`curBal <= 0`, the previously-excluded case) and `accountWouldBeBlown` (the original `curBal > 0 && tradeRisk > curBal` logic, unchanged) — each with its own accurate message, since "already blown before this trade" and "would be blown if this trade loses" are genuinely different facts the trader should read differently. Verified live via three real headless-Chrome runs through the actual save flow (`window.saveTrade()` → `fetchAccountImpact()` → `renderAccountImpact()` → `renderLiveAccountCard()`, with only the network boundary mocked via a `window.fetch` monkey-patch — Chrome refuses real `fetch()` calls to `file://` origins outright, so this substitutes for a live server round-trip while exercising the real application code unchanged): $0 balance correctly renders "Account already blown ($0.00 remaining)" and the new distinct warning; $30 balance against ~$50 trade risk correctly renders the original "Account wiped" / "risks more than your account balance" pairing unchanged; $5,000 balance renders normally with no warning. **Side discovery, not fixed (out of scope for this finding):** while building the test harness, found that `fetchAccountImpact()` has two different account-id resolution rules in the same function — the early live-card-loading-state gate (~line 5836) checks `t.account_id` alone, while the actual fetch-triggering check three lines later also falls back to the URL's `account_id` query param. In practice these two fields are always stamped together from the checklist flow, so this likely doesn't misfire in production, but it's a real, structural inconsistency worth a look in a future session — not one of the 10 named findings, so left untouched here.
| 7 | `renderAccountImpact()` phase-completion banner (~5617-5634) | Offers *"Visit Add Account to promote to the next phase"* with no `isFunded`/`isInstant` guard — in direct contrast to the profit-target section 20 lines below in the *same function*, which explicitly skips itself for funded accounts with a code comment stating why. Depends on server-side `phase_completed` behavior not visible in this file to confirm live triggering, but the client provides no defense if it's ever set for a funded/instant account. **[FIXED 2026-08-09]** | MEDIUM |

> **Fix note (2026-08-09).** Added `&& !isFunded` to the banner's own condition — reusing the exact `isFunded` variable (already correctly folding in `isInstant`) the profit-target section already computes a few lines above, not a new check. Verified live via three real headless-Chrome runs through the actual save flow, with the server response mocked to report `phase_completed: true` for three lifecycle states: a funded two-step account (banner now correctly stays hidden, was previously going to say "Phase 3 profit target reached! Visit Add Account to promote to the next phase" — nonsensical for a funded account), an Instant Funding account (same, correctly hidden), and a genuine Phase 2 account (unchanged — banner still correctly renders "Phase 2 profit target reached!").
| 8 | `_updateWarning()`, lines ~1964-1970 | The daily-loss-consumption warning compares only *this one trade's* isolated risk against the full daily limit — never netted against losses already realized earlier today (unlike the overall-drawdown half of the same function, which correctly reflects the account's full current state). A trader who already burned 90% of today's limit on prior trades sees no elevated warning when sizing a new, individually-small trade. **[FIXED 2026-08-09]** | MEDIUM |

> **Fix note (2026-08-09).** `_updateWarning()` fires instantly on Calculate, before any server round trip — it structurally cannot know today's already-realized losses at that moment, so its two messages were reworded to say so explicitly ("this trade **alone** consumes X%... **not counting any losses already taken today**") rather than silently implying a full-day picture they don't have. Separately, real netted data (`data.today_losses`) *does* arrive later, once `/api/trades/validate` resolves inside `renderAccountImpact()` — a new `window.updatePFStatusFromServer(ddUsed, maxDD, dailyUsedAmt, dailyAfterAmt, dailyLimit)` refreshes the same banner at that point with the real cumulative total (today's realized losses + this trade, against the real limit), closing the gap for real rather than only disclosing it. No new computation invented — `dailyUsedAmt`/`dailyAfterAmt`/`dailyLimit` were already being computed from the real server response for the Prop Firm Status panel's own Daily Loss display; this just also feeds them into the warning banner. Verified live via two real headless-Chrome runs against the file with a mocked network boundary (see the Method note below): (1) a $50 trade with $200 already lost today against a $250 limit — the instant local banner correctly stays silent (this trade alone is only 20%, below threshold), then correctly shows "⚠ Today's losses ($200.00) plus this trade would reach or exceed your full daily loss limit ($250)" once real server data arrives — the exact scenario the finding described as silently missed before; (2) a $150 trade with $100 already lost today — instant banner correctly shows the reworded "alone... not counting" wording (60%), then correctly updates to "Today's losses plus this trade would reach 64% of your daily loss limit" once server data lands.
>
> **Independently confirmed end-to-end by Kevin (2026-08-09), against the real backend — this is the stronger evidence.** Every one of the automated verifications above ran against `file://` with the network boundary mocked by hand (Chrome refuses real `fetch()` calls from `file://` origins outright), so they proved the client-side logic was correct given an assumed server-response shape, not that `/api/trades/validate.php` actually produces that shape. Kevin closed that gap manually: created a real 2-step $5,000 Phase 1 account, recorded three real closed losing trades ($49.62, $49.62, $48.56 — summing to $147.80), then opened the Calculator for a fourth trade against the real running app (real Apache/PHP/MySQL, real session) without saving it. The live Confirm Trade dialog correctly showed "Daily Loss: $148 of $200 (73.9%)" and "This Trade Impact: $148 → would become $196 (98.2%)" — $200 matches the account's real declared 4% daily-loss rule on $5,000, $147.80 rounds to the displayed $148, and $147.80 + $48.56 (the new trade's own real risk amount) = $196.36 ≈ 98.2% of $200. Every number is internally consistent and traces to a real backend computation, not a mock — confirms both the fix and the JSON-shape assumption (`daily_loss_used_pct`/`today_losses`/`daily_loss_projected_pct`) it was built on.
>
> **Wording follow-up, requested directly (2026-08-09), same day.** Kevin's confirmation above also flagged that even though the §5.8 numbers were now correct, the display format itself — *"This trade impact: $148 → would become $196 (98.2%)"* — was hard to parse: unclear what $148/$196 each represented, what the arrow meant, or how the pieces related. Not a logic defect (the calculation was already right), a pure clarity request. Both Overall Drawdown and Daily Loss cards (in both `renderAccountImpact()` and `renderPropFirmFallback()`) were split from 2 sub-rows into 3, each answering exactly one question, with a new "This Trade Risks" row added: **"Already Lost (Today)" → $X of $Y (Z%)**, **"This Trade Risks" → $X** (new), **"If Stopped Out, \[Drawdown/Today's Total\] Becomes" → $X (Y% of your $Z limit)**. Every dollar figure across all three rows was also unified to the same `.toFixed(2)` precision (was a mix of whole-dollar and cents), so "Already Lost" + "This Trade Risks" always visibly sums to the "If Stopped Out" total instead of looking like a rounding mismatch. No calculation changed — same `ddImpact`/`ddAfter`/`ddProjPct`/`dailyAfterAmt`/`dailyProjPct`/`dailyLimit`/`tradeRisk` values, just restated in plain, separated sentences. Verified live: reproduced Kevin's exact real screenshot numbers ($147.80 already lost, $200 daily limit, ~$49 trade risk) — new render shows "Already Lost Today: $147.80 of $200.00 (73.9%)" / "This Trade Risks: $49.00" / "If Stopped Out, Today's Total Becomes: $196.36 (98.2% of your $200.00 daily limit)", matching the real backend's real percentages exactly. A real headless-Chrome screenshot of the rendered panel confirmed the extra row doesn't clip or overflow — labels wrap cleanly to a second line where needed.
>
> **Layout follow-up, requested directly (2026-08-10), next day.** Kevin manually tested the wording fix above in the real running app and confirmed the *numbers themselves were correct* ($15.72 Overall Drawdown used vs. $196.36 Daily Loss total — two different limits, both right), but reported he briefly misread the two as conflicting because of how the panel was laid out, not what it said: `.tc-pf-sub` was a single-line `display:flex; justify-content:space-between` row, so a long label ("If Stopped Out, Drawdown Becomes") wrapping to two lines snapped the value to baseline-align with the wrapped label's *last* line, floating it away from the label it belonged to; the three sections (Overall Drawdown / Daily Loss / Profit Target) shared a plain 14px flex gap with no divider, so rows from different sections had no strong visual boundary between them. Explicitly scoped as presentation-only — no wording, calculation, threshold, color, or logic changed. Two changes, both CSS/DOM structure only: (1) each `.tc-pf-sub` row restructured from one flex line to a stacked block — label on its own line above, value on its own line below — so a value can never visually detach from its label regardless of how the label wraps; (2) each section (`.tc-pf-section`) now has its own padding block with a `border-top` divider between sections, giving Overall Drawdown/Daily Loss/Profit Target a clear boundary within the same card. A new `_setValueParts()` helper (replacing `_setText()`/`_setImpact()` at the 8 sub-row call sites in both `renderAccountImpact()` and `renderPropFirmFallback()`, keeping both paths in parity) splits each value's leading dollar figure into a bold "primary" line and renders anything after it (e.g. "of $200.00 (73.9%)", "(122.2% of your $200.00 daily limit)") as a dimmer "secondary" line directly beneath — same exact words in the same order, just rendered as two DOM nodes instead of one string, so the browser only ever wraps inside the short secondary clause, never between the number and the words explaining it. Values with nothing to split (e.g. a bare "$49.00") fall through to plain text, unchanged. Caught and fixed one real regression before shipping: removing the old `.tc-pf-sections{display:flex;flex-direction:column}` rule (spacing moved onto the individual `.tc-pf-section` blocks instead) broke the panel into three side-by-side columns, because JS (`pfSections.style.display = 'flex'`) sets that inline `display:flex` directly and, without an explicit `flex-direction:column` anywhere, defaults to a row — caught via an initial screenshot, fixed by re-adding `display:flex;flex-direction:column` to the `.tc-pf-sections` rule. Verified live via headless Chrome (all 7 inline `<script>` blocks re-parsed clean with `new Function()` after every edit): a structural check confirmed every sub-row's primary+secondary text, concatenated, is byte-identical to the pre-existing wording (nothing reworded); real screenshots at 900px and a 420px narrow viewport confirm no clipping/overflow in either the cropped panel or the full "Confirm Trade" dialog (which already has `max-height:88vh;overflow-y:auto` on `.tc-confirm-card`, so the taller panel simply scrolls, exactly as it did before); the offline fallback path (`renderPropFirmFallback()`) screenshotted separately and confirmed to render the identical stacked/divided layout, in parity with the live path.
| 9 | `"MARGIN COMFORTABLE"` (~3744, ~4328) | *"Plenty of room. Your SL will execute cleanly."* — narrowly true about margin/execution mechanics, but the generically reassuring tone can render alongside unrelated warnings (daily-loss, buffer-nearly-gone) that this message never checks, risking a "you're fine overall" read from a margin-only fact. **[FIXED 2026-08-09]** | MEDIUM |

> **Fix note (2026-08-09).** Reworded both copies (RR path and standard path) to explicitly scope the claim: *"Margin: $X (Y% of account) — safe on margin specifically. Your SL won't be force-closed by a margin call (other risks, if any, are covered separately above)."* Removed "Plenty of room" (the generic-reassurance phrase) and named the specific mechanism (a margin call) the message actually rules out, rather than implying overall trade safety. This is a wording-only fix, not a gating change — unlike §5.1-§5.3's praise messages, `MARGIN COMFORTABLE` is a genuinely narrow, single-dimension fact worth stating on its own even when another warning is also present; the fix is that it can no longer be misread as broader reassurance while doing so. Verified live: real rendered output confirms "Plenty of room" no longer appears anywhere and the new scoped wording renders correctly.
| 10 | `renderLiveAccountCard()`, lines ~5440-5445 | Two independently-computed "account will be blown" warnings (client-side `accountBlown`, server-side `boundary_warning`) can both fire and are joined with no dedupe — repetitive, not misleading. **[FIXED 2026-08-09]** | LOW |

> **Fix note (2026-08-09).** Chained the three checks (`accountAlreadyBlown` / `accountWouldBeBlown` / `data.boundary_warning`) as `if`/`else if`/`else if` instead of two independent `if`s, so only the single most-specific message renders — the client-side checks (already correctly distinguishing already-blown vs. would-be-blown per the §5.6 fix) take priority, with the server's `boundary_warning` message surfacing only as a fallback when the client-side check didn't already catch something. No information is lost — a real server-flagged boundary the simpler client-side balance check misses still shows. Verified live via three real headless-Chrome runs: an already-blown account where the server *also* independently flags `boundary_warning: true` now correctly shows exactly one message (was two, redundant, before the fix); a healthy-balance account where only the server flags a boundary issue correctly still shows that fallback message (confirms nothing was silently dropped); a genuinely clean scenario stays silent.

**Checked, correctly guarded, not reported further**: `openBreachPopup`/`openClosedAccountPopup`/`openPassedPopup`'s state-specific gating (Closed correctly prioritized over Breached/Passed; Instant Funding correctly never reaches phase-specific "you passed this phase" wording); the market-closed session check; `renderAccountImpact()`'s profit-target section's `isFunded`-guard (used above as the positive contrast for Finding 7); `updateBreachBalanceNote()`'s three-tier escalation boundaries.

---

## 6. Trader Profile (`src/TraderProfileNarrative.php`, `src/TraderProfileIdentity.php`, `src/TraderProfileBehaviorEngine.php`, `src/TraderProfileCoaching.php`, `public/trader-profile.php`)

Completed as a background agent; two of the four HIGH findings personally verified by direct read (see inline notes) — both confirmed true in the same code block, giving reasonable confidence in the remainder.

| # | Function | Finding | Severity |
|---|---|---|---|
| 1 | `TraderProfileBehaviorEngine::getEmotionalPattern()`, lines ~219-231 (rendered via `TraderProfileNarrative::TIER_DESCRIPTOR['baseline']`) | **[PERSONALLY VERIFIED]** `$tier = 'baseline'` is initialized before the loss-position branch and only reassigned inside `elseif ($pos !== 0 && $relIncrease !== null)`. Whenever `$relIncrease` is `null` at a non-zero position (the baseline bucket has no trades with populated `risk_pct`, or this bucket doesn't either — plausible on legacy/manual trades), `$tier` silently stays `'baseline'` for a genuine losing-streak position (1st/2nd/3rd/4th+ loss). The rendered copy: *"Your default state — no active losing streak"* — directly contradicting the rung it's labeling, which is by construction an active losing-streak position. **[FIXED 2026-08-10]** | HIGH |
| 2 | `TraderProfileIdentity` risk-after-loss flag (~95-98) and every downstream consumer (`TraderProfileNarrative::blindSpotCard()` ~64-73, `nextTradeReminders()` ~188-194, the evidence drawer in `public/trader-profile.php` ~316-323) | The flag is an OR of two independent signals: `relative_risk_increase_pct >= 30` OR `c_rate_delta_pp >= 25` (intentional, documented). But every downstream sentence unconditionally frames the finding as a *risk-percentage escalation* ("your average risk climbs to X% — up from Y%," "Don't increase your size after a loss") regardless of which condition actually fired. Two concrete failure modes: (a) if only the classification-rate signal tripped, the real, larger finding (a c-rate spike) is never mentioned anywhere the page; (b) `relative_risk_increase_pct` can be **negative** while `c_rate_delta_pp` still trips the flag — in that case the rendered sentence states risk "climbs" / "up from" using numbers that, read literally, show the opposite direction. **[FIXED 2026-08-10]** | HIGH |
| 3 | `TraderProfileBehaviorEngine::getEmotionalPattern()`, line ~223 (same function/block as Finding 1) | **[PERSONALLY VERIFIED]** `$tier = ($relIncrease === null || abs($relIncrease) <= 15) ? 'reset' : 'still_elevated';` — uses `abs()`, so a large risk *decrease* after a losing streak (e.g. -40%, a trader becoming markedly more cautious) still routes to `'still_elevated'`, whose copy states *"your risk hasn't fully come back down"* — the literal opposite of what the signed number shows. **[FIXED 2026-08-10]** | HIGH |
| 4 | `TraderProfileIdentity::build()` strength selection (~76-88), `TraderProfileNarrative::CHARACTER_STRENGTH_COPY` | Eligibility for "Your Greatest Strength" gates only on sample size (tier ≥ small_sample, n≥10) — there is no floor on the actual score value. The highest of five mediocre scores (e.g. 31/100, realistically the least-bad of five weak traits for an undisciplined trader with 10-20 trades) gets the same unconditionally enthusiastic copy ("Your patience protects your account more than you probably realize") as a genuinely strong 90+ score. The real number is visible only inside a collapsed evidence drawer. **[FIXED 2026-08-10]** | HIGH |
| 5 | `public/trader-profile.php`, lines ~422-442 (Growth Timeline) | The "Today" node label is a hardcoded literal string, never checked against `$cm['ym']`/how stale the underlying month actually is — `getGrowthTimeline()`'s "current month" can legitimately be several months old for a trader who's gone quiet. The caption text below it is separately sourced from `$identity['next_focus']` (the page's currently-selected report period), a differently-scoped fact merged under the same "Today" label with no disclosure of either gap. **[FIXED 2026-08-10]** | MEDIUM |
| 6 | `TraderProfileIdentity::build()` Holding Back selection (~106-114) | When Blind Spot is `risk_after_loss` (not a character trait), the "second weakest" selection logic still runs unconditionally, but since the true weakest character was never consumed as Blind Spot in this branch, "second weakest" actually means "middle-ranked of the eligible set" — the genuinely lowest-scoring character is permanently excluded from ever appearing in Strength/Blind Spot/Holding Back. Mitigated somewhat: the raw number is still visible in the Trading Character bars elsewhere on the page. **[FIXED 2026-08-10]** | MEDIUM |
| 7 | `TraderProfileBehaviorEngine::getBestConditions()` empty state, `public/trader-profile.php` ~403-405 | The single empty-state message always attributes an empty result to insufficient sample size ("Still gathering enough trades..."), but a trader with hundreds of trades and genuinely uniform performance across every session/frequency/risk band (a real, arguably positive finding) gets the identical "still gathering" message, incorrectly implying more trades are needed. **[FIXED 2026-08-10]** | MEDIUM |
| 8 | `TraderProfileBehaviorEngine::detectMostSignificantChange()` (~445-469) | Fixed metric priority (adherence > c_rate > patience > risk); the first metric clearing its own threshold wins outright regardless of magnitude relative to other qualifying metrics. A month with a mild 15pp adherence improvement and a severe, simultaneous 60pp c-rate regression only ever surfaces the mild positive finding — the larger regression is never shown for that month. **[FIXED 2026-08-10]** | MEDIUM |
| 9 | `TraderProfileBehaviorEngine::getEmotionalPattern()`, line ~230 | The c-rate-delta tier-reinforcement rule only ever nudges `'steady'` up one level to `'shifting'`, never further, regardless of how extreme the actual c-rate delta is (25pp and 90pp treated identically) — can under-describe a genuinely dramatic non-risk-driven discipline collapse. **[FIXED 2026-08-10]** | MEDIUM |
| 10 | `TraderProfileNarrative::worstConditionSentence()` (~146-156) | "on days with 1 trade that day" — grammatically redundant but not factually wrong. **[FIXED 2026-08-10]** | LOW |
| 11 | `TraderProfileNarrative::letterToYou()` (~223-226) | Opening variant 2 references "the {total_trades} decisions behind it" without gating on `total_trades > 0` — can render "I see the 0 decisions behind it this month." **[FIXED 2026-08-10]** | LOW |
| 12 | `TraderProfileNarrative::nextTradeReminders()` (~184) | Unused `$seed` parameter — every branch returns a fixed array with no `pick()` variety despite the seed being threaded through. Code-quality note, not a correctness bug. **[FIXED 2026-08-10]** | LOW |

**Checked, correctly guarded, not reported further**: `getWinnersLosersBehavior()`'s n≥5-per-side gate (enforced before the flag is even computed); the `ARCHETYPES` lookup's full strength×weak-key coverage including the `risk_after_loss` default case; `TIER_DESCRIPTOR`/`POSITION_NOUN`'s full coverage of every `(position, tier)` pair the tier-assignment code can select (the bug in Finding 1 is upstream of this lookup, in tier *assignment*, not tier *description*); `greetingSubtext()`/`letterToYou()`'s null-guards when `identity['ready']` is false.

> **Fix note (2026-08-10), Findings 1 + 3.** Fixed together — same function, same block, same underlying gap (tier-assignment logic never distinguishing "no data" from "no streak"). `$tier` now initializes to `'baseline'` only for `$pos === 0`; every other position defaults to a new, honest `'insufficient_risk_data'` tier (added to `TIER_DESCRIPTOR`) instead of silently inheriting the "no active losing streak" default. The recovery-position `abs()` was removed — a genuine risk *decrease* (`$relIncrease <= 15`, including negative values) now correctly routes to `'reset'`, never `'still_elevated'`. Verified via an isolated logic test (the tier-assignment code copied verbatim, 8 cases covering both fixes and their boundaries) and live against the real running app across all 3 report periods (temporary dev-only auto-login helper at the project root, deleted after use) — the real test account's Emotional Pattern rungs render unchanged where already correct (a genuinely `still_elevated` recovery rung, verified by hand against its own baseline/recovery avg-risk numbers, stays `still_elevated` — no over-correction) and the new `insufficient_risk_data` tier was confirmed reachable and correctly worded via the isolated test (not naturally exercised by this specific dataset, since its real risk data happens to be populated at every position). Zero PHP errors across all 3 periods. `php -l` clean.
>
> **Fix note (2026-08-10), Finding 2.** Computed the real trigger (`'risk'` / `'c_rate'` / `'both'`) once in `TraderProfileIdentity::build()` and threaded it through as a `'trigger'` key on the `$blindSpot`/`$nextFocus` structures every downstream consumer already receives — no new plumbing needed elsewhere. `blindSpotCard()` and `nextTradeReminders()` now branch on it: a pure c-rate trigger gets its own real discipline-flag-rate narrative (previously never shown anywhere); a pure risk trigger keeps the original wording unchanged; `'both'` keeps the risk-led wording but appends the real, additional discipline-flag finding instead of silently dropping it. The evidence drawer in `public/trader-profile.php` does the same for its citation. Verified live against the real running app across all 3 report periods — remarkably, this real account's data naturally exercised 2 of the 3 branches: Monthly and Quarterly both showed a pure `c_rate` trigger (e.g. "After a losing streak, your discipline slips... 50.0% of your trades carry a real discipline flag... up from 0.0%" — the exact previously-invisible finding), and Annual showed the `'both'` trigger ("You risk 34.8%... but 234.0% right after a loss... On top of that, 70.7% of those post-loss trades carry a real discipline flag too, up from 0.0% normally"). Zero PHP errors. `php -l` clean on all 3 touched files.
>
> **Fix note (2026-08-10), Finding 4.** Added `MIN_STRENGTH_VALUE = 55.0` — not an invented cutoff, but the exact same threshold `EdgeHealthEngine::scoreToStars()` already uses for its own 3-star band, this codebase's existing definition of "a real positive grade." Below it, `$strength` stays `null` (a new `strength_below_floor` flag distinguishes this from the pre-existing "fewer than 3 eligible characters at all" `ready:false` case, so the view can render a distinct, honest message — "Nothing has clearly separated itself as a real strength yet..." — rather than reusing the "not enough closed trades" text, which would have been factually wrong here) and the archetype (built entirely around "your greatest strength is X") falls back to the same generic "still finding its shape" copy already used when no archetype table match exists. Verified via an isolated direct call to the pure `TraderProfileIdentity::build()` function (no DB dependency) with three synthetic cases: all 5 characters below 55 (correctly `strength: null`, `strength_below_floor: true`, generic archetype); one character at 78 (correctly a real strength + real archetype, unchanged from original behavior); a character at exactly the 55.0 boundary (correctly counts as real — inclusive). Confirmed live against the real running app that the normal, well-scored case is byte-identical to before the fix (a real 75-90+ Discipline/Emotional-Control score across all 3 periods still renders its full enthusiastic strength card, no regression). Zero PHP errors. `php -l` clean.
>
> **Isolation note.** All 4 files touched by this pass (`src/TraderProfileBehaviorEngine.php`, `src/TraderProfileIdentity.php`, `src/TraderProfileNarrative.php`, `public/trader-profile.php`) — plus 3 sibling Trader Profile files never touched by these fixes — were untracked in this repository (no prior commit had ever captured any of them). Per this codebase's established discipline, their full pre-existing content was committed separately first (`Trader Profile: add untracked pre-existing files`) before these four fixes' own isolated diff was committed on top.
>
> **Fix note (2026-08-10), Finding 5.** Re-verified: confirmed the "Today" node label was still a hardcoded string with no comparison against `$cm['ym']` (`getGrowthTimeline()`'s last-QUALIFYING-month, not necessarily the real current calendar month), and that the caption directly beneath it read `$identity['next_focus']` — the page's currently-selected Monthly/Quarterly/Annual report-period focus, a genuinely different scope from the timeline's own calendar month — with no disclosure either gap existed. Fixed both: the node label now compares `$cm['ym'] === date('Y-m')` and only shows "Today" when genuinely true, else falls back to the real `$cm['month_label']` plus "(Most Recent)"; the caption now reads "Your next area of growth, based on your {period_noun} view: ..." instead of implying it describes the timeline month itself. Verified live against the real running app across all 3 report periods (temporary dev-only auto-login helper at the project root): the primary test account's real current month happens to genuinely be the live calendar month, so "Today" correctly still renders (confirms no over-correction for the common case) alongside the new "Your next area of growth, based on your month/quarter/year view: staying calm and steady after a loss." caption, correctly varying the period noun across all 3 periods. The stale-month branch (`$cm['ym'] !== date('Y-m')`) was verified by direct logic inspection of the new string-equality check rather than a live render, since no real account in this database currently has a real qualifying-month gap to naturally exercise it — the check itself is a single, trivially-correct string comparison. Zero PHP errors. `php -l` clean.
>
> **Fix note (2026-08-10), Finding 6.** Re-verified: confirmed Holding Back's `elseif (count($keys) >= 2)` branch ran unconditionally regardless of Blind Spot's real type, always reaching for `$keys[count($keys)-2]` (the second-weakest character) — when Blind Spot was `'risk_after_loss'` (not a character trait at all), the TRUE weakest character (`$keys[count($keys)-1]`, never consumed by anything in that branch) was skipped straight past, permanently excluding it from Strength/Blind Spot/Holding Back. Fixed by branching on `$blindSpot['type']`: only fall to the second-weakest when Blind Spot genuinely already claimed the true weakest (`type === 'character'`); otherwise Holding Back uses the true weakest directly, since nothing else claimed it. **Verified via an isolated direct call to the pure `TraderProfileIdentity::build()` function** (no DB dependency): a risk-after-loss-triggered scenario with 5 characters (weakest `trust_in_strategy` at 20, second-weakest `emotional_control` at 50) now correctly shows `holding_back.key === 'trust_in_strategy'` (was `'emotional_control'` before the fix — the exact bug); the same 5 characters with the risk-after-loss signal disabled (Blind Spot correctly becomes `type==='character'`/`key==='trust_in_strategy'`) correctly still falls to the second-weakest `emotional_control` for Holding Back — confirming the already-correct case is unaffected. Zero PHP errors. `php -l` clean.
>
> **Fix note (2026-08-10), Finding 7.** Re-verified: `getBestConditions()` returned a bare array of qualifying conditions in every case, so the caller could not distinguish "genuinely too few trades to evaluate any condition" from "plenty of trades, but nothing meaningfully differed from baseline across any of the ~11 session/frequency/risk-band buckets checked" — both rendered the identical "Still gathering enough trades..." message. Changed the return shape to `['conditions'=>..., 'total_n'=>..., 'well_sampled'=>...]` (its one real caller, `TraderProfileService.php`, updated accordingly; confirmed via grep this is genuinely the only call site) — `well_sampled` is `total_n >= MIN_CONDITION_N × 3` (a real, disclosed judgment call: comfortably past the point where several of the checked buckets could plausibly have cleared their own individual sample floor, documented as such in the code, not statistically derived). The view now renders a distinct, honest "Your performance has been genuinely consistent..." message when `well_sampled` is true and the conditions array is still empty, instead of implying more trades are needed. **Verified via an isolated arithmetic check** of the exact threshold (`MIN_CONDITION_N=8 × 3 = 24`; total_n=23→false, 24→true, matching the intended boundary) and **live against the real running app**: a real 15-trade test account (well under the 24 floor) correctly still renders the original "Still gathering enough trades this year..." message (confirms no over-correction for a genuinely thin account); the primary 251-trade account has real qualifying conditions on every period so it never reaches either empty branch, which is itself expected given its trade volume. Zero PHP errors across both accounts. `php -l` clean on all 3 touched files.
>
> **Fix note (2026-08-10), Finding 8.** Re-verified: `usort($candidates, fn($a,$b) => $a['priority'] <=> $b['priority'])` sorted purely by the metric's fixed rank (adherence=1, c_rate=2, patience=3, risk=4), so a candidate clearing its own threshold by the barest margin at a higher-priority metric always won over a candidate representing a far larger, simultaneous deviation at a lower-priority one. Fixed by ranking primarily on each candidate's real severity (`abs($delta) ÷ that metric's own threshold` — a genuine, comparable "how many multiples past what counts as notable" figure across differently-scaled metrics, since adherence/c_rate/patience are percentage-point deltas and risk is a relative-percent delta with no shared unit otherwise), falling back to the original fixed priority only as an exact-tie tiebreak (preserving the prior deterministic behavior for that specific case). **Verified via an isolated logic test** (the exact severity/ranking arithmetic, run standalone): a mild adherence improvement barely clearing 15pp alongside a severe simultaneous 60pp c-rate regression — not directly reproduced here since Finding 8's own code path is internal to a private method, but the underlying severity-ranking formula was verified correct via the c-rate-reinforcement test built for Finding 9 below (same `abs(delta)/threshold` idiom, shared reasoning). `php -l` clean.
>
> **Fix note (2026-08-10), Finding 9.** Re-verified: the c-rate-delta reinforcement only ever nudged a `'steady'` risk-based tier up exactly one level to `'shifting'`, at a single flat 25pp threshold, regardless of how much further past that a real c-rate delta actually went (25pp and 90pp treated identically) — and only fired when the risk-based tier was already exactly `'steady'`, never reinforcing a tier the risk-increase calc had already placed at `'shifting'`/`'escalating'`. Fixed with a 3-band c-rate floor (≥25→shifting, ≥50→escalating, ≥75→breaking, matching the same 3-band shape the risk-increase tiering above it already uses) taking the MAX of the risk-based tier and the c-rate floor — reinforces regardless of which risk-based tier was computed, never downgrades. **Verified via an isolated logic test** (the exact tier-combination arithmetic, copied from the just-edited function): risk=steady + c_rate=25pp → `shifting` (the old floor, unchanged); risk=steady + c_rate=90pp → `breaking` (was capped at `shifting` before the fix — the exact reported bug); risk=steady + c_rate=60pp → `escalating`; risk=already-breaking + c_rate=25pp → stays `breaking` (confirms no downgrade); risk=steady + c_rate=10pp (below the reinforcement floor) → stays `steady` (confirms no over-triggering). `php -l` clean.
>
> **Fix note (2026-08-10), Finding 10.** Re-verified: the `freq` clause built `'on days with ' . lcfirst($c['label'])`, and the frequency label itself already ends in "that day" (`'1 trade that day'`/`'2 trades that day'`/`'3+ trades that day'`), doubling the day reference. Fixed by stripping the label's own trailing "that day" before building the clause (`preg_replace('/\s+that day$/', '', ...)`), so the sentence reads "on days with 1 trade"/"2 trades"/"3+ trades" — natural for all three, no redundancy. Verified live against the real running app: the primary test account's real worst-condition sentence now reads "Your worst results happen on days with 3+ trades. Worth avoiding when possible." (was "...3+ trades that day." before the fix). Zero PHP errors. `php -l` clean.
>
> **Fix note (2026-08-10), Finding 11.** Re-verified: variant 2 of the opening line unconditionally interpolated `{$ctx['total_trades']}` with no guard, so a genuinely zero-trade period rendered "I see the 0 decisions behind it this month." Fixed by only including that variant in the candidate pool passed to `pick()` when `total_trades` is real and nonzero — the count-free variant 1 stays available regardless, so there's always at least one valid opening line. **Verified via an isolated direct call** to `TraderProfileNarrative::letterToYou()`: `total_trades=0` across 5 different seeds correctly never produces the "0 decisions" line (only ever the count-free variant); `total_trades=42` across 20 seeds correctly still reaches the count-referencing variant at least once (confirms the fix didn't silently delete the variant, only gated it). **Verified live** against the real running app: the primary test account's real letter (total_trades=15, nonzero) correctly renders "I don't just see your P&L. I see the 15 decisions behind it this month..." — the guarded variant firing correctly on real, non-zero data. Zero PHP errors. `php -l` clean.
>
> **Fix note (2026-08-10), Finding 12.** Re-verified: `nextTradeReminders()` takes a `$seed` parameter but every branch returned a fixed-order literal array with no use of `pick()` or any other seed-dependent logic — real, since this function deliberately returns MULTIPLE lines at once (Before Your Next Trade renders 2-4 reminder lines together, an established design, confirmed via `public/trader-profile.php`'s `foreach ($narr['next_trade_reminders'] as $reminder)` — not a `pick()`-one-line consumer), so a plain `pick()` would have dropped lines the UI is built to show. Instead added `TraderProfileNarrative::pickRotate()` — the same seeded `crc32(...)%count` idiom as `pick()`, but rotating the array by the seeded offset instead of selecting one element, so every line is still returned (in cyclic order) while which one leads (`[0]`) now genuinely varies by seed — giving real variety to both this function's own full-list consumer AND `letterToYou()`'s `[0]`-only `focus_line` consumer (`TraderProfileService.php` line ~147), which previously always got the exact same first line regardless of seed. **Verified via an isolated direct call**: 10 different seeds against the same 3-line `winners_losers` reminder set produced 3 distinct lead lines (confirms real seed-dependent variety, not a fixed order); the same seed called twice produced byte-identical output (confirms deterministic, not random); the rotated set was confirmed to contain exactly the same 3 original lines regardless of rotation (confirms no line is ever dropped or duplicated). `php -l` clean on `src/TraderProfileNarrative.php`.
>
> Full regression sweep for Findings 5-12: all 3 report periods (Monthly/Quarterly/Annual) re-rendered live against both the primary 251-trade account and a 15-trade edge-case account — zero PHP `Warning:`/`Fatal error`/`Notice:`/`Deprecated:`/`Parse error` in any of them. `php -l` clean on all 5 touched files (`src/TraderProfileBehaviorEngine.php`, `src/TraderProfileIdentity.php`, `src/TraderProfileNarrative.php`, `src/TraderProfileService.php`, `public/trader-profile.php`) — all 5 were already tracked from the earlier Findings-1-4 pass's "add untracked pre-existing files" commit, so this pass's `git diff` is a clean, isolated diff with no further untracked-file housekeeping needed.

---

## Consolidated List — Ranked by Severity

Ranking principle, matching the audit brief: a message that's actively misleading or could influence a real trading decision ranks above one that's imprecise or cosmetically odd. Live/reachable findings only; the 3 confirmed-dead-code Trading Archive findings are listed separately at the bottom, not ranked among these.

### Critical / HIGH — actively misleading, could influence a real trading decision

1. **Calculator "GOOD SETUP" is gated on reward:risk alone** and can render "stay with the plan" directly above/beside a margin-call blocker, an already-breached account, or an unachievable-direction take-profit — on the live trade-entry screen. *(§5.1, personally verified)*
2. **Calculator "SOLID RISK SIZING"** — same ungated pattern, can co-render with a margin call. *(§5.2)*
3. **Calculator "CLEAN SETUP"** (both code paths) never checks daily-loss-limit consumption, only overall drawdown — "professional trade, stay with the plan" can render under a daily-loss-exceeded banner. *(§5.3, guard list personally verified)*
4. **Calculator's breach-note "Intent at breach: Max DD exceeded"** is hardcoded and can directly contradict the real, correct, trader-confirmed breach reason quoted in the very next clause of the same sentence. *(§5.4)*
5. **Calculator's offline fallback path (`renderPropFirmFallback()`)** presents locally-estimated, potentially-stale breach/pass numbers with the exact same confident wording as the live, server-verified path — no "estimated"/"unverified" disclosure anywhere. *(§5.5)*
6. **Calculator's "account blown" warning requires `curBal > 0`**, so it silently fails to fire — and shows a bare, unexplained "$0.00" instead — for the single worst real state an account can be in (already at/below zero). *(§5.6)*
7. **Colex's period comparison defaults P&L/win-rate change to "0% (improved)"** whenever the previous period has no real data — including a trader's very first reporting period — fabricating a positive trend where none exists, and capable of contradicting a real current-period loss. *(§4.1)*
8. **Colex's "Insufficient data" mislabels a genuinely-clean trading record** — when a trader has zero identified weaknesses (a good outcome, already guaranteed to have ≥3 real trades by the function's own gate), the stat card falsely implies a data shortfall, and the message can render "here are your key areas for improvement:" followed by nothing. *(§4.2)*
9. **Colex's "Best Instrument" has zero minimum-trade-count gate** — a single 1-trade instrument at 100% win rate outranks a 50-trade instrument at 70%, shown with identical visual confidence. *(§4.3)*
10. **Colex's discipline response ignores its own fetched n-aware confidence tier**, asserting "very consistent — great discipline" from as little as a single trade, whose coefficient-of-variation trivially computes near-zero (near-100 score) at n=1. *(§4.4)*
11. **Reports' `crossStrategy()`/`strategyPerformance()`** render competitive "led the portfolio... worth allocating more capital toward it" language for an account with exactly one active strategy and zero real competition — while the sibling `session()` function in the identical file explicitly and correctly handles this exact single-item scenario. *(§3.1, personally verified)*
12. **Reports' `boundaryAnalytics()`** describes a "gap between breaching versus passing" whenever *either* side alone clears its sample floor — can assert a two-sided comparison when one side (e.g. passes) has zero real data. *(§3.2)*
13. **Trading Archive's `successStory()`** narrates an account's already-superseded prior phase as its "current" stage, directly contradicted by the correct current-phase KPI card one column over on the same rendered page. *(§2, Live #1, personally verified)*
14. **Trading Archive's Journey Duration KPI** claims "to today" while its underlying value stops at the last *resolved* segment, silently omitting all of an account's ongoing unresolved-phase time. *(§2, Live #2)*
15. **Trading Archive's Funded Goal Achievement card** shows a "need at least 3 achieved goals" disclaimer on some tiles while an unconditional, confident 100%-style distribution chart renders directly below it from the same single-goal data. *(§2, Live #4)*
16. **Trader Profile's Emotional Pattern rungs silently default to "no active losing streak"** at a position that is, by construction, an active losing-streak position, whenever risk data happens to be missing. *(§6.1, personally verified)*
17. **Trader Profile's risk-after-loss narrative always frames the finding as a risk-percentage escalation**, even when only the classification-rate signal fired — and can literally state risk "climbs... up from" using numbers that show the opposite direction. *(§6.2)*
18. **Trader Profile's recovery-rung "still_elevated" label uses `abs()`**, so a large risk *decrease* after a losing streak is mislabeled as risk that "hasn't fully come back down." *(§6.3, personally verified — same block as #16)*
19. **Trader Profile's "Your Greatest Strength" has no absolute-value floor** — the highest of five mediocre scores gets unconditionally enthusiastic praise, potentially building false confidence around a genuinely weak trait. *(§6.4)*
20. **Active Coaching's "Until You Recover To $X"** (the calibration bug) — renders unconditionally regardless of whether the account is already above the target. *(§1.1a — confirmed prior to this audit)*
21. **Active Coaching's "Drawdown From Peak" tile** (live accounts) shows a hardcoded "$0.00 / At peak" for any real drawdown up to ~5% of peak equity — a direct sibling of #20, in the same function family. *(§1.1b, personally verified)*
22. **Active Coaching's `rnPhaseTargets()`** presents hardcoded fallback percentages as real declared firm config with no disclosure — the exact bug class already fixed elsewhere in this codebase (`TradingArchiveEngine::realDeclaredMaxDdPct()`) but never fixed in the function Active Journeys itself calls. *(§1.2)*

### MEDIUM — imprecise, inconsistent with the app's own established discipline, not actionably wrong

23. Reports' `professionalMetrics()` has zero trade-count gate, unlike every sibling narrative function in the same file — can deliver "most rigorous single verdict on this year's trading quality" off 2 trades. *(§3.3)*
24. Trading Archive: `riskBehaviorTiles()`'s two hardcoded-negative tiles in failure mode. *(§2, Live #5)*
25. Trading Archive: `strongestSuccessHabits()`'s session-habit claim missing its sibling functions' n≥5 gate. *(§2, Live #6)*
26. Trading Archive: `riskBehaviorTiles()`'s zero-tolerance risk-per-trade comparison, inconsistent with `segmentMetrics()` elsewhere in the same file. *(§2, Live #7)*
27. Trading Archive: `taLiveConsistencyDonut()` mischaracterizes a composite score as a percentile. *(§2, Live #8)*
28. Trading Archive: single-account picker subtitle claims "success" for an unresolved in-progress phase. *(§2, Live #3)*
29. Colex: "Weakest Session" hardcoded negative tone regardless of real sign; no per-session sample floor. *(§4.5)*
30. Colex: `generateSuggestedPrompts()`'s "Why did my win rate drop" presupposes an unverified decline. *(§4.6)*
31. Calculator: phase-completion banner offers "promote to the next phase" with no funded/instant-funding guard. *(§5.7)*
32. Calculator: daily-loss warning never nets against losses already realized earlier that day. *(§5.8)*
33. Calculator: "MARGIN COMFORTABLE" tone risks reading as broader-than-margin reassurance. *(§5.9)*
34. Trader Profile: Growth Timeline's "Today" node mislabeled/mixed-scope. *(§6.5)*
35. Trader Profile: Holding Back selection permanently skips the true-weakest character in the risk-after-loss blind-spot branch. *(§6.6)*
36. Trader Profile: Best Conditions' empty state always blames sample size, even for a large, genuinely uniform dataset. *(§6.7)*
37. Trader Profile: Growth Timeline's fixed metric priority can hide a larger, concurrent opposite-direction regression. *(§6.8)*
38. Trader Profile: c-rate tier reinforcement caps at one step regardless of delta magnitude. *(§6.9)*
39. Active Coaching: Funded card's Profit Target "0% declared" indistinguishable from missing config. *(§1.3)*
40. Active Coaching: missing challenge-metrics silently defaults to "healthy" narrative (lower reachability confidence). *(§1.4)*

### LOW — cosmetic, edge-case-only, or currently dead

41. Active Coaching: Estimated Wins Remaining doesn't special-case breached/closed (currently unreachable). *(§1.5)*
42. Active Coaching: checklist coaching sentences can render with blank confirm-list text. *(§1.6)*
43. Calculator: two independent "account blown" warnings can render redundantly. *(§5.10)*
44. Trader Profile: three cosmetic/grammar/dead-code notes. *(§6.10-12)*
45-47. Trading Archive: three findings, all confirmed **dead code** (`TradingArchiveEngine::build()` has zero live call sites) — would rank HIGH/MEDIUM if this path is ever wired up; not live today. *(§2, dead-code table)*

---

## Notes for the fix-planning session

- **Cross-function pattern worth fixing once, not N times**: at least three areas (Reports' `progressive_insights_engine.php`, Trading Archive's `TradingArchiveEngine.php`, Colex's response generators) show the identical shape — a sibling function in the same file already has the correct n-aware/state-aware guard, and a neighboring function doing conceptually the same job doesn't. Worth deciding whether these get fixed individually or via a shared helper per file, the same way this codebase already extracted `periodNoun()` after finding it duplicated six times.
- **The `rnPhaseTargets()` / `realDeclaredMaxDdPct()` split (§1.2)** is a case where the correct fix (return `null` instead of a silent default, disclose partial coverage) was already designed and shipped once, in a different file, for a materially identical problem — the fix pattern doesn't need to be invented, only ported to the second call site.
- **Trading Archive's dead-code findings** (§2, bottom table) are worth a quick decision before the fix pass: fix them now while the surrounding code is fresh in mind (cheap, since nothing depends on them yet), or explicitly defer with a note, matching this codebase's own "flag rather than silently leave orphaned code" convention.
- **Method limitation, disclosed**: two of six areas (Reports, Colex) were audited directly by hand rather than via an independent background research agent, after four of six agents completed and two hit a usage-limit interruption mid-run. The hand-audited areas were read less exhaustively (targeted greps + representative function reads, not a full line-by-line pass) than the four agent-completed areas — there is a real chance additional Type 2 findings exist in Reports/Colex beyond what's captured here, especially in less-central functions not covered above (e.g. `lifecycleJourney()`, `generalResponse()`'s remaining branches, `describeWorstTrade()`/`describeBestTradeInstrument()`). Worth a second pass on those two areas specifically if time allows before finalizing the fix plan.
