follow-up(statistics): P3 cleanups from #110 review (Closed Days) #111

New issue

Closed

opened 2026-09-25 18:08:27 +00:00 by gabogg · 0 comments

gabogg commented

2026-09-25 18:08:27 +00:00

Owner

Follow-ups from the pass-2 review of #110 (Closed Days). All P3; the partial P2 (live schedule path duplicating the precedence rule) and the missing reference_business_days on the closed summary were fixed before merge.

Standards

Make calendar required on get_statistics_period_quality_async: every caller passes it, so the load fallback is dead code (the baseline's optional parameter is justified).
Move ScheduleCalendar / is_open_by_schedule out of occupancy_service.py into a small pure module next to CycleBounds (app/facility_time.py or a sibling), and make it a @dataclass(frozen=True) instead of a NamedTuple with behaviour (the calendar or await … fallback only works because a 2-tuple is truthy).
Primitive Obsession: ScheduleCalendar.exceptions is keyed by ISO strings and holds raw DB rows; key by date and keep only what classification needs.
Remaining bare literals in tests/test_statistics_closed_days.py: date(2026, 9, 7), the 4 * … + 2 * … day counts, timedelta(days=2), (1, 2, 3).
Raw calibration-log INSERT in tests (test_statistics_closed_days.py, also test_statistics_baselines.py on master): use record_calibration_log_async.
Nit: quoted "ScheduleCalendar" return annotation in occupancy_service.py.

Spec

usual_weekday comparison carries source_dates but no reference_business_days; the API README now scopes the field to period comparisons. Decide whether it should be len(source_dates) or stay absent.
Picker limit counting fully closed periods is not asserted on its own.

🤖 Generated with Claude Code

Follow-ups from the pass-2 review of #110 (Closed Days). All P3; the partial P2 (live schedule path duplicating the precedence rule) and the missing `reference_business_days` on the closed summary were fixed before merge. ## Standards - [ ] **Make `calendar` required** on `get_statistics_period_quality_async`: every caller passes it, so the load fallback is dead code (the baseline's optional parameter is justified). - [ ] **Move `ScheduleCalendar` / `is_open_by_schedule` out of `occupancy_service.py`** into a small pure module next to `CycleBounds` (`app/facility_time.py` or a sibling), and make it a `@dataclass(frozen=True)` instead of a `NamedTuple` with behaviour (the `calendar or await …` fallback only works because a 2-tuple is truthy). - [ ] **Primitive Obsession:** `ScheduleCalendar.exceptions` is keyed by ISO strings and holds raw DB rows; key by `date` and keep only what classification needs. - [ ] **Remaining bare literals in `tests/test_statistics_closed_days.py`:** `date(2026, 9, 7)`, the `4 * … + 2 * …` day counts, `timedelta(days=2)`, `(1, 2, 3)`. - [ ] **Raw calibration-log INSERT in tests** (`test_statistics_closed_days.py`, also `test_statistics_baselines.py` on master): use `record_calibration_log_async`. - [ ] **Nit:** quoted `"ScheduleCalendar"` return annotation in `occupancy_service.py`. ## Spec - [ ] **`usual_weekday` comparison** carries `source_dates` but no `reference_business_days`; the API README now scopes the field to period comparisons. Decide whether it should be `len(source_dates)` or stay absent. - [ ] **Picker `limit`** counting fully closed periods is not asserted on its own. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

gabogg added the

ready-for-agent

label

2026-09-25 18:08:27 +00:00

gabogg referenced this issue

2026-09-25 18:08:28 +00:00

feat(statistics): Closed Days are not business days (#103) #110

gabogg referenced this issue from a commit

2026-09-25 18:08:36 +00:00

feat(statistics): Closed Days are not business days (#103) (#110)

gabogg referenced this issue

2026-09-25 18:22:56 +00:00