From cb6b55aa6dd3a27ef550e578e138d492a4e6daa1 Mon Sep 17 00:00:00 2001 From: Gray Fay Date: Mon, 14 Sep 2026 00:10:52 -0400 Subject: [PATCH] fix: attribute shoreline addresses, and name the rows that cannot be placed Migration 23 attributed 985,631 of 985,634 blank regions in production and then stopped on three it could not place. TIGER state polygons stop at the waterline, so a lakefront or island address is legitimately outside every one of them by a few metres while being unambiguously in that state. A second pass attributes those from the nearest boundary within 500m -- comfortably wider than the waterline margin and far narrower than the distance to another state, so it cannot move an address across a border. Verified: a point 220m off Ohio's northern edge attributes to OH, one in the mid-Atlantic does not. Anything still unplaced is a coordinate error rather than a missing state, and the migration now names those rows -- id, county and coordinates -- in the error itself rather than only the log. That error surfaces on /health, so whoever decides what to do with them can see what they are without shell access. Three rows out of 5.8M should not be diagnosed by asking someone to go and find them. Co-Authored-By: Claude Opus 5 --- database/migrations.go | 90 +++++++++++++++++++++---- services/multistate_integration_test.go | 85 +++++++++++++++++++++++ 2 files changed, 163 insertions(+), 12 deletions(-) diff --git a/database/migrations.go b/database/migrations.go index a9ceb75..82241cf 100644 --- a/database/migrations.go +++ b/database/migrations.go @@ -1660,19 +1660,76 @@ func addRegionToAddressUniqueness() error { attributed, _ := res.RowsAffected() log.Printf("Migration 23: attributed %d of %d blank region(s) from their coordinates", attributed, blanks) - // Whatever is left sits outside every state boundary, which means the - // coordinates are wrong rather than the region being missing. Naming - // the count beats inventing a state for them. - var stranded int - if err := tx.QueryRow(` - SELECT COUNT(*) FROM ohio_addresses WHERE region IS NULL OR region = '' - `).Scan(&stranded); err != nil { - return fmt.Errorf("failed to recount blank regions: %w", err) + // A second pass for points that sit just outside a boundary. + // + // TIGER state polygons are land only, so a legitimate lakefront or + // coastal address -- Lake Erie shoreline, an island, a pier -- can fall + // outside every one of them by a few metres while being unambiguously + // in that state. Falling back to the nearest boundary within a short + // distance attributes those correctly; anything further away is a real + // coordinate error and is left for the check below. + res, err = tx.Exec(` + UPDATE ohio_addresses a + SET region = nearest.state_abbr + FROM ( + SELECT blank.id, s.state_abbr + FROM ohio_addresses blank + CROSS JOIN LATERAL ( + SELECT s2.state_abbr, s2.geometry + FROM us_states s2 + WHERE s2.geometry IS NOT NULL + ORDER BY s2.geometry <-> blank.geom + LIMIT 1 + ) s + WHERE (blank.region IS NULL OR blank.region = '') + AND ST_DWithin(blank.geom::geography, s.geometry::geography, $1) + ) nearest + WHERE a.id = nearest.id + `, shorelineToleranceMeters) + if err != nil { + return fmt.Errorf("failed to attribute blank regions from the nearest boundary: %w", err) + } + if n, _ := res.RowsAffected(); n > 0 { + log.Printf("Migration 23: attributed %d further row(s) from the nearest boundary within %.0fm", + n, shorelineToleranceMeters) + } + + // Whatever is left is a genuine coordinate error, not a missing state. + // The details go in the error rather than only the log, because that + // error is surfaced on /health -- so whoever has to decide what to do + // with these rows can see what they are without shell access. + rows, err := tx.Query(` + SELECT id, COALESCE(NULLIF(county, ''), '?'), + round(ST_X(geom)::numeric, 4), round(ST_Y(geom)::numeric, 4) + FROM ohio_addresses + WHERE region IS NULL OR region = '' + ORDER BY id + LIMIT 10 + `) + if err != nil { + return fmt.Errorf("failed to inspect rows with no region: %w", err) + } + var stranded []string + for rows.Next() { + var id int64 + var county string + var lng, lat float64 + if err := rows.Scan(&id, &county, &lng, &lat); err != nil { + rows.Close() + return fmt.Errorf("failed to read rows with no region: %w", err) + } + stranded = append(stranded, fmt.Sprintf("id=%d county=%s at (%.4f, %.4f)", id, county, lng, lat)) } - if stranded > 0 { - return fmt.Errorf("%d row(s) still have no region after attribution: their coordinates fall outside "+ - "every US state boundary, so the location is wrong rather than the state missing. "+ - "Fix or delete them, then re-run", stranded) + rows.Close() + if err := rows.Err(); err != nil { + return fmt.Errorf("failed to read rows with no region: %w", err) + } + + if len(stranded) > 0 { + return fmt.Errorf("%d row(s) still have no region: their coordinates are not within %.0fm of any US "+ + "state boundary, so the location is wrong rather than the state missing. Fix or delete them, "+ + "then re-run. Offending rows: %s", + len(stranded), shorelineToleranceMeters, strings.Join(stranded, "; ")) } } @@ -1819,6 +1876,15 @@ func RequireSchemaVersion(db *sql.DB, version int) error { "migrations are still running -- check /health and retry", applied.Int64, version) } +// shorelineToleranceMeters is how far outside a state boundary a point may sit +// and still be attributed to it. +// +// TIGER polygons stop at the waterline, so lakefront and island addresses are +// legitimately outside them by a small margin. 500m is comfortably more than +// that margin and far less than the distance to a neighbouring state, so it +// cannot silently move an address across a border. +const shorelineToleranceMeters = 500.0 + // SchemaVersionRegionUniqueness is migration 23, which keys address uniqueness // on (hash, region). Every ingest path's ON CONFLICT depends on the index it // creates. diff --git a/services/multistate_integration_test.go b/services/multistate_integration_test.go index 271dc6f..aa7ea63 100644 --- a/services/multistate_integration_test.go +++ b/services/multistate_integration_test.go @@ -529,3 +529,88 @@ func TestBlankRegionsAreAttributedFromCoordinates(t *testing.T) { t.Errorf("the Fort Wayne row was attributed %q, want IN -- stamping every blank OH is the bug this replaces", got["blank-in"]) } } + +// TIGER state polygons stop at the waterline, so a legitimate lakefront or +// island address can sit outside every state boundary by a few metres while +// being unambiguously in that state. Production had exactly three rows that +// ST_Contains could not place. +// +// The tolerance has to be wide enough for the waterline and far narrower than +// the distance to another state, so it can never move an address across a +// border. +func TestShorelineAddressesAttributeToTheNearestState(t *testing.T) { + db := setupMultiStateDB(t, false) + + if _, err := db.Exec(` + CREATE TABLE us_states ( + id BIGSERIAL PRIMARY KEY, + state_fips VARCHAR(2) NOT NULL UNIQUE, + state_abbr VARCHAR(2) NOT NULL UNIQUE, + state_name VARCHAR(255) NOT NULL UNIQUE, + geometry GEOMETRY(MULTIPOLYGON, 4326) + )`); err != nil { + t.Fatalf("create us_states: %v", err) + } + // Ohio's northern edge at 41.7; Lake Erie is above it. + if _, err := db.Exec(` + INSERT INTO us_states (state_fips, state_abbr, state_name, geometry) + VALUES ('39','OH','Ohio', ST_Multi(ST_MakeEnvelope(-85.0,38.4,-80.5,41.7,4326))) + `); err != nil { + t.Fatalf("seed boundary: %v", err) + } + + cases := []struct { + hash string + lng, lat float64 + wantState string + why string + }{ + {"on-land", -83.0, 40.0, "OH", "well inside the boundary"}, + {"shoreline", -82.7, 41.7020, "OH", "about 220m offshore, still Ohio"}, + {"at-sea", -40.0, 35.0, "", "mid-Atlantic, a real coordinate error"}, + } + for _, c := range cases { + if _, err := db.Exec(` + INSERT INTO ohio_addresses (hash, house_number, street, unit, city, district, region, postcode, county, geom, full_address) + VALUES ($1,'1','Main Street','','Somewhere','','','','Unknown', + ST_SetSRID(ST_MakePoint($2,$3),4326),'1 Main Street') + `, c.hash, c.lng, c.lat); err != nil { + t.Fatalf("seed %s: %v", c.hash, err) + } + } + + // Both attribution passes, in the order the migration runs them. + if _, err := db.Exec(` + UPDATE ohio_addresses a SET region = s.state_abbr + FROM us_states s + WHERE (a.region IS NULL OR a.region = '') + AND s.geometry IS NOT NULL AND ST_Contains(s.geometry, a.geom)`); err != nil { + t.Fatalf("contains pass: %v", err) + } + if _, err := db.Exec(` + UPDATE ohio_addresses a SET region = nearest.state_abbr + FROM ( + SELECT blank.id, s.state_abbr + FROM ohio_addresses blank + CROSS JOIN LATERAL ( + SELECT s2.state_abbr, s2.geometry FROM us_states s2 + WHERE s2.geometry IS NOT NULL + ORDER BY s2.geometry <-> blank.geom LIMIT 1 + ) s + WHERE (blank.region IS NULL OR blank.region = '') + AND ST_DWithin(blank.geom::geography, s.geometry::geography, 500) + ) nearest + WHERE a.id = nearest.id`); err != nil { + t.Fatalf("nearest pass: %v", err) + } + + for _, c := range cases { + var got string + if err := db.QueryRow(`SELECT region FROM ohio_addresses WHERE hash = $1`, c.hash).Scan(&got); err != nil { + t.Fatalf("read %s: %v", c.hash, err) + } + if got != c.wantState { + t.Errorf("%s (%s): region = %q, want %q", c.hash, c.why, got, c.wantState) + } + } +}