af-map — Prop-Build Analysis

Document Type: Critical Review & Analysis (companion to prop-build-template.md) Scope: Per-Repo / Per-Module Subject: af-map (AidFinder Mapping Service) Reviewer(s): Claude (automated code review) Date: 2026-04-09 Version: 0.1 Confidence Level: Medium What would raise confidence: access to CloudWatch metrics and SQS DLQ config in af-infra, a run against real Mapbox, and an interview with Marek Burda on the state-wide path.

Inputs Reviewed:

• Prop-build doc: data/af-map.yaml
• Source tree: /Users/andres/src/af/af-map/ (Go 1.26, ~4.7k LoC)
• Companion files: data/af-map/{api-examples,data-flow,deployment,runbook}.md

Part A — Per-Repo / Per-Module Analysis

A.1 Executive Summary

• Overall health: A small, focused, well-layered Go worker that does one thing (Postgres -> Mapbox tileset) reasonably well, with a clever streaming design but meaningful gaps in error handling, PII governance, and operational visibility.
• Top risk: Full household PII (names, addresses, demographics) is streamed to Mapbox and stored in vector tilesets with no application-level access controls or audit trail — see A.4.1 and A.11.
• Top win / thing worth preserving: io.Pipe-based streaming uploader in service/infrastructure/mapbox/tileset.go:26-69 wired through mapgeneration.go:77-92 — a genuinely memory-efficient design worth propagating.
• Single recommended next action: Add an explicit DLQ + Mapbox source cleanup-on-failure path, and justify-or-remove every //nolint:gosec (4 instances).
• Blocking unknowns: SQS DLQ / visibility-timeout settings live in af-infra and were not reviewed; no measured throughput or CloudWatch metrics available; FEMA Privacy Act applicability of source data is unknown.

A.2 Health Scorecard

Overall score: 3.28 (average of 18 rated dimensions; dim 7 N/A). Weighted reading: structural dimensions (1, 5, 8, 13, 16, 18) are solidly 4; operational and security dimensions (9, 17) drag the honest average into the low 3s and are where remediation should focus.

A.3 What's Working Well

• Strength: Streaming io.Pipe uploader — Postgres pagination batches are piped straight into the Mapbox multipart body, so memory is bounded regardless of disaster size.

  • Location: service/infrastructure/mapbox/tileset.go:26-69 + service/application/map-generation/mapgeneration.go:77-92
  • Why it works: avoids the obvious OOM footgun on large disasters; producer/consumer are decoupled by the pipe and errors propagate via pw.CloseWithError.
  • Propagate to: any other worker in the platform that materializes large Postgres result sets to external APIs (af-targeting is the obvious candidate).

• Strength: Clean hexagonal layering — cmd/map wires concrete infrastructure into application services that depend only on small interfaces (MapGenerationService, HouseholdService).

  • Location: service/application/map-generation/mapgeneration.go:17-26
  • Why it works: substitutable seams for testing; infrastructure is swappable without rewriting the application.
  • Propagate to: af-backend-go-api workers that currently couple handlers to concrete clients.

• Strength: Explicit count == 0 short-circuit before uploading.

  • Location: service/application/map-generation/mapgeneration.go:54-75
  • Why it works: avoids creating empty Mapbox sources/tilesets and the operational mess of cleaning them up.

A.4 What to Improve

A.4.1 P0 — PII exposure on Mapbox with no app-level audit or cleanup

• Problem: Full household PII (adult names, street addresses, lat/lon, demographics) is streamed to Mapbox vector tilesets and stored indefinitely. The published tileset properties include household_name, bq_household_id, and lat/lon. There is no application-level audit log of what was published when, and no automated cleanup of stale sources on failure.
• Evidence: service/infrastructure/mapbox/household.go:33-54 writes householdName (derived from BQAdultsFullNames) and coordinates into every GeoJSON feature; service/infrastructure/mapbox/tileset.go:61-66 returns error on non-200 but does NOT delete the partially-uploaded source; README manual cleanup curls confirm this is operator-driven. section_17_security_compliance.data_protection.pii_handling in data/af-map.yaml flags this as HIGH RISK.
• Suggested change: (a) emit a structured audit record on every UploadDeclaredHouseholdsToMapbox start/success/failure, including sourceID and row count; (b) add a deferred cleanup step that deletes the Mapbox source if publish fails; (c) confirm Mapbox ToS permits PII storage and minimize properties.
• Estimated effort: M
• Risk if ignored: regulatory exposure (FEMA Privacy Act if applicable), breach of Mapbox ToS, and inability to answer "what data was shared, when" during an incident.

A.4.2 P1 — No DLQ handling or poison-message protection in-app

• Problem: Message processing errors are logged and the message is left in the queue for SQS-default retry. There is no max-retry guard, no DLQ handoff in code, and no metric on failure count per message.
• Evidence: service/infrastructure/sqs/sqs.go:91-105 continues on both parse and handler errors without deleting; section_9_error_handling.circuit_breakers.applicable: false in YAML.
• Suggested change: Read ApproximateReceiveCount; after N failures, move to an explicit DLQ or delete with a loud error metric. Emit a CloudWatch metric for failed handles.
• Estimated effort: S
• Risk if ignored: a single poison DisasterNotification loops forever, consuming the single-threaded worker and blocking all other disasters.

A.4.3 P1 — Serialized single-message worker is a throughput bottleneck

• Problem: maxMessages = 1 (sqs.go:19) and a synchronous handler mean one disaster blocks the entire service. Mapbox upload can take up to 120 minutes (mapbox.go:16), so one bad disaster starves the queue.
• Evidence: service/infrastructure/sqs/sqs.go:19, service/infrastructure/mapbox/mapbox.go:16.
• Suggested change: Run N worker goroutines with bounded concurrency, or document and alert on SQS ApproximateAgeOfOldestMessage. Reduce the 120m timeout and rely on SQS visibility-timeout extension for long uploads.
• Estimated effort: M
• Risk if ignored: backed-up queue during multi-disaster events; no way to hit a reasonable tile-publish SLO.

A.4.4 P2 — Unjustified //nolint:gosec directives

• Problem: Four //nolint:gosec with no rationale (README §171 lists fixing them as tech debt).
• Evidence: service/application/map-generation/mapgeneration.go:51, domain/household/postgres/household.go:48,58,80.
• Suggested change: Add a one-line justification to each (likely int -> uint32 conversion for disaster_id) or refactor to eliminate the cast.
• Estimated effort: S
• Risk if ignored: security-review noise; real gosec findings will be drowned out.

A.4.5 P2 — is_updated flag parsed but unimplemented

• Problem: The DisasterNotification schema carries is_updated, and the README acknowledges incremental updates are not supported; every run is a full rebuild.
• Evidence: service/infrastructure/sqs/model/model.go:7-12 + section_19_roadmap.tech_debt[0] in YAML.
• Suggested change: Either delete the field until it's implemented, or implement the update path against Mapbox's tileset-source PATCH.
• Estimated effort: M
• Risk if ignored: contract rot; upstream callers will assume the flag does something.

A.5 Things That Don't Make Sense

1. Observation: The "state-based regen" path is gated on DisasterID == 0 rather than on the presence of StateName, which makes the branch implicit.

   • Location: service/application/map-generation/mapgeneration.go:50-92
   • Hypotheses considered: convenience; validator already enforces exactly-one-of at the boundary.
   • Question for author: can this become an explicit tagged union in the notification model?

2. Observation: The tileset ID in dev omits the environment prefix (helpishere.<disaster_id>) while stg/prod include it.

   • Location: service/infrastructure/mapbox/mapbox.go:46-52
   • Hypotheses considered: avoid breaking existing dev tileset; historical compatibility.
   • Question for author: why not always prefix (helpishere.dev.<id>) for uniformity?

A.6 Anti-Patterns Detected

A.6.1 Code-level

• God object / god function
• Shotgun surgery
• Feature envy
• Primitive obsession
• Dead code — is_updated field parsed and validated but never branched on.
• Copy-paste / duplication
• Magic numbers / unexplained constants — maxMessages = 1 (sqs.go:19), mapboxUploadMaxWaitingTime = 120m (mapbox.go:16).
• Deep nesting (>3 levels)
• Long parameter lists
• Boolean-flag parameters

A.6.2 Architectural

• Big ball of mud
• Distributed monolith
• Chatty services
• Leaky abstraction
• Golden hammer
• Vendor lock-in without exit strategy — Mapbox Tilesets API is baked in; no abstraction for a different tile backend.
• Stovepipe
• Missing seams for testing

A.6.3 Data

• God table
• EAV abuse
• Missing indexes on hot queries (not verified — schema upstream)
• N+1 queries
• Unbounded growth / no retention policy — Mapbox tilesets and sources are only deleted manually.
• Nullable-everything schemas
• Implicit coupling via shared database

A.6.4 Async / Ops

• Poison messages with no dead-letter queue — no in-app DLQ logic.
• Retry storms / no backoff
• Missing idempotency keys on non-idempotent ops — re-processing the same disaster_id grows the tileset (documented footgun).
• Hidden coupling via shared state
• Work queues without visibility / depth metrics — no CloudWatch custom metrics.

A.6.5 Security

• Secrets in code, .env committed, or logs
• Missing authn/z on internal endpoints
• Overbroad IAM roles
• Unvalidated input crossing a trust boundary (validator is present)
• PII/PHI in logs or error messages — PII storage in downstream Mapbox tileset (household.go:33-54); no boundary log/audit.
• Missing CSRF / XSS / SQLi / SSRF protections (no web surface)

A.6.6 Detected Instances

A.7 Open Questions

1. Q: Is the target SQS queue actually configured with a DLQ in af-infra, and with what maxReceiveCount?

   • Blocks: A.4.2, A.6.4 row 1.
   • Who can answer: af-infra owner.

2. Q: Does Mapbox ToS permit storing full PII in a published tileset? Is there a signed data-processing addendum?

   • Blocks: A.4.1, A.11.
   • Who can answer: legal / data-privacy owner.

3. Q: Are homeowners_declared rows derived from FEMA IA data?

   • Blocks: FEMA Privacy Act assessment.
   • Who can answer: af-backend-go-api data owner.

A.8 Difficulties Encountered

• Difficulty: No access to CloudWatch metrics, SQS configuration, or deployed task definitions at review time.

  • Impact on analysis: cannot verify whether a DLQ exists, cannot measure observed throughput or error rate, cannot confirm DATABASE_SSL_MODE=require in prod. All performance and resilience findings are structural, not empirical.
  • Fix that would help next reviewer: commit a read-only link to the CloudWatch dashboard and paste representative aws sqs get-queue-attributes output into data/af-map/runbook.md.

• Difficulty: The state-wide path is marked "experimental" but has no test coverage asserting it.

  • Impact on analysis: cannot say whether bq_households pagination terminates cleanly for large states.
  • Fix that would help next reviewer: an integration test against LocalStack exercising the DisasterID == 0 branch.

• Difficulty: Only skimmed the 428-line sqs_test.go; did not enumerate every case.

  • Impact on analysis: test-quality claims are directional, not definitive.

A.9 Risks & Unknowns

A.9.1 Known risks

A.9.2 Unknown unknowns

• Area not reviewed: actual SQS queue + DLQ configuration.
  • Reason: lives in af-infra; out of scope.
  • Best guess at risk level: medium — the repo assumes default retries.
• Area not reviewed: Postgres query plans and index coverage.
  • Reason: no DB access; schema owned upstream.
  • Best guess at risk level: medium — 50k-row pages over bq_households could be slow on a cold index.
• Area not reviewed: af-frontend Mapbox consumption.
  • Reason: out of scope for per-repo analysis.
  • Best guess at risk level: low for this repo.
• Area not reviewed: the 428-line sqs_test.go in detail.
  • Reason: time.
  • Best guess at risk level: low.

A.10 Technical Debt Register

A.11 Security Posture (lightweight STRIDE)

A.12 Operational Readiness

A.13 Test & Quality Signals

• Coverage (line / branch): not reported in repo (section_15_testing.unit.coverage_pct: null); CI uploads to Codecov.
• Trend: unknown.
• Flake rate: unknown.
• Slowest tests: unknown.
• Untested critical paths: Mapbox 3-step publish (no integration test); state-wide DisasterID == 0 branch.
• Missing test types: [ ] unit (partial) [x] integration [x] e2e [x] contract [x] load [x] security/fuzz

A.14 Performance & Cost Smells

• Hot paths: Postgres pagination -> io.Pipe -> Mapbox multipart upload.
• Suspected bottlenecks: serial 1-message SQS polling (sqs.go:19); Mapbox publish latency; 50k-row page size.
• Wasteful queries / loops: count query before upload is a small extra round-trip but worth it to skip empty.
• Oversized infra / idle resources: 2 vCPU / 8 GB Fargate for a mostly I/O-bound worker — worth measuring.
• Cache hit/miss surprises: N/A — no caches.

A.15 Bus-Factor & Knowledge Risk

• Who is the only person who understands X? Inferring from meta.authors (Marek Burda, Matúš Bafrnec); not verified.
• What breaks if they disappear tomorrow? The state-wide experimental path and Mapbox ToS context.
• What is undocumented tribal knowledge? The manual Mapbox cleanup workflow and the rationale for dev vs stg/prod naming asymmetry.
• Suggested knowledge-transfer actions: capture the Mapbox cleanup flow as a make clean-orphans script; record a note explaining (or fix) the naming asymmetry.

A.16 Compliance Gaps

A.17 Recommendations Summary