diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index 1b97728..9ba9fcd 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -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: `` +- [ ] 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 diff --git a/dash_app/callbacks/chart.py b/dash_app/callbacks/chart.py index 8242b6f..6d9cc2e 100644 --- a/dash_app/callbacks/chart.py +++ b/dash_app/callbacks/chart.py @@ -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 diff --git a/dash_app/components/chart_card.py b/dash_app/components/chart_card.py index 33fc9d0..09eafaa 100644 --- a/dash_app/components/chart_card.py +++ b/dash_app/components/chart_card.py @@ -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 diff --git a/dash_app/data/queries.py b/dash_app/data/queries.py index ce7a549..7cfc192 100644 --- a/dash_app/data/queries.py +++ b/dash_app/data/queries.py @@ -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) diff --git a/progress.txt b/progress.txt index a9b5074..36559b8 100644 --- a/progress.txt +++ b/progress.txt @@ -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.1–E.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 A–D 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.2–E.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