You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
383 lines
15 KiB
383 lines
15 KiB
# Diagnose no-plot trajectories Implementation Plan
|
|
|
|
**Goal:** Add an opt-in debug mode for the Trajectories tab that surfaces runtime early-returns and swallowed exceptions so we can diagnose why no Plotly chart is shown.
|
|
|
|
**Architecture:** Minimal, reversible instrumentation inside explorer.py and explorer_helpers.py. Add an opt-in UI toggle (checkbox + EXPLORER_DEBUG_TRAJECTORIES env var), extend the existing diagnostics/inspector helper to surface additional samples/counts, un-silence broad excepts to log exceptions and capture tracebacks into a diagnostics object accessible to tests and the UI (when debug enabled).
|
|
|
|
**Design:** thoughts/shared/designs/2026-03-30-diagnose-no-plot-trajectories-design.md
|
|
|
|
---
|
|
|
|
## Dependency Graph
|
|
|
|
```
|
|
Batch 1 (parallel): 1.1, 1.2 [foundation - no deps]
|
|
Batch 2 (parallel): 2.1 [core - depends on batch 1]
|
|
```
|
|
|
|
---
|
|
|
|
## Batch 1: Foundation (parallel - 2 implementers)
|
|
|
|
All tasks in this batch have NO dependencies and run simultaneously.
|
|
|
|
### Task 1.1: Extend diagnostics inspector
|
|
**File:** `explorer_helpers.py` (modify function `inspect_positions_for_issues`)
|
|
**Test:** `tests/test_explorer_helpers_diagnostics.py`
|
|
**Depends:** none
|
|
|
|
Purpose: add compact, structured diagnostics (mp_positions_sample, mp_positions_count, windows_with_no_positions) to the existing inspector output so both UI and tests can consume them.
|
|
|
|
Implementation decisions (gap-filling):
|
|
- Keep the function import-safe and pure (no Streamlit calls). Return additional keys under the same dict.
|
|
- Provide small, deterministic samples (sorted keys limited to 10) so tests are stable.
|
|
|
|
Estimate: 45-90 minutes
|
|
|
|
Verify: `pytest -q tests/test_explorer_helpers_diagnostics.py`
|
|
|
|
```python
|
|
# COMPLETE test code - tests/test_explorer_helpers_diagnostics.py
|
|
import numpy as np
|
|
from explorer_helpers import inspect_positions_for_issues
|
|
|
|
|
|
def test_inspect_positions_for_issues_basic():
|
|
positions_by_window = {
|
|
"w1": {"mp1": (1.0, 2.0), "mp2": (float('nan'), float('nan'))},
|
|
"w2": {},
|
|
}
|
|
party_map = {"mp1": "P1"}
|
|
d = inspect_positions_for_issues(positions_by_window, party_map)
|
|
|
|
# basic keys still present
|
|
assert d["windows_count"] == 2
|
|
assert isinstance(d["mp_id_set"], set)
|
|
# new diagnostics
|
|
assert "mp_positions_count" in d
|
|
assert d["mp_positions_count"] >= 1
|
|
assert "mp_positions_sample" in d
|
|
assert isinstance(d["mp_positions_sample"], list)
|
|
assert "windows_with_no_positions" in d
|
|
assert isinstance(d["windows_with_no_positions"], list)
|
|
|
|
```
|
|
|
|
```python
|
|
# COMPLETE implementation - explorer_helpers.py (function replacement)
|
|
def inspect_positions_for_issues(
|
|
positions_by_window: Dict[str, Dict[str, Tuple[float, float]]],
|
|
party_map: Dict[str, str],
|
|
) -> Dict[str, Any]:
|
|
"""Inspect positions_by_window for simple issues/summary.
|
|
|
|
Returns a dictionary with keys including the previous ones (windows_count,
|
|
window_labels, mp_id_set, party_map_count, parties_with_centroid_counts,
|
|
mismatched_mp_ids_sample) plus:
|
|
- mp_positions_count: int (num unique MP ids seen)
|
|
- mp_positions_sample: list[str] (sorted sample up to 10)
|
|
- windows_with_no_positions: list[str]
|
|
|
|
This helper remains pure and import-safe so unit tests can exercise it.
|
|
"""
|
|
windows = list(positions_by_window.keys())
|
|
windows_count = len(windows)
|
|
window_labels = sorted(windows)[:10]
|
|
|
|
mp_id_set: Set[str] = set()
|
|
parties_with_centroid_counts: Dict[str, int] = {}
|
|
mismatched: Set[str] = set()
|
|
windows_with_no_positions: List[str] = []
|
|
|
|
for win, pos in positions_by_window.items():
|
|
if not pos:
|
|
windows_with_no_positions.append(win)
|
|
continue
|
|
present_parties: Set[str] = set()
|
|
for ent in pos.keys():
|
|
if not ent:
|
|
continue
|
|
mp_id_set.add(ent)
|
|
party = party_map.get(ent)
|
|
if party is None:
|
|
# try stripping paren variant
|
|
party = party_map.get(_strip_paren(ent))
|
|
if party:
|
|
present_parties.add(party)
|
|
else:
|
|
mismatched.add(ent)
|
|
|
|
for p in present_parties:
|
|
parties_with_centroid_counts[p] = parties_with_centroid_counts.get(p, 0) + 1
|
|
|
|
mismatched_mp_ids_sample = sorted(list(mismatched))[:10]
|
|
|
|
mp_positions_sample = sorted(list(mp_id_set))[:10]
|
|
mp_positions_count = len(mp_id_set)
|
|
|
|
return {
|
|
"windows_count": windows_count,
|
|
"window_labels": window_labels,
|
|
"mp_id_set": mp_id_set,
|
|
"party_map_count": len(party_map),
|
|
"parties_with_centroid_counts": parties_with_centroid_counts,
|
|
"mismatched_mp_ids_sample": mismatched_mp_ids_sample,
|
|
"mp_positions_sample": mp_positions_sample,
|
|
"mp_positions_count": mp_positions_count,
|
|
"windows_with_no_positions": windows_with_no_positions,
|
|
}
|
|
|
|
```
|
|
|
|
Commit: `feat(explorer): extend diagnostic inspector to surface mp samples/counts`
|
|
|
|
---
|
|
|
|
### Task 1.2: Add tests and small helper for reading debug env var
|
|
**File:** `explorer.py` (add function `get_debug_trajectories_enabled`) **-- part of batch 2 core but small and independent**
|
|
**Test:** `tests/test_debug_flag.py`
|
|
**Depends:** none
|
|
|
|
Purpose: provide a single, testable helper that reads EXPLORER_DEBUG_TRAJECTORIES env var and returns a boolean. We use this consistently in UI code so tests can manipulate debug mode via env var.
|
|
|
|
Decision: implement conservative parsing ("1", "true", "True") as truthy. This function will be used by build_trajectories_tab and tests.
|
|
|
|
Estimate: 15-30 minutes
|
|
|
|
Verify: `pytest -q tests/test_debug_flag.py`
|
|
|
|
```python
|
|
# COMPLETE test code - tests/test_debug_flag.py
|
|
import os
|
|
import importlib
|
|
|
|
def test_get_debug_flag_on(monkeypatch):
|
|
monkeypatch.setenv("EXPLORER_DEBUG_TRAJECTORIES", "1")
|
|
import explorer
|
|
importlib.reload(explorer)
|
|
assert explorer.get_debug_trajectories_enabled() is True
|
|
|
|
|
|
def test_get_debug_flag_off(monkeypatch):
|
|
monkeypatch.delenv("EXPLORER_DEBUG_TRAJECTORIES", raising=False)
|
|
import explorer
|
|
importlib.reload(explorer)
|
|
assert explorer.get_debug_trajectories_enabled() is False
|
|
|
|
```
|
|
|
|
```python
|
|
# COMPLETE implementation to add into explorer.py
|
|
def get_debug_trajectories_enabled() -> bool:
|
|
"""Return whether the Trajectories debug mode is enabled via env var.
|
|
|
|
Truthy values: "1", "true", "True". Default False.
|
|
"""
|
|
val = os.getenv("EXPLORER_DEBUG_TRAJECTORIES", "")
|
|
return val in ("1", "true", "True")
|
|
|
|
```
|
|
|
|
Commit message: `chore(explorer): add get_debug_trajectories_enabled helper`
|
|
|
|
---
|
|
|
|
## Batch 2: Core Modules (parallel - 1 implementer)
|
|
|
|
These tasks depend on changes in Batch 1 (inspector additions and debug-flag helper). All tasks in this batch modify `explorer.py` (single-file microtask) and have a single test file.
|
|
|
|
### Task 2.1: Instrument trajectories UI and un-silence exceptions
|
|
**File:** `explorer.py` (update `select_trajectory_plot_data` exception handling, update `build_trajectories_tab` early-return instrumentation and try/except, add module-level diagnostics capture)
|
|
**Test:** `tests/test_diagnose_no_plot_trajectories.py`
|
|
**Depends:** 1.1, 1.2
|
|
|
|
Purpose: (A) Add opt-in debug UI binding to env var via checkbox and a DEBUG expander; (B) change helper-call swallow to log exceptions and include traceback in diagnostics; (C) instrument early-return gates (no positions, no mp_positions) to capture the reason and attach it to module-level diagnostics; (D) expose diagnostics to tests via attributes so tests can assert they were produced.
|
|
|
|
Decisions / gap-fills:
|
|
- Do not change public function signatures. To expose diagnostics to tests without changing signatures, set attributes on the function and module:
|
|
- select_trajectory_plot_data._last_diagnostics -> last inspector summary
|
|
- explorer._last_diagnostics -> diagnostics captured by build_trajectories_tab (early-returns or exceptions)
|
|
- Always call logger.exception(...) when an exception happens to preserve logs.
|
|
- Only call Streamlit UI functions to display tracebacks when debug mode is enabled.
|
|
|
|
Estimate: 2-4 hours
|
|
|
|
Verify: `pytest -q tests/test_diagnose_no_plot_trajectories.py`
|
|
|
|
```python
|
|
# COMPLETE test code - tests/test_diagnose_no_plot_trajectories.py
|
|
import traceback
|
|
import importlib
|
|
import explorer
|
|
from types import SimpleNamespace
|
|
|
|
|
|
def test_select_helper_exception_is_captured(monkeypatch):
|
|
# Force the inspector to raise and ensure diagnostics capture the traceback
|
|
def _boom(*a, **k):
|
|
raise RuntimeError("boom-inspector")
|
|
|
|
monkeypatch.setattr("explorer_helpers.inspect_positions_for_issues", _boom)
|
|
# call helper
|
|
fig, count, banner = explorer.select_trajectory_plot_data({}, {}, [], [])
|
|
# diagnostics should be attached to the function
|
|
d = getattr(explorer.select_trajectory_plot_data, "_last_diagnostics", None)
|
|
assert d is not None
|
|
assert "inspector_exception" in d
|
|
assert "boom-inspector" in d["inspector_exception"]
|
|
|
|
|
|
def test_build_trajectories_tab_early_return_sets_diagnostics(monkeypatch):
|
|
# Make load_positions return empty positions to trigger early return
|
|
monkeypatch.setattr(explorer, "load_positions", lambda db, ws: ({}, None))
|
|
# Ensure debug mode enabled via env var
|
|
monkeypatch.setenv("EXPLORER_DEBUG_TRAJECTORIES", "1")
|
|
importlib.reload(explorer)
|
|
# Call the tab builder (uses dummy Streamlit in tests)
|
|
explorer.build_trajectories_tab("/fake.db", "2025")
|
|
d = getattr(explorer, "_last_diagnostics", None)
|
|
assert d is not None
|
|
assert d.get("reason") == "no_positions"
|
|
|
|
```
|
|
|
|
```python
|
|
# COMPLETE implementation snippets to apply to explorer.py
|
|
import traceback
|
|
|
|
# Add near top-level (after imports in explorer.py)
|
|
_last_diagnostics: Optional[dict] = None
|
|
|
|
|
|
def get_debug_trajectories_enabled() -> bool:
|
|
val = os.getenv("EXPLORER_DEBUG_TRAJECTORIES", "")
|
|
return val in ("1", "true", "True")
|
|
|
|
|
|
# Replace the small inspector try/except in select_trajectory_plot_data with the
|
|
# following (complete function shown below replaces the existing select_trajectory_plot_data
|
|
# definition in explorer.py):
|
|
def select_trajectory_plot_data(
|
|
positions_by_window: Dict[str, Dict[str, Tuple[float, float]]],
|
|
party_map: Dict[str, str],
|
|
windows: List[str],
|
|
selected_parties: List[str],
|
|
smooth_alpha: float = 0.35,
|
|
mp_fallback_count: Optional[int] = None,
|
|
) -> Tuple[go.Figure, int, Optional[str]]:
|
|
"""Return (fig, trace_count, banner_text).
|
|
|
|
Helper used by build_trajectories_tab. Does not call Streamlit.
|
|
"""
|
|
if mp_fallback_count is None:
|
|
try:
|
|
mp_fallback_count = int(os.getenv("EXPLORER_MP_FALLBACK_COUNT", "20"))
|
|
except Exception:
|
|
mp_fallback_count = 20
|
|
|
|
# Compute per-party centroids aligned to windows
|
|
party_centroids, meta = compute_party_centroids(
|
|
positions_by_window, party_map, windows
|
|
)
|
|
|
|
# Use inspector to collect diagnostics (import-safe, pure helper).
|
|
try:
|
|
inspector_summary = inspect_positions_for_issues(positions_by_window, party_map)
|
|
except Exception as e:
|
|
# Do not silently swallow: log and capture traceback text so tests / UI
|
|
# can inspect it. Keep function import-safe (no Streamlit here).
|
|
tb = traceback.format_exc()
|
|
logger.exception("inspect_positions_for_issues failed: %s", e)
|
|
inspector_summary = {"inspector_exception": tb}
|
|
|
|
# expose diagnostics for tests without changing function signature
|
|
setattr(select_trajectory_plot_data, "_last_diagnostics", inspector_summary)
|
|
logger.debug("select_trajectory_plot_data inspector summary: %s", inspector_summary)
|
|
|
|
# ... rest of the original function remains unchanged (build fig/trace_count)
|
|
# (Implementation note: keep the rest identical to existing function.)
|
|
|
|
|
|
# Now update the call-site in build_trajectories_tab (replace the try/except around
|
|
# select_trajectory_plot_data invocation with the following snippet):
|
|
try:
|
|
fig2, trace_count2, banner_text = select_trajectory_plot_data(
|
|
positions_by_window, party_map, windows, selected_parties, smooth_alpha
|
|
)
|
|
if fig2 is not None:
|
|
fig = fig2
|
|
trace_count = trace_count2
|
|
if banner_text:
|
|
st.caption(banner_text)
|
|
except Exception as e:
|
|
# Do not silently pass. Log, capture traceback and (when debug enabled)
|
|
# surface to Streamlit.
|
|
tb = traceback.format_exc()
|
|
logger.exception("select_trajectory_plot_data raised: %s", e)
|
|
global _last_diagnostics
|
|
_last_diagnostics = {"build_exception": tb}
|
|
if get_debug_trajectories_enabled():
|
|
try:
|
|
st.exception(e)
|
|
except Exception:
|
|
# Streamlit may not be available in test env; fall back to text_area
|
|
try:
|
|
st.text_area("Trajectories exception", tb)
|
|
except Exception:
|
|
pass
|
|
|
|
|
|
# Instrument early-return gates (example: when positions_by_window is empty) by
|
|
# setting _last_diagnostics before returning. Replace the current block:
|
|
if not positions_by_window:
|
|
st.warning("Geen positiedata beschikbaar.")
|
|
global _last_diagnostics
|
|
_last_diagnostics = {"reason": "no_positions", "inspector": {}}
|
|
if get_debug_trajectories_enabled():
|
|
# call inspector and attach diagnostics when debug enabled
|
|
try:
|
|
_last_diagnostics["inspector"] = inspect_positions_for_issues(positions_by_window, {})
|
|
except Exception:
|
|
_last_diagnostics["inspector"] = {"error": "inspector_failed"}
|
|
return
|
|
|
|
# Note: make similar instrumentation for the `if not mp_positions:` early return
|
|
# inside the per-MP fallback path: set _last_diagnostics = {"reason": "no_mp_positions"}
|
|
|
|
```
|
|
|
|
Notes for implementer:
|
|
- Insert the two helper functions and the try/except replacement in the appropriate places of explorer.py. The select_trajectory_plot_data replacement above should replace the function body; keep the unchanged plotting logic intact after the diagnostic area.
|
|
- Add the module-level _last_diagnostics variable near the top of explorer.py (after imports).
|
|
|
|
Commit: `feat(explorer): instrument trajectories with debug diagnostics and un-silence helper exceptions`
|
|
|
|
---
|
|
|
|
## Verification & Manual checks
|
|
|
|
- Run unit tests for the modified files:
|
|
- pytest -q tests/test_explorer_helpers_diagnostics.py
|
|
- pytest -q tests/test_debug_flag.py
|
|
- pytest -q tests/test_diagnose_no_plot_trajectories.py
|
|
- Manual: run Streamlit locally with EXPLORER_DEBUG_TRAJECTORIES=1 and inspect the "DEBUG" expander in the Trajectories tab to see the diagnostics block and any surfaced tracebacks.
|
|
|
|
---
|
|
|
|
## Rollback plan
|
|
|
|
- All changes gated behind debug env var and small: revert the two modified files (explorer.py, explorer_helpers.py) to previous commit to remove instrumentation.
|
|
- Because public signatures are unchanged, rollout/revert is safe.
|
|
|
|
---
|
|
|
|
## Appendix — quick implementer checklist
|
|
|
|
1. Implement inspector changes (explorer_helpers.py) and run its tests.
|
|
2. Add get_debug_trajectories_enabled helper and tests.
|
|
3. Modify explorer.py: add _last_diagnostics, update select_trajectory_plot_data try/except, update build_trajectories_tab try/except and early-return instrumentation, add debug checkbox wiring in UI.
|
|
4. Add tests that monkeypatch inspector and load_positions and assert diagnostics are created.
|
|
|
|
---
|
|
|
|
Written: thoughts/shared/plans/2026-03-30-diagnose-no-plot-trajectories.md
|
|
|