feat(schedule): each business day remembers the schedule it was measured under #176
No reviewers
Labels
No labels
blocked
bug
enhancement
high-priority
low-priority
needs-info
needs-triage
ready-for-agent
ready-for-human
referenced
research
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gabogg/hikcentral!176
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/business-day-schedule-records"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Implements #113: each business day keeps the opening status, hours, source, and exception name resolved when its reset begins. Weekly and dated exception edits no longer change a started day's schedule. Older unrecorded days explicitly fall back to the current plan until an admin stamps or corrects them.
The schedule section now lets admins inspect a day and its audit history, stamp every completed day between the first and last counted event, and correct completed days with a reason. Overwrite requires a selected range and confirmation. Corrections change statistical interpretation immediately; calibration uses the original recorded schedule. Reset-time edits still recut historical business-day boundaries; date-effective reset records are tracked separately in #114.
Architectural impact
IMPORTEDas a source for later historical-flow work.Verification
python3 scripts/check_docs.py, andgit diff --checkpassed.Checklist
docs/standards/git-and-workflow.md.Issues closed on merge
Closes #113
gabogg referenced this pull request2026-09-28 10:28:14 +00:00
WIP: feat(schedule): each business day remembers the schedule it was measured underto feat(schedule): each business day remembers the schedule it was measured underCode review, pass 1 (
origin/master...0e65494, spec #113)Result: no P1s. 4 P2s (2 Standards, 2 Spec) and 11 P3s (8 Standards, 3 Spec). This is the first pass, so every finding gets fixed on the branch.
Verification: pytest 458 passed, frontend 242 passed, ruff clean, CI green. The branch is based on current master (
4029c79) and merges cleanly.Standards
Clean:
require_adminprotects the audit, stamp and correct routes. Errors useVALIDATION_ERRORper code-standards §3. All SQL is inOccupancyRepository. Stamp and correct each write the record and its audit row in oneBEGIN IMMEDIATE. The migration is idempotent with CHECK constraints. The audit list escapes values beforeinnerHTML. Tests use real sqlite and the ASGI client. EN and ES keys are complete.P2
app/static/js/i18n.js:169). The ESstampDaysHelpsays "los días cerrados" where it means completed days. CONTEXT.md defines Closed Day as a day the schedule declares closed, so the help text reads as the opposite. Use "días completados".app/static/index.html:796-799). The correction form reusesopenTableHeader,openTimeTableHeaderandcloseTimeTableHeader, which translate to mixed case ("Opening", "Apertura"). The<label>s have nouppercaseclass, but ui-design-guidelines §3.2 requires uppercase labels. The "is exception" checkbox also reuses the plural tab titlesubtabScheduleExceptions. Give it its own singular key.P3 (judgement calls)
3. Duplicated Code.
date.fromisoformat(facility_cycle_bounds(x, reset).label)appears atoccupancy_service.py:262,275,290,359,386,387. Usefacility_time.active_business_day().occupancy_repository.py:889and:946.BusinessDaySchedulemapping exists in both the service (~230) and the repository (:49).BusinessDaySchedule(source="MANUAL", recorded=True)(occupancy_controller.py:277), and the service builds the same object again (occupancy_service.py:367). Pass only the corrected fields.occupancy_controller.py:132,190,333,358). Move it next to the writes in the service, so a new edit route can't skip it.ScheduleSource(facility_time.py:49),ScheduleSourceValue(occupancy_models.py:324) and the DB CHECK. Keep one source of truth.get_active_schedule_info_asyncrunsensure_*and then loads the full calendar, including every record. Loops call it once per cycle (analytics_service.py:929,1038,occupancy_service.py:1467), and on every event (:717), so the cost grows with history. The monitor also reads the reset time on every 1 s tick (monitor_service.py:~98).last_schedule_day = Noneinmonitor_service.pyshould be annotateddate | None."Excepción de horario"and the Spanishday_namesmoved into the service (occupancy_service.py:226,391) unchanged. This predates the PR and overlaps #73 and #184.Spec
Checked and correct:
monitor_service.py:103-108).facility_cycle_bounds, so an edit between 00:00 and the reset freezes the previous day.schedule_for.P2
11. (a) The default stamp skips days with no counts. Spec: "by default fills only days that have no record" and "records can exist for any date, not only days with counted data". With no dates given, the service keeps only
has_datadays (occupancy_service.py:326). Closed and outage days with zero counts stay unrecorded, so a later weekly edit reclassifies them. That breaks "Editing the weekly schedule … never changes a … past day's classification". Default to the full span, including days with no data.12. (a) The one-reader test never touches statistics. Spec: "one-reader consistency (statistics and live view agree for the same day)". The test compares
ScheduleCalendarwithget_active_schedule_info_asynconly. Add an assertion through ananalytics_servicepath: the Closed Day list (analytics_service.py:439/811), or dwell and dayparts (:929/1038).P3
13. (a) Automatic freezes are not audit-logged. Spec: "every stamp or correction is audit-logged".
ensure_business_day_schedule_record_async(occupancy_repository.py:~838) writes no audit row. After a restart that spans the reset, the day is recorded late with the plan in force at that moment, and nothing shows it was late. Add an audit row with actorsystem, or a recorded-at timestamp that shows the lateness.14. (c) A correction can still reach calibration. Spec: "a correction never re-runs an audit or feeds old data into the current calibration". The completed-cycle audit reads the reader (
occupancy_service.py:1214). If an admin corrects yesterday before the auto-audit runs, the corrected hours feed k. Pin the audit to the hours recorded at freeze time, or document this window.15. (b) Scope creep, reasonable. The PR adds a
GET /schedule/days/{d}route, an/auditroute, a 10-year cap on stamp ranges, andschedule_source/schedule_recordedfields in the live info. The API docs count jumps from 66 to 80. The old 66 was stale, and only 4 routes are new.Summary
🤖 Generated with Claude Code
First review pass addressed
All 4 P2 findings and the actionable P3 findings from the first pass are addressed in the follow-up commit.
FREEZEaudit row with actorsystemand timestamp. Calibration uses the first schedule value recorded for a day, so later corrections change statistics without teaching the completed-cycle audit.The review's scope observation (#15) was advisory; the extra read/audit routes and stamp range cap remain documented and scoped to #113.
Ruff, formatting, JavaScript syntax, documentation checks, and the focused tests pass. The commit hook also reran the full pytest suite.
Please run the second review pass against this updated branch.
Code review, pass 2 (
origin/master...69ba31a, spec #113)Result: no P1 or P2 findings. All pass-1 findings are fixed, except #6, which is partly fixed. The 8 P3s (5 Standards, 3 Spec) are deferred to #186, following docs/standards/git-and-workflow.md ("Second pass, ordinary PR").
Verification: ruff is clean. pytest: 460 passed. Frontend tests: 242 passed.
tests/test_business_day_schedules.py: 8/8 passed. CI passed on69ba31a, and the branch is based on current master (4029c79).Standards
Pass-1 findings
i18n.js:170).scheduleExceptionSingularkey in EN and ES: fixed.active_business_day, a single upsert SQL constant, andBusinessDaySchedule.from_record.payload, and the service builds the MANUAL record: fixed.repo: fixed.ScheduleSourcenow generates its CHECK, but the audit action list is still written twice (#186 item 1).day_nameor the ISO date: fixed.The freeze write is atomic (
BEGIN IMMEDIATE). There is no new SQL outside the repositories, and the tests don't mock production state.New findings (P3, all in #186)
FREEZE, such as the local validation copy, would reject automatic freezes. Prod is safe because the table is new there.get_original_business_day_schedule_asyncdecodes the original fallback throughfrom_record, which forcesrecorded=True.database.py:344-351), a pattern others may copy.Spec
Pass-1 findings
get_statistics_period_quality_async(...).closed_days == 1.FREEZEaudit row (actorsystem) only when a record was actually inserted, so stamping stays safe to press twice.New findings (P3, all in #186)
N1 (a): "records can exist for any date, not only days with counted data". The default stamp stops at the last counted day, so a closed stretch after counting stopped stays on the fallback.
N2 (c): the "first recorded value" depends on the order of audit rows. A future IMPORTED writer without an audit row would break it. This needs a design decision.
N3 (b): three small behavior changes nobody asked for:
day_name.None of them breaks the working hours or the live label.
Summary
FREEZE.Merging.
🤖 Generated with Claude Code