chore(schedule): PR #175 second-pass P3 follow-ups (localization, test robustness, fallback name)

gabogg commented

2026-09-28 17:19:53 +00:00

Owner

Deferred P3 findings from the second review pass on #175 (schedule-exception naming, #108). None of them blocked the merge.

Standards

Two labels in the calibration panel are not localized. The <details> summary "CALIBRATION AND TRUST THRESHOLDS" and the "BASELINE OFFSET" label are still hardcoded English (app/static/index.html:672,678). Their sibling trust labels are translated. This predates #175.
- Acceptance: both strings come from EN/ES i18n keys and keep their uppercase rendering.
The uppercase test is brittle. In tests/test_i18n.py, the uppercase loop checks for the exact source string class="uppercase" data-i18n="occupancy.{key}". Reordering the attributes or adding a class breaks it even when the rendered output does not change. It also forces a redundant uppercase class onto inner <span>s whose parent <button> already has one (index.html:755).
- Acceptance: the test checks the uppercase styling on the element or its nearest labelled ancestor, independent of attribute order. The redundant span classes are removed.
One key does not follow the prefix of its neighbours. Eleven trust-grid keys use the trust*Label prefix, but their sibling is still exceptionMinEntriesLabel (app/static/js/i18n.js:125).
- Acceptance: rename it to trustExceptionMinEntriesLabel, or give a documented reason not to, and update its references in both EN and ES.
The fallback name is duplicated. The string "Excepción de horario" is hardcoded in both app/db/occupancy_repository.py:775 and app/services/occupancy_service.py:289,297.
- Acceptance: one shared constant, or it goes away through item 6.
The Holiday marker has only an indirect test. The assertion that the ES statistics legend says FERIADO was removed in #175. Only tests/frontend/test_statistics_deck_day.test.js:235 (kpis.visitors.tag) still covers the Holiday marker, which is separate from Schedule Exception (i18n.js:752,796,822).
- Acceptance: a direct assertion on the legend and marker strings, coordinated with #161, which owns Holiday identity.

Spec

The backend stores and shows Spanish text in every UI language. A nameless exception is saved as "Excepción de horario", and schedule_label appends "(Cerrado)" (occupancy_service.py:289-297). app.js:1849 then shows that label verbatim, so the English live card reads "Schedule Exception: Excepción de horario (Cerrado)". This conflicts with #73, which says stored data and payloads must not encode a language. The old "Feriado" default had the same problem.
- Acceptance: the backend returns language-neutral data (for example a null name plus a closed flag or code) and the frontend localizes it. Existing stored rows keep working. Needs a design decision: do this under #73, or settle the payload shape first.
An empty name now behaves differently. holiday.get("name") or … (occupancy_repository.py:775) replaces name: "" with the default name. The POST response (occupancy_controller.py:241) still echoes "", so the stored value and the response disagree. The admin UI rejects empty names, so the impact is small.
- Acceptance: either HolidayItem.name rejects empty strings (min_length=1), or the response returns the stored name. A test covers the chosen behavior.

Refs #175, #108, #73, #161.

🤖 Generated with Claude Code

Deferred P3 findings from the second review pass on #175 (schedule-exception naming, #108). None of them blocked the merge. ## Standards 1. **Two labels in the calibration panel are not localized.** The `<details>` summary "CALIBRATION AND TRUST THRESHOLDS" and the "BASELINE OFFSET" label are still hardcoded English (`app/static/index.html:672,678`). Their sibling trust labels are translated. This predates #175. - *Acceptance:* both strings come from EN/ES i18n keys and keep their uppercase rendering. 2. **The uppercase test is brittle.** In `tests/test_i18n.py`, the uppercase loop checks for the exact source string `class="uppercase" data-i18n="occupancy.{key}"`. Reordering the attributes or adding a class breaks it even when the rendered output does not change. It also forces a redundant `uppercase` class onto inner `<span>`s whose parent `<button>` already has one (`index.html:755`). - *Acceptance:* the test checks the uppercase styling on the element or its nearest labelled ancestor, independent of attribute order. The redundant span classes are removed. 3. **One key does not follow the prefix of its neighbours.** Eleven trust-grid keys use the `trust*Label` prefix, but their sibling is still `exceptionMinEntriesLabel` (`app/static/js/i18n.js:125`). - *Acceptance:* rename it to `trustExceptionMinEntriesLabel`, or give a documented reason not to, and update its references in both EN and ES. 4. **The fallback name is duplicated.** The string `"Excepción de horario"` is hardcoded in both `app/db/occupancy_repository.py:775` and `app/services/occupancy_service.py:289,297`. - *Acceptance:* one shared constant, or it goes away through item 6. 5. **The Holiday marker has only an indirect test.** The assertion that the ES statistics legend says `FERIADO` was removed in #175. Only `tests/frontend/test_statistics_deck_day.test.js:235` (`kpis.visitors.tag`) still covers the Holiday marker, which is separate from Schedule Exception (`i18n.js:752,796,822`). - *Acceptance:* a direct assertion on the legend and marker strings, coordinated with #161, which owns Holiday identity. ## Spec 6. **The backend stores and shows Spanish text in every UI language.** A nameless exception is saved as `"Excepción de horario"`, and `schedule_label` appends `"(Cerrado)"` (`occupancy_service.py:289-297`). `app.js:1849` then shows that label verbatim, so the English live card reads "Schedule Exception: Excepción de horario (Cerrado)". This conflicts with #73, which says stored data and payloads must not encode a language. The old "Feriado" default had the same problem. - *Acceptance:* the backend returns language-neutral data (for example a null name plus a closed flag or code) and the frontend localizes it. Existing stored rows keep working. **Needs a design decision:** do this under #73, or settle the payload shape first. 7. **An empty name now behaves differently.** `holiday.get("name") or …` (`occupancy_repository.py:775`) replaces `name: ""` with the default name. The POST response (`occupancy_controller.py:241`) still echoes `""`, so the stored value and the response disagree. The admin UI rejects empty names, so the impact is small. - *Acceptance:* either `HolidayItem.name` rejects empty strings (`min_length=1`), or the response returns the stored name. A test covers the chosen behavior. Refs #175, #108, #73, #161. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

gabogg added this to the Historical passenger-flow backfill milestone

2026-09-28 17:19:53 +00:00

gabogg added the

needs-triage

label

2026-09-28 17:19:53 +00:00

gabogg referenced this issue

2026-09-28 17:20:18 +00:00

fix(schedule): call dated overrides schedule exceptions #175

gabogg referenced this issue

2026-09-28 18:10:01 +00:00

feat(schedule): each business day remembers the schedule it was measured under #176

gabogg commented

2026-10-02 15:42:52 +00:00

Author

Owner

Triage decision, 2026-10-02 (maintainer)

Items 4 and 6 move to #73 (language-agnostic payloads). #73 is the designed home for language-neutral backend data, so fixing only this corner here would create two patterns.
- The backend returns neutral data: a null name and a closed flag/code.
- The frontend localizes it ("Schedule exception" / "Excepción de horario", "Closed" / "Cerrado").
- Existing stored rows keep working.
Item 5 (direct Holiday-marker test) moves to #178. It owns Holiday identity and is rewriting the markers.
Item 7, an empty exception name:
- HolidayItem.name rejects "" (min_length=1), and "no name" is null.
- The display name for a nameless exception comes from the frontend (see item 6 / #73), not from a stored Spanish default.
- A test covers it.
Item 3: rename exceptionMinEntriesLabel → trustExceptionMinEntriesLabel in EN and ES.
Items 1 and 2: as written.

Remaining scope here: items 1, 2, 3 and 7. Relabelled ready-for-agent.

🤖 Generated with Claude Code

## Triage decision, 2026-10-02 (maintainer) - **Items 4 and 6 move to #73** (language-agnostic payloads). #73 is the designed home for language-neutral backend data, so fixing only this corner here would create two patterns. - The backend returns neutral data: a null name and a `closed` flag/code. - The frontend localizes it ("Schedule exception" / "Excepción de horario", "Closed" / "Cerrado"). - Existing stored rows keep working. - **Item 5 (direct Holiday-marker test) moves to #178.** It owns Holiday identity and is rewriting the markers. - **Item 7, an empty exception name:** - `HolidayItem.name` rejects `""` (`min_length=1`), and "no name" is `null`. - The display name for a nameless exception comes from the frontend (see item 6 / #73), not from a stored Spanish default. - A test covers it. - **Item 3:** rename `exceptionMinEntriesLabel` → `trustExceptionMinEntriesLabel` in EN and ES. - **Items 1 and 2:** as written. **Remaining scope here:** items 1, 2, 3 and 7. Relabelled `ready-for-agent`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

gabogg referenced this issue

2026-10-02 15:42:52 +00:00

refactor(i18n): language-agnostic identifiers and payloads — i18n with English fallback, not a Spanish default #73

gabogg referenced this issue

2026-10-02 15:42:53 +00:00

chore(schedule): PR #176 second-pass P3 follow-ups (audit actions, freeze migration, default stamp span) #186

gabogg added

ready-for-agent

and removed

needs-triage

labels

2026-10-02 15:42:56 +00:00