Skip to content

refactor: replace OverallStatus with typed component fields in AccessibilityProfile - #113

Merged
koinsaari merged 1 commit into
mainfrom
refactor/accessibility-component-fields
Jul 26, 2026
Merged

refactor: replace OverallStatus with typed component fields in AccessibilityProfile#113
koinsaari merged 1 commit into
mainfrom
refactor/accessibility-component-fields

Conversation

@koinsaari

Copy link
Copy Markdown
Contributor

Summary

  • Replaces the single OverallStatus enum with named, structured component fields (Entrance, Pathways, Restroom, Parking, Elevator) on AccessibilityProfile
  • Each component carries typed properties, per-field audit flags computed on write, and read-time inheritance metadata (IsInherited, SourceID)
  • SourceReports stores raw external opinions verbatim — no server-side judgment; clients decide trust level
  • Removes DetectConflicts and the 422 hard-conflict response — no OverallStatus means no self-contradictions to enforce
  • Fixes AccessibilityProfile.UpdatedAt leaking "0001-01-01T00:00:00Z" into request bodies (was omitempty, changed to omitzero) which caused kin-openapi to reject valid writes with HTTP 400

Changed

  • pkg/models/accessibility.go — new typed component structs, SourceReports, omitzero on UpdatedAt
  • api/openapi.yaml + internal/api/v1/server.gen.go — schema updated, DetectConflicts removed
  • internal/a11y/engine.goWithAuditFlags dispatches per typed component; DetectConflicts removed
  • internal/validation/validation.go — validation updated for new model
  • internal/place/repository.go — write path updated
  • internal/sources/osm/a11y.go — OSM transformer writes SourceReports instead of OverallStatus
  • cmd/ingestion/ — batcher and main updated
  • All integration and unit tests updated

Test plan

  • All 371 integration tests pass (go test -tags integration ./...)
  • Unit tests pass (go test ./...)
  • CI passes

🤖 Generated with Claude Code

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown

Claude encountered an error after 1m 21s —— View job


Reviewing PR #113

  • Read REVIEW.md and CLAUDE.md
  • Fetch full diff (git diff origin/main...HEAD)
  • Review core invariants (pure data layer, deterministic AuditFlags, validation ordering, no HTTP framework)
  • Review SQL/GORM parameterization and JSONB Valuer/Scanner symmetry
  • Review test coverage for new/changed exported functions
  • Post findings and set commit status

@koinsaari
koinsaari force-pushed the refactor/accessibility-component-fields branch from 53695eb to 4f72e02 Compare July 26, 2026 15:11
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown

Claude finished @koinsaari's task in 4m 11s —— View job


Reviewing PR #113

  • Read REVIEW.md and CLAUDE.md
  • Fetch full diff (git diff origin/main...HEAD)
  • Review core invariants (pure data layer, deterministic AuditFlags, validation ordering, no HTTP framework)
  • Review SQL/GORM parameterization and JSONB Valuer/Scanner symmetry
  • Review test coverage for new/changed exported functions
  • Post findings and set commit status (blocked — see note below)

Findings

🔴 Important — internal/db/migrations/000002_create_accessibility_profiles.up.sql was edited in place instead of adding a new migration

This PR modifies an already-numbered, previously-merged migration (000002) to swap overall_status TEXT / components JSONB for the new source_reports/entrance/pathways/restroom/parking/elevator columns, rather than adding 000008_...up.sql. The project uses golang-migrate (internal/db, iofs source), which tracks applied versions in schema_migrations by number only — it will not re-run 000002 on any database that already applied it (any long-lived dev/staging/prod environment, or a container that persists across deploys). Those databases will keep the old columns and silently diverge from the GORM model and this PR's own integration tests (e.g. internal/place/repository_integration_test.go, which now assert on Entrance.IsLevel and would get a "column does not exist" error against a stale schema). testcontainers-go recreates a fresh DB per test run, so CI won't catch this — but any environment that isn't torn down and recreated will break on every accessibility read/write.

Fix: revert 000002_*.sql to its original content and add a new migration (000008_...) that alters accessibility_profiles to the new column set.

Fix this →

🟡 Nit — stale package/root READMEs still document removed behavior

README.md, cmd/api/README.md, internal/a11y/README.md, and internal/sources/osm/README.md still describe OverallStatus, DetectConflicts, and the 422 hard-conflict response, all of which this PR removes. .github/copilot-instructions.md also references DetectConflicts/422 as if still current. Worth a follow-up pass so contributors relying on these docs aren't misled about the current write-path contract.

🟡 Nit — unrelated .gitignore addition

.gitignore gains graphify-out/, which is unrelated to the component-fields refactor and probably belongs in a separate change.

No issues found with SQL/GORM parameterization (all queries in internal/place/repository.go use placeholders), JSONB Valuer/Scanner symmetry (each new *Props type and SourceReports implements both consistently), validation ordering (validation.Place still runs before engine.WithAuditFlags in CreatePlace), or the removal of DetectConflicts/422 itself — that removal is coherent given OverallStatus no longer exists to contradict, and the OpenAPI spec, generated server, and README-adjacent 422 schema were all removed together. Test coverage for the new typed components (internal/a11y/engine_test.go, internal/sources/osm/a11y_test.go, internal/place/repository_integration_test.go) looks thorough across create/update/inherit/skip-verified paths.

I was not able to run go build/go test in this sandbox (commands require approval that isn't available here) — please confirm CI is green.

Commit status: attempted to set claude-code-review to failure on 4f72e029 via the GitHub API, but the action's token returned 403 Resource not accessible by integration — the app doesn't have statuses: write permission in this workflow. A maintainer will need to grant that permission for future runs.

…ibilityProfile

Replaces the single OverallStatus enum with named, structured component
fields (Entrance, Pathways, Restroom, Parking, Elevator). Each component
carries typed properties, per-field audit flags computed on write, and
read-time inheritance metadata (IsInherited, SourceID). SourceReports now
stores raw external opinions verbatim without server-side judgment.

Removes DetectConflicts and the 422 hard-conflict response — no
OverallStatus means no self-contradictions to enforce. Updates OpenAPI
schema, generated server code, OSM transformer, validation, engine,
repository, and all integration tests.

Fixes: AccessibilityProfile.UpdatedAt used omitempty (did not omit zero
time.Time), leaking a readOnly field into request bodies and causing
kin-openapi to reject valid writes with HTTP 400.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@koinsaari
koinsaari force-pushed the refactor/accessibility-component-fields branch from 4f72e02 to ed1360e Compare July 26, 2026 15:24
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown

Claude Code is working…

I'll analyze this and get back to you.

View job run

@koinsaari
koinsaari merged commit ac86298 into main Jul 26, 2026
1 of 9 checks passed
@koinsaari
koinsaari deleted the refactor/accessibility-component-fields branch July 26, 2026 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant