OSM tag-to-profile mapping + TransformNode wiring

[koinsaari]

Summary

• Adds internal/sources/osm/a11y.go with a pure mapTagsToProfile function that translates POI-node accessibility tags into a *models.AccessibilityProfile (or nil when no signal is present)
• Covers the full v1 tag set: wheelchair, toilets:wheelchair, capacity:disabled, parking:disabled, automatic_door, step_count/entrance:step_count, ramp/ramp:wheelchair, elevator
• TransformNode now sets Place.Accessibility via mapTagsToProfile; signature unchanged
• 23 unit tests for the mapper + 2 new transformer tests; all 276 non-integration tests pass

Test plan

• go test ./internal/sources/osm/... -v — all 71 tests pass
• go test ./internal/... ./pkg/... ./cmd/api/... — 276 tests pass
• CI green

Part of #10.

🤖 Generated with Claude Code

[claude]

Claude finished @koinsaari's task in 5m 37s —— View job

---

Code Review

• Gather context (REVIEW.md, CLAUDE.md, PR diff)
• Review changed files
• Post findings and set commit status

---

Summary

The mapTagsToProfile mapper itself is clean, well-structured, and correctly handles the tag semantics. The unit test coverage is thorough. One architectural invariant is violated when wiring it into the ingestion pipeline — see the Important finding below. Two nits follow.

---

🔴 Important — WithAuditFlags is never called; ingestion profile persistence is uncharted

File: internal/sources/osm/transformer.go:43 and cmd/ingestion/main.go (canonical pipeline)

Setting Accessibility: mapTagsToProfile(tags) in TransformNode attaches a populated *models.AccessibilityProfile to each Place before it enters the batcher. That profile is then handed to UpsertBatch, which calls GORM Create with clause.OnConflict. Two problems follow:

1. Audit flags are never computed. internal/a11y.WithAuditFlags is not called anywhere in the canonical pipeline before (or after) the profile is persisted. The architectural invariant requires AuditFlags to be computed by internal/a11y on every write. A node tagged step_count=2, ramp:wheelchair=no produces HasStep=true, HasRamp=false — but without WithAuditFlags, the FlagEntranceStepNoRamp flag is never set, and DetectConflicts would pass a profile that the API write path would reject.

2. UpsertBatch was not designed to persist the Accessibility association. Its DoUpdates columns (name, lat, lng, category, rank, tags, external_ids, status, updated_at) contain nothing accessibility-related. On re-import (the idempotent OSM full-import), on-conflict rows will leave any previously written profile untouched. For new places, GORM's batch-create association behaviour without FullSaveAssociations: true is not guaranteed to insert the accessibility_profiles row — meaning profiles may be silently dropped. The integration test at cmd/ingestion/main_integration_test.go never queries accessibility_profiles, so this would not be caught by CI.

The canonical pipeline needs an explicit post-upsert step that calls engine.WithAuditFlags(p.Accessibility) followed by placesRepo.UpsertProfileIngestion(ctx, p.ID, p.Accessibility) for every place where Accessibility != nil — mirroring the API write path order (validation → a11y engine → persist) and respecting the user_verified guard already built into UpsertProfileIngestion. Until that plumbing is in place, setting Accessibility in TransformNode attaches data that either silently disappears or lands in the DB without audit flags.

Fix this →

---

🟡 Nit 1 — ramp=yes is silently ignored; intent not documented

File: internal/sources/osm/a11y.go:130–138

The generic ramp key is only handled for ramp=no (sets HasRamp=false). ramp=yes falls through without producing any signal and without setting any=true, so a node with only {"ramp": "yes"} returns nil from mapTagsToProfile. The PR description states "covers … ramp/ramp:wheelchair", which implies both keys are handled. If the intent is to be conservative (a non-wheelchair-specific ramp does not imply wheelchair accessibility), add a test documenting the ramp=yes → nil behaviour so future maintainers don't treat it as a bug to fix.

🟡 Nit 2 — capacity:disabled=0 omits Count on the inaccessible component

File: internal/sources/osm/a11y.go:97–101

When capacity:disabled=0 the function returns a ComponentParking with HasDisabledSpaces=false but no Count field. A downstream consumer cannot distinguish "confirmed zero spaces" from "count unknown." Setting Count: &n (n=0) on the inaccessible-path component would preserve the information that was explicitly present in the source tag.

---

Note: attempted to set claude-code-review commit status to failure but the token lacks statuses:write permission (HTTP 403).

[claude]

Claude finished @koinsaari's task in 3m 49s —— View job

---

Code Review

• Gather context (REVIEW.md, CLAUDE.md, PR diff)
• Review changed files
• Post findings and set commit status

---

Summary

The mapTagsToProfile mapper and its tests are clean and correct. The fix commit (79ed26a) properly separates the profile from Place to avoid the unsafe association path that the previous review flagged. Both previous nits have been addressed. However the fix exposes a new 🔴 Important gap: the profile is now computed and then silently discarded before it touches any storage layer.

---

🔴 Important — computed profile is discarded in source.go; OSM accessibility data never reaches the database

File: internal/sources/osm/source.go:51

p, _, err := TransformNode(node.ID, node.Lat, node.Lng, node.Tags, category)

The _ throws away the *models.AccessibilityProfile that TransformNode now returns. Every OSM node goes through mapTagsToProfile — steps, ramps, wheelchair status, elevator, parking — and the result is silently dropped before the place enters the batcher. After a full import, no accessibility profiles derived from OSM tags will exist in the database. The mapper is dead code in the pipeline.

The root blocker is that sources.Sink accepts only models.Place; there is no channel to carry profiles through the batcher into a post-upsert persistence step. Completing the wiring requires either:

1. Preferred — a post-upsert pass in runCanonical (cmd/ingestion/main.go): after b.flushNow, iterate over the flushed places, call internal/a11y.Engine.WithAuditFlags(profile) on each non-nil profile, then call placesRepo.UpsertProfileIngestion(ctx, p.ID, profile). This mirrors the API write path order (mapper → audit flags → persist) and respects the user_verified guard already in UpsertProfileIngestion. The batcher would need to carry (place, profile) pairs rather than just place, or profiles could be buffered in a parallel slice.

2. Alternative — extend sources.Sink to func(context.Context, models.Place, *models.AccessibilityProfile) error, propagate through the batcher, and persist after each flush.

Until one of these is wired, adding mapTagsToProfile to the ingestion path has no observable effect on the registry.

Fix this →

---

Previous nits — both resolved

• ramp=yes silently ignored: a11y_test.go:170–175 now has an explicit subtest "ramp=yes alone returns nil (non-wheelchair-specific ramp is not a signal)" that documents and pins the conservative intent. ✅
• capacity:disabled=0 missing Count: a11y.go:104 now sets Count: &n (n=0) on the inaccessible-path component, and a11y_test.go:115–118 verifies it. ✅