Repository navigation
fix(analysis): merge event matching into the Analysis page - #441
Merged
wpfleger96 merged 2 commits intoNov 1, 2026
Merged
wpfleger96 merged 2 commits into
wpfleger96 merged 2 commits into
Conversation
Owner
Author
ScreenshotsEvent Comparison cardMatched and Sensitivity now sit on the Analysis page, under its page-level experimental banner. A missed device hypopnea shows Event ExplorerThe match section is gone, so the page shows device-scored events only. |
wpfleger96
added a commit
that referenced
this pull request
Oct 31, 2026
This was referenced Oct 31, 2026
wpfleger96
force-pushed
the
wpfleger96/merge-event-matching
branch
from
November 1, 2026 19:08
d9833b7 to
f6077fd
Compare
wpfleger96
force-pushed
the
wpfleger96/ui-experimental-pages
branch
from
November 1, 2026 19:08
552ec5a to
0efd518
Compare
/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
force-pushed
the
wpfleger96/ui-experimental-pages
branch
from
November 1, 2026 20:09
0efd518 to
d829a38
Compare
wpfleger96
force-pushed
the
wpfleger96/merge-event-matching
branch
from
November 1, 2026 20:09
f6077fd to
38e7dba
Compare
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



GET /sessions/{id}/events/matchreports 0 matched. It adds the storedtimestamp_startto each SNORE event's relative time and compares the result with absolute device times, buttimestamp_startis0.0on 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/comparehas the right time base but disagrees with it in other ways: it uses any-match false negatives, counts every device event type inmachine_event_count, has no matched count, and hardcodes the 5 s tolerance.This PR makes
compare_eventsthe one implementation and deletes/events/match. It now pools apneas and hypopneas on both sides and usesmatch_events_by_start_timewithEVENT_MATCH_TOLERANCE_SECONDS, the batch-validation matcher, so each event is in at most one pair. The result gainsmatched_events, the nameEventValidationResultalready 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_eventsnow finds its row throughlatest_analysis_row.Removed:
/events/match,EventMatchResult,EventService.match_events,get_machine_apnea_hypopnea_times,MATCHABLE_MACHINE_EVENT_TYPES,split_by_tolerance_match,HasStartTimeandgetEventMatch.convert_machine_eventsalready keeps only the matchable types.Changes that show up in numbers:
machine_event_counton the card and insnore waveform comparenow 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.H. It used to beunknown, becauseHypopneaEventhas noevent_type.snore waveform compareprints the matched count, and its Machine line says "(apneas and hypopneas only)".snore analysisMissed 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 andsnore waveform comparedoesn't, so the two can differ on those sessions.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.tscloses the gap the REST test can't see: a default page calling an experimental route. It calls everyui/src/apiexport against a fake adapter to find the wrappers that hit experimental operations, then scans all ofui/srcexceptapi,testsandtypes. The operation list lives inui/src/tests/experimentalOperations.json, andEXPERIMENTAL_OPERATIONSreads the same file, so a new tagged route is covered on both sides.cancelAnalysisJobis the one exception, sinceImportViewneeds it and it reads no metrics.No migration, re-analysis, re-parse or
recompute-daysis needed.Stack: #439 → #440 → this PR