fix: attribute shoreline addresses, and name the rows that cannot be placed

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.

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)
+		}
+	}
+}