Skip to content

fix(analysis): merge event matching into the Analysis page - #441

Merged
wpfleger96 merged 2 commits into
wpfleger96/ui-experimental-pagesfrom
wpfleger96/merge-event-matching
Nov 1, 2026
Merged

wpfleger96 merged 2 commits into
wpfleger96/ui-experimental-pagesfrom
wpfleger96/merge-event-matching

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Oct 31, 2026 •

Copy link
Copy Markdown
Owner

GET /sessions/{id}/events/match reports 0 matched. It adds the stored timestamp_start to each SNORE event's relative time and compares the result with absolute device times, but timestamp_start is 0.0 on stored analysis results, so nothing lines up. On all 205 analyzed sessions I checked locally it matched nothing, which is the 0 matched and 0% sensitivity on Event Explorer, and it also made every device event a false negative and every SNORE event a false positive.

/waveforms/compare has the right time base but disagrees with it in other ways: it uses any-match false negatives, counts every device event type in machine_event_count, has no matched count, and hardcodes the 5 s tolerance.

This PR makes compare_events the one implementation and deletes /events/match. It now pools apneas and hypopneas on both sides and uses match_events_by_start_time with EVENT_MATCH_TOLERANCE_SECONDS, the batch-validation matcher, so each event is in at most one pair. The result gains matched_events, the name EventValidationResult already uses. Matched and Sensitivity move to the Event Comparison card on the Analysis page, and Event Explorer drops its match section, so it's now a device-only page. The card names its detection mode, follows the mode toggle, loads after a run from the page, and hides when the selected mode has no comparison. compare_events now finds its row through latest_analysis_row.

Removed: /events/match, EventMatchResult, EventService.match_events, get_machine_apnea_hypopnea_times, MATCHABLE_MACHINE_EVENT_TYPES, split_by_tolerance_match, HasStartTime and getEventMatch. convert_machine_events already keeps only the matchable types.

Changes that show up in numbers:

  • Sensitivity goes from 0% to its real value. Across 200 analyzed local sessions that's 347 of 1,132 device apneas and hypopneas, about 31%.
  • machine_event_count on the card and in snore waveform compare now counts only apneas and hypopneas, so it drops (1,149 to 1,132 on those sessions). The card says so, because the summary card on the same page uses the same label for all device event types.
  • False-negative and false-positive totals can shift under one-to-one matching. On those sessions the totals didn't move (785 and 5,289), and new counts equal the old matching logic with the time base corrected on all 205 sessions.
  • A missed device hypopnea now reports type H. It used to be unknown, because HypopneaEvent has no event_type.
  • snore waveform compare prints the matched count, and its Machine line says "(apneas and hypopneas only)".
  • snore analysis Missed and Extra lists are now one-to-one and time-ordered, so they can be longer than before. It still includes RERAs when the device scored them and snore waveform compare doesn't, so the two can differ on those sessions.
  • False-positive and false-negative tables now page at 25 rows through a shared TablePager, which Events by Mode uses too.

These local analysis rows are STALE_VERSION, so the exact percentages will move after re-analysis. The sensitivity number is still SNORE's experimental detection scored against device events, not a clinical measurement.

ui/src/tests/experimentalBoundary.test.ts closes the gap the REST test can't see: a default page calling an experimental route. It calls every ui/src/api export against a fake adapter to find the wrappers that hit experimental operations, then scans all of ui/src except api, tests and types. The operation list lives in ui/src/tests/experimentalOperations.json, and EXPERIMENTAL_OPERATIONS reads the same file, so a new tagged route is covered on both sides. cancelAnalysisJob is the one exception, since ImportView needs it and it reads no metrics.

No migration, re-analysis, re-parse or recompute-days is needed.

Stack: #439 → #440 → this PR

@wpfleger96

Copy link
Copy Markdown
Owner Author

Screenshots

Event Comparison card

Matched and Sensitivity now sit on the Analysis page, under its page-level experimental banner. A missed device hypopnea shows H instead of unknown. Values here are mock data. Mobile below.

01-event-comparison-desktop

01-event-comparison-mobile

Event Explorer

The match section is gone, so the page shows device-scored events only.

02-event-explorer-desktop

/events/match always reported 0 matched: it added the stored timestamp_start,
which is 0.0, to relative times and compared them with absolute device times.
/waveforms/compare disagreed with it anyway, using any-match false negatives,
counting every device event type, and hardcoding the tolerance.

compare_events is now the one implementation, on the batch-validation matcher.
Its numbers move to the experimental Analysis page, so Event Explorer no longer
calls an experimental route. A boundary test keeps default pages off them.
`snore analysis` listed missed and extra events from any-match lists, so it could disagree with `snore waveform compare` on one session. Neither list is stored, so no re-analysis is needed.
The Analysis page card is the only home for Matched and Sensitivity, so it must appear after a run from the page, follow the selected mode, and page its tables.
The experimental operation list moves to JSON that the UI boundary test also reads, so a newly tagged route cannot escape the default-page scan.
The compare response's `matched` becomes `matched_events`, as in `EventValidationResult`.
@wpfleger96
wpfleger96 force-pushed the wpfleger96/ui-experimental-pages branch from 0efd518 to d829a38 Compare November 1, 2026 20:09
@wpfleger96
wpfleger96 force-pushed the wpfleger96/merge-event-matching branch from f6077fd to 38e7dba Compare November 1, 2026 20:09
@wpfleger96
wpfleger96 merged commit 6cc54d3 into wpfleger96/ui-experimental-pages Nov 1, 2026
4 checks passed
@wpfleger96
wpfleger96 deleted the wpfleger96/merge-event-matching branch November 1, 2026 20:40
wpfleger96 added a commit that referenced this pull request Nov 1, 2026
Day detail, the Dashboard, Stats and reports show SNORE's own FL and
RERA numbers and its effectiveness and trend judgements next to device
data. `GET /days/{date}` also runs the breath lookup on every load to
produce them. This PR moves that data to separate pages and endpoints,
so a person only sees it by going there. Nothing is deleted, and
analysis still runs on import.

| Default surface | Moved to |
|---|---|
| Day detail and `DayDetail`: `fl_class_ge4_pct`, `rera_index`,
`rera_count` and their reasons | `/days/:date/experimental` and `GET
/days/{date}/experimental` |
| Dashboard, Stats, `TherapySummary`, report templates: `effectiveness`,
`ahi_trend_direction` | Therapy Assessment card on the Analysis page and
`GET /stats/experimental` |

REST URLs stay put. Operations that return experimental data now carry a
FastAPI `experimental` tag: the analysis and validation routers,
`/events/match`, `/waveforms/compare` and the two new endpoints.
`tests/unit/test_provenance_tags.py` rejects any Experimental field
reachable from an untagged operation, and pins the tagged list in
`EXPERIMENTAL_OPERATIONS` so a route can't be tagged to escape the
check. Tagging is now required on enum and `Literal` response fields
too, so a judgement like `effectiveness` can't go untagged. Plain string
fields still aren't checked. The sidebar group "Analysis" is renamed
"Experimental".

**Changes that show up:**
- `effectiveness` and `ahi_trend_direction` are retagged from Derived to
Experimental, and their API types are now enums. They're SNORE's
judgements, not arithmetic on device data.
- The Therapy Assessment card covers all recorded data and says so. The
old Stats badge followed the Range toggle.
- `GET /days/{date}` no longer touches breath rows, so it answers the
same whether or not analysis has run.
- `snore stats` drops the Effectiveness line and the trend direction in
the chart title.
- `/stats/experimental` takes no `days_limit` and always returns 200. An
empty range gives `{"effectiveness": "unknown", "ahi_trend_direction":
null}`.
- `/stats/summary`, `/stats/periods`, `/stats/trends` and
`/stats/records` return 422 for `days_limit` outside 0 to 36500. Very
large values used to return 500 and negative ones an empty result. No
client sends either.
- `ApneaEvent.event_type` is now tagged Experimental.
- `DayDetailView` reloads when the date changes and drops stale
responses, like the new page.
- A browser tab left open across the deploy shows an undefined
effectiveness badge until reload.

**Left as is:**
- `DayExperimentalMetrics` has no `device_id`, and `DayDetailView` has
no device query, so on a two-device date the page shows the lowest
device id, the same row `GET /days/{date}` picks by default.
- The leak "Assessment" line in `snore stats` (`well controlled` or
`elevated`) is also a SNORE judgement. I left it for now since it wasn't
part of this split.
- Event Explorer still calls `/events/match`. The next PR in the stack
removes that.
- `ImportView` still calls `DELETE /analysis/jobs/{job_id}`, which is
tagged `experimental` with the rest of the analysis router. It only
cancels a job and reads no metrics.

Apple Health stays on the therapy pages, and Records and Rx best/worst
stay default.

No migration, re-analysis, re-parse or `recompute-days` is needed.

Stack: #439 → this PR → #441
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant