From 62c2cc8a7bec5db3ad231b793b91a9bc7ca31860 Mon Sep 17 00:00:00 2001 From: Suriyakumarvijayanayagam Date: Thu, 24 Sep 2026 13:44:51 +0530 Subject: [PATCH] A visit is #1042, not 4cc216ca-dad3-4958-bb96-5f5a82022cf8 Every other thing in this product a person refers to already had a readable reference: a shop is chennai, a camera cam1, a customer V-42, a person their email. An audit of every list response found exactly one gap, and it was the row people look at most - the arrivals feed showed a visit as 36 hex characters. 012 argued no route takes a visit id so none was needed. That is true of routing and false of everything else: it is what the feed shows, what a support conversation quotes, and what somebody reading an API response judges the product by. Migration 014 mirrors the visitor scheme exactly - per client, so it discloses no platform-wide volume, and beside the uuid rather than instead of it. A stored counter is affordable on the busiest table because visits from one tenant are already serialised by the consumer's SetOrderMatters(true), so it adds no contention that was not already there. A derived reference was the alternative and does not work: several people through one door share occurred_at to the microsecond, which is the collision 004 exists to handle. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KGcjxF1cNLcuwc3DAPcnfj --- server/internal/api/refs.go | 14 +++++++ server/internal/api/refs_test.go | 15 +++++++ server/internal/api/types.go | 5 +++ server/internal/store/api_arrivals.go | 7 ++-- server/internal/store/store.go | 9 ++++- server/migrations/014_visit_numbers.sql | 52 +++++++++++++++++++++++++ 6 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 server/migrations/014_visit_numbers.sql diff --git a/server/internal/api/refs.go b/server/internal/api/refs.go index d184325..b46373a 100644 --- a/server/internal/api/refs.go +++ b/server/internal/api/refs.go @@ -47,6 +47,20 @@ func VisitorRef(number int64) string { return VisitorRefPrefix + strconv.FormatInt(number, 10) } +// VisitRefPrefix marks a visit reference. "#" rather than a letter because a +// visit is a numbered event, not a named thing, and it reads correctly in a +// sentence: "visit #1042 at chennai". +const VisitRefPrefix = "#" + +// VisitRef is what a person quotes for one visit. Empty for a visit recorded +// before 014, which had no number - absent rather than wrong. +func VisitRef(number int64) string { + if number <= 0 { + return "" + } + return VisitRefPrefix + strconv.FormatInt(number, 10) +} + // ParseVisitorRef accepts "V-42", "v-42" and bare "42". // // Bare digits are accepted because a shop assistant reading a number off a diff --git a/server/internal/api/refs_test.go b/server/internal/api/refs_test.go index b4b56af..f514545 100644 --- a/server/internal/api/refs_test.go +++ b/server/internal/api/refs_test.go @@ -140,3 +140,18 @@ func TestAnArrivalCarriesTheShopReferenceItCanBeFilteredBy(t *testing.T) { t.Fatalf("the platform-wide visit counter leaked into the feed: %s", body) } } + +// A visit reference is what a person quotes; the uuid is what a machine +// de-duplicates on. Both travel, neither replaces the other. +func TestVisitRefIsReadableAndAbsentWhenUnnumbered(t *testing.T) { + if got := VisitRef(1042); got != "#1042" { + t.Errorf("VisitRef(1042) = %q, want #1042", got) + } + // Visits recorded before 014 have no number. Absent, never "#0" - a + // reference that looks real and is not is worse than none. + for _, n := range []int64{0, -1} { + if got := VisitRef(n); got != "" { + t.Errorf("VisitRef(%d) = %q, want empty", n, got) + } + } +} diff --git a/server/internal/api/types.go b/server/internal/api/types.go index 966a0e3..573f91d 100644 --- a/server/internal/api/types.go +++ b/server/internal/api/types.go @@ -268,6 +268,11 @@ type AgentPrincipal struct { // audit log to do it. type Arrival struct { VisitID string `json:"visit_id"` + // VisitRef is what a person quotes - "#1042" - per client, beside the uuid + // rather than instead of it. Every other thing in the product a person + // refers to has one: a shop is `chennai`, a camera `cam1`, a customer + // `V-42`. A visit had only 36 hex characters. + VisitRef string `json:"visit_ref,omitempty"` // Seq is this visit's position in the feed - assigned by the server when it // learned of the visit, not by the camera. It drives the cursor and the // ordering, and it is `json:"-"` on purpose. diff --git a/server/internal/store/api_arrivals.go b/server/internal/store/api_arrivals.go index 9a9e67b..91bbf18 100644 --- a/server/internal/store/api_arrivals.go +++ b/server/internal/store/api_arrivals.go @@ -12,7 +12,7 @@ import ( // mean the first poll of a feed and every poll after it returned different // shapes, which is the kind of bug that only shows up under load. const arrivalColumns = ` - vi.id::text, vi.seq, vi.occurred_at, vi.site_id::text, si.name, si.slug, vi.camera_id, + vi.id::text, vi.seq, COALESCE(vi.number, 0), vi.occurred_at, vi.site_id::text, si.name, si.slug, vi.camera_id, vi.is_new_visitor, vi.similarity, vi.quality, vi.attributes, vi.image_key, COALESCE(vi.visitor_id::text, ''), COALESCE(vs.number, 0), @@ -112,13 +112,14 @@ func (s *Store) Arrivals(ctx context.Context, q api.ArrivalQuery) ([]api.Arrival var at time.Time var sim, qual *float64 var imageKey string - var number int64 - if err := rows.Scan(&a.VisitID, &a.Seq, &at, &a.SiteID, &a.Site, &a.SiteSlug, &a.CameraID, + var number, visitNumber int64 + if err := rows.Scan(&a.VisitID, &a.Seq, &visitNumber, &at, &a.SiteID, &a.Site, &a.SiteSlug, &a.CameraID, &a.IsNew, &sim, &qual, &a.Attributes, &imageKey, &a.VisitorID, &number, &a.Label, &a.Name); err != nil { return nil, err } a.VisitorRef = api.VisitorRef(number) + a.VisitRef = api.VisitRef(visitNumber) a.OccurredAt = at.UTC().Format(time.RFC3339Nano) if sim != nil { a.Similarity = *sim diff --git a/server/internal/store/store.go b/server/internal/store/store.go index 17a509d..528dd36 100644 --- a/server/internal/store/store.go +++ b/server/internal/store/store.go @@ -113,10 +113,15 @@ func (s *Store) RecordVisit(ctx context.Context, site ingest.Site, // visitor for a visit we already recorded. var visitID string err = tx.QueryRow(ctx, ` + WITH n AS ( + UPDATE clients SET visit_seq = visit_seq + 1 + WHERE id = $1 RETURNING visit_seq + ) INSERT INTO visits (client_id, site_id, source_event_id, occurred_at, camera_id, is_new_visitor, similarity, quality, - attributes, image_key) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, COALESCE($9, '{}'::jsonb), $10) + attributes, image_key, number) + SELECT $1, $2, $3, $4, $5, $6, $7, $8, COALESCE($9, '{}'::jsonb), $10, n.visit_seq + FROM n ON CONFLICT (client_id, source_event_id) DO NOTHING RETURNING id::text`, site.ClientID, site.SiteID, v.EventID, v.OccurredAt, v.CameraID, diff --git a/server/migrations/014_visit_numbers.sql b/server/migrations/014_visit_numbers.sql new file mode 100644 index 0000000..d8493a1 --- /dev/null +++ b/server/migrations/014_visit_numbers.sql @@ -0,0 +1,52 @@ +-- A visit reference a person can use, beside the key a machine uses. +-- +-- Every other thing in this product a person refers to already has one: a shop +-- is `chennai`, a camera is `cam1`, a customer is `V-42`, a person is their +-- email. A visit had only its uuid: +-- +-- "visit_id": "4cc216ca-dad3-4958-bb96-5f5a82022cf8" +-- +-- 012 argued that this was acceptable because no route takes a visit id and +-- nobody says one out loud. That is true of routing and false of everything +-- else: it is what the arrivals feed shows, what a support conversation has to +-- quote, and what somebody reading an API response judges the product by. The +-- owner asked for it twice. +-- +-- `#1042`, per client, mirroring `V-42` exactly and for the same three reasons +-- (speakable, per-tenant so it discloses no platform-wide volume, and a +-- reference beside the key rather than a replacement for it - eleven tables +-- reference visits.id). +-- +-- Why a stored counter is affordable on the hottest table in the schema: +-- allocating it row-locks the client for the length of one insert, and visits +-- from one tenant are ALREADY serialised - the MQTT consumer sets +-- SetOrderMatters(true) precisely so that `seq` is a commit order. So this +-- adds no contention a tenant did not already have, and tenants never block +-- each other. A derived reference was the alternative and does not work: +-- several people through one door share occurred_at to the microsecond, which +-- is the very collision 004 exists to handle. + +ALTER TABLE clients ADD COLUMN IF NOT EXISTS visit_seq bigint NOT NULL DEFAULT 0; +ALTER TABLE visits ADD COLUMN IF NOT EXISTS number bigint; + +-- Existing rows get their numbers in the order the server learned of them, +-- which is what `seq` means - not occurred_at, which is the camera's clock and +-- arrives out of order after a site has been offline. +WITH numbered AS ( + SELECT id, row_number() OVER (PARTITION BY client_id ORDER BY seq) AS n + FROM visits +) +UPDATE visits v SET number = numbered.n + FROM numbered + WHERE v.id = numbered.id AND v.number IS NULL; + +UPDATE clients c + SET visit_seq = GREATEST(c.visit_seq, COALESCE( + (SELECT max(number) FROM visits WHERE client_id = c.id), 0)); + +CREATE UNIQUE INDEX IF NOT EXISTS visits_client_number_idx + ON visits (client_id, number); + +COMMENT ON COLUMN visits.number IS + 'Per-client visit number, shown as #1042. A public reference beside the ' + 'uuid key, never a replacement for it.';