Skip to content

feat(history): add calendar month selection - #1866

Open
bferanmi806-sketch wants to merge 5 commits into
srcfl:masterfrom
bferanmi806-sketch:feat/1318-calendar-month-history
Open

bferanmi806-sketch wants to merge 5 commits into
srcfl:masterfrom
bferanmi806-sketch:feat/1318-calendar-month-history

Conversation

@bferanmi806-sketch

@bferanmi806-sketch bferanmi806-sketch commented Oct 26, 2026 •

Copy link
Copy Markdown

Summary

Implements #1318: the four shared history tiles and pie view can show this month, last month, or a month selected with the native picker. The API uses calendar days in the box's timezone and keeps the current month partial. EV-cost endpoints are unchanged.

Month selection now waits for the box's runtime timezone and recalculates the current month at interaction time, including month boundaries. A custom selection no longer highlights This month. Month charts scroll horizontally at a readable day width, with a subtle directional cue only while days extend beyond the visible area; Week returns to the compact layout, and the chart region supports keyboard scrolling.

Validation

  • Final Linux web suite: 903 tests passed; focused history regressions: 11 passed. JavaScript syntax and diff whitespace checks passed.

  • Real local Go API with a fresh isolated data directory, seeded through the built-in backfill utility (1100 days, five-minute samples, seed 1866): verified populated import, production, consumption and export bars plus pie totals. Checked current/last month, February 2024 (29 days), partial current month, and missing-data behavior.

  • Desktop and 390px mobile browser review: readable chart labels, horizontal chart scrolling without page overflow, correct custom-month highlight, and populated pie. Before/after screenshots retained for the author's human review.

  • A pass-through proxy held September's actual API response for 2.5 seconds. Logs confirmed October arrived first; both bars and pie retained October after the stale September response arrived.

  • Timezone helper tests cover differing box/browser calendar months at a UTC boundary, year rollover and invalid metadata. Browser timezone emulation and physical touch scrolling were unavailable. ArrowRight scrolling worked. Five status-poll warnings coincided with restarting the test proxy; no other browser warnings/errors were captured.

  • The prior full Go API and make verify passed on the signed pre-follow-up head. The follow-up Go checks are listed below. The earlier verify run skipped its Avahi container run and release Docker-context checks because the test container had no Docker daemon.

  • Follow-up verification: the full Linux API package test run passed; the helper-only test TestPlannerFailureCrashHelper was skipped. The timezone/month regression passed, and the API comment documents that month takes precedence over days. The go vet ./internal/api check passed, and the full Go tree built for linux/amd64. The Linux test binary was cross-compiled on Windows and executed in WSL.

Integration and review

This PR is no longer a draft. Human visual review is complete; see the Nov 10 follow-up comment and screenshots. Actual hunk comparisons found no shared changed lines with #1859, #1862, #1863 or #1751. The closest edits are #1859's live-history rendering beside the selector, #1751's daily response fields after the month range setup, and #1546's EV card insertion after the history cake. Preserve both sets of changes when integrating and rerun the relevant checks after those PRs land.

Fork Actions runs have required upstream approval (action_required, zero jobs/logs). The repository policy endpoint returned 403, so the exact approval rule could not be confirmed. A maintainer needs to inspect and approve the runs if required. No reviewers have been requested.

Visual comparison

Populated energy history from the actual local API. Month charts now scroll horizontally to keep each day's labels readable; the mobile view no longer highlights This month when a custom month is selected.

View Before After
Desktop Desktop before Desktop after
Mobile viewport Mobile before Mobile after

Mobile captures use a browser viewport, not a physical phone. These images are retained for human visual review; additional viewport screenshots and results are in the Nov 10 follow-up comment.

Add this-month, last-month, and a month picker to the shared history tiles. Aggregate each month using local calendar boundaries and keep the current month partial.

Signed-off-by: Balogun Feranmi <bferanmi806@gmail.com>
Signed-off-by: Balogun Feranmi <bferanmi806@gmail.com>
@bferanmi806-sketch
bferanmi806-sketch force-pushed the feat/1318-calendar-month-history branch from ffcdb6e to 20f9229 Compare October 28, 2026 15:35
Signed-off-by: Balogun Feranmi <bferanmi806@gmail.com>
Signed-off-by: Balogun Feranmi <bferanmi806@gmail.com>
@bferanmi806-sketch
bferanmi806-sketch marked this pull request as ready for review October 28, 2026 21:01

@miravoss26 miravoss26 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds calendar-month history to the four history tiles and the pie: month=YYYY-MM on /api/energy/daily, This month / Last month / month picker in the UI, with a timezone-safe month helper and tests.

  • Question go/internal/api/api.go (handleEnergyDaily): inside if month != "", loc, err := time.LoadLocation(...) is a new loc scoped to that block. The outer loc (from now.Location()) is what the rest of the handler sees. If code further down builds day boundaries with time.Date(..., loc) rather than from todayMidnight, month requests would be bucketed in the server zone, not the box zone. I could not see the rest of the handler in this diff, so worth confirming, ideally with a test where box tz differs from process tz.
  • Low days is silently ignored when month is set. Fine, but a 400 or a comment would make it explicit.
  • Low The PR text says it is still pending the author's visual review. Treat the UI part as unreviewed by a human.
  • Security screen: no secrets, no new dependencies, no new hosts. month is strictly parsed (2006-01 round-trip), future months rejected.

Needs a human decision on the loc shadowing question before merge; the rest looks sound from my read.

@bferanmi806-sketch

Copy link
Copy Markdown
Author

Hi @miravoss26, thanks for the detailed review!

You're right about the "loc" shadowing. I checked the remainder of "handleEnergyDaily", and the outer "loc" isn't used after the month block. The handler rebuilds "todayMidnight" using the box timezone, and the daily loop derives its boundaries using "todayMidnight.AddDate()", which preserves that location. "calendarMonthBounds" also receives "now" after conversion to the box timezone.

So the month calculations should be correct despite the shadowing. I'll make the variable scoping clearer and add a regression test with different server and box timezones to verify this explicitly.

For "days", the month parameter intentionally takes precedence. I'll document that behaviour.

Good catch on the PR description too. I'll update the outdated draft wording and clarify the visual review status.

Thanks again for taking the time to review this!

@bferanmi806-sketch

Copy link
Copy Markdown
Author

@miravoss26 Follow-up pushed in 16a813390611c14f175630a425d75c0e3e475784. I clarified the process and box timezone variables and kept month calculations in the box timezone; the API comment now documents that month takes precedence over days. The new API regression runs with Los Angeles as the process timezone and Stockholm as the box timezone, then checks all March 2024 local day boundaries, including the DST transition and days=1 precedence.

The full Linux API package run passed; its helper-only TestPlannerFailureCrashHelper was skipped. go vet ./internal/api and the full Linux/amd64 Go build passed. The test binary was cross-compiled with the Windows Go toolchain and executed in WSL. GitHub reports no check runs for this commit. Human visual review remains outstanding.

@miravoss26 miravoss26 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds a calendar-month selector to the history tiles; /api/energy/daily?month=YYYY-MM takes precedence over days.

  • I read the Go side (handler, calendarMonthBounds, tests). Month parsing is strict, future months are rejected, boundaries use the box timezone and the current month stays partial. DST and leap-year cases are covered, and so is the process-TZ vs box-TZ split. No security concerns: input is a parsed YYYY-MM, no new deps or hosts.
  • I did not review the frontend half of the diff (picker, chart scrolling) line by line.

From my read the API side is fine. A human should look at the UI behaviour, and this is an external contributor PR, so a maintainer call on scope.

@bferanmi806-sketch

Copy link
Copy Markdown
Author

Manual visual review completed on PR head 16a8133 using the running app with isolated synthetic history. At 1440px desktop and 390px mobile, the month picker and Last month/custom month states worked: November 2026 (current month), October 2026 (31 days), and February 2024 (29 days). All four bar cards updated, and the pie view matched the month totals. At 390px, the page stayed within the viewport; horizontal scrolling remained inside each chart tile (285px viewport, 1334px chart content). ArrowRight advanced the chart and updated the directional cue. Runtime timezone was Europe/London. No confirmed UI defects; no UI code changed. This is browser/viewport evidence, not physical touch-device verification.

Desktop bars — November 2026 month selector and all four daily charts:
clipboard

Mobile bars after horizontal keyboard scroll:
clipboard

Mobile pie view for February 2024:
clipboard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants