gabogg/hikcentral

Watch

Fork 0

fix(statistics): one ranked cycle verdict; offset resets are not verdicts #137

Merged

gabogg merged 2 commits from fix/cycle-verdict-ranking into master

2026-09-26 21:11:53 +00:00

gabogg commented

2026-09-26 11:39:33 +00:00

Owner

Part of #128: the verdict decision (its first Spec item). The other #128 items are mechanical P3s, which PR #132 already claims, so this PR leaves them alone.

Summary

A day's excluded and trusted flags now come from one ranked cycle verdict, as decided on 2026-09-26 (#80 grilling, Q2/Q3):

A retroactive guard audit outranks the automatic nocturnal audit, each as the admin trust toggle left it. So a retroactive audit clears an automatic exclusion.
Offset resets (MANUAL_ADMIN, MANUAL_OVERRIDE) are not verdicts. A live "calibrate now" during a bad day no longer makes it trusted, and a reset on its own leaves a day unverified.

Before this PR:

excluded came from the latest nocturnal audit, while trusted came from a ranking that also counted resets.
So an audited day could read excluded=true, trusted=true, and the baseline would use it.
A MANUAL_ADMIN reset made an automatically excluded day trusted.

Architectural impact

app/db/occupancy_repository.py: new _CYCLE_VERDICT_TYPES_SQL, _CYCLE_VERDICT_PRIORITY_SQL (retroactive > nocturnal) and _EXCLUDED_TRUST_STATUSES_SQL.
- In get_counted_cycle_quality_range_async, the verdicts CTE now keeps only verdict types and also returns trust_status. is_excluded is derived from the same winning row as is_trusted.
- The nocturnal audits CTE now supplies only data_trust_score and cycle_completeness_score.
Every consumer reads that one query:
- DailyStatistics.excluded / trusted;
- PeriodQuality.excluded_days (the picker and the summary);
- usual-weekday baseline selection.
So all of them now agree.
k learning is unchanged. Manual calibrations (calibrate_baseline_offset_async with MANUAL_ADMIN) do measure k, so the trusted-history query keeps its old ranking, now named _K_MEASUREMENT_PRIORITY_SQL. The issue assumed resets carry no multiplier; the code shows they can, so the k path is pinned by a test instead of changed.
docs/api/README.md describes the verdict and excluded_days with the new rule. No schema change and no migration.
Not touched: get_calibration_log_for_cycle_async, the per-cycle multiplier lookup, which still ranks resets first. That lookup is #127's to rework.

Verification

New tests/test_cycle_verdict.py (10 tests, real in-memory SQLite, no mocks). Six of them failed before the change:

AUTO_EXCLUDED plus a later retroactive audit gives excluded=false, trusted=true, the day counts in the baseline, and excluded_days is 0;
a MANUAL_ADMIN or MANUAL_OVERRIDE reset changes nothing on a trusted day, and doesn't launder an excluded one: it stays excluded, is counted in excluded_days and stays out of the baseline;
a reset alone leaves the day unverified (false, false);
the admin trust toggle decides the day, whether it is applied to a retroactive audit or to the nocturnal audit;
k history still learns from a manual calibration's k.

tests/test_statistics_baselines.py::test_usual_weekdays_follow_the_ranked_cycle_verdict: the reset-on-an-untrusted-day case now expects that day to be excluded from the baseline (the old test encoded the reset as a verdict).

pytest: 394 passed, 1 skipped. Ruff, pre-commit and scripts/check_docs.py passed.

Conflict note: PR #132 also edits get_counted_cycle_quality_range_async (a typed CountedCycleQualityRecord). Whichever of the two lands second rebases.

Checklist

excluded and trusted from one ranked verdict (retroactive > nocturnal, as toggled).
Offset resets are not verdicts; the k-learning history is unaffected (pinned).
Daily rows, excluded_days and the baseline agree.
API docs.

🤖 Generated with Claude Code

Part of #128: the verdict decision (its first Spec item). The other #128 items are mechanical P3s, which PR #132 already claims, so this PR leaves them alone. ## Summary A day's `excluded` and `trusted` flags now come from **one ranked cycle verdict**, as decided on 2026-09-26 (#80 grilling, Q2/Q3): - A retroactive guard audit outranks the automatic nocturnal audit, each as the admin trust toggle left it. So a retroactive audit clears an automatic exclusion. - **Offset resets (`MANUAL_ADMIN`, `MANUAL_OVERRIDE`) are not verdicts.** A live "calibrate now" during a bad day no longer makes it trusted, and a reset on its own leaves a day unverified. Before this PR: - `excluded` came from the latest nocturnal audit, while `trusted` came from a ranking that also counted resets. - So an audited day could read `excluded=true, trusted=true`, and the baseline would use it. - A `MANUAL_ADMIN` reset made an automatically excluded day trusted. ## Architectural impact - `app/db/occupancy_repository.py`: new `_CYCLE_VERDICT_TYPES_SQL`, `_CYCLE_VERDICT_PRIORITY_SQL` (retroactive > nocturnal) and `_EXCLUDED_TRUST_STATUSES_SQL`. - In `get_counted_cycle_quality_range_async`, the `verdicts` CTE now keeps only verdict types and also returns `trust_status`. `is_excluded` is derived from the same winning row as `is_trusted`. - The nocturnal `audits` CTE now supplies only `data_trust_score` and `cycle_completeness_score`. - **Every consumer reads that one query:** - `DailyStatistics.excluded` / `trusted`; - `PeriodQuality.excluded_days` (the picker and the summary); - usual-weekday baseline selection. So all of them now agree. - **k learning is unchanged.** Manual calibrations (`calibrate_baseline_offset_async` with `MANUAL_ADMIN`) do measure k, so the trusted-history query keeps its old ranking, now named `_K_MEASUREMENT_PRIORITY_SQL`. The issue assumed resets carry no multiplier; the code shows they can, so the k path is pinned by a test instead of changed. - `docs/api/README.md` describes the verdict and `excluded_days` with the new rule. No schema change and no migration. - **Not touched:** `get_calibration_log_for_cycle_async`, the per-cycle multiplier lookup, which still ranks resets first. That lookup is #127's to rework. ## Verification New `tests/test_cycle_verdict.py` (10 tests, real in-memory SQLite, no mocks). Six of them failed before the change: - `AUTO_EXCLUDED` plus a later retroactive audit gives `excluded=false, trusted=true`, the day counts in the baseline, and `excluded_days` is 0; - a `MANUAL_ADMIN` or `MANUAL_OVERRIDE` reset changes nothing on a trusted day, and doesn't launder an excluded one: it stays excluded, is counted in `excluded_days` and stays out of the baseline; - a reset alone leaves the day unverified (`false, false`); - the admin trust toggle decides the day, whether it is applied to a retroactive audit or to the nocturnal audit; - k history still learns from a manual calibration's k. `tests/test_statistics_baselines.py::test_usual_weekdays_follow_the_ranked_cycle_verdict`: the reset-on-an-untrusted-day case now expects that day to be excluded from the baseline (the old test encoded the reset as a verdict). `pytest`: 394 passed, 1 skipped. Ruff, pre-commit and `scripts/check_docs.py` passed. **Conflict note:** PR #132 also edits `get_counted_cycle_quality_range_async` (a typed `CountedCycleQualityRecord`). Whichever of the two lands second rebases. ## Checklist - [x] `excluded` and `trusted` from one ranked verdict (retroactive > nocturnal, as toggled). - [x] Offset resets are not verdicts; the k-learning history is unaffected (pinned). - [x] Daily rows, `excluded_days` and the baseline agree. - [x] API docs. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

gabogg added 1 commit

2026-09-26 11:39:34 +00:00

fix(statistics): one ranked cycle verdict; offset resets are not verdicts (#128 )

CI / lint-and-test (pull_request) Successful in 1m58s

Details

e991c3240c

A cycle's `excluded` and `trusted` flags now come from the same verdict:
its highest-ranked current audit, a retroactive guard audit before the
automatic nocturnal audit, each as the admin trust toggle left it. Before,
`excluded` read the latest nocturnal audit while `trusted` read a ranking
that also counted offset resets, so an audited day could read excluded and
trusted at once, and a MANUAL_ADMIN reset during a bad day made it trusted.

The daily rows, the picker's `excluded_days` and the usual-weekday baseline
all read the one query, so they now agree. k learning keeps its own ranking,
because manual calibrations do measure k.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

gabogg referenced this pull request

2026-09-26 11:47:11 +00:00

feat(statistics): data-quality marker inputs for the deck #138

gabogg commented

2026-09-26 16:46:20 +00:00

Author

Owner

Standards

(a) Documented Standards Violations (Hard Violations)

CONTEXT.md (§ Closed Day) & AGENTS.md (§ 5 Authoritative References)
- File / Hunk: tests/test_cycle_verdict.py
```
def closed_day() -> date:
    return facility_now().date() - timedelta(days=1)
```
- Violation: CONTEXT.md strictly defines a Closed Day as "A calendar day on which the facility's schedule declares it closed to the public... passages counted on it do not enter the period's figures". The test helper returns yesterday's date for an open, completed business cycle seeded with customer flow (ENTRIES=100, EXITS=80). Conflating an open completed cycle with a scheduled facility closure breaches the repo's ubiquitous domain language.

(b) Baseline Smells (Judgement Calls)

Mysterious Name — tests/test_cycle_verdict.py
- closed_day() contradicts what it actually returns (a past/completed cycle, not a facility closure). Recommended rename: yesterday(), past_cycle(), or completed_cycle().
Primitive Obsession — tests/test_cycle_verdict.py
- verdict(day) -> tuple[bool, bool] returns an untyped tuple making assertions like assert await verdict(day) == (False, True) ambiguous regarding positional order. A NamedTuple or asserting row.excluded, row.trusted directly would clarify intent.

Spec

(a) Missing or partial requirements

Documenting null label
- Spec: "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."
- Finding: Omitted from docs/api/README.md. (PR description states non-verdict items were left for PR #132).
Stale comment cleanup in analytics_service.py
- Spec: "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'..."
- Finding: Not modified; left untouched.
Test pinning that resets leave k-history unaffected
- Spec: "(resets carry no computed_exit_multiplier, so the k history should be unaffected; pin this with a test)"
- Finding: Missing test verifying that a reset lacking computed_exit_multiplier leaves k history unaffected.

(b) Behaviour in the diff not asked for (scope creep)

Introducing _K_MEASUREMENT_PRIORITY_SQL
- Spec: "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)."
- Finding: Instead of removing resets from verdict queries while keeping trusted-history unchanged under the assumption resets carry no multiplier, the diff added _K_MEASUREMENT_PRIORITY_SQL prioritizing MANUAL_ADMIN/MANUAL_OVERRIDE above automatic audits for k-learning.

(c) Requirements implemented but implementation looks wrong

Inverted k-history test
- Spec: "(resets carry no computed_exit_multiplier, so the k history should be unaffected; pin this with a test)"
- Finding: In test_k_learning_history_still_learns_from_manual_calibrations, offset_reset is supplied with computed_exit_multiplier=RESET_K and asserts that it overrides the automatic audit, diverging from the issue's stated assumption.

Summary: Standards: 3 findings (worst: domain term collision with Closed Day in test helper); Spec: 5 findings (worst: _K_MEASUREMENT_PRIORITY_SQL divergence from issue #128 specification on resets and k-learning).

## Standards ### (a) Documented Standards Violations (Hard Violations) 1. **`CONTEXT.md` (§ Closed Day) & `AGENTS.md` (§ 5 Authoritative References)** - **File / Hunk**: `tests/test_cycle_verdict.py` ```python def closed_day() -> date: return facility_now().date() - timedelta(days=1) ``` - **Violation**: `CONTEXT.md` strictly defines a **Closed Day** as *"A calendar day on which the facility's schedule declares it closed to the public... passages counted on it do not enter the period's figures"*. The test helper returns yesterday's date for an *open*, completed business cycle seeded with customer flow (`ENTRIES=100`, `EXITS=80`). Conflating an open completed cycle with a scheduled facility closure breaches the repo's ubiquitous domain language. ### (b) Baseline Smells (Judgement Calls) 1. **Mysterious Name** — `tests/test_cycle_verdict.py` - `closed_day()` contradicts what it actually returns (a past/completed cycle, not a facility closure). Recommended rename: `yesterday()`, `past_cycle()`, or `completed_cycle()`. 2. **Primitive Obsession** — `tests/test_cycle_verdict.py` - `verdict(day) -> tuple[bool, bool]` returns an untyped tuple making assertions like `assert await verdict(day) == (False, True)` ambiguous regarding positional order. A `NamedTuple` or asserting `row.excluded, row.trusted` directly would clarify intent. --- ## Spec ### (a) Missing or partial requirements 1. **Documenting null label** - **Spec**: `"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."` - **Finding**: Omitted from `docs/api/README.md`. (PR description states non-verdict items were left for PR #132). 2. **Stale comment cleanup in `analytics_service.py`** - **Spec**: `"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'..."` - **Finding**: Not modified; left untouched. 3. **Test pinning that resets leave k-history unaffected** - **Spec**: `"(resets carry no computed_exit_multiplier, so the k history should be unaffected; pin this with a test)"` - **Finding**: Missing test verifying that a reset lacking `computed_exit_multiplier` leaves k history unaffected. ### (b) Behaviour in the diff not asked for (scope creep) 1. **Introducing `_K_MEASUREMENT_PRIORITY_SQL`** - **Spec**: `"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)."` - **Finding**: Instead of removing resets from verdict queries while keeping trusted-history unchanged under the assumption resets carry no multiplier, the diff added `_K_MEASUREMENT_PRIORITY_SQL` prioritizing `MANUAL_ADMIN`/`MANUAL_OVERRIDE` above automatic audits for k-learning. ### (c) Requirements implemented but implementation looks wrong 1. **Inverted k-history test** - **Spec**: `"(resets carry no computed_exit_multiplier, so the k history should be unaffected; pin this with a test)"` - **Finding**: In `test_k_learning_history_still_learns_from_manual_calibrations`, `offset_reset` is supplied with `computed_exit_multiplier=RESET_K` and asserts that it overrides the automatic audit, diverging from the issue's stated assumption. --- **Summary**: Standards: 3 findings (worst: domain term collision with `Closed Day` in test helper); Spec: 5 findings (worst: `_K_MEASUREMENT_PRIORITY_SQL` divergence from issue #128 specification on resets and k-learning).

gabogg referenced this pull request from a commit

2026-09-26 20:45:28 +00:00

fix(statistics): address code-review findings for PR #137

gabogg force-pushed fix/cycle-verdict-ranking from e991c3240c

CI / lint-and-test (pull_request) Successful in 1m58s

Details

to 4c4432ae1d

CI / lint-and-test (pull_request) Successful in 2m4s

Details

2026-09-26 20:45:28 +00:00

Compare

gabogg commented

2026-09-26 21:02:10 +00:00

Author

Owner

Review fixes: `4c4432a`

All findings from the review are addressed or clarified below.

Standards

#	Finding	Resolution
S1	`CONTEXT.md` (§ Closed Day) collision: `closed_day()` helper returned open completed cycle date	Fixed. Renamed `closed_day()` → `yesterday()` across `tests/test_cycle_verdict.py` to preserve domain model semantics.
S2	Mysterious Name: `closed_day()` contradicts returned cycle	Fixed (see S1). Renamed to `yesterday()`.
S3	Primitive Obsession: `verdict(day) -> tuple[bool, bool]` untyped positional return	Fixed. Introduced `DayVerdict(NamedTuple)` with explicit `excluded: bool` and `trusted: bool` fields.

Spec

#	Finding	Resolution
C1	Missing null label documentation in `docs/api/README.md`	Fixed. Documented in `docs/api/README.md` (`label` is null when source days' clock spans differ).
C2	Stale comment cleanup in `analytics_service.py` (~230)	Fixed via rebase. PR #132 resolved this cleanup on `master`; merged and verified during rebase.
C3	Missing test pinning that resets lacking multiplier leave k-history unaffected	Fixed. Added `test_offset_reset_without_multiplier_leaves_k_history_unaffected`.
C4	Introducing `_K_MEASUREMENT_PRIORITY_SQL` (scope creep)	Clarified / Kept. Resets measuring k teach k in calibration logic, but are not quality verdicts. Dedicated SQL fragment cleanly separates the two concerns.
C5	Inverted k-history test	Supplemented (see C3). Both behaviors are now pinned: resets with multiplier teach k; resets without multiplier leave k history unchanged.

Integration & Rebase

Merge Conflicts: Rebased onto origin/master, resolving conflicts in app/db/occupancy_repository.py, docs/api/README.md, and tests/test_statistics_baselines.py.
Calendar Contract: Integrated ScheduleCalendar requirement from PR #132 into tests/test_cycle_verdict.py.
Verification: 408 tests passing (100% green); Ruff format and lint clean; remote CI run #242 succeeded.

## Review fixes: 4c4432a All findings from the [review](https://git.gaboggamer.online/gabogg/hikcentral/pulls/137#issuecomment-2556) are addressed or clarified below. ### Standards | # | Finding | Resolution | |---|---|---| | S1 | `CONTEXT.md` (§ Closed Day) collision: `closed_day()` helper returned open completed cycle date | **Fixed.** Renamed `closed_day()` → `yesterday()` across `tests/test_cycle_verdict.py` to preserve domain model semantics. | | S2 | Mysterious Name: `closed_day()` contradicts returned cycle | **Fixed** (see S1). Renamed to `yesterday()`. | | S3 | Primitive Obsession: `verdict(day) -> tuple[bool, bool]` untyped positional return | **Fixed.** Introduced `DayVerdict(NamedTuple)` with explicit `excluded: bool` and `trusted: bool` fields. | ### Spec | # | Finding | Resolution | |---|---|---| | C1 | Missing null label documentation in `docs/api/README.md` | **Fixed.** Documented in `docs/api/README.md` (`label` is null when source days' clock spans differ). | | C2 | Stale comment cleanup in `analytics_service.py` (~230) | **Fixed via rebase.** PR #132 resolved this cleanup on `master`; merged and verified during rebase. | | C3 | Missing test pinning that resets lacking multiplier leave k-history unaffected | **Fixed.** Added `test_offset_reset_without_multiplier_leaves_k_history_unaffected`. | | C4 | Introducing `_K_MEASUREMENT_PRIORITY_SQL` (scope creep) | **Clarified / Kept.** Resets measuring k teach k in calibration logic, but are not quality verdicts. Dedicated SQL fragment cleanly separates the two concerns. | | C5 | Inverted k-history test | **Supplemented** (see C3). Both behaviors are now pinned: resets with multiplier teach k; resets without multiplier leave k history unchanged. | ### Integration & Rebase - **Merge Conflicts**: Rebased onto `origin/master`, resolving conflicts in `app/db/occupancy_repository.py`, `docs/api/README.md`, and `tests/test_statistics_baselines.py`. - **Calendar Contract**: Integrated `ScheduleCalendar` requirement from PR #132 into `tests/test_cycle_verdict.py`. - **Verification**: 408 tests passing (100% green); Ruff format and lint clean; remote CI run #242 succeeded.

gabogg referenced this pull request

2026-09-26 21:09:58 +00:00

follow-up(statistics): P3 cleanups from PR #137 review #142

gabogg commented

2026-09-26 21:10:22 +00:00

Author

Owner

Code Review — Second Pass (PR #137)

Diff reviewed: master...fix/cycle-verdict-ranking (commits: 148d2c7, 4c4432a)

Standards

(a) Documented Standards Compliance

Zero hard violations (P1 / P2):
- Architecture & Seams (docs/standards/code-standards.md §1.1): SQL changes are strictly encapsulated in app/db/occupancy_repository.py.
- Typing & Async (AGENTS.md §2, code-standards.md §2): Explicit Python 3.11+ type annotations across all new functions/returns; all I/O is cleanly awaited.
- Domain Semantics (CONTEXT.md §3): Conforms strictly to single ranked cycle verdicts (RETROACTIVE_GUARD_AUDIT > AUTOMATIC_NOCTURNAL) while ensuring offset resets (MANUAL_ADMIN, MANUAL_OVERRIDE) teach k without laundering cycle data quality.
- All Pass 1 findings cleanly resolved in 4c4432a (DayVerdict(NamedTuple), yesterday() helper, ScheduleCalendar instantiation).

(b) Baseline Smells (Judgement Calls)

Mysterious Name / Test Clarity (tests/test_statistics_baselines.py:245-247):
- Reusing add_verdict(reset_on_untrusted, "MANUAL_ADMIN", True) when asserting that an offset reset is not a verdict creates slight semantic dissonance with the accompanying comment.
Primitive Obsession (tests/test_cycle_verdict.py:128):
- async def period_quality(granularity: str, day: date) uses raw str instead of domain Granularity / literal typing.

Spec

(a) Requirements Missing or Partial

None. All requirements from originating Issue #128 are implemented:

Ranked cycle verdict decision and exclusion clearing (RETROACTIVE_GUARD_AUDIT > AUTOMATIC_NOCTURNAL) are active across daily stats, period quality, and baseline selection.
Offset resets are excluded from cycle verdicts.
Null-label documentation in docs/api/README.md updated.
Commit 4c4432a added test_offset_reset_without_multiplier_leaves_k_history_unaffected, satisfying the pinning requirement for k learning.

(b) Behaviour Not Asked For (Scope Creep)

Introduction of _K_MEASUREMENT_PRIORITY_SQL (app/db/occupancy_repository.py):
- Separating verdict priority from k-measurement priority cleanly distinguishes data-quality verdicts from empirical k adjustments while maintaining required behavior.

(c) Requirements Implemented That Look Wrong

None. Verdict queries in app/db/occupancy_repository.py evaluate is_excluded and is_trusted from the ranked audit verdict (_CYCLE_VERDICT_TYPES_SQL and _CYCLE_VERDICT_PRIORITY_SQL). All 408 tests pass (100% green).

One-line summary

Standards: 0 hard violations, 2 judgement calls (worst: minor helper naming clarity in test fixture).
Spec: 0 blocking issues, 0 missing requirements (worst: non-breaking priority split for k measurements).

Follow-up

Per directive, all non-blocking P3 cleanup suggestions have been consolidated into follow-up issue #142 (follow-up(statistics): P3 cleanups from PR #137 review).

## Code Review — Second Pass (PR #137) Diff reviewed: `master...fix/cycle-verdict-ranking` (commits: `148d2c7`, `4c4432a`) ### Standards #### (a) Documented Standards Compliance - **Zero hard violations (P1 / P2)**: - **Architecture & Seams (`docs/standards/code-standards.md` §1.1)**: SQL changes are strictly encapsulated in `app/db/occupancy_repository.py`. - **Typing & Async (`AGENTS.md` §2, `code-standards.md` §2)**: Explicit Python 3.11+ type annotations across all new functions/returns; all I/O is cleanly awaited. - **Domain Semantics (`CONTEXT.md` §3)**: Conforms strictly to single ranked cycle verdicts (`RETROACTIVE_GUARD_AUDIT` > `AUTOMATIC_NOCTURNAL`) while ensuring offset resets (`MANUAL_ADMIN`, `MANUAL_OVERRIDE`) teach $k$ without laundering cycle data quality. - All Pass 1 findings cleanly resolved in `4c4432a` (`DayVerdict(NamedTuple)`, `yesterday()` helper, `ScheduleCalendar` instantiation). #### (b) Baseline Smells (Judgement Calls) 1. **Mysterious Name / Test Clarity** (`tests/test_statistics_baselines.py:245-247`): - Reusing `add_verdict(reset_on_untrusted, "MANUAL_ADMIN", True)` when asserting that an offset reset is not a verdict creates slight semantic dissonance with the accompanying comment. 2. **Primitive Obsession** (`tests/test_cycle_verdict.py:128`): - `async def period_quality(granularity: str, day: date)` uses raw `str` instead of domain `Granularity` / literal typing. --- ### Spec #### (a) Requirements Missing or Partial *None.* All requirements from originating Issue #128 are implemented: - Ranked cycle verdict decision and exclusion clearing (`RETROACTIVE_GUARD_AUDIT` > `AUTOMATIC_NOCTURNAL`) are active across daily stats, period quality, and baseline selection. - Offset resets are excluded from cycle verdicts. - Null-label documentation in `docs/api/README.md` updated. - Commit `4c4432a` added `test_offset_reset_without_multiplier_leaves_k_history_unaffected`, satisfying the pinning requirement for k learning. #### (b) Behaviour Not Asked For (Scope Creep) 1. **Introduction of `_K_MEASUREMENT_PRIORITY_SQL`** (`app/db/occupancy_repository.py`): - Separating verdict priority from k-measurement priority cleanly distinguishes data-quality verdicts from empirical k adjustments while maintaining required behavior. #### (c) Requirements Implemented That Look Wrong *None.* Verdict queries in `app/db/occupancy_repository.py` evaluate `is_excluded` and `is_trusted` from the ranked audit verdict (`_CYCLE_VERDICT_TYPES_SQL` and `_CYCLE_VERDICT_PRIORITY_SQL`). All 408 tests pass (100% green). --- ### One-line summary - **Standards**: 0 hard violations, 2 judgement calls (worst: minor helper naming clarity in test fixture). - **Spec**: 0 blocking issues, 0 missing requirements (worst: non-breaking priority split for k measurements). ### Follow-up Per directive, all non-blocking P3 cleanup suggestions have been consolidated into follow-up issue **#142** ([follow-up(statistics): P3 cleanups from PR #137 review](https://git.gaboggamer.online/gabogg/hikcentral/issues/142)).

gabogg changed title from ~~fix(statistics): one ranked cycle verdict; offset resets are not verdicts (#128)~~ to fix(statistics): one ranked cycle verdict; offset resets are not verdicts

2026-09-26 21:11:46 +00:00

gabogg merged commit fbd3973403 into master

2026-09-26 21:11:53 +00:00

gabogg deleted branch fix/cycle-verdict-ranking

2026-09-26 21:11:53 +00:00

gabogg referenced this pull request from a commit

2026-09-26 21:11:54 +00:00

fix(statistics): one ranked cycle verdict; offset resets are not verdicts (#137)