follow-up(statistics): P3 cleanups from #126 review (dwell multiplier, labels, excluded/trusted flags) #128

New issue

Closed

opened 2026-09-26 01:41:04 +00:00 by gabogg · 0 comments

gabogg commented

2026-09-26 01:41:04 +00:00

Owner

Non-blocking findings from the pass-2 review of #126 (#106), moved here under the review policy so the PR could merge. See the review comment on #126 for details.

Design point settled in the 2026-09-26 grilling for the statistics deck RFC (#80): see Decision under the first Spec item. All items are now ready for implementation.

Spec

excluded and trusted can disagree. is_excluded reads the latest AUTOMATIC_NOCTURNAL log (occupancy_repository.py get_counted_cycle_quality_range_async, audits CTE), while is_trusted reads the highest-priority current verdict (verdicts CTE). Supersession only runs between nocturnal logs, so an AUTO_EXCLUDED day with a later trusted RETROACTIVE_GUARD_AUDIT (or a MANUAL_ADMIN reset during the day) reads excluded=true, trusted=true, and the usual-weekday baseline would use it.

Decision (maintainer, 2026-09-26):
1. One ranked verdict decides everything. excluded is derived from the same winning verdict as trusted: a retroactive guard audit, then the nocturnal audit as set by the admin trust toggle (MANUAL_TRUSTED / MANUAL_EXCLUDED), then the automatic nocturnal verdict. A retroactive audit therefore clears an automatic exclusion. This applies to DailyStatistics.excluded / trusted, the deck's data-quality marker, the usual-weekday baseline, and the picker's excluded_days.
2. Offset resets are not verdicts. MANUAL_ADMIN and MANUAL_OVERRIDE logs (live offset recalibrations) say nothing about the day's data quality and must not rank as a cycle verdict. Remove them from _CYCLE_VERDICT_PRIORITY_SQL (or filter them out of every verdict query). That fragment also drives the trusted-history query used for k learning, so check that query still returns what calibration expects (resets carry no computed_exit_multiplier, so the k history should be unaffected; pin this with a test).
3. Update docs/api/README.md (the excluded / trusted / excluded_days wording) and CONTEXT.md if it describes verdict priority.
Tests: AUTO_EXCLUDED + later retroactive audit → excluded=false, trusted=true, counted in the baseline and not in excluded_days; TRUSTED nocturnal + a MANUAL_ADMIN reset → the reset changes nothing; AUTO_EXCLUDED + MANUAL_ADMIN reset → still excluded and untrusted.
Document the null label. docs/api/README.md ("Each usual daypart carries its clock label… the label is the same for all of them") should say that label is null when source days' clock spans differ.
Stale comment at analytics_service.py (~230, usual-weekday candidate selection): "oldest first, as the range query expects; the trust verdict is then read only until enough days pass". Both halves are false now: the range query accepts any order and trust comes from the batch. The sorted(candidates) can go unless order matters elsewhere.

Standards

Typing (code-standards §2.2/§2.3): occupancies(response) in tests/test_statistics_dwell_labels_excluded.py has no annotations; _shared_label(usable: list[tuple[datetime.date, list[Any]]], …) could use the daypart model type instead of Any.
Duplicated Code in tests: the event-seeding loop in the verdict-matrix test copies seed_day's; extract a seed_events(day) helper.
Test gaps found by mutation checks:
- changing > MIN_PLAUSIBLE_EXIT_MULTIPLIER to > 0 still passes, so add a case at 0.5 or 0.4 to pin the boundary;
- reverting label=_shared_label(usable, index) to usable[0][1][index].label still passes, so test the helper through the response, not only as a unit.
Minor: float(log["computed_exit_multiplier"]) is repeated in _cycle_exit_multiplier_async and the hourly series; the log helper could return the parsed multiplier.
Performance, kept on purpose in pass 1: the daypart week view makes one calibration-log query per day (7 extra). Batch it if the week view becomes hot.

🤖 Generated with Claude Code

Non-blocking findings from the pass-2 review of #126 (#106), moved here under the review policy so the PR could merge. See the review comment on #126 for details. **Design point settled** in the 2026-09-26 grilling for the statistics deck RFC (#80): see *Decision* under the first Spec item. All items are now ready for implementation. ## Spec - [ ] **`excluded` and `trusted` can disagree.** `is_excluded` reads the latest `AUTOMATIC_NOCTURNAL` log (`occupancy_repository.py` `get_counted_cycle_quality_range_async`, `audits` CTE), while `is_trusted` reads the highest-priority current verdict (`verdicts` CTE). Supersession only runs between nocturnal logs, so an `AUTO_EXCLUDED` day with a later trusted `RETROACTIVE_GUARD_AUDIT` (or a `MANUAL_ADMIN` reset during the day) reads `excluded=true, trusted=true`, and the usual-weekday baseline would use it. **Decision (maintainer, 2026-09-26):** 1. **One ranked verdict decides everything.** `excluded` is derived from the same winning verdict as `trusted`: a retroactive guard audit, then the nocturnal audit as set by the admin trust toggle (`MANUAL_TRUSTED` / `MANUAL_EXCLUDED`), then the automatic nocturnal verdict. A retroactive audit therefore clears an automatic exclusion. This applies to `DailyStatistics.excluded` / `trusted`, the deck's data-quality marker, the usual-weekday baseline, and the picker's `excluded_days`. 2. **Offset resets are not verdicts.** `MANUAL_ADMIN` and `MANUAL_OVERRIDE` logs (live offset recalibrations) say nothing about the day's data quality and must not rank as a cycle verdict. Remove them from `_CYCLE_VERDICT_PRIORITY_SQL` (or filter them out of every verdict query). That fragment also drives the trusted-history query used for k learning, so check that query still returns what calibration expects (resets carry no `computed_exit_multiplier`, so the k history should be unaffected; pin this with a test). 3. Update `docs/api/README.md` (the `excluded` / `trusted` / `excluded_days` wording) and `CONTEXT.md` if it describes verdict priority. **Tests:** `AUTO_EXCLUDED` + later retroactive audit → `excluded=false, trusted=true`, counted in the baseline and not in `excluded_days`; `TRUSTED` nocturnal + a `MANUAL_ADMIN` reset → the reset changes nothing; `AUTO_EXCLUDED` + `MANUAL_ADMIN` reset → still excluded and untrusted. - [ ] **Document the null label.** `docs/api/README.md` ("Each usual daypart carries its clock `label`… the label is the same for all of them") should say that `label` is `null` when source days' clock spans differ. - [ ] **Stale comment** at `analytics_service.py` (~230, usual-weekday candidate selection): "oldest first, as the range query expects; the trust verdict is then read only until enough days pass". Both halves are false now: the range query accepts any order and trust comes from the batch. The `sorted(candidates)` can go unless order matters elsewhere. ## Standards - [ ] **Typing (code-standards §2.2/§2.3):** `occupancies(response)` in `tests/test_statistics_dwell_labels_excluded.py` has no annotations; `_shared_label(usable: list[tuple[datetime.date, list[Any]]], …)` could use the daypart model type instead of `Any`. - [ ] **Duplicated Code in tests:** the event-seeding loop in the verdict-matrix test copies `seed_day`'s; extract a `seed_events(day)` helper. - [ ] **Test gaps found by mutation checks:** - changing `> MIN_PLAUSIBLE_EXIT_MULTIPLIER` to `> 0` still passes, so add a case at 0.5 or 0.4 to pin the boundary; - reverting `label=_shared_label(usable, index)` to `usable[0][1][index].label` still passes, so test the helper through the response, not only as a unit. - [ ] **Minor:** `float(log["computed_exit_multiplier"])` is repeated in `_cycle_exit_multiplier_async` and the hourly series; the log helper could return the parsed multiplier. - [ ] **Performance, kept on purpose in pass 1:** the daypart week view makes one calibration-log query per day (7 extra). Batch it if the week view becomes hot. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

gabogg added the

needs-triage

label

2026-09-26 01:41:04 +00:00

gabogg referenced this issue from a commit

2026-09-26 01:41:36 +00:00

fix(statistics): daypart dwell multiplier, usual daypart labels, per-day excluded flag (#106) (#126)

gabogg added

ready-for-agent

and removed

needs-triage

labels