Skip to content

feat(web,api): day log complete history without layer filter - #112

Merged
atulmgupta merged 2 commits into
mainfrom
day-log
Sep 16, 2026
Merged

atulmgupta merged 2 commits into
mainfrom
day-log

Conversation

@atulmgupta

Copy link
Copy Markdown
Contributor

Summary

  • Day log now loads the full event history for the selected day (all layers by default).
  • Frontend paginates until total_events; truncated is only for signal/gear caps.
  • SI fields displayed via useUnits; drive accent uses quieter cyan #0891b2.
  • Guideline review: missing DayLogLayer import, TS fragment, timezone, Text titles, test virtualizer mock.

Test plan

  • go test ./internal/api/daylog/ ./internal/database/daylog/
  • npx tsc --noEmit
  • Vitest day-log files 23/23
  • CI green

Default all event layers, paginate until total_events, and keep
truncated only for signal/gear caps. Display SI via useUnits, quieter
drive accent, and guideline-clean page/timeline components.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 15, 2026 22:36

Copilot AI commented Sep 15, 2026

Copy link
Copy Markdown

One or more custom setup steps configured for this repository failed during this Copilot code review run:

Verify TypeScript build

Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.

Note

You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical event-decoding defects and unresolved pagination consistency and completeness risks remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Expands Day Log to load complete history across all layers by default, with paginated retrieval, client-side filtering, virtualization, SI formatting, and updated localization.

Changes:

  • Extends backend event assembly, pagination, mappings, and documentation.
  • Adds frontend search, category filters, details, timezone handling, and virtualization.
  • Updates API types, translations, controls, source displays, and tests.
File summaries
File Reviewed change / final findings
web/src/i18n/en/locale-vehicles.json Updates generated vehicle translations.
web/src/i18n/en.json Adds Day Log translations.
web/src/features/vehicles/pages/DayLogPage.tsx Wires the complete-history page and removes layer URL filters.
web/src/features/vehicles/pages/DayLogPage.test.tsx Tests all-layer rendering and filtering.
web/src/features/vehicles/lib/daylog.ts Adds categories and display helpers. Moderate (1 vote): advertised time search cannot match event timestamps.
web/src/features/vehicles/lib/daylog.test.ts Tests Day Log helpers.
web/src/features/vehicles/components/daylog/DayLogTimeline.tsx Adds filtering, search, virtualization, and details. Moderate (2 votes): time search omits timestamps and timezone from memo dependencies.
web/src/features/vehicles/components/daylog/DayLogSources.tsx Displays source completeness information.
web/src/features/vehicles/components/daylog/DayLogControls.tsx Maintains date and vehicle controls.
web/src/api/types.ts Defines complete-history response fields. Nit (1 vote): DayLogLayer documentation still describes layers as off by default.
web/src/api/hooks/useDayLog.ts Fetches all event pages. Critical (2 votes): offset pagination can duplicate or omit events as data changes. Moderate (2 votes): empty pages can silently produce an incomplete successful result.
web/src/api/hooks/useDayLog.test.ts Tests pagination behavior.
internal/database/daylog/repo.go Adds Day Log queries and typed signal reads. Moderate (1 vote): the baseline query excludes unknown security event types and can lose prior transition context.
internal/database/daylog/repo_test.go Tests repository query shapes.
internal/api/daylog/handler.go Handles complete-history pagination and layer defaults. Nit (2 votes): the handler lacks the API tracing span and propagated instrumentation.
internal/api/daylog/handler_test.go Tests layer and pagination contracts.
internal/api/daylog/events.go Builds, sorts, and pages timeline events. Critical (2 votes): HVAC values read from IntValue although production stores booleans. Critical (1 vote): enum mappings read only IntValue while production values use StrValue. Moderate (1 vote): pagination occurs after full fan-out, merge, and sort for every page.
internal/api/daylog/events_test.go Tests event assembly and paging.
internal/api/daylog/doc.go Documents the API taxonomy and pagination.
Review details

Suppressed comments (4)

internal/api/daylog/events.go:437

  • limit/offset are applied only after all source rows have been queried, edge-detected, merged, and sorted. Since the frontend now makes one request per page, a large day repeats the full database fan-out and sort for every page (up to the signal/gear caps), so this pagination does not bound server work and can hit the 15-second handler timeout. Page or cursor source reads, or cache one stable assembled snapshot for the sequence.
	total := len(events)
	limit := in.Limit
	if limit <= 0 {
		limit = dayLogDefaultLimit
	}

internal/database/daylog/repo.go:319

  • SecurityEvents intentionally returns unknown event_type rows, but this baseline query only loads the three currently known types. For an unknown/generic security row whose previous state exists before the window, BuildTimeline starts from at nil, so the new all-security-events behavior still loses the transition context. Select the latest prior row for every event type rather than applying this allowlist, and update the query arguments accordingly.
	rows, err := r.pool.Query(ctx, dayLogSecurityPrevSQL, vehicleID, windowStart, []string{"locked", "sentry_mode", "valet_mode_enabled"})

web/src/api/types.ts:3664

  • The response now documents omitted layers as “all layers,” but the DayLogLayer comment above still says layers are off by default. Update that stale alias documentation so callers do not infer the old filtered-by-default behavior.
  /** Requested optional layers (echo). Omitted upstream means all layers. */
  layers: string[]

web/src/features/vehicles/lib/daylog.ts:243

  • The UI placeholder promises time search, but this keyword set never includes event.ts (or the formatted clock time), so entering a visible timestamp such as 15:04:05 cannot match any row. Add the event timestamp in the same representation users see, or remove “time” from the placeholder.
export function eventKeywords(event: DayLogEvent): string {
  const parts: string[] = [event.type, event.source, event.layer, categoryOf(event.type)];
  const payload = event.payload ?? {};
  for (const key of ['door', 'window', 'component', 'field', 'event_type', 'from', 'to', 'gear', 'version', 'status', 'start_place', 'end_place']) {
    const v = payload[key];
    if (typeof v === 'string' && v !== '') parts.push(v);
    else if (typeof v === 'number' || typeof v === 'boolean') parts.push(String(v));
  }
  return parts.join(' ').toLowerCase();
  • Files reviewed: 19/19 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +712 to +722
fromShort, fromOK := enumShort(turnSignalShort, edge.From.IntValue)
toShort, toOK := enumShort(turnSignalShort, edge.To.IntValue)
// The direction component comes from the NEW state; the
// previous state rides in from/to for the transition line.
if toOK {
e.Payload["component"] = toShort
}
e.Payload["from"] = enumDisplay(fromShort, fromOK, edge.From.IntValue)
e.Payload["to"] = enumDisplay(toShort, toOK, edge.To.IntValue)
e.Payload["from_value"] = typedSignalValue(edge.From)
e.Payload["to_value"] = typedSignalValue(edge.To)
Comment on lines +749 to +750
toShort, toOK := enumShort(hvacShort, edge.To.IntValue)
fromShort, fromOK := enumShort(hvacShort, edge.From.IntValue)
Comment on lines +51 to +54
while (events.length < total) {
const params = new URLSearchParams(base);
params.set('offset', String(offset));
const page = await request<DayLogResponse>(`/day-log?${params.toString()}`, { signal });
Comment on lines +55 to +56
const got = page.events ?? [];
if (got.length === 0) break;
Comment on lines +75 to +78
const title = t(`dayLog.events.${e.type}`, e.type).toLowerCase();
return title.includes(query) || eventKeywords(e).includes(query);
});
}, [all, hidden, query, t]);
// means every layer: the default view is the complete history.
// Returns 200 with an empty events array for a quiet day, 404 for an
// unknown vehicle.
func (h *Handler) Get(w http.ResponseWriter, r *http.Request) {
The history-fail case can paint the playback heading before both
QueryError copies land. Wait for two Can't reach server strings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 15, 2026 23:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Event-state reconstruction defects and inefficient, inconsistent pagination remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (14)

Previously missed (2) — in code that hasn't changed since the last review.

internal/api/daylog/handler.go:300

  • These limit/offset values are applied only after the handler has queried every source and BuildTimeline has assembled and sorted the entire day. The frontend now requests one page at a time until total_events, so a large day causes the same DB fan-out, signal-cap scan, event construction, and sort to be repeated for every page; move pagination into the read model or introduce a server-side snapshot/cursor so pages reuse one assembly.
    internal/database/daylog/repo.go:319
  • The builder deliberately emits unrecognized security event types as generic events, and the security schema documents types beyond these three (for example airbag_deployed and crash_state). Restricting the pre-window baseline query to only locked, sentry_mode, and valet_mode_enabled means a generic event that is the first row of the selected day loses an available from state. Seed the baseline for every event type (or use the same complete event-type registry) and add a regression case for a generic type.

internal/api/daylog/doc.go:68

  • This new contract says signal_log stores proto enum numbers and that the timeline's numeric maps are the source of truth, but the touched production fields do not follow that shape: turn/window values are persisted in str_value and HvacPower in bool_value. This documentation both misleads API consumers and explains why the current reader misses real transitions; document the field-specific canonical types instead.
// signal_log stores proto enum NUMBERS. The timeline labels them with
// read-side display maps (turnSignalShort, windowShort, hvacShort,
// gearShortForm) that cite their generated sources
// (internal/tesla/protomodel/enum_parsers_gen.go + the proto). These
// maps do not import proto bindings and do not re-run the codec's
// prefix-trim; the parity test asserts every entry against the
// generated String() output. Unknown numbers render raw, never guessed.

internal/api/daylog/events.go:750

  • HvacPower is canonicalized to a Go bool and persisted in signal_log.bool_value, so real rows have nil IntValue here. This makes every HVAC transition fall through to hvac_off and drops its from/to states; use BoolValue (toBool/setBoolFromTo) instead of the enum map, and update the fixtures to use bool rows.
	case "HvacPower":
		{
			toShort, toOK := enumShort(hvacShort, edge.To.IntValue)
			fromShort, fromOK := enumShort(hvacShort, edge.From.IntValue)

internal/api/daylog/events.go:720

  • Fleet enum values are canonical short strings in signal_log.str_value, but this mapping only reads IntValue. Consequently real turn-signal transitions have no component and nil from/to labels instead of rendering Left/Right/Off; read StrValue (while preserving an unknown raw fallback) here.
	case "LightsTurnSignal":
		{
			e := mk("turn_signal", LayerTurnSignals)
			fromShort, fromOK := enumShort(turnSignalShort, edge.From.IntValue)
			toShort, toOK := enumShort(turnSignalShort, edge.To.IntValue)
			// The direction component comes from the NEW state; the
			// previous state rides in from/to for the transition line.
			if toOK {
				e.Payload["component"] = toShort
			}
			e.Payload["from"] = enumDisplay(fromShort, fromOK, edge.From.IntValue)
			e.Payload["to"] = enumDisplay(toShort, toOK, edge.To.IntValue)

internal/api/daylog/events.go:375

  • This new branch maps valet_mode_enabled to valet_on/valet_off, but the package contract still says ValetMode transitions are out of v1 scope and intentionally not mapped (internal/api/daylog/doc.go:85-86). Update the API taxonomy documentation to describe these emitted event types and payloads, or remove the mapping so the documented contract and response behavior agree.
		case "valet_mode_enabled":
			typ := "valet_unknown"
			if s.ToState != nil {
				switch *s.ToState {
				case "true":

internal/api/daylog/events.go:874

  • gearShortForm explicitly supports both full proto tokens and short tokens, but this comparison happens before normalization. A stored ShiftStateD followed by D is the same gear state yet produces a false gear event; compare the normalized forms and keep the raw values only for payload provenance.
		if t.Gear != prev.Gear {
			out = append(out, gearEdge{From: prev, To: t})

internal/api/daylog/handler.go:255

  • This expanded handler performs the full day-log fan-out, but it still has no otel.Tracer("api") span, span error recording, or trace ID in its error logs. That breaks the established API observability pattern used by handlers such as internal/api/drives/listing.go:21-47, making slow page requests and source failures hard to correlate; add the span at entry and record each early-return error.
	signalRows, err := h.repo.SignalRows(ctx, p.vehicleID, SignalFieldsForLayers(p.layers), p.start, p.end, dayLogSignalRowCap+1)
	if err != nil {
		log.Error().Err(err).Int64("vehicle_id", p.vehicleID).Msg("daylog: signal query failed")
		httpx.WriteError(w, http.StatusInternalServerError, "failed to load signal edges")
		return

web/src/api/hooks/useDayLog.ts:56

  • When a subsequent page is empty before total_events, this break returns a successful result containing only a partial history while leaving truncated false. The UI cannot distinguish that from a complete day, so surface a query error (or an explicit incomplete flag) for a stalled/inconsistent page instead of silently accepting missing events.
    if (got.length === 0) break;

web/src/api/hooks/useDayLog.ts:54

  • Each loop iteration issues another request, but the handler reruns the complete source fan-out and rebuilds/sorts the entire timeline before applying this offset. A multi-page day therefore repeats the same database work and O(E log E) merge for every page, leading to minutes of sequential loading and potentially exhausting the route rate limit; page a stable server-side snapshot/cursor or assemble once and page that result.
  while (events.length < total) {
    const params = new URLSearchParams(base);
    params.set('offset', String(offset));
    const page = await request<DayLogResponse>(`/day-log?${params.toString()}`, { signal });

web/src/api/hooks/useDayLog.ts:54

  • The all-pages loop uses offsets against a dataset that can change while today’s telemetry is being ingested. If a new event is inserted before the next offset, the next response can skip an existing event (or duplicate one), yet the loop still stops when it reaches the earlier total_events; use a stable server-side snapshot/cursor or another consistency mechanism for the complete-history fetch.
  let offset = events.length;
  while (events.length < total) {
    const params = new URLSearchParams(base);
    params.set('offset', String(offset));
    const page = await request<DayLogResponse>(`/day-log?${params.toString()}`, { signal });

web/src/features/vehicles/components/daylog/DayLogTimeline.tsx:76

  • The placeholder advertises time search, but eventKeywords contains no timestamp and the predicate only checks the localized title plus metadata/payload. Searching for a visible timestamp such as 08:04:05 therefore returns no match; include the event's localized display time in this predicate.
      return title.includes(query) || eventKeywords(e).includes(query);

web/src/features/vehicles/components/daylog/DayLogTimeline.tsx:99

  • Adding the virtualized list here leaves DayLogTimeline.tsx at 546 lines while it still combines filter controls, query states, virtualization, row rendering, expanded payload details, and a large event-formatting switch. The frontend component guideline caps component files at 200 lines; extract the row/details and describeEvent logic into focused files so the list remains maintainable and independently testable.
  const virtualizer = useVirtualizer({
    count: rows.length,
    getScrollElement: () => parentRef.current,
    estimateSize: () => 64,
    overscan: 12,

web/src/features/vehicles/components/daylog/DayLogTimeline.tsx:153

  • When the capped signal/gear inputs contain only baselines or unchanged samples, BuildTimeline can return no events while truncated is true. This branch renders only the empty state; the truncation warning below the non-empty branch is unreachable, so the user is told nothing was recorded without being told records were omitted. Render the cap warning in the empty branch or outside the all.length conditional.
        ) : all.length === 0 ? (
          <EmptyState
            icon={<Icons.moon className="h-8 w-8" />}
            message={t('dayLog.timeline.empty', 'Nothing recorded this day.')}
            description={t(
  • Files reviewed: 20/20 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment on lines 822 to +830
if window, ok := windowFieldSuffix[edge.Field]; ok {
e := mk("window", LayerDoorsWindow)
e.Payload["window"] = window
e.Payload["value"] = typedSignalValue(edge.Row)
fromShort, fromOK := enumShort(windowShort, edge.From.IntValue)
toShort, toOK := enumShort(windowShort, edge.To.IntValue)
e.Payload["from"] = enumDisplay(fromShort, fromOK, edge.From.IntValue)
e.Payload["to"] = enumDisplay(toShort, toOK, edge.To.IntValue)
e.Payload["from_value"] = typedSignalValue(edge.From)
e.Payload["to_value"] = typedSignalValue(edge.To)

Copilot AI commented Sep 15, 2026

Copy link
Copy Markdown

One or more custom setup steps configured for this repository failed during this Copilot code review run:

Verify TypeScript build

Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.

Note

You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.

@atulmgupta
atulmgupta merged commit 878b14e into main Sep 16, 2026
23 of 24 checks passed
@atulmgupta
atulmgupta deleted the day-log branch September 16, 2026 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants