Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a persistent geocoding cache to reduce repeated Mapbox forward-geocode requests during address-based geocoding, plus a CLI backfill command and feature tests.
Changes:
- Introduces a new
geocodeCachetable/model/migration and wires it into the Kysely DB types. - Updates address geocoding to consult and populate the cache (4-week TTL).
- Adds a
populateGeocodeCacheCLI command and a Vitest suite covering cache hit + expiry behaviour.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/resources/addresses.csv | Adds address CSV fixture data. |
| tests/feature/geocodeCache.test.ts | Adds integration tests validating cache hit and expiry behaviour. |
| src/server/services/database/schema.ts | Documents new geocodeCache table and exposes it on the DB schema interface. |
| src/server/services/database/index.ts | Registers geocodeCache table type in the Kysely Database interface. |
| src/server/models/GeocodeCache.ts | Adds Kysely table typing for the cache table. |
| src/server/mapping/geocode.ts | Implements read-through/write-through caching for Mapbox address geocoding. |
| src/server/commands/populateGeocodeCache.ts | Adds command to backfill cache from existing address-geocoded data sources. |
| migrations/1774658921006_geocode_cache.ts | Creates the geocodeCache table. |
| bin/cmd.ts | Exposes the new populateGeocodeCache command via CLI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+345
to
+349
| logger.debug(`Geocode cache hit for "${address}"`); | ||
| return cached.point; | ||
| } | ||
|
|
||
| logger.info(`Geocode cache miss for "${address}", calling Mapbox API`); |
Comment on lines
+29
to
+40
| const entries: { address: string; point: Point | null }[] = []; | ||
| for (const record of records) { | ||
| const json = record.json as Record<string, unknown>; | ||
| const address = addressColumns | ||
| .map((c) => json[c] || "") | ||
| .filter(Boolean) | ||
| .join(", ") | ||
| .trim(); | ||
| if (!address) continue; | ||
|
|
||
| entries.push({ address, point: record.geocodePoint as Point | null }); | ||
| } |
Comment on lines
+1
to
+4
| import { sql, type Kysely } from "kysely"; | ||
|
|
||
| export async function up(db: Kysely<any>): Promise<void> { | ||
| await db.schema |
Comment on lines
+336
to
+347
| const mapboxGeocode = async (address: string): Promise<Point | null> => { | ||
| const cached = await db | ||
| .selectFrom("geocodeCache") | ||
| .select("point") | ||
| .where("address", "=", address) | ||
| .where("createdAt", ">", sql<Date>`now() - interval '4 weeks'`) | ||
| .executeTakeFirst(); | ||
|
|
||
| if (cached) { | ||
| logger.debug(`Geocode cache hit for "${address}"`); | ||
| return cached.point; | ||
| } |
Comment on lines
+53
to
+65
| let inserted = 0; | ||
| for (let i = 0; i < deduplicated.length; i += batchSize) { | ||
| const batch = deduplicated.slice(i, i + batchSize); | ||
| await db | ||
| .insertInto("geocodeCache") | ||
| .values(batch) | ||
| .onConflict((oc) => oc.column("address").doNothing()) | ||
| .execute(); | ||
| inserted += batch.length; | ||
| } | ||
|
|
||
| logger.info( | ||
| ` ${dataSource.name}: inserted ${inserted} cache entries from ${records.length} records`, |
| await db.schema | ||
| .createTable("geocodeCache") | ||
| .addColumn("address", "text", (col) => col.primaryKey()) | ||
| .addColumn("point", sql`geography`) |
|
|
||
| export interface GeocodeCacheTable { | ||
| address: string; | ||
| point: Point | null; |
Comment on lines
+368
to
+383
| const point: Point | null = results.features?.length | ||
| ? { | ||
| lng: results.features[0].geometry.coordinates[0], | ||
| lat: results.features[0].geometry.coordinates[1], | ||
| } | ||
| : null; | ||
|
|
||
| await db | ||
| .insertInto("geocodeCache") | ||
| .values({ address, point }) | ||
| .onConflict((oc) => | ||
| oc.column("address").doUpdateSet({ point, createdAt: sql`now()` }), | ||
| ) | ||
| .execute(); | ||
|
|
||
| return point; |
45cc8c2 to
386a6eb
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.