refactor: remove Trends tab from Patient Pathways (Task E.1)

Remove trends tab, metric toggle, render helper, and callback I/O
from Patient Pathways view. Trends will be re-added as a standalone
3rd view in E.2-E.4. Preserved get_trend_data() and create_trend_figure()
for reuse.
This commit is contained in:
Andrew Charlwood
2026-02-07 22:07:01 +00:00
parent 03ebaa057d
commit d052d2b16d
5 changed files with 162 additions and 67 deletions
+95 -5
View File
@@ -10,19 +10,27 @@ Comprehensive review and improvement of all Plotly charts in the Dash dashboard.
### What Changes
- `src/visualization/plotly_generator.py` — shared styling constants, bug fixes, new chart functions
- `src/data_processing/pathway_queries.py` — new query functions for Tier 3 analytics
- `src/data_processing/pathway_queries.py` — new/modified query functions
- `dash_app/data/queries.py` — thin wrappers for new queries
- `dash_app/callbacks/chart.py`heatmap metric toggle, new tab support
- `dash_app/callbacks/chart.py`remove Trends tab, fix chart height
- `dash_app/callbacks/trust_comparison.py` — trust color palette, heatmap metric toggle
- `dash_app/components/chart_card.py` — new tab definitions, metric toggle component
- `dash_app/callbacks/trends.py` — NEW: Trends view callbacks (directorate overview + drug drill-down)
- `dash_app/callbacks/__init__.py` — register new trends callbacks
- `dash_app/components/chart_card.py` — remove Trends tab, metric toggle cleanup
- `dash_app/components/trust_comparison.py` — metric toggle component
- `dash_app/components/trends.py` — NEW: Trends landing + detail components
- `dash_app/components/sidebar.py` — add Trends nav item
- `dash_app/callbacks/navigation.py` — 3-way view switching
- `dash_app/callbacks/filters.py` — add nav-trends input
- `dash_app/app.py` — add trends-view to layout, add selected_trends_directorate to app-state
- `dash_app/assets/nhs.css` — chart height CSS for responsive sizing
### What Stays (DO NOT MODIFY)
- Pipeline/analysis logic: `pathway_pipeline.py`, `transforms.py`, `diagnosis_lookup.py`, `pathway_analyzer.py`
- Database schema and `pathway_nodes` table
- CLI refresh command
- CLI refresh command and `cli/compute_trends.py`
- Existing callback chain architecture (app-state → chart-data → UI)
- Two-view architecture (Patient Pathways + Trust Comparison)
- Trust Comparison view (unchanged)
---
@@ -241,6 +249,78 @@ Comprehensive review and improvement of all Plotly charts in the Dash dashboard.
- **Checkpoint**: Timeline tab shows when each drug cohort was active
---
## Phase E: Trends View Redesign + Chart Height
### E.1 Remove Trends tab from Patient Pathways
- [x] Remove `("trends", "Trends")` from `TAB_DEFINITIONS` in `dash_app/components/chart_card.py`
- [x] Remove `trends-metric-wrapper` div and `trends-metric-toggle` SegmentedControl from `chart_card.py`
- [x] Remove `_render_trends()` helper from `dash_app/callbacks/chart.py`
- [x] Remove `elif active_tab == "trends"` dispatch case from `update_chart()`
- [x] Remove `Output("trends-metric-wrapper", "style")` and `Input("trends-metric-toggle", "value")` from `update_chart()` callback signature — updated ALL 4 return paths to return 3 values instead of 4
- [x] Remove thin wrapper `get_trend_data()` from `dash_app/data/queries.py` (will be re-imported by the new Trends view callbacks)
- [x] Keep `get_trend_data()` in `pathway_queries.py` — it's still used by the new Trends view
- [x] Keep `create_trend_figure()` in `plotly_generator.py` — it's still used by the new Trends view
- **Checkpoint**: Patient Pathways has 9 tabs (Icicle through Doses, no Trends). `python run_dash.py` starts cleanly. PASSED.
### E.2 Add Trends sidebar nav item + view container
- [ ] Add `"trends"` icon SVG to `_ICONS` dict in `dash_app/components/sidebar.py` — use a line chart icon: `<polyline points="22 12 18 12 15 21 9 3 6 12 2 12"/>`
- [ ] Add `_sidebar_item("Trends", "trends", active=False, item_id="nav-trends")` to sidebar children
- [ ] Add `html.Div(id="trends-view", style={"display": "none"}, children=[...])` to `app.py` layout inside `view-container`, after `trust-comparison-view`
- [ ] Update `switch_view()` in `dash_app/callbacks/navigation.py`:
- Add `Output("trends-view", "style")` and `Output("nav-trends", "className")` — now 3 views, 3 nav items (6 outputs total)
- Handle 3-way switching: `"patient-pathways"`, `"trust-comparison"`, `"trends"`
- [ ] Update `update_app_state()` in `dash_app/callbacks/filters.py`:
- Add `Input("nav-trends", "n_clicks")`
- Add `elif triggered_id == "nav-trends": active_view = "trends"` case
- **Checkpoint**: 3 sidebar items visible. Clicking "Trends" switches to empty trends view. `python run_dash.py` starts cleanly.
### E.3 Create Trends landing page — directorate-level trends
- [ ] Create `dash_app/components/trends.py`:
- `make_trends_landing()` — container with title, description, metric toggle (`dmc.SegmentedControl` id: `trends-view-metric-toggle`, options: Patients / Cost per Patient / Cost per Patient p.a.), and `dcc.Graph(id="trends-overview-chart")` wrapped in `dcc.Loading`
- `make_trends_detail()` — hidden container with back button (id: `trends-back-btn`), title (id: `trends-detail-title`), same metric toggle, and `dcc.Graph(id="trends-detail-chart")` wrapped in `dcc.Loading`
- [ ] Update `get_trend_data()` in `pathway_queries.py` to support `group_by` parameter:
- `group_by="drug"` (default, existing behavior): one line per drug
- `group_by="directory"`: one line per directory (aggregate drugs within each directory)
- When `group_by="directory"`: `SELECT period_end, directory AS name, SUM(...) ... GROUP BY period_end, directory`
- [ ] Update thin wrapper in `dash_app/data/queries.py` to pass `group_by` param
- [ ] Create `dash_app/callbacks/trends.py` with `register_trends_callbacks(app)`:
- Callback to render directorate-level chart: Input `app-state` + `trends-view-metric-toggle` → Output `trends-overview-chart` figure. Calls `get_trend_data(group_by="directory", metric=...)` → `create_trend_figure(data, title, metric)`.
- Only fires when `active_view == "trends"` and `selected_trends_directorate` is None.
- [ ] Register in `dash_app/callbacks/__init__.py`
- [ ] Rename "Cost" label to "Cost per Patient" in the metric toggle options (value stays `total_cost`)
- [ ] Wire `trends-view` div in `app.py` to contain `make_trends_landing()` + `make_trends_detail()`
- **Checkpoint**: Trends view shows directorate-level line chart. Metric toggle switches y-axis. Lines show one per directorate.
### E.4 Add drug drill-down within Trends view
- [ ] Add `selected_trends_directorate` key (default `None`) to `app-state` initial data in `app.py`
- [ ] Add directorate selection callback in `dash_app/callbacks/trends.py`:
- Clicking a line/trace on the overview chart sets `selected_trends_directorate` in app-state
- Use `clickData` from `trends-overview-chart` as Input
- Extract directorate name from the clicked trace's `name` attribute
- Update `app-state` with `selected_trends_directorate`
- [ ] Add landing/detail toggle callback:
- Input: `app-state` → show/hide `trends-landing` vs `trends-detail`
- When `selected_trends_directorate` is set: hide landing, show detail with title "[Directorate] — Drug Trends"
- [ ] Add detail chart callback:
- Input: `app-state` + `trends-view-metric-toggle` → Output `trends-detail-chart`
- Calls `get_trend_data(directory=selected, metric=..., group_by="drug")` → `create_trend_figure()`
- Only fires when `selected_trends_directorate` is not None
- [ ] Add back button callback:
- Clicking `trends-back-btn` clears `selected_trends_directorate` in app-state → returns to landing
- **Checkpoint**: Click a directorate line → drill into drug-level trends. Back button returns to overview. `python run_dash.py` starts cleanly.
### E.5 Fix chart height to fill viewport
- [ ] In `create_trend_figure()` in `plotly_generator.py`: remove explicit `height=500`, let `autosize=True` (from `_base_layout()`) handle it
- [ ] For ALL Patient Pathways chart functions (icicle, sankey, heatmap, funnel, depth, scatter, network, timeline, doses): review and remove fixed `height=...` values where appropriate. Replace with responsive height:
- For charts with dynamic height (e.g. `max(400, n_bars * 28 + 150)`): keep the dynamic calculation but ensure minimum is high enough to fill viewport
- For charts with fixed `height=500`: remove it
- [ ] Add CSS rule to ensure `#pathway-chart .js-plotly-plot, #pathway-chart .plot-container` have `height: 100%` to propagate the flex container height to the Plotly div
- [ ] Verify the existing CSS flex chain propagates correctly: `.chart-card` → `.dash-loading-callback` → `#chart-container` → `#pathway-chart`
- [ ] Rename "Cost" to "Cost per Patient" in any remaining metric toggle labels (heatmap toggles in `chart_card.py` and `trust_comparison.py`)
- **Checkpoint**: Charts fill available viewport height in Patient Pathways. No fixed 500px cutoff. `python run_dash.py` starts cleanly.
---
## Completion Criteria
@@ -273,6 +353,16 @@ Comprehensive review and improvement of all Plotly charts in the Dash dashboard.
- [x] Drug timeline shows Gantt-style cohort activity
- [x] `python run_dash.py` starts cleanly
### Phase E
- [ ] Trends tab removed from Patient Pathways (9 tabs remain)
- [ ] 3rd sidebar item "Trends" visible and functional
- [ ] Trends landing page shows directorate-level line chart with metric toggle
- [ ] Clicking a directorate drills into drug-level trends
- [ ] Back button returns to directorate overview
- [ ] Charts fill available viewport height (no fixed 500px cutoff)
- [ ] "Cost" renamed to "Cost per Patient" in metric toggles
- [ ] `python run_dash.py` starts cleanly
---
## Key Reference Files
+6 -35
View File
@@ -417,25 +417,6 @@ def _render_doses(app_state, title):
return create_dosing_distribution_figure(data, title)
def _render_trends(app_state, title, metric="patients"):
"""Build the temporal trends line chart."""
from dash_app.data.queries import get_trend_data
from visualization.plotly_generator import create_trend_figure
selected_dirs = (app_state or {}).get("selected_directorates") or []
selected_drugs = (app_state or {}).get("selected_drugs") or []
directory = selected_dirs[0] if len(selected_dirs) == 1 else None
drug = selected_drugs[0] if len(selected_drugs) == 1 else None
try:
data = get_trend_data(metric=metric, directory=directory, drug=drug)
except Exception:
log.exception("Failed to load trend data")
return _empty_figure("Failed to load trend data.")
return create_trend_figure(data, title, metric=metric)
def register_chart_callbacks(app):
"""Register tab switching, pathway data loading, and chart rendering callbacks."""
@@ -515,21 +496,18 @@ def register_chart_callbacks(app):
Output("pathway-chart", "figure"),
Output("chart-subtitle", "children"),
Output("heatmap-metric-wrapper", "style"),
Output("trends-metric-wrapper", "style"),
Input("chart-data", "data"),
Input("active-tab", "data"),
Input("app-state", "data"),
Input("heatmap-metric-toggle", "value"),
Input("trends-metric-toggle", "value"),
)
def update_chart(chart_data, active_tab, app_state, heatmap_metric, trends_metric):
def update_chart(chart_data, active_tab, app_state, heatmap_metric):
"""Render the active tab's chart from chart-data nodes."""
active_tab = active_tab or "icicle"
chart_type = (app_state or {}).get("chart_type", "directory")
# Show/hide metric toggles based on active tab
# Show/hide metric toggle based on active tab
heatmap_toggle_style = {} if active_tab == "heatmap" else {"display": "none"}
trends_toggle_style = {} if active_tab == "trends" else {"display": "none"}
if chart_type == "indication":
subtitle = "Trust \u2192 Indication \u2192 Drug \u2192 Patient Pathway"
@@ -537,24 +515,17 @@ def register_chart_callbacks(app):
subtitle = "Trust \u2192 Directorate \u2192 Drug \u2192 Patient Pathway"
if not chart_data:
return no_update, no_update, heatmap_toggle_style, trends_toggle_style
return no_update, no_update, heatmap_toggle_style
error_msg = chart_data.get("error")
if error_msg:
return _empty_figure(error_msg), subtitle, heatmap_toggle_style, trends_toggle_style
# Trends tab doesn't depend on chart-data nodes
if active_tab == "trends":
title = _generate_chart_title(app_state) if app_state else ""
metric = trends_metric or "patients"
fig = _render_trends(app_state, title, metric=metric)
return fig, subtitle, heatmap_toggle_style, trends_toggle_style
return _empty_figure(error_msg), subtitle, heatmap_toggle_style
if not chart_data.get("nodes"):
return _empty_figure(
"No matching pathways found.\n"
"Try adjusting your filters."
), subtitle, heatmap_toggle_style, trends_toggle_style
), subtitle, heatmap_toggle_style
# Lazy rendering — only compute the active tab's chart
title = _generate_chart_title(app_state) if app_state else ""
@@ -609,4 +580,4 @@ def register_chart_callbacks(app):
tab_label = dict(TAB_DEFINITIONS).get(active_tab, active_tab)
fig = _empty_figure(f"{tab_label} chart — coming soon")
return fig, subtitle, heatmap_toggle_style, trends_toggle_style
return fig, subtitle, heatmap_toggle_style
-18
View File
@@ -14,7 +14,6 @@ TAB_DEFINITIONS = [
("network", "Network"),
("timeline", "Timeline"),
("doses", "Doses"),
("trends", "Trends"),
]
# Full set retained for Trust Comparison dashboard (Phase 10.8)
@@ -95,23 +94,6 @@ def make_chart_card():
),
],
),
# Trends metric toggle — visible only when trends tab active
html.Div(
id="trends-metric-wrapper",
style={"display": "none"},
children=[
dmc.SegmentedControl(
id="trends-metric-toggle",
data=[
{"value": "patients", "label": "Patients"},
{"value": "total_cost", "label": "Cost"},
{"value": "cost_pp_pa", "label": "Cost p.a."},
],
value="patients",
size="xs",
),
],
),
],
),
# Chart area with loading spinner
-8
View File
@@ -30,7 +30,6 @@ from data_processing.pathway_queries import (
get_drug_network as _get_drug_network,
get_drug_timeline as _get_drug_timeline,
get_dosing_distribution as _get_dosing_distribution,
get_trend_data as _get_trend_data,
)
DB_PATH = Path(__file__).resolve().parents[2] / "data" / "pathways.db"
@@ -252,10 +251,3 @@ def get_dosing_distribution(
return _get_dosing_distribution(DB_PATH, date_filter_id, chart_type, directory, trust)
def get_trend_data(
metric: str = "patients",
directory: Optional[str] = None,
drug: Optional[str] = None,
) -> list[dict]:
"""Time-series trend data from pathway_trends table."""
return _get_trend_data(DB_PATH, metric, directory, drug)
+61 -1
View File
@@ -640,6 +640,66 @@ Working Dash application with 2 views (Patient Pathways + Trust Comparison), 13
- `sqlite3` table existence check via `SELECT name FROM sqlite_master WHERE type='table' AND name='pathway_trends'` is the clean way to handle the table-doesn't-exist case.
- Adding a new Output/Input to an existing callback requires updating ALL return paths (4 return sites in update_chart).
### Next iteration should:
- ALL TASKS ARE COMPLETE. Check completion criteria in IMPLEMENTATION_PLAN.md.
- See Manual Intervention below — Phase E tasks added.
### Blocked items:
- None — all tasks complete
## Manual Intervention — 2026-02-07
### Reason: Redesign temporal trends as standalone view + fix chart height
### Changes made:
- `IMPLEMENTATION_PLAN.md` — added Phase E with 5 tasks (E.1E.5), updated "What Changes" section, added Phase E completion criteria
- `guardrails.md` — added guardrails for 3-view navigation and Trends view state
- `progress.txt` — this entry
### Tasks reset: None (all Phase AD tasks remain complete)
### Tasks added:
- E.1: Remove Trends tab from Patient Pathways
- E.2: Add Trends sidebar nav item + view container (3rd top-level view)
- E.3: Create Trends landing page — directorate-level overview chart with metric toggle
- E.4: Add drug drill-down within Trends view (click directorate → drug-level trends)
- E.5: Fix chart height to fill viewport + rename "Cost" to "Cost per Patient"
### Context for next iteration:
- Start with E.1 (remove Trends from Patient Pathways) — this is a cleanup task that simplifies the codebase before adding the new view
- E.1 involves removing the trends tab from TAB_DEFINITIONS, removing the trends-metric-wrapper/toggle from chart_card.py, removing _render_trends() and its dispatch case from chart.py, and cleaning up the update_chart() callback signature (remove trends Output/Input). CRITICAL: update ALL return paths in update_chart() when removing the trends toggle style output.
- After E.1, E.2 adds the 3rd sidebar item and empty view container. Key files: sidebar.py (add icon + nav item), app.py (add trends-view div), navigation.py (3-way switch_view), filters.py (add nav-trends Input)
- E.3 creates the new Trends view components and callbacks. The existing `get_trend_data()` in pathway_queries.py needs a `group_by` parameter added. `create_trend_figure()` in plotly_generator.py is reused as-is.
- E.4 adds drill-down using the same landing/detail toggle pattern as Trust Comparison (selected_trends_directorate in app-state)
- E.5 fixes chart height by removing fixed height values and relying on CSS flex + responsive=True
- The existing `get_trend_data()` query already supports directory filter and drug filter. For directorate-level grouping, add a `group_by="directory"` parameter that changes the SQL GROUP BY from drug to directory.
- Keep `create_trend_figure()` — it already handles any number of named series
### New guardrails added:
- "3-view navigation pattern" — when modifying switch_view or update_app_state, update ALL outputs/return paths for 3 views
- "Trends view state in app-state" — selected_trends_directorate must be in app-state initial data
## Iteration 17 — 2026-02-07
### Task: E.1 — Remove Trends tab from Patient Pathways
### Why this task:
- E.1 is the first Phase E task. Progress.txt explicitly recommended it. It's a cleanup prerequisite — the Trends tab must be removed from Patient Pathways before re-adding it as a standalone 3rd view in E.2E.4.
### Status: COMPLETE
### What was done:
- **chart_card.py**: Removed `("trends", "Trends")` from `TAB_DEFINITIONS` (now 9 tabs: Icicle through Doses). Removed `trends-metric-wrapper` div and `trends-metric-toggle` SegmentedControl from chart card header.
- **chart.py**: Removed `_render_trends()` helper function (~17 lines). Removed `Output("trends-metric-wrapper", "style")` and `Input("trends-metric-toggle", "value")` from `update_chart()` callback. Removed `trends_metric` parameter. Removed `trends_toggle_style` variable and the entire `if active_tab == "trends"` dispatch block. Updated ALL 4 return paths from 4-tuple to 3-tuple (figure, subtitle, heatmap_toggle_style).
- **queries.py**: Removed `get_trend_data` import from the import block and removed the `get_trend_data()` thin wrapper function.
- **Preserved**: `get_trend_data()` in `pathway_queries.py` and `create_trend_figure()` in `plotly_generator.py` — both still needed for the new Trends view.
### Validation results:
- Tier 1 (Code): `from dash_app.app import app` OK. `python run_dash.py` starts cleanly on http://127.0.0.1:8050/.
- Tier 2 (Visual): 9 tabs visible (Icicle, Sankey, Heatmap, Funnel, Depth, Scatter, Network, Timeline, Doses). No Trends tab.
- Tier 3 (Functional): Callback signature now has 3 Outputs, 4 Inputs (was 4 Outputs, 5 Inputs). All 4 return paths updated correctly.
### Files changed:
- `dash_app/components/chart_card.py` — removed trends tab + metric toggle
- `dash_app/callbacks/chart.py` — removed _render_trends + trends dispatch + trends I/O
- `dash_app/data/queries.py` — removed get_trend_data import + wrapper
- `IMPLEMENTATION_PLAN.md` — marked E.1 subtasks [x]
### Committed: [pending]
### Patterns discovered:
- Removing a callback Output/Input required updating exactly 4 return paths: (1) no chart_data, (2) error_msg, (3) no nodes, (4) final return with fig. Guardrail about counting return paths before/after was essential.
- The `dmc` import in chart_card.py is still needed for the heatmap metric toggle — only the trends toggle was removed.
### Next iteration should:
- Do Task E.2: Add Trends sidebar nav item + view container. Key files to read:
1. `dash_app/components/sidebar.py` — add "trends" icon + 3rd nav item
2. `dash_app/app.py` — add `trends-view` div to layout, add `selected_trends_directorate` to app-state initial data
3. `dash_app/callbacks/navigation.py` — update `switch_view()` for 3 views (6 outputs: 3 view styles + 3 nav classNames)
4. `dash_app/callbacks/filters.py` — add `Input("nav-trends", "n_clicks")` to `update_app_state()`
- CRITICAL: The `switch_view()` callback must handle 3 views with 6 Outputs. Read the existing 2-view implementation first to understand the pattern, then extend to 3.
- CRITICAL: `update_app_state()` has multiple return paths — ensure ALL handle the new "trends" active_view case.
### Blocked items:
- None