gabogg/hikcentral
Watch
1
0
Fork
You've already forked hikcentral
 0
Code Issues 113 Pull requests 8 Projects Releases Packages Wiki Activity Actions

feat(statistics-deck): Day view (Room, Desk, Laptop) (#134) #149

Merged
gabogg merged 4 commits from feat/deck-day-view into master 2026-09-27 14:37:01 +00:00
Conversation 5 Commits 4 Files changed 8 +1730 -3
gabogg commented 2026-09-26 23:12:47 +00:00
Owner
Copy link

Closes #134

Problem

The statistics deck shell (#133, PR #144) renders labelled placeholders for the Day tab. The Day period needs its real content (RFC #80 §4): six KPIs and four graphs about one closed business day, laid out separately for the Room, Desk and Laptop profiles, with the data-quality marker (RFC §4.1) and the comparison rules ("—" with a reason, never 0%).

Approach

  • New view module app/static/js/src/ui/statistics_deck/views/day_view.js. It calls registerView('day', renderDayView) when imported, and is imported once from index.html next to the StatisticsDeck import.
  • Route JSON is mapped to KPI and graph inputs by exported pure functions (dayKpis, hourlyGraph, peopleInsideGraph, entranceShare, daypartSplit, dayTier, comparisonText, formatters). renderDayView only builds the DOM and the Chart.js graphs from those inputs.
  • Routes: periods/day/{date}/summary, timeseries/hourly?baseline=usual_weekday, periods/day/{date}/entrances, dwell/dayparts?baseline=usual_weekday, plus the shell's loadDays() for the day's daily row (tier, holiday). A profile only fetches what it shows (Laptop skips entrances and dayparts). Each route that fails marks only its own cells.
  • Layouts (DAY_LAYOUTS), each keeping the top of the ranking:
    • Room: 6 KPIs; lead row visitors by hour (3fr) + people inside (2fr); supporting row entrance share + daypart split.
    • Desk: 4 KPIs (visitors, vs usual, peak, average visit); lead row visitors by hour; supporting row people inside + entrance share.
    • Laptop: 3 KPIs (visitors, vs usual, peak); one graph, visitors by hour.
  • Marker: gap-estimated buckets are hatched with the shell's deckQualityMarks plugin on "visitors by hour" and "people inside". An Unreliable day (excluded or missing) greys out every graph: muted series, and every bucket hatched. The badge is the shell's. No per-KPI marks, no margins (margin_upper/margin_lower ignored).
  • Clock labels come from the server only: hour buckets, daypart spans, busiest hour. The peak's hour is the server bucket label holding peak.timestamp_epoch; nothing is formatted from the browser clock. Numbers use the deck language's locale, and so do the Chart.js axes.
  • i18n: statisticsDeck.views.day.* and statisticsDeck.comparison.reason.<ComparisonReason> (all 10 codes), es and en, at the end of each statisticsDeck block.
  • CSS: app/static/css/statistics-deck-day.css, view-specific rules only.

Verification

  • node --test tests/frontend/*.test.js: 159 pass. The new tests/frontend/test_statistics_deck_day.test.js has 33 tests. They cover every Acceptance item: the profile layouts and rankings, route URLs, KPI mapping, "—" with a reason for no last-week data, no usual-weekday baseline (KPI and hourly graph), no last-year data (entrance YoY note and per-camera reason) and zero/closed references, gap buckets hatched exactly on both hourly graphs (also end to end through the rendered chart configs), an excluded day greying every graph cell, closed days, failed routes, cleanup, and es/en key parity.
  • TZ=America/Caracas pytest -q: 417 passed. ruff check . and ruff format --check . clean. python3 scripts/check_docs.py clean.
  • Real browser (Chromium via Playwright, dev DB copy, operator login): Room, Desk and Laptop at 1920×1080, 1920×960 and 760×600. Every run had no page, root, row or cell overflow, nothing leaving a cell, and a 38 px top bar.
    • Live data: 2026-09-14 (en) and 2026-09-09 (es). Both are excluded days with no usual weekday and no last-week or last-year data, so the live runs show the greyed-out state and the "—" reasons.
    • Patched responses (Playwright route interception on the live JSON): an OK Estimate day with two gap buckets, a baseline, last-year data, a null usual daypart span, a holiday and an ESTIMATED top entrance. These covered the shapes the dev DB lacks, and I inspected the screenshots in en and es.

Decisions

  1. Last year on the Visitors KPI is shown only when the summary carries last_year (RFC: "last year only when it has data"). The summary route omits it when there is no data. When it is present without a change (e.g. CLOSED_PERIOD), it shows "—" with the reason. The "no last-year data" acceptance case shows on the entrance-share graph: one note VS LAST YEAR — no data last year when the route has no year data at all, and a per-camera "—" with NO_CAMERA_DATA_LAST_YEAR.
  2. Gap hatching covers the whole hour column on "visitors by hour", not just the bar. A gap bucket often has a short or empty bar, so the hatch would be invisible. This uses the shell plugin unchanged: the marks anchor on an invisible zero point series, whose elements the plugin treats as columns.
  3. Gap buckets are passed to the plugin as the Unreliable tier, since that is the tier the plugin hatches (charts.js documents hatching for "a bucket that overlaps an ingestion gap").
  4. Daypart labels come only from UsualDaypart.label, per #134. A null label, or no baseline at all, shows no span.
  5. Room KPI row uses minmax(96px, 12fr) for the Day tab, so a label, the figure and two comparison lines fit on short screens (RFC §3.5 lets a view adjust its split). At 1080 and 960 it is the shell's 12 % as before.
  6. Entrance share is an HTML bar list, not a canvas, so each row can carry its YoY text. It shows the top 5 entrances plus one "others (n)" row.
  7. Addition beyond #134: HOLIDAY tag. A holiday is tagged on the Visitors tile. #134 asks for holidays on per-day graphs, and the Day view has none. The maintainer chose to keep the tag.
  8. Top entrance is a record (review C-5): the day summary route now withholds top_entrance on an unreliable day, as it does busiest_hour (app/services/analytics_service.py, with a test in tests/test_statistics_marker_inputs.py and a note in docs/api/README.md). The tile then shows "—" with the reason.

Open questions

  • The summary route never returns NO_DATA_LAST_YEAR; it drops last_year instead. If the maintainer wants the Visitors tile to always carry a last-year line with "—", the view only needs summary.last_year to be kept with its reason.

Known duplication left for consolidation

These are local to day_view.js on purpose, per the coordination rules with #135 and #136:

  • formatCount, formatPercent, formatChange, formatDwell, numberLocale;
  • comparisonText, reasonText, baselineReasonText ("—" with a reason);
  • the KPI tile builder kpiCell;
  • the entrance-share mapping and HTML list (entranceShare, entranceList);

Outside the view files, this PR changes one <link> and one import in index.html, adds the i18n blocks, and makes the server-side top-entrance records fix (decision 8). No shell module was edited. This branch is rebased on #150 (fix/deck-shell-view-support), whose shell APIs it uses (ctx.createChart, deckLocale, shell colours, the rejecting loadDays). Until #150 merges, this PR's diff against master includes #150's two commits.

🤖 Generated with Claude Code

Closes #134 ## Problem The statistics deck shell (#133, PR #144) renders labelled placeholders for the Day tab. The Day period needs its real content (RFC #80 §4): six KPIs and four graphs about one closed business day, laid out separately for the Room, Desk and Laptop profiles, with the data-quality marker (RFC §4.1) and the comparison rules ("—" with a reason, never 0%). ## Approach - New view module `app/static/js/src/ui/statistics_deck/views/day_view.js`. It calls `registerView('day', renderDayView)` when imported, and is imported once from `index.html` next to the StatisticsDeck import. - Route JSON is mapped to KPI and graph inputs by exported pure functions (`dayKpis`, `hourlyGraph`, `peopleInsideGraph`, `entranceShare`, `daypartSplit`, `dayTier`, `comparisonText`, formatters). `renderDayView` only builds the DOM and the Chart.js graphs from those inputs. - Routes: `periods/day/{date}/summary`, `timeseries/hourly?baseline=usual_weekday`, `periods/day/{date}/entrances`, `dwell/dayparts?baseline=usual_weekday`, plus the shell's `loadDays()` for the day's daily row (tier, holiday). A profile only fetches what it shows (Laptop skips entrances and dayparts). Each route that fails marks only its own cells. - Layouts (`DAY_LAYOUTS`), each keeping the top of the ranking: - **Room**: 6 KPIs; lead row visitors by hour (3fr) + people inside (2fr); supporting row entrance share + daypart split. - **Desk**: 4 KPIs (visitors, vs usual, peak, average visit); lead row visitors by hour; supporting row people inside + entrance share. - **Laptop**: 3 KPIs (visitors, vs usual, peak); one graph, visitors by hour. - Marker: gap-estimated buckets are hatched with the shell's `deckQualityMarks` plugin on "visitors by hour" and "people inside". An Unreliable day (excluded or missing) greys out every graph: muted series, and every bucket hatched. The badge is the shell's. No per-KPI marks, no margins (`margin_upper`/`margin_lower` ignored). - Clock labels come from the server only: hour buckets, daypart spans, busiest hour. The peak's hour is the server bucket label holding `peak.timestamp_epoch`; nothing is formatted from the browser clock. Numbers use the deck language's locale, and so do the Chart.js axes. - i18n: `statisticsDeck.views.day.*` and `statisticsDeck.comparison.reason.<ComparisonReason>` (all 10 codes), es and en, at the end of each `statisticsDeck` block. - CSS: `app/static/css/statistics-deck-day.css`, view-specific rules only. ## Verification - `node --test tests/frontend/*.test.js`: 159 pass. The new `tests/frontend/test_statistics_deck_day.test.js` has 33 tests. They cover every Acceptance item: the profile layouts and rankings, route URLs, KPI mapping, "—" with a reason for no last-week data, no usual-weekday baseline (KPI and hourly graph), no last-year data (entrance YoY note and per-camera reason) and zero/closed references, gap buckets hatched exactly on both hourly graphs (also end to end through the rendered chart configs), an excluded day greying every graph cell, closed days, failed routes, cleanup, and es/en key parity. - `TZ=America/Caracas pytest -q`: 417 passed. `ruff check .` and `ruff format --check .` clean. `python3 scripts/check_docs.py` clean. - Real browser (Chromium via Playwright, dev DB copy, operator login): Room, Desk and Laptop at 1920×1080, 1920×960 and 760×600. Every run had no page, root, row or cell overflow, nothing leaving a cell, and a 38 px top bar. - Live data: 2026-09-14 (en) and 2026-09-09 (es). Both are excluded days with no usual weekday and no last-week or last-year data, so the live runs show the greyed-out state and the "—" reasons. - Patched responses (Playwright route interception on the live JSON): an OK Estimate day with two gap buckets, a baseline, last-year data, a null usual daypart span, a holiday and an ESTIMATED top entrance. These covered the shapes the dev DB lacks, and I inspected the screenshots in en and es. ## Decisions 1. **Last year on the Visitors KPI** is shown only when the summary carries `last_year` (RFC: "last year only when it has data"). The summary route omits it when there is no data. When it is present without a change (e.g. `CLOSED_PERIOD`), it shows "—" with the reason. The "no last-year data" acceptance case shows on the entrance-share graph: one note `VS LAST YEAR — no data last year` when the route has no year data at all, and a per-camera "—" with `NO_CAMERA_DATA_LAST_YEAR`. 2. **Gap hatching covers the whole hour column** on "visitors by hour", not just the bar. A gap bucket often has a short or empty bar, so the hatch would be invisible. This uses the shell plugin unchanged: the marks anchor on an invisible zero point series, whose elements the plugin treats as columns. 3. **Gap buckets are passed to the plugin as the Unreliable tier**, since that is the tier the plugin hatches (charts.js documents hatching for "a bucket that overlaps an ingestion gap"). 4. **Daypart labels** come only from `UsualDaypart.label`, per #134. A null label, or no baseline at all, shows no span. 5. **Room KPI row** uses `minmax(96px, 12fr)` for the Day tab, so a label, the figure and two comparison lines fit on short screens (RFC §3.5 lets a view adjust its split). At 1080 and 960 it is the shell's 12 % as before. 6. **Entrance share** is an HTML bar list, not a canvas, so each row can carry its YoY text. It shows the top 5 entrances plus one "others (n)" row. 7. **Addition beyond #134: HOLIDAY tag.** A holiday is tagged on the Visitors tile. #134 asks for holidays on per-day graphs, and the Day view has none. The maintainer chose to keep the tag. 8. **Top entrance is a record** (review C-5): the day summary route now withholds `top_entrance` on an unreliable day, as it does `busiest_hour` (`app/services/analytics_service.py`, with a test in `tests/test_statistics_marker_inputs.py` and a note in `docs/api/README.md`). The tile then shows "—" with the reason. ## Open questions - The summary route never returns `NO_DATA_LAST_YEAR`; it drops `last_year` instead. If the maintainer wants the Visitors tile to always carry a last-year line with "—", the view only needs `summary.last_year` to be kept with its reason. ## Known duplication left for consolidation These are local to `day_view.js` on purpose, per the coordination rules with #135 and #136: - `formatCount`, `formatPercent`, `formatChange`, `formatDwell`, `numberLocale`; - `comparisonText`, `reasonText`, `baselineReasonText` ("—" with a reason); - the KPI tile builder `kpiCell`; - the entrance-share mapping and HTML list (`entranceShare`, `entranceList`); Outside the view files, this PR changes one `<link>` and one `import` in `index.html`, adds the i18n blocks, and makes the server-side top-entrance records fix (decision 8). No shell module was edited. This branch is rebased on #150 (`fix/deck-shell-view-support`), whose shell APIs it uses (`ctx.createChart`, `deckLocale`, shell colours, the rejecting `loadDays`). Until #150 merges, this PR's diff against master includes #150's two commits. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(statistics-deck): Day view (Room, Desk, Laptop) (#134)
All checks were successful
CI / lint-and-test (pull_request) Successful in 2m10s
Details
59db50f7c5 
Registers the Day view with the deck shell. Each profile is its own
layout: Room 6 KPIs and 4 graphs, Desk 4 and 3, Laptop 3 and 1, keeping
the top of the RFC §4 ranking.

- KPIs from the day summary: visitors vs the same weekday last week
  (and last year when that data exists), vs the Usual Weekday Baseline,
  peak people inside, average visit (Mean Dwell), busiest hour and top
  entrance with ESTIMATED attribution shown as such.
- Graphs: visitors by hour vs the usual weekday, people inside, entrance
  share with year-on-year change, daypart split vs usual labelled with
  the usual clock span (none when it is null).
- A missing reference shows "—" with its reason, never 0%.
- Gap-estimated buckets hatch their whole hour column on both hourly
  graphs; an Unreliable day greys out every graph. Closed days show as
  closed.
- Route JSON to KPI and graph inputs is exported pure functions, tested
  in tests/frontend/test_statistics_deck_day.test.js.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gabogg commented 2026-09-27 09:28:42 +00:00
Author
Owner
Copy link

Code review: PR #149 (feat/deck-day-view @ 59db50f vs merge base 08b6d1c)

Standards sources: AGENTS.md, docs/standards/code-standards.md, docs/standards/ui-design-guidelines.md, ADR 0002, the deck shell's exported helpers, and the Fowler smell baseline. Duplication across the Day, Week and Month views is known and will be consolidated after merge, so it isn't flagged here. Spec: #134 and RFC §3–§5.

Standards

Verified: 28/28 tests pass and node --check is clean; there is no innerHTML. Facility times come from the server's bucket_time_label, and every chart is destroyed in cleanup behind an alive guard. Request count is 4 plus loadDays in Room and 2 plus loadDays in Laptop.

# Pri Finding
S-1 P1 The Promise.allSettled([...]).then(...) chain has no .catch, and cells are filled without a try/catch. A throwing mapper or chart leaves the remaining cells on LOADING and stops the rest, which contradicts the PR's "each route that fails marks only its own cells".
S-2 P2 period.business_days === 0 and the view-owned !period/closed states re-implement isClosedPeriod and the shell's empty states (now shell-owned in #150).
S-3 P2 resolveViewColors() copies the shell's token read and adds a second cache. #150 adds cyan and primary.
S-4 P2 numberLocale(tr) hard-codes locales (#150 adds ctx.locale). The daypart tick callback (v) => `${v}%` ignores the deck locale (12.5% in Spanish).
S-5 P2 §3.3: numbers in the KPI lines (visitors, usual reference, comparison %, peak hour) and the entrance YoY cells are plain text, not <data>.
S-6 P3 Possible Repeated Switches: fillGraph's id cascade, the id === 'inside' ? … source pick, and GRAPH_TITLE all branch on the graph id. Use one GRAPHS table.
S-7 P3 Possible Duplicated Code within the file: the KPI header is built twice, `— ${reason}` appears three times, and the legend wrapper is built twice.
S-8 P2 §2.1: .sd-day-greyed .sd-day-bar i hard-codes rgba(124, 135, 151, 0.55), duplicating the shell hatch.

Spec

Verified:

  • KPI and graph ranks per profile (Room 6/4, Desk 4/3, Laptop 3/1).
  • Visitors vs same_weekday_last_week; the last-year line only when the summary includes it.
  • "—" with a reason, never 0%; ESTIMATED shown on the top entrance and share rows.
  • No margin_* and no confidence bands; people inside from cumulative_occupancy.
  • Closed days shown as closed; es/en parity.
# Pri Finding
C-1 P3 comparisonText sets dailyAverage, but kpiCell never renders it. Render it or drop it.
C-2 P3 Unrequested scope: "Holidays are marked on per-day graphs". The Day view has none, so the HOLIDAY tag on the Visitors tile is a new rule. It's harmless; keep it but note it.
C-3 — The Day-only 96 px KPI-row minimum is allowed by §3.5. No action.
C-4 P2 #134: "each daypart labelled with its clock span (UsualDaypart.label; when it is null, show no span)". The view falls back to the day's own DwellDaypart.label, an agent-made departure from the issue text. Follow the issue.
C-5 P2 §4.1 "Records … never come from an Unreliable day": top entrance is a record (busiest entrance), like busiest hour, which the server already withholds when reliable is empty (analytics_service.py:628). Per "Records come from the route", add the same reliable guard for top entrance on the server, with a test.
C-6 P3 Gap buckets marked TIER_UNRELIABLE (full-column hatch) look the same as a greyed Unreliable day. The gapNote caption mitigates this, so it's acceptable. No action.
C-7 P3 The entrance list's grey-out uses a local hatch gradient, not the shared style (same root as S-8).

Maintainer decision (applies to Day, Week and Month): when there is no last-year data, hide the last-year line, per RFC §5.1 "last year only when it has data". Day already does this.

Summary. Standards: 8 findings, worst S-1 (an unhandled rejection leaves cells on LOADING, P1). Spec: 5 actionable findings, worst C-4/C-5 (daypart-label fallback against the issue text; top entrance on an Unreliable day). First pass, so all findings get fixed on this branch. S-2, S-3 and S-4 build on the shell-support PR #150.

🤖 Generated with Claude Code

# Code review: PR #149 (`feat/deck-day-view` @ 59db50f vs merge base 08b6d1c) **Standards** sources: AGENTS.md, `docs/standards/code-standards.md`, `docs/standards/ui-design-guidelines.md`, ADR 0002, the deck shell's exported helpers, and the Fowler smell baseline. Duplication across the Day, Week and Month views is known and will be consolidated after merge, so it isn't flagged here. **Spec**: #134 and RFC §3–§5. ## Standards Verified: 28/28 tests pass and `node --check` is clean; there is no `innerHTML`. Facility times come from the server's `bucket_time_label`, and every chart is destroyed in cleanup behind an `alive` guard. Request count is 4 plus `loadDays` in Room and 2 plus `loadDays` in Laptop. | # | Pri | Finding | |---|---|---| | S-1 | **P1** | The `Promise.allSettled([...]).then(...)` chain has no `.catch`, and cells are filled without a try/catch. A throwing mapper or chart leaves the remaining cells on LOADING and stops the rest, which contradicts the PR's "each route that fails marks only its own cells". | | S-2 | P2 | `period.business_days === 0` and the view-owned `!period`/closed states re-implement `isClosedPeriod` and the shell's empty states (now shell-owned in #150). | | S-3 | P2 | `resolveViewColors()` copies the shell's token read and adds a second cache. #150 adds cyan and primary. | | S-4 | P2 | `numberLocale(tr)` hard-codes locales (#150 adds `ctx.locale`). The daypart tick callback `` (v) => `${v}%` `` ignores the deck locale (`12.5%` in Spanish). | | S-5 | P2 | §3.3: numbers in the KPI lines (visitors, usual reference, comparison %, peak hour) and the entrance YoY cells are plain text, not `<data>`. | | S-6 | P3 | Possible Repeated Switches: `fillGraph`'s id cascade, the `id === 'inside' ? …` source pick, and `GRAPH_TITLE` all branch on the graph id. Use one `GRAPHS` table. | | S-7 | P3 | Possible Duplicated Code within the file: the KPI header is built twice, `` `— ${reason}` `` appears three times, and the legend wrapper is built twice. | | S-8 | P2 | §2.1: `.sd-day-greyed .sd-day-bar i` hard-codes `rgba(124, 135, 151, 0.55)`, duplicating the shell hatch. | ## Spec Verified: - KPI and graph ranks per profile (Room 6/4, Desk 4/3, Laptop 3/1). - Visitors vs `same_weekday_last_week`; the last-year line only when the summary includes it. - "—" with a reason, never 0%; ESTIMATED shown on the top entrance and share rows. - No `margin_*` and no confidence bands; people inside from `cumulative_occupancy`. - Closed days shown as closed; es/en parity. | # | Pri | Finding | |---|---|---| | C-1 | P3 | `comparisonText` sets `dailyAverage`, but `kpiCell` never renders it. Render it or drop it. | | C-2 | P3 | Unrequested scope: *"Holidays are marked on per-day graphs"*. The Day view has none, so the HOLIDAY tag on the Visitors tile is a new rule. It's harmless; keep it but note it. | | C-3 | — | The Day-only 96 px KPI-row minimum is allowed by §3.5. No action. | | C-4 | **P2** | #134: *"each daypart labelled with its clock span (UsualDaypart.label; when it is null, show no span)"*. The view falls back to the day's own `DwellDaypart.label`, an agent-made departure from the issue text. Follow the issue. | | C-5 | **P2** | §4.1 *"Records … never come from an Unreliable day"*: top entrance is a record (busiest entrance), like busiest hour, which the server already withholds when `reliable` is empty (`analytics_service.py:628`). Per *"Records come from the route"*, add the same `reliable` guard for top entrance on the server, with a test. | | C-6 | P3 | Gap buckets marked `TIER_UNRELIABLE` (full-column hatch) look the same as a greyed Unreliable day. The `gapNote` caption mitigates this, so it's acceptable. No action. | | C-7 | P3 | The entrance list's grey-out uses a local hatch gradient, not the shared style (same root as S-8). | **Maintainer decision (applies to Day, Week and Month):** when there is no last-year data, **hide** the last-year line, per RFC §5.1 *"last year only when it has data"*. Day already does this. --- **Summary.** Standards: 8 findings, worst **S-1** (an unhandled rejection leaves cells on LOADING, P1). Spec: 5 actionable findings, worst **C-4/C-5** (daypart-label fallback against the issue text; top entrance on an Unreliable day). First pass, so all findings get fixed on this branch. S-2, S-3 and S-4 build on the shell-support PR #150. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
gabogg force-pushed feat/deck-day-view from 59db50f7c5
All checks were successful
CI / lint-and-test (pull_request) Successful in 2m10s
Details
to 5d3ba7e4f2
All checks were successful
CI / lint-and-test (pull_request) Successful in 2m5s
Details
2026-09-27 09:49:38 +00:00 Compare
gabogg commented 2026-09-27 09:50:13 +00:00
Author
Owner
Copy link

Review fixes: all findings addressed in 5d3ba7e

The branch is rebased on #150 (fix/deck-shell-view-support) and force-pushed with lease.

# Fix
S-1 (P1) Each KPI tile and graph fills on its own inside try/catch. The allSettled chain ends in a .catch that turns any cell still pending into "could not be loaded". The test uses a throwing peak mapper and a throwing chart; the other tiles and graphs still fill and nothing stays on LOADING.
S-2 Dropped the view's no-period and closed-day states (and the unused views.day.closed key); the shell owns them in #150. ctx.loadDays() now rejects on failure, and the marker then falls back to the period's counts (tested).
S-3 resolveViewColors() is gone. Colours come from currentDeckColors() (cyan, primary, muted).
S-4 Charts go through ctx.createChart, which sets the deck locale. Formatting uses the shell's deckLocale, and the old numberLocale is gone. Daypart ticks use formatTickPercent in the deck locale ("12,5%" in Spanish, tested).
S-5 Numbers in KPI lines and entrance YoY cells go through withData into <data> elements (tested).
S-6 One GRAPHS table (source route plus fill) replaces the id cascade, the source pick and GRAPH_TITLE. KPIs use one KPI_BUILDERS table.
S-7 Shared kpiHeader, missingText ("— reason") and legend helpers.
S-8 The local hatch colour is removed; greyed entrance bars take the shell's .sd-swatch-unreliable.
C-1 A DAILY_AVERAGE change is rendered as "+4.0% · DAILY AVG" (tested).
C-2 HOLIDAY tag kept; the PR description now lists it as an addition beyond #134.
C-3 No action (allowed by §3.5).
C-4 Daypart spans come only from UsualDaypart.label. A null label, or no baseline, shows no span.
C-5 Server: the day summary withholds top_entrance when no day is reliable, as it does busiest_hour. There is a new backend test in tests/test_statistics_marker_inputs.py, and docs/api/README.md is updated. The tile shows "— unreliable day, no records".
C-6 No action (the gap caption distinguishes it).
C-7 Same fix as S-8.

Verification:

  • Frontend: 159 pass (Day view: 33).
  • TZ=America/Caracas pytest -q: 417 passed.
  • ruff check, ruff format --check and check_docs.py are clean.
  • Playwright re-run of Room, Desk and Laptop at 1920×1080, 1920×960 and 760×600: live dev data in en and es, plus patched responses for an OK day with gaps, a baseline and last-year data. Every run passed with no page, root, row or cell overflow and a 38 px top bar, and I checked the screenshots.

🤖 Generated with Claude Code

## Review fixes: all findings addressed in 5d3ba7e The branch is rebased on #150 (`fix/deck-shell-view-support`) and force-pushed with lease. | # | Fix | |---|---| | S-1 (P1) | Each KPI tile and graph fills on its own inside try/catch. The `allSettled` chain ends in a `.catch` that turns any cell still pending into "could not be loaded". The test uses a throwing `peak` mapper and a throwing chart; the other tiles and graphs still fill and nothing stays on LOADING. | | S-2 | Dropped the view's no-period and closed-day states (and the unused `views.day.closed` key); the shell owns them in #150. `ctx.loadDays()` now rejects on failure, and the marker then falls back to the period's counts (tested). | | S-3 | `resolveViewColors()` is gone. Colours come from `currentDeckColors()` (`cyan`, `primary`, `muted`). | | S-4 | Charts go through `ctx.createChart`, which sets the deck locale. Formatting uses the shell's `deckLocale`, and the old `numberLocale` is gone. Daypart ticks use `formatTickPercent` in the deck locale ("12,5%" in Spanish, tested). | | S-5 | Numbers in KPI lines and entrance YoY cells go through `withData` into `<data>` elements (tested). | | S-6 | One `GRAPHS` table (source route plus `fill`) replaces the id cascade, the source pick and `GRAPH_TITLE`. KPIs use one `KPI_BUILDERS` table. | | S-7 | Shared `kpiHeader`, `missingText` ("— reason") and `legend` helpers. | | S-8 | The local hatch colour is removed; greyed entrance bars take the shell's `.sd-swatch-unreliable`. | | C-1 | A `DAILY_AVERAGE` change is rendered as "+4.0% · DAILY AVG" (tested). | | C-2 | HOLIDAY tag kept; the PR description now lists it as an addition beyond #134. | | C-3 | No action (allowed by §3.5). | | C-4 | Daypart spans come only from `UsualDaypart.label`. A null label, or no baseline, shows no span. | | C-5 | Server: the day summary withholds `top_entrance` when no day is reliable, as it does `busiest_hour`. There is a new backend test in `tests/test_statistics_marker_inputs.py`, and `docs/api/README.md` is updated. The tile shows "— unreliable day, no records". | | C-6 | No action (the gap caption distinguishes it). | | C-7 | Same fix as S-8. | Verification: - Frontend: 159 pass (Day view: 33). - `TZ=America/Caracas pytest -q`: 417 passed. - `ruff check`, `ruff format --check` and `check_docs.py` are clean. - Playwright re-run of Room, Desk and Laptop at 1920×1080, 1920×960 and 760×600: live dev data in en and es, plus patched responses for an OK day with gaps, a baseline and last-year data. Every run passed with no page, root, row or cell overflow and a 38 px top bar, and I checked the screenshots. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
gabogg commented 2026-09-27 10:01:20 +00:00
Author
Owner
Copy link

Code review, second pass: PR #149 (Day) @ 5d3ba7e

All first-pass fixes are verified. 76 node tests pass, and tests/test_statistics_marker_inputs.py passes 9/9.

  • Server change (C-5): correct. The same reliable guard as busiest_hour, a test that fails without the fix, and docs/api/README.md updated.
  • C-4: correct. Spans come only from UsualDaypart.label.
  • Shell API (#150): adopted.

Standards

# Pri Finding
S2-1 P2 §3.3: the S-5 fix wraps formatted figures with withData, which splits on \d+. "+5.2%" becomes <data value=5> + <data value=2>, and Spanish "1.234" becomes 1 + 234. The test at test_statistics_deck_day.test.js:538 locks in the wrong values. withData is for whole counts; wrap formatted figures in one <data value={raw}> each.
S2-2 P3 chart() keeps a ctx.Chart && ctx.createChart ? … : null workaround. If Chart.js is missing, the cell shows an empty box instead of the error text.
S2-3 P3 daypartConfig re-spreads deckChartOptions().scales.y.ticks because the shell merges scales.y shallowly. A deep tick merge in the shell would fix this for all views.
S2-4 P3 When fillGraph throws, the header keeps its legend and notes over the error, and sd-day-greyed stays on.

Spec

# Pri Finding
C2-1 P2 Maintainer decision / RFC §5.1 "last year only when it has data": with no last-year data at all, the entrance-share graph still shows "— no data last year". Hide the note along with the column, and keep "—" plus the reason only for single rows missing last year when others have it.

Verdict: mergeable after S2-1 and C2-1. The P3s are filed as a follow-up issue.

🤖 Generated with Claude Code

# Code review, second pass: PR #149 (Day) @ 5d3ba7e All first-pass fixes are verified. 76 node tests pass, and `tests/test_statistics_marker_inputs.py` passes 9/9. - **Server change (C-5):** correct. The same `reliable` guard as `busiest_hour`, a test that fails without the fix, and `docs/api/README.md` updated. - **C-4:** correct. Spans come only from `UsualDaypart.label`. - **Shell API (#150):** adopted. ## Standards | # | Pri | Finding | |---|---|---| | S2-1 | **P2** | §3.3: the S-5 fix wraps formatted figures with `withData`, which splits on `\d+`. "+5.2%" becomes `<data value=5>` + `<data value=2>`, and Spanish "1.234" becomes `1` + `234`. The test at `test_statistics_deck_day.test.js:538` locks in the wrong values. `withData` is for whole counts; wrap formatted figures in one `<data value={raw}>` each. | | S2-2 | P3 | `chart()` keeps a `ctx.Chart && ctx.createChart ? … : null` workaround. If Chart.js is missing, the cell shows an empty box instead of the error text. | | S2-3 | P3 | `daypartConfig` re-spreads `deckChartOptions().scales.y.ticks` because the shell merges `scales.y` shallowly. A deep tick merge in the shell would fix this for all views. | | S2-4 | P3 | When `fillGraph` throws, the header keeps its legend and notes over the error, and `sd-day-greyed` stays on. | ## Spec | # | Pri | Finding | |---|---|---| | C2-1 | **P2** | Maintainer decision / RFC §5.1 *"last year only when it has data"*: with no last-year data at all, the entrance-share graph still shows "— no data last year". Hide the note along with the column, and keep "—" plus the reason only for single rows missing last year when others have it. | **Verdict:** mergeable after S2-1 and C2-1. The P3s are filed as a follow-up issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
gabogg commented 2026-09-27 10:07:48 +00:00
Author
Owner
Copy link

Second-pass fixes in c00716c

# Fix
S2-1 Each formatted figure (comparison changes, counts, the usual reference and its day count, busiest-hour and top-entrance visitors, entrance YoY cells) is now one <data value={raw}>formatted</data>, built from phrase() parts. withData is no longer used. The test that locked in the split values now asserts raw values and text, e.g. ['5.24', '+5.2%'] and ['11000.4', '11,000']. A new test checks that Spanish 1.234 stays one figure.
C2-1 With no last-year data at all, the entrance share hides the year-on-year column and shows no note; the unused yoyMissing key is removed. A single entrance missing last year while others have it still shows "—" with its reason. Tests cover both the pure mapping and the rendered view.

S2-2 to S2-4 are untouched (filed as an issue).

Checks:

  • Frontend: 160 pass (Day view: 34).
  • TZ=America/Caracas pytest -q: 417 passed.
  • ruff check, ruff format --check and check_docs.py are clean.
  • I did not re-run the Playwright browser check for this commit.

🤖 Generated with Claude Code

## Second-pass fixes in c00716c | # | Fix | |---|---| | S2-1 | Each formatted figure (comparison changes, counts, the usual reference and its day count, busiest-hour and top-entrance visitors, entrance YoY cells) is now one `<data value={raw}>formatted</data>`, built from `phrase()` parts. `withData` is no longer used. The test that locked in the split values now asserts raw values and text, e.g. `['5.24', '+5.2%']` and `['11000.4', '11,000']`. A new test checks that Spanish `1.234` stays one figure. | | C2-1 | With no last-year data at all, the entrance share hides the year-on-year column and shows no note; the unused `yoyMissing` key is removed. A single entrance missing last year while others have it still shows "—" with its reason. Tests cover both the pure mapping and the rendered view. | S2-2 to S2-4 are untouched (filed as an issue). Checks: - Frontend: 160 pass (Day view: 34). - `TZ=America/Caracas pytest -q`: 417 passed. - `ruff check`, `ruff format --check` and `check_docs.py` are clean. - I did not re-run the Playwright browser check for this commit. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(statistics-deck): one <data> per figure, hide an empty last-year column (#149 review)
All checks were successful
CI / lint-and-test (pull_request) Successful in 2m4s
Details
c00716c6a2 
- S2-1: formatted figures (changes, counts, the usual reference, YoY
  cells) render as one <data value={raw}> each instead of going through
  withData, which split "+5.2%" and "1.234" into digit runs. Copy with
  figures is built from `phrase()` parts.
- C2-1: with no last-year data at all the entrance share shows neither
  the year-on-year column nor a note (RFC §5.1); a single entrance
  missing last year still shows "—" with its reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test(statistics-deck): Day expects the canonical comparison-reason texts
All checks were successful
CI / lint-and-test (pull_request) Successful in 2m8s
Details
cb4c8e7da8 
After merging master (Month #147 and Week #148), master's Month wording is
the canonical statisticsDeck.comparison.reason.* set (maintainer decision).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gabogg force-pushed feat/deck-day-view from cb4c8e7da8
All checks were successful
CI / lint-and-test (pull_request) Successful in 2m8s
Details
to b500dde28a
All checks were successful
CI / lint-and-test (pull_request) Successful in 2m10s
Details
2026-09-27 14:36:51 +00:00 Compare
gabogg commented 2026-09-27 14:36:59 +00:00
Author
Owner
Copy link

Master merged in (b500dde) after Month (#147) and Week (#148). I resolved index.html, keeping all three views' stylesheets and imports, and i18n.js: master's views.month/views.week plus the canonical comparison-reason texts (Month wording, the maintainer's decision), with Day's final views.day block added. No Day key was lost, and es/en parity holds. The Day tests now expect the canonical texts. The full suite passes on top of master (417), and ruff and check_docs are clean.

**Master merged in (b500dde)** after Month (#147) and Week (#148). I resolved `index.html`, keeping all three views' stylesheets and imports, and `i18n.js`: master's `views.month`/`views.week` plus the canonical comparison-reason texts (Month wording, the maintainer's decision), with Day's final `views.day` block added. No Day key was lost, and es/en parity holds. The Day tests now expect the canonical texts. The full suite passes on top of master (417), and ruff and check_docs are clean.
gabogg merged commit 92fe600e40 into master 2026-09-27 14:37:01 +00:00
gabogg deleted branch feat/deck-day-view 2026-09-27 14:37:01 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
blocked
Waits on the issues listed in its "Blocked by:" line before implementation

  
bug
Category: something is broken; behaviour differs from the spec or docs

  
enhancement
Category: new feature or improvement, including refactors, chores and docs

  
high-priority
Address before ordinary-priority work; independent of triage readiness and blockers

  
low-priority
Address after ordinary-priority work; independent of triage readiness and blockers

  
needs-info
Waiting on reporter for more information

  
needs-triage
Maintainer needs to evaluate this issue

  
ready-for-agent
Fully specified, ready for an AFK agent

  
ready-for-human
Requires human implementation

  
referenced
Previously closed issue reopened or referenced by an open issue; visible history marker

  
research
Needs investigation (internet or local resources) before it can be triaged down; used alongside a category and a triage role

  
wontfix
Will not be actioned

No labels
blocked
bug
enhancement
high-priority
low-priority
needs-info
needs-triage
ready-for-agent
ready-for-human
referenced
research
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gabogg/hikcentral!149
No description provided.