Fix diagnostics/cohorts tabs + add flag descriptions & SQL tooltips #1

Merged
firman-admin merged 8 commits from fix/diagnostics-cohorts-flaginfo into master 2026-07-21 10:50:53 +00:00
Owner

Summary

Three small, independently-reviewed fixes/features bundled from the same working session, plus doc/changelog upkeep:

  • Diagnostics tab fix: residual chart (#ch_residuals) x-axis labels are now index-aligned to the underlying series. Previously assumed a "first N months" slice, which mismatched actual/predicted values whenever a mid-series month was filtered out of one metric.
  • Cohorts tab fix: chart was always blank (COHORT_DATA.cohorts — a key that never existed — instead of the real .rows). Now aggregates loan_count/cum_loss_count across the is_repeat/credit_score_bin/loan_duration_bin breakouts per (cohort_month, mob). Also fixes the "curves" view's MoB lookup, which assumed MoB values are a contiguous 1..N sequence aligned to array position (same bug shape as the portSeries fix in 8161f05); now looks up by actual MoB value, matching the "heatmap" view.
  • Flag descriptions + SQL feature: hover tooltip on flag pills/labels, and full description + literal SQL CASE WHEN logic in the Add/Edit comparison-line flyout, for all 4 segmentation flags — sourced from create_wo_forecast_raw_v2.sql.
  • Dashboard-ux-improvements review-fix: KPI card label wording corrected to match spec (carried over from the previous feature's review pass).
  • Retroactive PROPOSAL/SPEC/understand docs for all three items under .planning/, progress.md brought current, CHANGELOG.md backfilled through 0.5.0 (also covering three previously-shipped-but-undocumented versions).

Known issues — flagged, not fixed in this PR

  • Heatmap canvas destruction (Cohorts tab): the "heatmap" view replaces #ch_cohort's parent innerHTML with a <table>, destroying the canvas element. Switching back to "curves"/"snapshot" afterward will fail until the page is reloaded. Pre-existing bug (not introduced here); a low-risk fix approach (sibling container + display toggle instead of overwriting the canvas's parent) has been scoped but not implemented — needs a go/no-go.
  • Cohort aggregation-across-segments assumption: aggregating cumulative loss across all three segment breakouts into one line per cohort month was a judgment call, not a specified requirement — there's no segment-filter UI today. Options (keep as-is / add a Triage-style flag-pill breakdown / add a single "break down by" dropdown) are laid out in .planning/SPEC-fix-cohorts-tab.md and progress.md, pending a product decision.
  • is_graduated's SQL transcription fidelity issue found during review has already been fixed in this PR (see commit 9c4a13f).

Review

Independent spec-reviewer and quality-reviewer subagent passes were run against the live diff (not against the authors' own claims) for all three features — no CRITICAL/blocking findings. Full findings recorded in .planning/SPEC-fix-diagnostics-tab.md, .planning/SPEC-fix-cohorts-tab.md, .planning/SPEC-flag-descriptions-and-sql.md.

Test plan

  • Diagnostics tab: confirm residual chart renders with correct labels, including when a metric has gaps mid-series
  • Cohorts tab: confirm "curves" and "heatmap" views render non-blank data; confirm curves MoB alignment looks correct
  • Triage tab + Add/Edit comparison line flyout: confirm flag tooltips and SQL blocks display correctly for all 4 flags
  • KPI cards: confirm label wording for each method (FPG/TPG/DPD30/Loss/Final Forecast)
## Summary Three small, independently-reviewed fixes/features bundled from the same working session, plus doc/changelog upkeep: - **Diagnostics tab fix**: residual chart (`#ch_residuals`) x-axis labels are now index-aligned to the underlying series. Previously assumed a "first N months" slice, which mismatched actual/predicted values whenever a mid-series month was filtered out of one metric. - **Cohorts tab fix**: chart was always blank (`COHORT_DATA.cohorts` — a key that never existed — instead of the real `.rows`). Now aggregates `loan_count`/`cum_loss_count` across the `is_repeat`/`credit_score_bin`/`loan_duration_bin` breakouts per `(cohort_month, mob)`. Also fixes the "curves" view's MoB lookup, which assumed MoB values are a contiguous 1..N sequence aligned to array position (same bug shape as the `portSeries` fix in 8161f05); now looks up by actual MoB value, matching the "heatmap" view. - **Flag descriptions + SQL feature**: hover tooltip on flag pills/labels, and full description + literal SQL CASE WHEN logic in the Add/Edit comparison-line flyout, for all 4 segmentation flags — sourced from `create_wo_forecast_raw_v2.sql`. - **Dashboard-ux-improvements review-fix**: KPI card label wording corrected to match spec (carried over from the previous feature's review pass). - Retroactive PROPOSAL/SPEC/understand docs for all three items under `.planning/`, `progress.md` brought current, `CHANGELOG.md` backfilled through `0.5.0` (also covering three previously-shipped-but-undocumented versions). ## Known issues — flagged, not fixed in this PR - **Heatmap canvas destruction** (Cohorts tab): the "heatmap" view replaces `#ch_cohort`'s parent `innerHTML` with a `<table>`, destroying the canvas element. Switching back to "curves"/"snapshot" afterward will fail until the page is reloaded. Pre-existing bug (not introduced here); a low-risk fix approach (sibling container + display toggle instead of overwriting the canvas's parent) has been scoped but not implemented — needs a go/no-go. - **Cohort aggregation-across-segments assumption**: aggregating cumulative loss across all three segment breakouts into one line per cohort month was a judgment call, not a specified requirement — there's no segment-filter UI today. Options (keep as-is / add a Triage-style flag-pill breakdown / add a single "break down by" dropdown) are laid out in `.planning/SPEC-fix-cohorts-tab.md` and `progress.md`, pending a product decision. - `is_graduated`'s SQL transcription fidelity issue found during review has already been fixed in this PR (see commit `9c4a13f`). ## Review Independent spec-reviewer and quality-reviewer subagent passes were run against the live diff (not against the authors' own claims) for all three features — no CRITICAL/blocking findings. Full findings recorded in `.planning/SPEC-fix-diagnostics-tab.md`, `.planning/SPEC-fix-cohorts-tab.md`, `.planning/SPEC-flag-descriptions-and-sql.md`. ## Test plan - [ ] Diagnostics tab: confirm residual chart renders with correct labels, including when a metric has gaps mid-series - [ ] Cohorts tab: confirm "curves" and "heatmap" views render non-blank data; confirm curves MoB alignment looks correct - [ ] Triage tab + Add/Edit comparison line flyout: confirm flag tooltips and SQL blocks display correctly for all 4 flags - [ ] KPI cards: confirm label wording for each method (FPG/TPG/DPD30/Loss/Final Forecast)
Adds FLAG_INFO with a human-readable description and the literal SQL
CASE WHEN logic for each of the 4 segmentation flags, transcribed from
create_wo_forecast_raw_v2.sql. Wired in as a hover tooltip on flag
pills/labels everywhere they appear, and as full description + SQL
text in the Add/Edit comparison-line flyout, per user-specified
placement.

Retroactively documented in .planning/PROPOSAL-flag-descriptions-and-sql.md,
understand-flag-descriptions-and-sql.md, SPEC-flag-descriptions-and-sql.md.
Quality-reviewer flagged that renderKPIs' inline label map duplicated
methodName() and disagreed with it for 'fcst' ("Forecast" vs "Final
Forecast"). Corrects the map to lowercase labels per SPEC's literal
scenario strings and adds avgLabel (appends " rate" only for Loss),
matching R2's Avg-card wording exactly while keeping KPI-card and
chart-title label spaces intentionally distinct.

Findings recorded in SPEC-dashboard-ux-improvements.md.
computeBacktest() built the residual chart's x-axis labels via a
naive "first N months" slice (allSeries.slice(0, tpgPts.length)), but
tpgPts/dpdPts/lossPts are filtered arrays that can skip any month in
the middle, not just the end — so whenever a mid-series month was
filtered from one metric, every later label was shifted relative to
its actual data point.

Replaces with full-length, index-aligned arrays (tpgResid, dpdResid,
lossResid) — one entry per month in allSeries, null where a month
doesn't qualify — paired with a single residualLabels =
allSeries.map(s => s.m). Same null-gap + spanGaps:true pattern already
used elsewhere in the file. Per-point filter conditions and the
actual-minus-predicted formula are unchanged from the original
tpgPts/dpdPts/lossPts construction (verified index-for-index
identical); the stats table above the chart is untouched.

Retroactively documented in PROPOSAL-fix-diagnostics-tab.md,
SPEC-fix-diagnostics-tab.md (includes independent spec-reviewer +
quality-reviewer verification).
renderCohortsTab() read COHORT_DATA.cohorts, a key that never exists —
export_cohort.py writes {flags, rows}, so cohorts always resolved to
[] and the chart/heatmap/curves views were silently blank. Rows are
also broken out by is_repeat/credit_score_bin/loan_duration_bin with
no pre-aggregated rate, not flat {cohort, value} records.

Reads COHORT_DATA.rows and aggregates loan_count/cum_loss_count across
those three segment breakouts per (cohort_month, mob) — a
count-weighted sum-then-divide, not an average of segment rates —
before computing value = cum_loss_count/loan_count*100. NOTE:
aggregating across all segments (vs. adding a segment-filter UI) is a
judgment call, not a user-specified requirement — flagged as an open
assumption pending user sign-off in PROPOSAL-fix-cohorts-tab.md.

Also fixes the "curves" view's MoB lookup, which assumed MoB values
are a contiguous 1..N sequence aligned to the label array's index
(cohorts.find(... c.mob === i + 1)) — not reliably true, since MoB can
be capped per cohort by loan_duration. Now parses the actual MoB
number from the label and looks up by value, matching how the
"heatmap" view already did it correctly. Same bug shape as the
portSeries stale-reference fix (8161f05) and the diagnostics-tab
residual fix earlier in this branch; grepped the rest of the file for
a third instance — none found.

Known separate issue, NOT fixed here: the "heatmap" view destroys the
#ch_cohort canvas by replacing its parent's innerHTML with a <table>,
so switching back to curves/snapshot afterward breaks. Flagged in
SPEC-fix-cohorts-tab.md, left for a follow-up decision.

Retroactively documented in PROPOSAL-fix-cohorts-tab.md,
SPEC-fix-cohorts-tab.md (includes independent spec-reviewer +
quality-reviewer verification).
progress.md hadn't been touched since the codebase-hygiene phase.
Brings it current across dashboard-refresh, raw-table-rebuild,
dashboard-ux-improvements, and the three just-committed items (KPI
label review-fix, diagnostics residual-alignment fix, cohorts
data/aggregation fix, flag-descriptions-and-sql feature), plus the
open items needing user sign-off (cohort-aggregation assumption,
heatmap-canvas-destruction issue).
Fixes stale "not yet committed" language left over from before this
session's commits landed, and records findings from exploring the
three open issues flagged during review (heatmap canvas destruction,
cohort-aggregation-across-segments assumption, is_graduated SQL
paraphrase) — options laid out, nothing implemented pending user
decision on the two design-judgment items.
Quality-reviewer/spec-reviewer verification of the flag-descriptions
feature found the is_graduated tooltip's SQL text was a paraphrase,
not a verbatim match: create_wo_forecast_raw_v2.sql's graduation CTE
is actually 4 independently OR'd sub-conditions (LOWER(prev_flow) LIKE
'%campaign%' AND NOT LIKE '%bad%', the same test on graduation_flow,
plus two separate exact-match checks against 'xtra_limit_500k-manual')
but the transcribed string collapsed this to a 2-condition
prev_flow/graduation_flow shorthand and dropped LOWER(). Semantically
equivalent, not verbatim — no correctness bug, but worth tightening
since this text is shown directly to users as "the actual SQL logic."

Spells out all 4 OR'd conditions with LOWER() explicit, matching the
source structurally. Text-only change to the FLAG_INFO constant, no
logic/schema change. Updates SPEC-flag-descriptions-and-sql.md's R3
verification from PARAPHRASED to MATCH.
CHANGELOG.md hadn't been touched since 0.1.0 (codebase-hygiene) despite
three more features (dashboard-refresh 0.2.0, raw-table-rebuild 0.3.0,
dashboard-ux-improvements 0.4.0) already shipped to origin/master, plus
today's batch (0.5.0): flag-descriptions-and-sql feature, diagnostics
and cohorts tab fixes, and the dashboard-ux-improvements KPI label
review-fix. 0.5.0's entry explicitly calls out the two known,
unresolved cohorts-tab issues (heatmap canvas destruction,
aggregation-across-segments assumption) as tracked follow-ups rather
than burying them.

Also fixes two pre-existing typos in the 0.1.0 entry (dropped leading
character: "equirements.txt" -> "requirements.txt", "ill_forecasts.py"
-> "fill_forecasts.py") and drops the file's leading BOM, consistent
with 0c980f3's prior BOM-removal convention.
firman-admin deleted branch fix/diagnostics-cohorts-flaginfo 2026-07-21 10:50:53 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
firman-admin/risk-segmentation!1
No description provided.