From 26f70cc3f19385e9419b0d06666d79fc3db6aa5c Mon Sep 17 00:00:00 2001 From: ev <4164774+ev-sc@users.noreply.github.com> Date: Wed, 23 Sep 2026 00:08:26 +0200 Subject: [PATCH 1/5] Index data_record postcode look-ups FilterType.EXACT on the postcode column was a sequential scan over every record in the data source. The expression index matches the filter's lower(json->>'postcode') exactly, so the planner uses it with no code change. Co-Authored-By: Claude Opus 5.5 --- ...790100000000_data_record_postcode_index.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 migrations/1790100000000_data_record_postcode_index.ts diff --git a/migrations/1790100000000_data_record_postcode_index.ts b/migrations/1790100000000_data_record_postcode_index.ts new file mode 100644 index 00000000..b98adeb0 --- /dev/null +++ b/migrations/1790100000000_data_record_postcode_index.ts @@ -0,0 +1,28 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { type Kysely, sql } from "kysely"; + +/** + * Index postcode look-ups on data_record. + * + * The expression matches what FilterType.EXACT generates in + * src/server/repositories/DataRecord.ts — lower(json->>column) = search — so + * `?filter={"type":"EXACT","column":"postcode","search":"LS1 1AA"}` becomes an + * index scan instead of a sequential scan over every record in the source. + * + * Deliberately NOT a partial index (WHERE json ? 'postcode'): the EXACT + * predicate doesn't imply that condition, so the planner can't use a partial + * index for it and silently falls back to the sequential scan. + * + * Not CONCURRENTLY, because migrations run inside a transaction. Building it + * blocks writes to data_record (not reads) for the duration of the build. + */ +export async function up(db: Kysely): Promise { + await sql` + CREATE INDEX IF NOT EXISTS data_record_postcode_idx + ON data_record (data_source_id, (lower(json->>'postcode'))); + `.execute(db); +} + +export async function down(db: Kysely): Promise { + await sql`DROP INDEX IF EXISTS data_record_postcode_idx;`.execute(db); +} From 65fc3e4f8053c554ab764002e3d03751b9f094fc Mon Sep 17 00:00:00 2001 From: ev <4164774+ev-sc@users.noreply.github.com> Date: Wed, 23 Sep 2026 00:10:02 +0200 Subject: [PATCH 2/5] Don't geocode blank coordinates to 0,0 Number("") is 0, so a Coordinates-geocoded record with empty latitude and longitude cells passed the isNaN check and was placed at 0,0 in the Gulf of Guinea. Treat blank values as missing, so the record is left ungeocoded. Co-Authored-By: Claude Opus 5.5 --- src/server/mapping/geocode.ts | 16 ++++++++++++++-- tests/unit/server/mapping/geocode.test.ts | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/server/mapping/geocode.ts b/src/server/mapping/geocode.ts index 605a10cd..5c6dcc03 100644 --- a/src/server/mapping/geocode.ts +++ b/src/server/mapping/geocode.ts @@ -244,8 +244,20 @@ const geocodeRecordByCoordinates = async ( throw new Error(`Missing longitude column "${longitudeColumn}" in row`); } - const lat = Number(dataRecordJson[latitudeColumn]); - const lng = Number(dataRecordJson[longitudeColumn]); + const rawLat = dataRecordJson[latitudeColumn]; + const rawLng = dataRecordJson[longitudeColumn]; + // Number("") and Number(null) are 0, which would silently place a record + // with no coordinates at 0,0 in the Gulf of Guinea. + const isBlank = (v: unknown) => + v === null || v === undefined || String(v).trim() === ""; + if (isBlank(rawLat) || isBlank(rawLng)) { + throw new Error( + `Missing coordinates: latitude=${rawLat}, longitude=${rawLng}`, + ); + } + + const lat = Number(rawLat); + const lng = Number(rawLng); if (isNaN(lat) || isNaN(lng)) { throw new Error( diff --git a/tests/unit/server/mapping/geocode.test.ts b/tests/unit/server/mapping/geocode.test.ts index b38484fb..6f24e87f 100644 --- a/tests/unit/server/mapping/geocode.test.ts +++ b/tests/unit/server/mapping/geocode.test.ts @@ -27,4 +27,24 @@ describe("geocode", () => { expect(result?.centralPoint?.lng).toBeCloseTo(-0.8, 0); expect(result?.areas[AreaSetCode.PC]).toBe("HP20 2QB"); }); + + test("geocodeRecord by coordinates returns null for blank coordinates rather than 0,0", async () => { + const geocodingConfig = { + type: GeocodingType.Coordinates as const, + latitudeColumn: "latitude", + longitudeColumn: "longitude", + }; + + for (const [latitude, longitude] of [ + ["", ""], + ["", "-1.5"], + [null, null], + ]) { + const result = await geocodeRecord( + { externalId: "test-blank", json: { latitude, longitude } }, + geocodingConfig, + ); + expect(result).toBeNull(); + } + }); }); From 703a61d6464b012061c8be9c96cca51e5825451e Mon Sep 17 00:00:00 2001 From: ev <4164774+ev-sc@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:47:02 +0200 Subject: [PATCH 3/5] Document the Turbopack Google Fonts cache failure in dev Turbopack can cache a Google Fonts response that uses dynamic-subset /l/font?kit=...&skey=... URLs, which its font loader can't parse. In dev the resulting compile error makes every route, API included, return 500. Clearing .next/dev fixes it. Co-Authored-By: Claude Opus 5.5 --- README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/README.md b/README.md index e9a5634d..dbcee9a7 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,21 @@ 3. Open the dev server at https://localhost:3000 4. Log in with username `hello@commonknowledge.coop` and password `1234`. +### Troubleshooting: every page and API route returns 500 in dev + +If `npm run dev` fails with `Can't resolve '@vercel/turbopack-next/internal/font/google/font'` +and `next/font/google queries have exactly one entry`, Turbopack has cached a Google +Fonts response that uses dynamic-subset URLs (`fonts.gstatic.com/l/font?kit=…&skey=…`). +It can't parse the `&` in those URLs, and in dev that one compile error turns every +route into a 500, the REST API included. Stop the dev server, delete the cache and +start again: + +```bash +rm -rf .next/dev +``` + +`next dev --webpack` isn't affected, so it also works as a stopgap. + ### Migrations - Create with `npm run kysely migrate:make [name]` From dde74241eae3a3b6a00c3c957b4d3b62d99ae1fc Mon Sep 17 00:00:00 2001 From: ev <4164774+ev-sc@users.noreply.github.com> Date: Wed, 23 Sep 2026 08:29:39 +0200 Subject: [PATCH 4/5] Let the postcode index be pre-built CONCURRENTLY Document the manual CONCURRENTLY build for deploys that can't take the brief write block, and drop an INVALID index left by a failed concurrent build instead of letting IF NOT EXISTS accept it. Co-Authored-By: Claude Opus 5.5 --- ...790100000000_data_record_postcode_index.ts | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/migrations/1790100000000_data_record_postcode_index.ts b/migrations/1790100000000_data_record_postcode_index.ts index b98adeb0..40f819ad 100644 --- a/migrations/1790100000000_data_record_postcode_index.ts +++ b/migrations/1790100000000_data_record_postcode_index.ts @@ -14,9 +14,29 @@ import { type Kysely, sql } from "kysely"; * index for it and silently falls back to the sequential scan. * * Not CONCURRENTLY, because migrations run inside a transaction. Building it - * blocks writes to data_record (not reads) for the duration of the build. + * blocks writes to data_record (not reads) for the duration of the build: + * 3.3 s on a 7 GB, 1.95M-row data_record locally. To avoid even that, build it + * by hand before deploying and this migration becomes a no-op: + * + * CREATE INDEX CONCURRENTLY IF NOT EXISTS data_record_postcode_idx + * ON data_record (data_source_id, (lower(json->>'postcode'))); + * + * A failed CONCURRENTLY build leaves an INVALID index behind under the same + * name, which IF NOT EXISTS would silently accept, so drop that first. */ export async function up(db: Kysely): Promise { + await sql` + DO $$ + BEGIN + IF EXISTS ( + SELECT 1 FROM pg_index i + JOIN pg_class c ON c.oid = i.indexrelid + WHERE c.relname = 'data_record_postcode_idx' AND NOT i.indisvalid + ) THEN + DROP INDEX data_record_postcode_idx; + END IF; + END $$; + `.execute(db); await sql` CREATE INDEX IF NOT EXISTS data_record_postcode_idx ON data_record (data_source_id, (lower(json->>'postcode'))); From 4b0bc1d37a676a183126163eef212039dedbb97c Mon Sep 17 00:00:00 2001 From: ev <4164774+ev-sc@users.noreply.github.com> Date: Wed, 23 Sep 2026 09:01:07 +0200 Subject: [PATCH 5/5] Drop the duplicate data_record covering index Production has idx_data_record_source_id_covering, a hand-made index identical to the migration-managed data_record_source_id_covering_id (btree (data_source_id) INCLUDE (id), 350 MB each). It is the only exact duplicate in the production database. Dropping it frees ~350 MB and one index's write cost. IF EXISTS, as no other environment has it. Co-Authored-By: Claude Opus 5.5 --- ...op_duplicate_data_record_covering_index.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 migrations/1790200000000_drop_duplicate_data_record_covering_index.ts diff --git a/migrations/1790200000000_drop_duplicate_data_record_covering_index.ts b/migrations/1790200000000_drop_duplicate_data_record_covering_index.ts new file mode 100644 index 00000000..6c676f2b --- /dev/null +++ b/migrations/1790200000000_drop_duplicate_data_record_covering_index.ts @@ -0,0 +1,35 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { type Kysely, sql } from "kysely"; + +/** + * Drop a duplicate of data_record_source_id_covering_id. + * + * Production has two identical indexes, both + * btree (data_source_id) INCLUDE (id), 350 MB each: + * + * data_record_source_id_covering_id - created by migration + * 1770310811295_data_record_id_covering_index + * idx_data_record_source_id_covering - created by hand; it appears nowhere + * in this repo's history + * + * Identical definitions give the planner nothing to choose between. The + * hand-made one had 7 scans against 3,123 for the migration one (stats since + * 2026-04-08), and it backs no constraint. Dropping it frees ~350 MB and + * removes one index to maintain on every data_record write. + * + * IF EXISTS: other environments never had it. + * + * DROP INDEX takes a brief ACCESS EXCLUSIVE lock on data_record. The drop + * itself is instant, but it has to wait for running queries on the table to + * finish, and the migration CLI's 10 s lock_timeout bounds that wait. + */ +export async function up(db: Kysely): Promise { + await sql`DROP INDEX IF EXISTS idx_data_record_source_id_covering;`.execute( + db, + ); +} + +export async function down(): Promise { + // Nothing to restore: the migration-managed data_record_source_id_covering_id + // is the same index, and recreating the duplicate would reintroduce the waste. +}