Skip to content

Commit 7c56d76

Browse files
atulmguptaCopilot
andcommitted
fix(api): /signals/observations latest-first + unbounded since
The driving-dynamics cold-signal panels (G-Force, Pedal Usage, Autopilot & Cruise) and the SignalLogWidget event feed surfaced empty / stale values because /signals/observations had two coordinated bugs: 1. The repo ordered ASC by ts. The frontend latestNumeric() helper reads data[0] expecting the most recent observation, so callers asking with limit:1 got the OLDEST row in the window, and SignalLogWidget showed the oldest 20 events of the day instead of the most recent 20. 2. The handler defaulted since to now-24h. Cold signals (Lateral/Long Acceleration, PedalPosition, BrakePedalPos, BrakePedal, CruiseSetSpeed, CruiseFollowDistance) only emit while driving, so the panels went empty when the car had been parked > 24h despite the data being on disk. Fix: - Repo: ListByName + ListByVehicle now ORDER BY ts DESC and accept zero time.Time{} bounds to mean 'unbounded' on that side. Extracted the SQL composition into buildObservationQuery (covered by 5 new unit tests). The (vehicle_id, signal_name, ts DESC) index serves the bounded LIMIT case in index-only fashion. - Handler: omit the lower/upper bound from the predicate when since/until query params are absent. Reject non-RFC3339 since/until with 400 instead of silently widening to 'all history' (subtle behavior regression flagged by rubber-duck review). Affected callers (all want latest-first): GForcePanel, PedalUsage, AutopilotSection, SignalLogWidget, PowersharePage. SignalCatalogWidget's per-signal counts now reflect 'recently active signals' (newest 100 of all-time) instead of 'oldest 100 of last 24h' — strictly more useful for the widget's UX. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 303aeb7 commit 7c56d76

3 files changed

Lines changed: 146 additions & 27 deletions

File tree

internal/api/signal_catalog_handler.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,17 @@ func (h *SignalCatalogHandler) ListCatalog(w http.ResponseWriter, r *http.Reques
9696
// Query params:
9797
// - vehicle_id (required): int64
9898
// - signal_name (optional): when set, narrows to one signal name
99-
// - since / until (optional, RFC3339): time window (default: last 24h)
99+
// - since / until (optional, RFC3339): time window. When omitted the
100+
// corresponding bound is unconstrained, so callers asking for `limit=1`
101+
// get the most recent observation regardless of age — required for the
102+
// cold-signal panels (G-force / pedals / cruise) on /driving-dynamics
103+
// where signals only emit while driving and may be hours-to-days old.
104+
// Invalid RFC3339 values produce 400 instead of being silently ignored.
100105
// - limit (optional): cap, 1..1000, default 100
101106
//
107+
// Results are ordered most recent first (ts DESC) — the frontend
108+
// `latestNumeric()` helper reads `data[0]` as the latest reading.
109+
//
102110
// Used by SignalLogWidget, SignalCatalogWidget, PowersharePage, and the
103111
// driving-dynamics components.
104112
func (h *SignalCatalogHandler) ListObservations(w http.ResponseWriter, r *http.Request) {
@@ -118,18 +126,22 @@ func (h *SignalCatalogHandler) ListObservations(w http.ResponseWriter, r *http.R
118126
limit = 1000
119127
}
120128

121-
now := time.Now().UTC()
122-
since := now.Add(-24 * time.Hour)
123-
until := now
129+
var since, until time.Time
124130
if s := q.Get("since"); s != "" {
125-
if t, err := time.Parse(time.RFC3339, s); err == nil {
126-
since = t
131+
t, err := time.Parse(time.RFC3339, s)
132+
if err != nil {
133+
writeError(w, http.StatusBadRequest, "since must be RFC3339")
134+
return
127135
}
136+
since = t
128137
}
129138
if u := q.Get("until"); u != "" {
130-
if t, err := time.Parse(time.RFC3339, u); err == nil {
131-
until = t
139+
t, err := time.Parse(time.RFC3339, u)
140+
if err != nil {
141+
writeError(w, http.StatusBadRequest, "until must be RFC3339")
142+
return
132143
}
144+
until = t
133145
}
134146

135147
signalName := q.Get("signal_name")

internal/database/signal_observation_repo.go

Lines changed: 55 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package database
33
import (
44
"context"
55
"fmt"
6+
"strings"
67
"time"
78

89
"github.com/jackc/pgx/v5"
@@ -16,6 +17,37 @@ type SignalObservationRepo struct {
1617
db *DB
1718
}
1819

20+
// buildObservationQuery composes the SELECT for ListByVehicle / ListByName.
21+
// signalName == "" omits the signal_name predicate. from.IsZero() omits the
22+
// lower time bound. to.IsZero() omits the upper time bound. ORDER BY is
23+
// always ts DESC (newest first) so callers reading data[0] get the latest
24+
// observation. The returned args slice is positional and matches the $N
25+
// placeholders in the query string.
26+
func buildObservationQuery(vehicleID int64, signalName string, from, to time.Time, limit int) (string, []any) {
27+
args := []any{vehicleID}
28+
var b strings.Builder
29+
b.WriteString(`SELECT vehicle_id, ts, signal_name, value_numeric, value_text, value_bool, source
30+
FROM signal_observations
31+
WHERE vehicle_id = $1`)
32+
33+
if signalName != "" {
34+
args = append(args, signalName)
35+
fmt.Fprintf(&b, ` AND signal_name = $%d`, len(args))
36+
}
37+
if !from.IsZero() {
38+
args = append(args, from)
39+
fmt.Fprintf(&b, ` AND ts >= $%d`, len(args))
40+
}
41+
if !to.IsZero() {
42+
args = append(args, to)
43+
fmt.Fprintf(&b, ` AND ts <= $%d`, len(args))
44+
}
45+
46+
args = append(args, limit)
47+
fmt.Fprintf(&b, ` ORDER BY ts DESC LIMIT $%d`, len(args))
48+
return b.String(), args
49+
}
50+
1951
// NewSignalObservationRepo constructs a SignalObservationRepo bound to db.
2052
func NewSignalObservationRepo(db *DB) *SignalObservationRepo {
2153
return &SignalObservationRepo{db: db}
@@ -88,17 +120,21 @@ func (r *SignalObservationRepo) GetLatest(ctx context.Context, vehicleID int64,
88120
return &o, nil
89121
}
90122

91-
// ListByVehicle returns signal observations for a vehicle within the inclusive
92-
// time window [from, to], ordered by ts ASC and capped by limit.
123+
// ListByVehicle returns signal observations for a vehicle, ordered most
124+
// recent first (ts DESC) and capped by limit. Time bounds [from, to] are
125+
// applied only when non-zero; passing time.Time{} for either bound omits
126+
// that side of the predicate so callers can request "the latest N
127+
// observations regardless of age" (used by the cold-signal panels on
128+
// /driving-dynamics, the SignalLogWidget event feed, etc.).
129+
//
130+
// DESC ordering matches the frontend `latestNumeric()` helper which reads
131+
// `data[0]` as "most recent". Switching from ASC to DESC also exploits
132+
// the (vehicle_id, signal_name, ts DESC) compression order so the
133+
// hypertable can satisfy LIMIT-bounded scans without an extra sort.
93134
func (r *SignalObservationRepo) ListByVehicle(ctx context.Context, vehicleID int64, from, to time.Time, limit int) ([]models.SignalObservation, error) {
94-
const query = `
95-
SELECT vehicle_id, ts, signal_name, value_numeric, value_text, value_bool, source
96-
FROM signal_observations
97-
WHERE vehicle_id = $1 AND ts BETWEEN $2 AND $3
98-
ORDER BY ts ASC
99-
LIMIT $4`
135+
query, args := buildObservationQuery(vehicleID, "", from, to, limit)
100136

101-
rows, err := r.db.Pool.Query(ctx, query, vehicleID, from, to, limit)
137+
rows, err := r.db.Pool.Query(ctx, query, args...)
102138
if err != nil {
103139
return nil, fmt.Errorf("signal-observations-repo-list-by-vehicle: %w", err)
104140
}
@@ -127,18 +163,18 @@ func (r *SignalObservationRepo) ListByVehicle(ctx context.Context, vehicleID int
127163
}
128164

129165
// ListByName returns signal observations for a vehicle filtered by signal
130-
// name within the inclusive time window [from, to], ordered by ts ASC and
131-
// capped by limit. signal_name is the FK into signal_catalog.name (ADR-009),
132-
// so an explicit join is unnecessary for filtering.
166+
// name, ordered most recent first (ts DESC) and capped by limit. Time bounds
167+
// [from, to] are applied only when non-zero; passing time.Time{} for either
168+
// bound omits that side of the predicate. signal_name is the FK into
169+
// signal_catalog.name (ADR-009), so an explicit join is unnecessary for
170+
// filtering.
171+
//
172+
// The (vehicle_id, signal_name, ts DESC) idx_signal_obs_vehicle_signal_ts
173+
// index serves this query in index-only fashion for any limit.
133174
func (r *SignalObservationRepo) ListByName(ctx context.Context, vehicleID int64, name string, from, to time.Time, limit int) ([]models.SignalObservation, error) {
134-
const query = `
135-
SELECT vehicle_id, ts, signal_name, value_numeric, value_text, value_bool, source
136-
FROM signal_observations
137-
WHERE vehicle_id = $1 AND signal_name = $2 AND ts BETWEEN $3 AND $4
138-
ORDER BY ts ASC
139-
LIMIT $5`
175+
query, args := buildObservationQuery(vehicleID, name, from, to, limit)
140176

141-
rows, err := r.db.Pool.Query(ctx, query, vehicleID, name, from, to, limit)
177+
rows, err := r.db.Pool.Query(ctx, query, args...)
142178
if err != nil {
143179
return nil, fmt.Errorf("signal-observations-repo-list-by-name: %w", err)
144180
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package database
2+
3+
import (
4+
"strconv"
5+
"strings"
6+
"testing"
7+
"time"
8+
)
9+
10+
// Regression coverage for buildObservationQuery — the SQL builder behind
11+
// SignalObservationRepo.ListByName and ListByVehicle. The shape of these
12+
// queries is the contract the cold-signal panels (G-force / pedals / cruise
13+
// on /driving-dynamics) and SignalLogWidget rely on; getting it wrong
14+
// renders the panels empty or stale.
15+
16+
func TestBuildObservationQuery_ByNameAlwaysDESC(t *testing.T) {
17+
q, _ := buildObservationQuery(42, "PedalPosition", time.Time{}, time.Time{}, 1)
18+
if !strings.Contains(q, "ORDER BY ts DESC") {
19+
t.Fatalf("expected ORDER BY ts DESC, got %q", q)
20+
}
21+
if strings.Contains(q, "ORDER BY ts ASC") {
22+
t.Fatalf("must not order ASC — frontend reads data[0] as latest: %q", q)
23+
}
24+
}
25+
26+
func TestBuildObservationQuery_OmitsTimeBoundsWhenZero(t *testing.T) {
27+
q, args := buildObservationQuery(42, "PedalPosition", time.Time{}, time.Time{}, 1)
28+
if strings.Contains(q, "ts >=") || strings.Contains(q, "ts <=") || strings.Contains(q, "BETWEEN") {
29+
t.Fatalf("zero time bounds must omit ts predicates entirely, got %q", q)
30+
}
31+
// args = [vehicle_id, signal_name, limit]
32+
if len(args) != 3 {
33+
t.Fatalf("expected 3 args [vehicle, signal_name, limit], got %d: %v", len(args), args)
34+
}
35+
}
36+
37+
func TestBuildObservationQuery_AppliesBothTimeBoundsWhenSet(t *testing.T) {
38+
from := time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC)
39+
to := time.Date(2026, 5, 2, 0, 0, 0, 0, time.UTC)
40+
q, args := buildObservationQuery(42, "PedalPosition", from, to, 100)
41+
if !strings.Contains(q, "ts >=") || !strings.Contains(q, "ts <=") {
42+
t.Fatalf("expected both ts >= and ts <= predicates, got %q", q)
43+
}
44+
// args = [vehicle_id, signal_name, from, to, limit]
45+
if len(args) != 5 {
46+
t.Fatalf("expected 5 args, got %d: %v", len(args), args)
47+
}
48+
}
49+
50+
func TestBuildObservationQuery_OmitsSignalNameWhenEmpty(t *testing.T) {
51+
q, args := buildObservationQuery(42, "", time.Time{}, time.Time{}, 20)
52+
if strings.Contains(q, "signal_name =") {
53+
t.Fatalf("empty signal_name must omit signal_name predicate, got %q", q)
54+
}
55+
// args = [vehicle_id, limit]
56+
if len(args) != 2 {
57+
t.Fatalf("expected 2 args [vehicle, limit], got %d: %v", len(args), args)
58+
}
59+
}
60+
61+
func TestBuildObservationQuery_PlaceholdersAreSequential(t *testing.T) {
62+
from := time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC)
63+
q, args := buildObservationQuery(42, "PedalPosition", from, time.Time{}, 50)
64+
// Expect $1=vehicle, $2=signal, $3=from, $4=limit
65+
for i := 1; i <= len(args); i++ {
66+
marker := "$" + strconv.Itoa(i)
67+
if !strings.Contains(q, marker) {
68+
t.Fatalf("expected placeholder %s in query, got %q", marker, q)
69+
}
70+
}
71+
}

0 commit comments

Comments
 (0)