Bug
internal/place/repository.go UpsertBatch's ON CONFLICT DO UPDATE column list includes external_ids:
DoUpdates: clause.AssignmentColumns([]string{
"name", "lat", "lng", "category", "rank", "tags", "external_ids", "status", "updated_at",
}),
internal/sources/osm/transformer.go TransformNode only ever populates ExternalIDs with the OSM entry:
ExternalIDs: models.ExternalIDs{
"osm": models.ExternalRef{ID: fmt.Sprintf("node/%d", osmID), Confidence: 1.0},
},
Any subsequent OSM full-import or diff-sync run over an already-ingested node overwrites the entire external_ids JSONB column with just {"osm": ...}, silently destroying any Wheelmap (or other future external source) ref that AttachExternalRef's jsonb_set had previously attached to that place.
This contradicts the documented design (AGENTS.md: "AttachExternalRef is the only path that writes it") and defeats the point of using jsonb_set for surgical per-source updates in the first place.
Why it wasn't caught
No existing test re-runs UpsertBatch on a place that already has a non-OSM external_ids entry attached. TestAttachExternalRef_ExistingDifferentKey / TestAttachExternalRef_ExistingSameKey (internal/place/repository_integration_test.go) seed once, attach once, check once — none re-ingest afterward.
Fix
Drop external_ids from UpsertBatch's DoUpdates list. OSM's own entry only needs to be written once, at insert time (Create already sets it via the row's own ExternalIDs field); on conflict it should be left untouched, exactly like the code comment already claims.
Suggested test
Extend the existing AttachExternalRef integration tests (or add a new one) to: seed via UpsertBatch, attach a wheelmap ref, re-run UpsertBatch with the same (osm_id, osm_type), then assert the wheelmap entry survives.
Bug
internal/place/repository.goUpsertBatch'sON CONFLICT DO UPDATEcolumn list includesexternal_ids:internal/sources/osm/transformer.goTransformNodeonly ever populatesExternalIDswith the OSM entry:Any subsequent OSM
full-importordiff-syncrun over an already-ingested node overwrites the entireexternal_idsJSONB column with just{"osm": ...}, silently destroying any Wheelmap (or other future external source) ref thatAttachExternalRef'sjsonb_sethad previously attached to that place.This contradicts the documented design (
AGENTS.md: "AttachExternalRefis the only path that writes it") and defeats the point of usingjsonb_setfor surgical per-source updates in the first place.Why it wasn't caught
No existing test re-runs
UpsertBatchon a place that already has a non-OSMexternal_idsentry attached.TestAttachExternalRef_ExistingDifferentKey/TestAttachExternalRef_ExistingSameKey(internal/place/repository_integration_test.go) seed once, attach once, check once — none re-ingest afterward.Fix
Drop
external_idsfromUpsertBatch'sDoUpdateslist. OSM's own entry only needs to be written once, at insert time (Createalready sets it via the row's ownExternalIDsfield); on conflict it should be left untouched, exactly like the code comment already claims.Suggested test
Extend the existing
AttachExternalRefintegration tests (or add a new one) to: seed viaUpsertBatch, attach awheelmapref, re-runUpsertBatchwith the same(osm_id, osm_type), then assert thewheelmapentry survives.