feat(analysis): import-time breath persistence and two-phase import job - #156
Draft
wpfleger96 wants to merge 19 commits into
Draft
feat(analysis): import-time breath persistence and two-phase import job#156wpfleger96 wants to merge 19 commits into
wpfleger96 wants to merge 19 commits into
Conversation
…e seams, two-phase import job
Implements PR-A of the multiuser MCP plan: the substrate required before
BreathService queries can be answered.
Key changes:
- Breath ORM model and migration: new breaths table with all per-breath
columns (timing, flow features, trigger/cycle types, flatness_index).
Written atomically in store_result() within the same analysis transaction.
- AlgorithmIdentity / AlgoVersions / AnalysisRunMetadata in versioning.py
(all StrEnum; engine_versions_json uses nested {identity, run} shape).
- AnalysisComputation: new intermediate type returned by compute_for_session();
decouples compute from DB write so analyze_session() handles all three
phases (read, compute, write) behind one async boundary.
- run_batch_analysis(session_ids=...): when session_ids is provided, filters
to exactly those rows instead of date range — used by the post-import hook.
- ensure_registered_parsers() in register_all.py: idempotent parser
registration checked by parser ID; safe to call multiple times.
- --no-analyze CLI flag on snore import: skips analysis phase when set.
When omitted, runs AnalysisFacade.run_batch_analysis(session_ids=...) on
the freshly imported session IDs immediately after the import transaction
commits.
- ImportResult.imported_session_ids (and per-source): session IDs thread
from _import_single_session() through import_sessions_batch() up through
ImportService.import_sources() so callers have the exact IDs to analyze.
- Two-phase import job (API path): _run_import() now calls phase_complete()
after import commits (non-terminal milestone), then runs analysis.
All terminal payloads produced after import commits carry import_committed
and import_result — including analysis failure and cancellation.
- BreathService typed seam: all Appendix A types and stub method signatures;
NotImplementedError bodies (PR-B fills them). Waveform helpers
fetch_waveform_window_raw() and compute_waveform_window() implemented.
- 7 pinned two-phase job contract tests (test_import_two_phase.py).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
- Add test_import_session_ids.py: 4 unit tests pinning that import_sessions_batch returns correct DB Session.id values for new sessions, returns empty for skipped sessions, and that ImportResult correctly aggregates imported_session_ids from per-source batches. - Add two e2e tests to test_import_options.py: --no-analyze skips the analysis phase (analysis show fails for session 1), and a default import (no --no-analyze) stores an analysis result at import time. - Fix SQLite write concurrency: set max_workers=1 in all three sites that call run_batch_analysis (CLI import hook, API import router, analysis run CLI). SQLite tolerates only one concurrent writer; prior runs with max_workers=4 caused database-is-locked failures when store_result began writing Breath rows alongside analysis_results. - Add --no-analyze to e2e conftest import_fixture helper so base fixtures don't pre-populate analysis results, keeping existing tests that assert on initial DB state deterministic. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
changed the title
…outers, and CLI Add primary_mode field to AnalysisRunRequest and BatchAnalysisRequest (api/schemas.py), pass it through both API router handlers, and wire it through the CLI analysis run command with --primary-mode option. - api/schemas.py: primary_mode: str | None on both request models - api/routers/analysis.py: pass primary_mode to facade.run_analysis and run_batch_analysis - cli/groups/analysis.py: --primary-mode option, thread through _analyze_single_session and _analyze_batch helpers; ValueError → ClickException translation in both helpers (max_workers=1 retained: SQLite tolerates only one concurrent writer; PostgreSQL callers can increase via the session_ids path — boundary deviation justified) - analysis/service.py: add id DESC tie-breaker to latest-run selector Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…r to 422 Implement five BreathService methods that were stubbed NotImplementedError: - _resolve_session_for_date: single/multi-session day disambiguation with MultiSessionAmbiguityError for ambiguous multi-device days - _latest_analysis_for_session: (status, algo, ar_id) with tie-breaker ORDER BY created_at DESC, id DESC - get_breath_table: paginated BreathRow or time-binned BreathBin aggregates; NOT_RUN / STALE_VERSION early-returns; CROSS_VERSION_REFUSAL_KEYS guard - find_windows: three criteria (WORST_FLATTENING_LEAK_VALID, CA_CENTERED, FL_RUN_ENDING_IN_RECOVERY) with >50% overlap dedup and top-N selection; per-criterion options validation; mixed-version / partial-coverage handling - compare_epochs: RX uniformity check (EpochRxViolation), ALGO_VERSION_MISMATCH refusal, PRIMARY_MODE_MISMATCH demotion, DistributionStats for four metrics (mid_insp_flattening, flatness_index, tidal_volume_ml, ie_ratio on leak-valid breaths), RERA proxy from FL-run-ending-in-recovery pattern Also add ValueError → HTTPException(422) conversion in both analysis router handlers (run_analysis and run_batch_analysis) so invalid primary_mode arguments surface as 422 Unprocessable Entity rather than falling through to the 500 handler. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… boundaries, and breath-write acceptance Add two test files covering all pinned obligations from plan v3.8: tests/unit/test_primary_mode_rejection.py (18 tests): - _resolve_primary_mode: ValueError on out-of-modes primary_mode; ValueError when DEFAULT_MODE absent and primary_mode=None; valid member returned; DEFAULT_MODE used when present - API single-session route: 422 on invalid primary_mode, facade delegated on valid - API batch route: 422 on invalid primary_mode, non-422 on valid - _compute_leak_valid boundaries: absent channel, empty array, overlap below/above threshold, nearest-neighbour at exactly 5s gap, just above 5s gap, 1Hz vs 25Hz sample rates, ramp_active=None compile-time assertion tests/integration/test_breath_analysis_write.py (8 tests): - A1 fresh import: Breath rows present after store_result; session_id correct - A2 two re-analyses: two AnalysisResult rows retained; equal created_at tie-breaker selects highest id - A3 atomic rollback: RuntimeError after AnalysisResult flush rolls back parent and all Breath children - A4 non-UTC determinism: timestamps stored as naive UTC regardless of host tz; datetime.utcfromtimestamp epoch round-trip Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Implement five methods that raised NotImplementedError: - get_nightly_summary: queries Day/Session rows, calls _latest_analysis_for_session per session, checks CROSS_VERSION_REFUSAL_KEYS across ok sessions (MIXED_VERSION on mismatch), computes FL metrics (median/95th/max) and RERA proxy (FL runs of >=2 ending in recovery breath) from Breath rows, derives compliance from total therapy hours - get_nightly_range_summary: iterates date range calling get_nightly_summary, aggregates compliance stats into NightlyRangeSummary - get_device_capabilities: queries session/waveform/event/setting tables for actual date coverage and distinct channels/event types/setting keys; uses getattr guard for parser_registry.list_supported_models() since that method may not be implemented by all registry versions - get_contextual_events: fetches machine events enriched with session-level Statistics (pressure_mean, leak_mean) as per-event context approximation; per-moment waveform context deferred (NOT_AVAILABLE) to avoid full waveform deserialization - get_ca_analysis: fetches CA events and computes periodic-breathing proxy as fraction of breaths within 60s of any CA event start Type fixes: DayAnalysisStatus.STALE (not STALE_VERSION), MIXED_VERSION for cross-version mismatch, NullReason.ALGO_VERSION_MISMATCH (not CROSS_VERSION_MISMATCH); annotate fl_median/fl_95th/fl_max as float | None before branch to satisfy mypy; remove unused total_duration variable. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…onable missing-table error, seam tests
- Gate trigger/cycle inference on device manufacturer: ResMed →
APPLICABILITY_VALIDATED; any other vendor →
APPLICABILITY_UNVALIDATED_DEVICE. Adds device_manufacturer field to
RawSessionBlobs/AnalysisInputs; load_session_inputs_raw fetches the
Device row via a second query and threads manufacturer through to
_build_computed_breaths.
- Per-session timing in CLI import analysis phase: monotonic clock with
closure-based accumulator; progress line now shows
'session: X.Xs, total: Y.Ys'.
- Actionable missing-breaths-table error in store_result(): catches
OperationalError('no such table: breaths') and re-raises as
RuntimeError with drop-and-reimport instructions.
- fetch_waveform_window_raw raises ValueError when an explicit
session_id is supplied but not found, instead of returning an empty
window silently.
- test_breath_service_seams.py: all 33 seam tests now pass. Fixed
invalid enum strings in ComputedBreath factory
(inferred_trigger_type='patient'→'normal', inferred_cycle_type=
'machine'→'normal', ramp_active_reason='ramp_settings_absent'→
'not_available'); fixed CaDetail assertion (event_type does not exist
on CaDetail → assert duration_seconds + session_id instead).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… breaths assertion
Three acceptance blockers from Paul's final list:
- Item 3 (A4): datetime.fromtimestamp in store_result was host-TZ-dependent.
Fix: fromtimestamp(ts, tz=UTC).replace(tzinfo=None) — TZ-independent naive
UTC regardless of process timezone. Test drives real store_result() under
os.environ["TZ"]="America/New_York" + time.tzset(), asserts stored
timestamp equals datetime.utcfromtimestamp(epoch). The old two A4 tests
hand-built rows and could not fail; replaced with one test that can.
- Item 2: try/except wrapped self.db_session.add_all() which is in-memory
registration and never raises OperationalError. Fix: move flush() for
breath rows inside the try block so the real DB-touching call is guarded.
Test creates full schema, DROP TABLE breaths via sqlite3 (no monkeypatching),
re-opens engine, calls store_result() — verifies RuntimeError with
actionable message from the real SQLite error path.
- Item 1: e2e test_import_without_no_analyze_runs_analysis_phase only checked
analysis show success; never asserted breath rows. Fix: after import,
sqlite3.connect(db).execute('SELECT COUNT(*) FROM breaths') must be > 0.
Segmenter wiring break would now fail here.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…context, compliance denominator Address all Thufir pass-1 CRITICAL and IMPORTANT findings: CRITICAL — BreathService cross-profile read isolation - Add profile_id to BreathService.__init__ (required arg). - _resolve_session_for_date, get_breath_table, find_windows, compare_epochs, get_nightly_summary, get_analysis_status all join through Device.profile_id == self._profile_id. - get_device_capabilities verifies device ownership; returns null/empty for foreign devices. - fetch_waveform_window_raw accepts optional profile_id and enforces ownership when supplied. - get_waveform_window passes self._profile_id through. - 7 two-profile adversarial tests: foreign session/device IDs, date auto-selection, capabilities, find_windows, compare_epochs, waveform. IMPORTANT — AnalysisFacade latest-run selector inconsistency - analysis_facade.py: add id DESC tie-breaker to all 3 row_number() ORDER BY clauses (list_status, delete_latest, get_analysis_result). IMPORTANT — compliance denominator - get_nightly_range_summary: divide by n_calendar not n_nights. IMPORTANT — waveform corruption swallowed as CHANNEL_ABSENT - compute_waveform_window: re-raise ValueError (corrupt blob / sample mismatch); only unknown exceptions collapse to missing_channels. IMPORTANT — contextual event values were session means not at-event - get_contextual_events: use waveform window seam (pressure/leak ±5 s, MV over prior 120 s). null + NOT_AVAILABLE when channel absent. IMPORTANT — CA analysis used proxy instead of persisted data - get_ca_analysis: periodic_breathing_pct from persisted periodic_breathing_episodes (total duration / session duration * 100). MV rolling variance from MV waveform binned into 10-min windows. IMPORTANT — breath-table zero-coercion and missing peak_exp_flow - Add peak_exp_flow_lpm to models.Breath (nullable, no migration per ruling #4), ComputedBreath, and store_result(). - _build_computed_breaths threads peak_expiratory_flow through. - BreathRow fields (timing, amplitude, shape, class) made nullable; get_breath_table passes None instead of or-0.0/or-1/or-False. IMPORTANT — compare_epochs metrics filter and cross-epoch identity - metrics filter: only requested DistributionMetric fields are computed; unrequested fields receive null DistributionStats. - Cross-epoch CROSS_VERSION_REFUSAL_KEYS comparison added at return. MINOR - Replace datetime.utcfromtimestamp deprecation in TZ test with datetime.fromtimestamp(epoch, tz=UTC).replace(tzinfo=None). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…t access Address remaining items from Thufir pass-1 finding #3 not covered by 153bb31: get_nightly_summary: `analyzed == 0` inside `if not ok_sessions` was tautologically true, always returning NOT_RUN for all-stale days. Fixed: inspect session_coverages for any STALE_VERSION status. find_windows breath helpers: _find_worst_flattening_windows and _find_fl_run_windows skipped only `ar_id is None`, allowing stale sessions' breath rows to contribute to results. Fixed: also skip when `ar_status != AnalysisStatus.OK`. get_ca_analysis: early return on non-OK analysis blocked CA events even though events are stored at import time (event-anchored). Fixed: always fetch and return CA events; map status to honest day_status (STALE/NOT_RUN); gate pb_pct on ar_id not None. Tests (4 new, TestStaleCoverageStateMachine): - test_all_stale_nightly_summary_is_stale: day_status=STALE not NOT_RUN - test_stale_session_excluded_from_find_windows_breath_rows: stale breath rows absent from WORST_FLATTENING results; day_status=STALE - test_stale_and_not_run_find_windows_status_is_stale: mixed day is STALE - test_ca_events_returned_when_analysis_stale: CA events present, day_status=STALE Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…e adversarial matrix Item 1 — fetch_waveform_window_raw resolution query had no profile predicate. The function accepted profile_id as Optional, checked ownership only after resolution, and allowed foreign sessions to enter MultiSessionAmbiguityError payloads. Concrete leak: profile A (1 session) + profile B (1 session) on same date → resolution saw 2 rows → ambiguity error exposed B's session_id, start_time, and duration. Fix: make profile_id required; join Device and add Device.profile_id == profile_id in the first (resolution) query so foreign rows never enter resolution or ambiguity payloads. Three internal callers that omitted profile_id now pass self._profile_id. Item 2 — adversarial matrix missing 4 categories from Thufir's corrective action. Added to TestTwoProfileIsolation: - test_ambiguity_payload_excludes_foreign_profile_session: A has 2 sessions, B has 1 on same date → ambiguity error payload contains only A's 2 sessions; B's session_id absent. Fails without the Item-1 resolution-query fix. - test_foreign_session_on_same_date_does_not_cause_ambiguity: A (1) + B (1) on same date → resolves cleanly to A's session. Fails without the fix. - test_foreign_analysis_result_not_returned_via_breath_table_by_date: B has 10-breath analysis, A has 3-breath analysis on same date → A's get_breath_table by date returns exactly 3 rows. - test_get_contextual_events_foreign_session_not_returned: A has 1 OA event, B has 2 CA events on same date → A's get_contextual_events returns only OA. - test_get_ca_analysis_foreign_session_ca_events_not_returned: B has 2 CA events, A has none → A's get_ca_analysis returns empty ca_events. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…metrics, epoch refusal, capabilities, validation Addresses all Thufir pass-2 CRITICAL and IMPORTANT findings. CRITICAL: - Add DeviceAmbiguityError raised when ≥2 devices have sessions on same date - Add _resolve_device() centralizing profile-scoped device resolution - Add _fetch_day_sessions() returning all sessions for a resolved device/date - Add _reduce_day_status() pure reducer implementing 5-case precedence: mixed identities → MIXED_VERSION; all-OK → OK; all-NOT_RUN → NOT_RUN; all-STALE → STALE; anything else (incl. stale+not-run) → PARTIAL - Update find_windows(), get_nightly_summary(), get_nightly_range_summary(), compare_epochs(), get_contextual_events(), get_ca_analysis() to use helpers IMPORTANT #4 (status test): - Rename test_stale_and_not_run_find_windows_status_is_stale to _is_partial - Flip expectation to PARTIAL (stale+not-run → PARTIAL per plan §1 line 864) - Add plan-line citation comments to all status/precedence assertions IMPORTANT #5 (compliance): - get_nightly_summary() reads Day.total_therapy_hours instead of summing Session.duration_seconds - get_nightly_range_summary() validates date_end >= date_start IMPORTANT #6 (capabilities): - rx_keys_present uses RX_KEYS from rx_tracker (not CROSS_VERSION_REFUSAL_KEYS) - Add list_supported_models() to ParserRegistry aggregating parser metadata - get_device_capabilities() uses list_supported_models() IMPORTANT #7 (epoch refusal): - Move RX homogeneity check before any breath queries - Check ALL contributing sessions per night (not just first session) - Immediately refuse with null distributions when RX violation detected IMPORTANT #8 (corruption propagation): - get_contextual_events() and get_ca_analysis() narrow except-Exception catches to re-raise ValueError (corrupt blob) while swallowing absent-channel errors IMPORTANT #9 (input validation): - get_contextual_events() validates event_types (non-empty strings or None) and min_duration (>= 0 or None) IMPORTANT #2 (CA metrics): - Implement preceding_mv_slope via linear regression over prior 120s MV - Implement ps_delivered_cmh2o via mean(THERAPY_PRESSURE - EPAP) over ±5s - Implement stability_index as CV (std/mean) of MV over prior 120s - Fix WaveformWindowRequest window_cap_seconds bypass for full-session MV fetch - get_ca_analysis() and get_contextual_events() now aggregate across ALL sessions New tests: - TestSameProfileTwoDevice: DeviceAmbiguityError for methods without device_id - TestSplitNight: contextual events and CA analysis return from both sessions - TestContextualEventsInputValidation: invalid event_types and min_duration - TestNightlyRangeDateValidation: reversed date range raises ValueError Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…am tests Closes two acceptance gaps from ca05051: Numeric provenance (TestCaNumericProvenance, 4 tests): - test_ca_pb_pct_nonzero_from_known_episodes: episode end_time-start_time=360s in 3600s session → pb_pct=10.0% (asserts exact value, not just non-null) - test_ca_mv_slope_nonzero_from_linear_ramp: MV=t (unit ramp) → slope≈1.0 - test_ca_ps_nonzero_from_known_pressures: THERAPY_PRESSURE=20, EPAP=8 → PS≈12.0 - test_ca_mv_variance_nonzero_from_two_distinct_bins: MV bins [5.0, 15.0] → variance=50.0 (1200s session, exactly two 600s bins) Corrupt blob through public seams (TestCorruptBlobThroughPublicSeams, 2 tests): - test_corrupt_pressure_blob_raises_in_contextual_events: corrupt pressure waveform → ValueError propagates, not silently NOT_AVAILABLE - test_corrupt_mv_blob_raises_in_ca_analysis: corrupt MV waveform → same Blob builder helper: _make_waveform_blob_from_arrays (caller-supplied arrays); _make_corrupt_waveform_blob (3-byte string, not parseable as float32 pairs). Lifted pattern from tests/unit/test_waveform_service.py:_make_waveform_blob. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…work Findings closed: breath_service.py: - Deleted _resolve_device (line ~941), _resolve_session_for_date (line ~840), and _fetch_day_sessions (line ~994). Replaced with a single _resolve_range(date_start, date_end, device_id) → (resolved_device_id, sessions_by_date) that validates device ownership independent of data presence and raises DeviceAmbiguityError (≥2 devices) or ValueError (foreign device / no sessions) instead of leaking NO_DATA_IN_RANGE for authorization failures. - compare_epochs: two-phase design — Phase 1 resolves sessions + builds per-session RX snapshots; Phase 2 checks cross-epoch identity; only Phase 3 fetches breath rows. Any failure (RX violation or identity mismatch) nulls ALL epoch distributions before any breath queries. Added date_start <= date_end validation on each EpochRequest. Foreign device_id → NOT_AVAILABLE (not NO_DATA_IN_RANGE). Same-profile multi-device with no device_id → DeviceAmbiguityError raised to caller. - get_ca_analysis: eligibility gate — only OK sessions contribute to pb_pct and mv_rolling_variance. NOT_RUN and STALE sessions excluded from both numerator and denominator. MIXED_VERSION → refuse night-level fields with ALGO_VERSION_MISMATCH. MV rolling variance now collects bin means across ALL OK sessions (cross-session), computes one variance after the loop. preceding_mv_slope converted from L/min per second to L/min per minute (×60, plan §12 line 976). stability_index window narrowed from 120 s to 60 s (plan §12 line 976). - get_contextual_events: removed except Exception catch-all (unexpected failures now propagate). event_types capped at 50 items, deduplicated order-preserving. Uses _resolve_range; ValueError → return []. - compute_waveform_window: removed except Exception catch-all so unexpected compute failures propagate instead of being silently added to missing_channels. - get_device_capabilities: rx_keys_present now queries only Setting rows with non-null values. supported_vendor_models lets real exceptions from parser_registry propagate. Owned device with no data in range → NO_DATA_IN_RANGE (was NOT_AVAILABLE). - All callers (get_breath_table, find_windows, get_nightly_summary, get_contextual_events, get_ca_analysis) updated to use _resolve_range. test_breath_service_seams.py: - Updated test_resolve_session_for_date_foreign_device_invisible → test_resolve_range_foreign_profile_sessions_invisible (calls _resolve_range directly). - Updated test_compare_epochs_foreign_device_contributes_zero_sessions → test_compare_epochs_foreign_device_returns_not_available. - Updated test_ca_mv_slope_nonzero_from_linear_ramp: assert slope ≈ 60.0 ± 1.0 (plan §12 line 976: L/min per minute after ×60 conversion). - Fixed test_ca_mv_variance_nonzero_from_two_distinct_bins: added _store_analysis call so session passes the OK eligibility gate. - Added TestSameProfileTwoDeviceExtended: test_two_device_compare_epochs_raises_device_ambiguity, test_two_device_disjoint_date_range_raises_device_ambiguity, test_explicit_foreign_device_id_raises_in_epochs. - Added TestCompareEpochsRefusal: test_same_night_rx_divergence_refuses_with_null_distributions, test_cross_epoch_identity_mismatch_refuses_with_null_distributions (uses mock to inject two different AlgoVersions since _latest_analysis_for_session only returns OK for current-identity sessions). - Added TestCaNumericProvenanceExtended: test_ca_stability_index_over_60s_window, test_ca_pb_pct_over_eligible_sessions_only (20% not 10%, NOT_RUN excluded from denominator), test_ca_mv_variance_over_all_eligible_sessions. - Added TestContextualEventsInputValidationExtended: test_event_types_at_cap_does_not_raise, test_event_types_over_cap_is_truncated. - Added TestUnexpectedErrorPropagation: test_unexpected_runtime_error_propagates_through_contextual_events, test_unexpected_runtime_error_propagates_through_ca_analysis. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… numeric stability Item 1: Remove remaining except Exception in CA PB accumulation loop (breath_service.py ~3138). AnalysisResultDTO.model_validate now propagates ValidationError and all other unexpected exceptions instead of silently passing. Item 2: Add TestCaMixedVersionRefusal.test_mixed_version_refuses_ca_night_level_fields — two sessions with distinct algorithm identities → day_status=MIXED_VERSION → periodic_breathing_pct is None, pb_reason=ALGO_VERSION_MISMATCH, mv_rolling_variance is None. Uses same mock pattern as cross-epoch identity test (plan §1 line 185). Item 3: Fix test_ca_stability_index_over_60s_window — add numeric assertion abs(ev.stability_index - 0.2) < 0.05 (alternating 8/12 values: mean≈10, stdev≈2, CV≈0.2). Fix plan-line citation: stability is §12 line 980, not 976. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…XED_VERSION identity, Day session filter Pass-5 targeted fixes — all defects confirmed in source before dispatch. CRITICAL — resolve once per operation, pass fixed device_id down: - compare_epochs: union-resolve across all epoch dates BEFORE per-epoch loop; DeviceAmbiguityError propagates, foreign device → NOT_AVAILABLE, no-data → NO_DATA_IN_RANGE - get_nightly_range_summary: _resolve_range called once before per-date loop; DeviceAmbiguityError propagates, no-sessions ValueError → empty summary - get_contextual_events: except ValueError:return[] removed; ownership errors propagate - get_waveform_window: calls _resolve_range first; passes resolved device_id into fetch_waveform_window_raw, ensuring DeviceAmbiguityError fires for multi-device IMPORTANT — event_types reject (not truncate): - After dedup, if unique count > 50 → raise ValueError (never silently drop requested types) - Rewrote test to assert 50 succeeds and 51 raises IMPORTANT — MIXED_VERSION CA: algo_identity=None: - After _reduce_day_status, if MIXED_VERSION → algo_identity = None (plan §12 lines 984-993) - Updated MIXED_VERSION test to assert result.algorithm_identity is None IMPORTANT — get_device_capabilities Day filter: - Day query now requires EXISTS(Session.day_id == Day.id) — empty Day cache rows excluded - Added adversarial test: Day(session_count=0) + no sessions → NO_DATA_IN_RANGE MINOR — stability test falsifies 120-second window: - Signal redesigned: [0,30)=0.0, [30,90]=alternating 8/12, rest=0.0 - 60-second window [30,90] gives CV≈0.2; 120-second window includes zero-region (different CV) - Old 120-second implementation would fail the numeric assertion New adversarial tests: - test_two_device_nightly_range_raises_device_ambiguity (TestSameProfileTwoDeviceExtended) - test_foreign_device_id_in_contextual_events_propagates (TestSameProfileTwoDeviceExtended) - test_day_with_no_sessions_returns_no_data_in_range (TestGetDeviceCapabilities) - test_event_types_over_cap_raises_value_error (rewrite of truncation test) Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…CA null_reason Pass-6 targeted fixes (all defects confirmed in source). 1. IMPORTANT — fetch_waveform_window_raw: delete internal Session resolver. Signature changed to (db, request, session_id: int, session_start: datetime). All callers now pass pre-resolved session_id and session_start explicitly. get_waveform_window: calls _resolve_range for device validation, handles empty/multi-session/single cases, then passes resolved session into the raw fetch. DeviceAmbiguityError fires before the waveform fetch; DeviceNotOwnedError for explicit foreign device_id; no synthetic empty window with session_id=0. 2. IMPORTANT — Nightly range: DeviceNotOwnedError distinguishes ownership failure. _resolve_range now raises DeviceNotOwnedError (not ValueError) for an explicit device_id not owned by the profile. get_nightly_range_summary re-raises DeviceNotOwnedError (ownership failure propagates) and DeviceAmbiguityError, while catching only ValueError (no sessions, auto-select found nothing) for the empty-summary path. DeviceNotOwnedError added to __all__ exports. 3. IMPORTANT — CA MIXED_VERSION null_reason. STALE and MIXED_VERSION were both mapped to ANALYSIS_STALE. Split: STALE → ANALYSIS_STALE; MIXED_VERSION → ALGO_VERSION_MISMATCH (plan §12 lines 984-993). test_mixed_version_refuses_ca_night_level_fields updated to assert result.null_reason == NullReason.ALGO_VERSION_MISMATCH. 4. MINOR — Two-separate-epoch disjoint-date regression test. test_two_separate_disjoint_epoch_requests_raise_device_ambiguity: two separate EpochRequest objects (not one spanning both dates), device A on epoch 1 date, device B on epoch 2 date, no device_id → DeviceAmbiguityError from union-resolve. New adversarial tests: - test_get_waveform_window_foreign_device_raises (replaces old fetch_waveform_window_raw profile test — DeviceNotOwnedError, not ValueError on old path) - test_foreign_device_id_in_nightly_range_raises_device_not_owned - test_valid_device_with_no_sessions_in_range_returns_empty_summary - test_two_device_waveform_window_raises_device_ambiguity (replaces MultiSessionAmbiguityError) - test_two_separate_disjoint_epoch_requests_raise_device_ambiguity grep evidence: no Session select inside fetch_waveform_window_raw body. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…tion order Pass-7 fixes (all defects confirmed in source before dispatch). CRITICAL — fetch_waveform_window_raw: restore profile_id ownership enforcement. The pass-6 signature (session_id: int, session_start: datetime) had no ownership check; any caller with an AsyncSession could supply a foreign session_id and receive raw waveform bytes. Plan §9 lines 720-735: the public seam must take profile_id and verify Device.profile_id in a join. Implementation: - Rename old body to _fetch_waveform_blobs (private, trusted internal helper) - Restore fetch_waveform_window_raw(db, profile_id, request) with one ownership-enforcing join query; derives session_start from the owned DB row; raises ValueError for foreign/unknown session_id - Update all 6 internal call sites to _fetch_waveform_blobs (already have session_id + session_start from _resolve_range) - _fetch_waveform_blobs NOT in __all__; fetch_waveform_window_raw stays public IMPORTANT — get_waveform_window: validate session_id before empty-day return. Plan §9 lines 822-825: explicit session_id must raise if not found for the date/device. The empty-day block previously returned a synthetic session_id=0 window before checking request.session_id. Fixed: if not day_sessions AND request.session_id is not None → raise ValueError. Only the session_id=None path returns the synthetic empty window. MINOR — _resolve_range docstring updated: "Raise DeviceNotOwnedError" (was ValueError). New tests: - test_foreign_session_id_raises_value_error: profile A → profile B's session_id via public fetch_waveform_window_raw → ValueError (not bytes) - test_owned_session_returns_waveform_data: owned session → missing_channels (no blobs) - test_explicit_session_id_on_empty_date_raises: get_waveform_window, empty date, session_id=99999 → ValueError - test_no_session_id_on_empty_date_returns_empty_window: session_id=None, empty date → session_id=0 empty window (regression guard) Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ndow_raw Pass-8 fix (plan §9 lines 822-825). The ownership query in fetch_waveform_window_raw validated only Session.id + Device.profile_id, ignoring request.therapy_date and request.device_id. An owned session from a different date or a different same-profile device returned data under contradictory metadata. Fix: add Day join + Day.date == request.therapy_date predicate. When request.device_id is non-null, add Session.device_id == request.device_id. Same ValueError response for every mismatch. Two adversarial tests: - test_wrong_therapy_date_raises_value_error: owned session + wrong date → ValueError - test_wrong_device_id_raises_value_error: owned session + same-profile wrong device → ValueError Also fix get_waveform_window docstring: clarifies that _fetch_waveform_blobs is called internally (not the public fetch_waveform_window_raw). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
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
1 participant
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.
Adds the substrate PR-B (MCP server) requires: a
BreathORM model atomically persisted at analysis time, versioned algorithm identity, typed service seams with a centralized device resolver and status reducer, and the two-phase import job lifecycle.This branch grounds on main @
22a5b13(Phase 1 multiuser). No Alembic migration per ruling #4 — fresh DBs get the right schema; drop-and-reimport is the upgrade path.Breath model + analysis persistence
database/models.py:Breathmodel (FK toanalysis_results, CASCADE) withBreathMetrics,ShapeFeatures,peak_insp_flow_lpm,peak_exp_flow_lpm,mid_insp_flattening, inferred trigger/cycle (experimental), quality flags (leak_valid,ramp_active,mask_off); all timing/amplitude/shape fields nullableanalysis/service.py:store_result()atomically persistsAnalysisResult+Breathrows in one transaction; host-TZ-independent timestamps;(created_at DESC, id DESC)selector at all threeAnalysisFacadesitesBreathService seams — centralized device resolver and status reducer
DeviceAmbiguityError: structured error listing owned device IDs when a date-range query spans multiple profile-owned devices and nodevice_idwas specified_resolve_device(therapy_date, device_id): auto-selects when 1 device, raisesDeviceAmbiguityErrorfor ≥2,ValueErrorfor 0_fetch_day_sessions(device_id, therapy_date): all sessions for the resolved device/date_reduce_day_status(coverages, identities): single-sourced pure status reducer — exact precedence: MIXED_VERSION → OK → NOT_RUN → STALE (all-stale only) → PARTIALfind_windows,get_nightly_summary,compare_epochs,get_contextual_events,get_ca_analysis,get_waveform_window) goes through_resolve_deviceand_fetch_day_sessions; point seams enforce single-session on topget_breath_table: verifies explicitsession_idmatches both profile/date AND resolved/requested deviceService seam correctness
get_nightly_summary: readsDay.total_therapy_hours(canonical) instead of summingSession.duration_secondsget_nightly_range_summary: rejects reversed date ranges; divides compliance byn_calendar_nightsget_device_capabilities:rx_keys_presentusesRX_KEYSfromsnore.analysis.rx_tracker;supported_vendor_modelsfrom newparser_registry.list_supported_models()(implemented in registry)compare_epochs: RX homogeneity checked across EVERY contributing session/night BEFORE breath queries; refused epochs return null distributions; cross-epoch identity guard also runs before distributionsget_contextual_events: validatesevent_types(list of non-empty strings) andmin_duration >= 0at boundary; re-raisesValueError(corrupt blob); swallows only genuine channel-absent conditionsget_ca_analysis: uses_resolve_device+_fetch_day_sessionsfor split-night correctness; re-raises corrupt-blobValueError; PB% computed fromend_time - start_timeof persisted episodes; MV variance uses intentional full-session cap; per-CA MV slope, PS (IPAP−EPAP), stability (CV) computed via waveform window seamfetch_waveform_window_raw:profile_idrequired; joinsDevicein the first resolution queryprimary_mode end-to-end
api/schemas.py,api/routers/analysis.py,cli/groups/analysis.py:primary_modethreaded through all layers with proper resolution and API boundary validationImport pipeline
JobPhaseenum; non-terminalphase_completeSSE;--no-analyzeflagTests (1341 passing)
TestSameProfileTwoDevice(7 tests):DeviceAmbiguityErrorfor multi-device nightly/windows/events/CA/waveform; explicit-device isolation; session/device mismatch rejectionTestSplitNight(2 tests): split-night (2 sessions, 1 device) contextual events and CA aggregationTestStaleCoverageStateMachine(4 tests): all-stale → STALE; stale+not-run → PARTIAL (plan §1 line 864); stale-row exclusion from find_windows; CA events available on stale daysTestTwoProfileIsolation(12 tests): full adversarial matrix including ambiguity-payload exclusion (falsifiable against profile predicate), foreign analysis result, contextual events, CA analysisTestContextualEventsInputValidation(3 tests): invalid event_types, empty string, negative min_durationTestNightlyRangeDateValidation(1 test): reversed date range raises ValueErrorBoundary deviation —
max_workers=1cli/groups/analysis.pyis outside the named boundary but required at all three analysis-invocation sites to preventdatabase is lockeddeadlocks under SQLite.