diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index f4bd8d26..f6a74490 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -429,6 +429,8 @@ created_at, updated_at UNIQUE(device_id, date) ``` +After any membership change a `days` row survives only if at least one `sessions` row (enabled or disabled) still references it; `DayManager.recalculate_day` prunes orphaned rows after session deletion, import-time replacement, and `snore db recompute-days` (migration `017` removed shells left by earlier versions). `session_count` counts enabled sessions, so a day whose sessions are all disabled persists with `session_count = 0` and reset aggregates. Requesting a fully deleted day by date therefore returns not-found rather than a zeroed row. + **sessions** ```sql id, device_id (FK devices), day_id (FK days), diff --git a/src/snore/cli/groups/db.py b/src/snore/cli/groups/db.py index 1e460dce..88eea8f7 100644 --- a/src/snore/cli/groups/db.py +++ b/src/snore/cli/groups/db.py @@ -170,7 +170,10 @@ async def _run() -> None: @db.command("recompute-days") @db_option @click.confirmation_option( - prompt="Re-derive every Day's metrics from stored session statistics?" + prompt=( + "Re-derive every Day's metrics from stored session statistics " + "and delete Day rows no session references?" + ) ) def recompute_days(db: str | None) -> None: """Recompute all Day aggregates from stored session statistics (no reparse). @@ -178,7 +181,8 @@ def recompute_days(db: str | None) -> None: Re-runs day aggregation over the ``Statistics`` already stored for each session, refreshing every Day row across all devices and profiles. Use after an aggregation-formula change (e.g. a new weighting) to update - historical Day rows without re-importing raw device data. + historical Day rows without re-importing raw device data. Day rows that + no session references are deleted rather than recomputed. Days are processed in chunks, each committed in its own gated write transaction (like ``cleanup-orphans``), so the SQLite write lock is @@ -218,6 +222,7 @@ async def _run() -> None: console=console, ) as progress: task = progress.add_task("Recomputing days", total=len(day_ids)) + pruned = 0 for chunk in iter_id_chunks(day_ids): async with write_gate(), session_scope(immediate=True) as session: days = ( @@ -226,10 +231,14 @@ async def _run() -> None: .all() ) for day in days: - await DayManager.aggregate_day_statistics(day, session) + if not await DayManager.recalculate_day(day, session): + pruned += 1 progress.update(task, advance=len(chunk)) - print_success(f"Recomputed {len(day_ids)} day(s) from session statistics") + summary = f"Recomputed {len(day_ids) - pruned} day(s) from session statistics" + if pruned: + summary += f"; pruned {pruned} orphaned day(s)" + print_success(summary) asyncio.run(_run()) diff --git a/src/snore/database/day_manager.py b/src/snore/database/day_manager.py index c86ee8ec..1363be60 100644 --- a/src/snore/database/day_manager.py +++ b/src/snore/database/day_manager.py @@ -6,7 +6,7 @@ from datetime import date, datetime, time, timedelta -from sqlalchemy import select +from sqlalchemy import exists, select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import joinedload @@ -220,10 +220,6 @@ async def aggregate_day_statistics(cls, day: Day, db_session: AsyncSession) -> N day, spec.name, cls._weighted_average(stat_pairs, spec.name) ) - # Private alias kept for existing callers (importers.py, tests) that still - # use the pre-rename name. - _aggregate_day_statistics = aggregate_day_statistics - @classmethod async def link_session_to_day( cls, @@ -255,12 +251,35 @@ async def link_session_to_day( return day @classmethod - async def recalculate_day(cls, day: Day, db_session: AsyncSession) -> None: + async def recalculate_day(cls, day: Day, db_session: AsyncSession) -> bool: """ - Recalculate aggregated statistics for a day. + Recalculate a day after its session membership changed. + + Lifecycle rule: after any membership change a Day row survives only if + at least one Session row, enabled or disabled, still references it. + Deleting the last such session orphans the day and the row is pruned + here (re-import recreates it via ``link_session_to_day``). Disabling + the last enabled session does not orphan the day: the disabled Session + still points at it through the composite ``(day_id, device_id)`` FK, so + the row stays with ``session_count == 0`` and reset aggregates. Args: day: Day object to recalculate db_session: SQLAlchemy async database session + + Returns: + True if the day still exists, False if it was pruned. """ + # The existence probe is a Core statement and does not trigger + # autoflush, so flush first: a pending Session that references this + # day would otherwise be invisible and the day deleted from under it. + await db_session.flush() + referenced = await db_session.scalar( + select(exists().where(SessionModel.day_id == day.id)) + ) + if not referenced: + await db_session.delete(day) + await db_session.flush() + return False await cls.aggregate_day_statistics(day, db_session) + return True diff --git a/src/snore/database/importers.py b/src/snore/database/importers.py index 2d6e896c..551205ad 100644 --- a/src/snore/database/importers.py +++ b/src/snore/database/importers.py @@ -532,12 +532,9 @@ async def import_sessions_batch( for day_id in batch_day_ids: day_record = await db.get(models.Day, day_id) if day_record: - await DayManager._aggregate_day_statistics(day_record, db) - # A replaced session may have been the sole occupant of its - # Day row. Delete orphan Day rows so they don't appear in - # day listings with zero sessions. - if day_record.session_count == 0: - await db.delete(day_record) + # A replaced session may have been the sole occupant of + # its Day row; recalculate_day prunes the orphaned row. + await DayManager.recalculate_day(day_record, db) # Force and overlap-replace imports delete existing sessions (cascading # to their waveforms) before inserting replacements, so SQLite may reuse diff --git a/src/snore/database/migrations/versions/017_sessions_day_id_index.py b/src/snore/database/migrations/versions/017_sessions_day_id_index.py new file mode 100644 index 00000000..c133a882 --- /dev/null +++ b/src/snore/database/migrations/versions/017_sessions_day_id_index.py @@ -0,0 +1,59 @@ +"""Index ``sessions.day_id`` and delete orphaned ``days`` rows. + +``DayManager.aggregate_day_statistics`` and the orphan probe in +``DayManager.recalculate_day`` both look sessions up by ``day_id``. Without an +index each is a full ``sessions`` scan, so ``snore db recompute-days`` costs +O(days x sessions). The model-side ``Index`` covers fresh DBs via +``create_all``; this migration covers pre-existing DBs. Both use the name +``ix_sessions_day_id`` so the ``test_migration_schema_drift`` parity check +stays green. + +Before ``recalculate_day`` pruned orphans, deleting every session of a day +left a zero-session ``days`` shell behind. The one-off DELETE below removes +those historical shells so the lifecycle rule holds for existing databases +without a manual ``recompute-days`` run. Days whose sessions are all disabled +are still referenced and are kept. + +Revision ID: 017_sessions_day_id_index +Revises: 016_validation_runs +""" + +from collections.abc import Sequence + +import sqlalchemy as sa + +from alembic import op + +revision: str = "017_sessions_day_id_index" +down_revision: str | Sequence[str] | None = "016_validation_runs" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + +_INDEX = "ix_sessions_day_id" +_TABLE = "sessions" + + +def upgrade() -> None: + bind = op.get_bind() + from sqlalchemy import inspect as sa_inspect # noqa: PLC0415 + + existing_indexes = {ix["name"] for ix in sa_inspect(bind).get_indexes(_TABLE)} + if _INDEX not in existing_indexes: + op.create_index(_INDEX, _TABLE, ["day_id"]) + + op.execute( + sa.text( + "DELETE FROM days WHERE id NOT IN " + "(SELECT day_id FROM sessions WHERE day_id IS NOT NULL)" + ) + ) + + +def downgrade() -> None: + # The orphan-row DELETE is not reversible; only the index is dropped. + bind = op.get_bind() + from sqlalchemy import inspect as sa_inspect # noqa: PLC0415 + + existing_indexes = {ix["name"] for ix in sa_inspect(bind).get_indexes(_TABLE)} + if _INDEX in existing_indexes: + op.drop_index(_INDEX, table_name=_TABLE) diff --git a/src/snore/database/models.py b/src/snore/database/models.py index 8778590a..dd664c10 100644 --- a/src/snore/database/models.py +++ b/src/snore/database/models.py @@ -564,7 +564,11 @@ def __repr__(self) -> str: class Day(Base): - """Daily aggregated statistics (OSCAR-compatible pre-calculated cache).""" + """Daily aggregated statistics (OSCAR-compatible pre-calculated cache). + + Rows no Session references are pruned by ``DayManager.recalculate_day``, + which documents the lifecycle rule. + """ __tablename__ = "days" @@ -730,6 +734,10 @@ class Session(Base): # start_time range scan. Run per imported session; index prevents full table # scan growth as diagnostic-blip sessions accumulate. Index("ix_sessions_device_id_start_time", "device_id", "start_time"), + # Day aggregation and the orphan-day probe both look sessions up by + # day_id; without this index each is a full sessions scan, making + # ``db recompute-days`` O(days x sessions). + Index("ix_sessions_day_id", "day_id"), ) def __repr__(self) -> str: diff --git a/src/snore/services/breath/capabilities.py b/src/snore/services/breath/capabilities.py index 5dcaba76..ea0e2f82 100644 --- a/src/snore/services/breath/capabilities.py +++ b/src/snore/services/breath/capabilities.py @@ -70,7 +70,8 @@ async def get_device_capabilities( ) # Date range of actual data — only days with at least one Session count - # as "imported nights"; empty Day rows never widen the reported range. + # as "imported nights". DayManager.recalculate_day prunes orphaned Day + # rows, so this predicate is defence-in-depth against hand-edited data. day_stmt = select(models.Day).where( models.Day.device_id == device_id, exists().where(models.Session.day_id == models.Day.id), diff --git a/src/snore/services/session_service.py b/src/snore/services/session_service.py index b3a239c7..4fd335f5 100644 --- a/src/snore/services/session_service.py +++ b/src/snore/services/session_service.py @@ -357,7 +357,8 @@ async def delete_sessions(self, session_ids: list[int]) -> int: Day aggregates for the affected days are recalculated after the DELETE (mirrors ``set_session_enabled``), so a day left with fewer sessions is - re-aggregated and a day left with none has its statistics reset. + re-aggregated and a day left with no sessions at all is pruned (see + ``DayManager.recalculate_day``). """ # Dedupe: chunked IN-binds don't implicitly de-duplicate like a single IN. session_ids = list(dict.fromkeys(session_ids)) @@ -405,11 +406,15 @@ async def delete_sessions(self, session_ids: list[int]) -> int: # a reused id never serves a deleted row's arrays. clear_waveform_array_cache() + # day_ids were collected under the profile filter; the join repeats it + # as defence-in-depth because recalculate_day can delete the row. for chunk in iter_id_chunks(list(day_ids)): days = ( ( await self.db_session.execute( - select(models.Day).where(models.Day.id.in_(chunk)) + select(models.Day) + .join(models.Device, models.Day.device_id == models.Device.id) + .where(models.Day.id.in_(chunk), self._profile_filter()) ) ) .scalars() diff --git a/tests/integration/test_cli_commands.py b/tests/integration/test_cli_commands.py index cdbf3328..a8234a95 100644 --- a/tests/integration/test_cli_commands.py +++ b/tests/integration/test_cli_commands.py @@ -375,6 +375,60 @@ def test_recompute_days_rederives_ahi( assert len(ahi_values) == 10 assert all(v == pytest.approx(5.2) for v in ahi_values) + def test_recompute_days_prunes_orphans_and_keeps_disabled_only_days( + self, cli_runner, populated_test_db, db_session + ): + """Under the production engine (FK enforcement on), recompute-days + deletes a Day whose sessions are gone and keeps a Day whose only + session is disabled, reporting the pruned count.""" + orphan_day_id, disabled_day_id = ( + db_session.execute( + text( + "SELECT day_id FROM sessions WHERE device_session_id IN " + "('test_session_0', 'test_session_1') ORDER BY device_session_id" + ) + ) + .scalars() + .all() + ) + db_session.execute( + text("DELETE FROM sessions WHERE device_session_id = 'test_session_0'") + ) + db_session.execute( + text( + "UPDATE sessions SET enabled = 0 " + "WHERE device_session_id = 'test_session_1'" + ) + ) + db_session.commit() + + result = cli_runner.invoke( + cli, + ["db", "recompute-days", "--db", str(populated_test_db)], + input="y\n", + ) + + assert result.exit_code == 0 + assert "Recomputed 9 day(s)" in result.output + assert "pruned 1 orphaned day(s)" in result.output + + db_session.expire_all() + remaining = db_session.execute(text("SELECT id FROM days")).scalars().all() + assert orphan_day_id not in remaining + assert len(remaining) == 9 + disabled_count = db_session.execute( + text("SELECT session_count FROM days WHERE id = :id"), + {"id": disabled_day_id}, + ).scalar_one() + assert disabled_count == 0 + assert ( + db_session.execute( + text("SELECT COUNT(*) FROM sessions WHERE day_id = :id"), + {"id": disabled_day_id}, + ).scalar_one() + == 1 + ) + def test_recompute_days_empty_database(self, cli_runner, temp_db): """recompute-days on an empty database reports zero days.""" asyncio.run(init_database(str(temp_db))) diff --git a/tests/unit/test_breath_service_seams.py b/tests/unit/test_breath_service_seams.py index 34029043..d44f2285 100644 --- a/tests/unit/test_breath_service_seams.py +++ b/tests/unit/test_breath_service_seams.py @@ -988,7 +988,8 @@ async def test_day_with_no_sessions_returns_no_data_in_range( """get_device_capabilities returns NO_DATA_IN_RANGE for a Day row with no sessions. Empty Day cache rows (session_count=0, no Session children) must not inflate - nights_with_data or produce a non-null actual date range. + nights_with_data or produce a non-null actual date range. DayManager now + prunes such rows, so this models legacy or hand-edited data. plan §13 lines 949-961: actual endpoints derived from dates with ≥1 session. """ from snore.database import models as _m # noqa: PLC0415 diff --git a/tests/unit/test_day_manager.py b/tests/unit/test_day_manager.py index ac62af35..fbf56bb6 100644 --- a/tests/unit/test_day_manager.py +++ b/tests/unit/test_day_manager.py @@ -6,11 +6,14 @@ are aggregated across multiple sessions. """ -from datetime import date, datetime +from datetime import date, datetime, timedelta import pytest +from sqlalchemy import select + from snore.database.day_manager import DayManager +from snore.database.models import Day, Session class TestDaySplitLogic: @@ -210,7 +213,7 @@ async def test_empty_day_resets_statistics( session.day_id = None await async_db_session.flush() - await DayManager._aggregate_day_statistics(day, async_db_session) + await DayManager.aggregate_day_statistics(day, async_db_session) assert day.session_count == 0 assert day.total_therapy_hours == 0.0 @@ -247,7 +250,7 @@ async def test_empty_day_resets_epap_statistics( session.day_id = None await async_db_session.flush() - await DayManager._aggregate_day_statistics(day, async_db_session) + await DayManager.aggregate_day_statistics(day, async_db_session) assert day.epap_min is None assert day.epap_max is None @@ -501,3 +504,93 @@ async def test_zero_usage_hours_contributes_nothing_to_total( day = await DayManager.link_session_to_day(session, device.id, async_db_session) assert day.total_therapy_hours == pytest.approx(0.0, abs=0.001) + + +class TestDayPruning: + """Test DayManager.recalculate_day's orphan-pruning lifecycle rule.""" + + async def test_recalculate_day_prunes_unreferenced_day( + self, async_db_session, async_test_device, async_test_session_factory + ): + """A day no Session row references is deleted, not reset in place.""" + device = async_test_device + + session = await async_test_session_factory( + device_id=device.id, + start_time=datetime(2024, 11, 5, 22, 0, 0), + duration_hours=8.0, + ) + day = await DayManager.link_session_to_day(session, device.id, async_db_session) + day_id = day.id + + await async_db_session.delete(session) + await async_db_session.flush() + + assert await DayManager.recalculate_day(day, async_db_session) is False + assert await async_db_session.get(Day, day_id) is None + + async def test_recalculate_day_keeps_day_with_only_disabled_sessions( + self, async_db_session, async_test_device, async_test_session_factory + ): + """A disabled session still references its day, so the row survives + with zeroed aggregates rather than being pruned.""" + device = async_test_device + + session = await async_test_session_factory( + device_id=device.id, + start_time=datetime(2024, 11, 5, 22, 0, 0), + duration_hours=8.0, + ahi=5.0, + ) + day = await DayManager.link_session_to_day(session, device.id, async_db_session) + day_id = day.id + + session.enabled = False + await async_db_session.flush() + + assert await DayManager.recalculate_day(day, async_db_session) is True + + await async_db_session.flush() + async_db_session.expire_all() + stored = ( + await async_db_session.execute(select(Day).where(Day.id == day_id)) + ).scalar_one() + assert stored.session_count == 0 + assert stored.ahi is None + + async def test_recalculate_day_sees_pending_session_before_pruning( + self, async_db_session, async_test_device, async_test_session_factory + ): + """A Session added but not yet flushed still counts as a reference. + + The existence probe is a Core statement that bypasses autoflush; without + an explicit flush the pending session would be invisible and the day + deleted out from under it. + """ + device = async_test_device + + first = await async_test_session_factory( + device_id=device.id, + start_time=datetime(2024, 11, 5, 22, 0, 0), + duration_hours=8.0, + ) + day = await DayManager.link_session_to_day(first, device.id, async_db_session) + day_id = day.id + + await async_db_session.delete(first) + await async_db_session.flush() + + start = datetime(2024, 11, 6, 2, 0, 0) + pending = Session( + device_id=device.id, + device_session_id="pending_session", + start_time=start, + end_time=start + timedelta(hours=4), + duration_seconds=4 * 3600, + day_id=day_id, + ) + async_db_session.add(pending) + + assert await DayManager.recalculate_day(day, async_db_session) is True + assert await async_db_session.get(Day, day_id) is day + assert day.session_count == 1 diff --git a/tests/unit/test_import_overlap_guard.py b/tests/unit/test_import_overlap_guard.py index 609eb7d5..32c30969 100644 --- a/tests/unit/test_import_overlap_guard.py +++ b/tests/unit/test_import_overlap_guard.py @@ -446,6 +446,49 @@ async def test_replacement_deletes_orphan_day_row( assert len(days) == 1 assert days[0].session_count == 1 + async def test_replacement_keeps_day_with_disabled_sibling( + self, async_db_session, importer, device + ): + """A disabled session still references its Day, so replacing the day's + only enabled session must not prune the row. + + The old ``session_count == 0`` check counted enabled sessions only, so + the ORM delete would have tried to null the sibling's FK and aborted the + import with an IntegrityError.""" + # Enabled session on therapy day 20, replaced below by one on day 19. + await _seed_session( + async_db_session, device, "20260120_130000", _dt(20, 13), _dt(20, 19) + ) + # Disabled sibling on therapy day 20 that survives the replacement. + disabled = await _seed_session( + async_db_session, device, "20260120_210000", _dt(20, 21), _dt(20, 22) + ) + disabled.enabled = False + await async_db_session.flush() + + # 11:00-20:00 covers the 13:00-19:00 session and lands on therapy day 19. + incoming = _make_unified(_SERIAL, "20260120_merged", _dt(20, 11), _dt(20, 20)) + full_importer = SessionImporter(profile_id=importer.profile_id) + imported, skipped, failed, _ = await full_importer.import_sessions_batch( + [incoming], db=async_db_session + ) + assert (imported, skipped, failed) == (1, 0, 0) + assert not await _session_exists(async_db_session, "20260120_130000") + assert await _session_exists(async_db_session, "20260120_210000") + + days = ( + ( + await async_db_session.execute( + select(models.Day) + .where(models.Day.device_id == device.id) + .order_by(models.Day.date) + ) + ) + .scalars() + .all() + ) + assert [(d.date.day, d.session_count) for d in days] == [(19, 1), (20, 0)] + class TestOldFormatPurge: """Old noon-bucket rows (e.g. ``20260130_merged``) are purged unconditionally. diff --git a/tests/unit/test_session_service.py b/tests/unit/test_session_service.py index e66a0461..fbbc8046 100644 --- a/tests/unit/test_session_service.py +++ b/tests/unit/test_session_service.py @@ -401,10 +401,10 @@ async def test_delete_one_session_recomputes_day( assert day.obstructive_apneas == 8 assert day.pressure_mean == pytest.approx(12.0) - async def test_delete_all_sessions_resets_day_stats( + async def test_delete_all_sessions_prunes_day( self, async_db_session, async_test_device, async_test_session_factory ): - """Deleting every session of a day resets the day's aggregates.""" + """Deleting every session of a day removes the orphaned Day row.""" from snore.database.day_manager import DayManager base = datetime(2025, 3, 1, 22, 0, 0) @@ -436,23 +436,51 @@ async def test_delete_all_sessions_resets_day_stats( assert day.session_count == 2 assert day.epap_mean is not None + day_id = day.id + service = SessionService(async_db_session, profile_id=1) deleted = await service.delete_sessions([s1.id, s2.id]) assert deleted == 2 await async_db_session.flush() - await async_db_session.refresh(day) - assert day.session_count == 0 - assert day.total_therapy_hours == 0.0 - assert day.obstructive_apneas == 0 - assert day.central_apneas == 0 - assert day.hypopneas == 0 - assert day.reras == 0 - assert day.ahi is None - assert day.epap_mean is None - assert day.pressure_mean is None - assert day.leak_median is None - assert day.spo2_mean is None + assert await async_db_session.get(Day, day_id) is None + + async def test_delete_last_enabled_session_keeps_day_with_disabled_sibling( + self, async_db_session, async_test_device, async_test_session_factory + ): + """Deleting a day's only enabled session while a disabled sibling + remains keeps the Day row at session_count 0 and the sibling intact.""" + from snore.database.day_manager import DayManager + + base = datetime(2025, 3, 1, 22, 0, 0) + enabled = await async_test_session_factory( + async_test_device.id, base, duration_hours=4.0, usage_hours=4.0, ahi=8.0 + ) + disabled = await async_test_session_factory( + async_test_device.id, base + timedelta(hours=5), duration_hours=2.0 + ) + disabled.enabled = False + await async_db_session.flush() + + await DayManager.link_session_to_day( + enabled, async_test_device.id, async_db_session + ) + day = await DayManager.link_session_to_day( + disabled, async_test_device.id, async_db_session + ) + assert day.session_count == 1 + day_id, disabled_id = day.id, disabled.id + + service = SessionService(async_db_session, profile_id=1) + assert await service.delete_sessions([enabled.id]) == 1 + + await async_db_session.flush() + async_db_session.expire_all() + stored = await async_db_session.get(Day, day_id) + assert stored is not None + assert stored.session_count == 0 + assert stored.ahi is None + assert await async_db_session.get(Session, disabled_id) is not None async def test_delete_spanning_multiple_days_recomputes_each( self, async_db_session, async_test_device, async_test_session_factory @@ -476,7 +504,7 @@ async def test_delete_spanning_multiple_days_recomputes_each( usage_hours=4.0, ahi=4.0, ) - # Day B: one session — deleting it empties and resets the day. + # Day B: one session — deleting it orphans the day, which is pruned. b1 = await async_test_session_factory( async_test_device.id, base + timedelta(days=2), @@ -495,6 +523,7 @@ async def test_delete_spanning_multiple_days_recomputes_each( assert day_a.id != day_b.id assert day_a.session_count == 2 assert day_b.session_count == 1 + day_b_id = day_b.id service = SessionService(async_db_session, profile_id=1) deleted = await service.delete_sessions([a1.id, b1.id]) @@ -502,11 +531,9 @@ async def test_delete_spanning_multiple_days_recomputes_each( await async_db_session.flush() await async_db_session.refresh(day_a) - await async_db_session.refresh(day_b) assert day_a.session_count == 1 assert day_a.ahi == pytest.approx(4.0) - assert day_b.session_count == 0 - assert day_b.ahi is None + assert await async_db_session.get(Day, day_b_id) is None async def test_delete_across_chunk_boundary_recomputes_days( self, @@ -577,6 +604,7 @@ async def test_delete_across_chunk_boundary_recomputes_days( c1, async_test_device.id, async_db_session ) assert day_a.session_count == 4 + day_b_id, day_c_id = day_b.id, day_c.id service = SessionService(async_db_session, profile_id=1) deleted = await service.delete_sessions([a1.id, a2.id, a3.id, b1.id, c1.id]) @@ -599,14 +627,10 @@ async def test_delete_across_chunk_boundary_recomputes_days( assert survivor == 1 await async_db_session.refresh(day_a) - await async_db_session.refresh(day_b) - await async_db_session.refresh(day_c) assert day_a.session_count == 1 assert day_a.ahi == pytest.approx(2.0) - assert day_b.session_count == 0 - assert day_b.ahi is None - assert day_c.session_count == 0 - assert day_c.ahi is None + assert await async_db_session.get(Day, day_b_id) is None + assert await async_db_session.get(Day, day_c_id) is None async def test_delete_preview_sums_counts_and_sorts_across_chunks( self, @@ -854,6 +878,42 @@ async def test_set_session_enabled_toggle( await async_db_session.refresh(session) assert session.enabled is False + async def test_disable_last_session_keeps_day_and_reenable_restores_stats( + self, async_db_session, async_test_device, async_test_session_factory + ): + """Disabling a day's only session keeps its Day row (the disabled + session still references it) with session_count reset to 0, and + re-enabling it restores the aggregates.""" + from snore.database.day_manager import DayManager + + session = await async_test_session_factory( + async_test_device.id, + datetime(2025, 3, 1, 22, 0, 0), + duration_hours=6.0, + usage_hours=6.0, + ahi=5.0, + ) + day = await DayManager.link_session_to_day( + session, async_test_device.id, async_db_session + ) + assert day.session_count == 1 + + service = SessionService(async_db_session, profile_id=1) + await service.set_session_enabled(session.id, False) + + await async_db_session.flush() + await async_db_session.refresh(day) + assert day.session_count == 0 + assert day.total_therapy_hours == 0.0 + assert day.ahi is None + + await service.set_session_enabled(session.id, True) + + await async_db_session.flush() + await async_db_session.refresh(day) + assert day.session_count == 1 + assert day.ahi == pytest.approx(5.0) + async def test_set_session_enabled_not_found(self, async_db_session): """Raises ValueError if session not found.""" service = SessionService(async_db_session, profile_id=1)