follow-up(statistics-deck): P3 cleanups from PR #144 review #145

New issue

Open

opened 2026-09-26 22:30:17 +00:00 by gabogg · 0 comments

gabogg commented

2026-09-26 22:30:17 +00:00

Owner

Follow-ups from the second-pass review of #144 (review comment). All are P3. Each names its fix, so the design is settled.

Standards

S2-2 statistics_deck.js loadOlder: a failed LOAD OLDER, or a failed − past the oldest period, fails silently (.catch(() => null)). Set a per-tab olderError flag and show statisticsDeck.picker.error next to the button, as S-5 does for the first page.
S2-3 Format the gap labels in the service, like the sibling HourlyVisitors.label. Make GapInterval.start_label/end_label plain str fields filled in _gaps_within (analytics_service.py), and drop the computed_field and the facility_time import from app/schemas/statistics.py.
S2-4 charts.js: add a comment that currentDeckColors() and cachedHatch are read once per page, on the assumption that there is no runtime theme switch.
S2-5 statistics_deck.js: state the invariant that periods === null means the first page isn't loaded yet, next to the shared tab.loading slot. Alternatively, fold loadPeriods/loadOlder into one fetchPage(before).

Spec

C2-1 docs/api/README.md: document GapInterval.start_label/end_label (facility-time HH:MM), and the before query param on GET /api/statistics/periods (it returns periods older than the given period start; a misaligned value gets 422).

Verification

Frontend tests for S2-2. pytest, ruff and scripts/check_docs.py stay green.

Follow-ups from the second-pass review of #144 ([review comment](https://git.gaboggamer.online/gabogg/hikcentral/pulls/144#issuecomment-2637)). All are P3. Each names its fix, so the design is settled. ## Standards - [ ] **S2-2** `statistics_deck.js` `loadOlder`: a failed **LOAD OLDER**, or a failed `−` past the oldest period, fails silently (`.catch(() => null)`). Set a per-tab `olderError` flag and show `statisticsDeck.picker.error` next to the button, as S-5 does for the first page. - [ ] **S2-3** Format the gap labels in the service, like the sibling `HourlyVisitors.label`. Make `GapInterval.start_label`/`end_label` plain `str` fields filled in `_gaps_within` (`analytics_service.py`), and drop the `computed_field` and the `facility_time` import from `app/schemas/statistics.py`. - [ ] **S2-4** `charts.js`: add a comment that `currentDeckColors()` and `cachedHatch` are read once per page, on the assumption that there is no runtime theme switch. - [ ] **S2-5** `statistics_deck.js`: state the invariant that `periods === null` means the first page isn't loaded yet, next to the shared `tab.loading` slot. Alternatively, fold `loadPeriods`/`loadOlder` into one `fetchPage(before)`. ## Spec - [ ] **C2-1** `docs/api/README.md`: document `GapInterval.start_label`/`end_label` (facility-time `HH:MM`), and the `before` query param on `GET /api/statistics/periods` (it returns periods older than the given period start; a misaligned value gets 422). ## Verification Frontend tests for S2-2. `pytest`, `ruff` and `scripts/check_docs.py` stay green.

gabogg added the

ready-for-agent

label

2026-09-26 22:30:17 +00:00