Skip to content

fix(nomantim): modify address autocomplete to only include California addresses#217

Open
v7ncentng wants to merge 12 commits into
benevolentbandwidth:mainfrom
v7ncentng:update-address-autofill
Open

fix(nomantim): modify address autocomplete to only include California addresses#217
v7ncentng wants to merge 12 commits into
benevolentbandwidth:mainfrom
v7ncentng:update-address-autofill

Conversation

@v7ncentng

@v7ncentng v7ncentng commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Restricted address autocomplete to California-only results and increased the suggestion limit to 4 addresses at a time.

Motivation

  • The delivery optimizer serves California-based routes, so surfacing out-of-state addresses in autocomplete adds noise and invites data entry errors.

Changes

Frontend

  • app/ui/next.config.ts: Added turbopack: { root: __dirname } so Turbopack resolves modules relative to app/ui/ rather than a parent directory
  • app/ui/src/app/components/AddressGeocoder/utils/nominatim.ts:
    • Added California bounding box (viewbox=-124.48,42.01,-114.13,32.53) with bounded=1 to restrict Nominatim search to California at the API level
    • Added post-filter on address.state === "California" as a safety net for any edge-case results that slip through the viewbox

Validation

Frontend

  • npm --prefix app/ui run lint
  • npm --prefix app/ui run format:check
  • npm --prefix app/ui run typecheck
  • npm --prefix app/ui run test
  • npm --prefix app/ui run build
  • npm --prefix app/mobile run lint
  • npm --prefix app/mobile run typecheck

Backend

  • cmake --preset dev
  • .github/scripts/check-backend-static.sh build/dev
  • cmake --build --preset dev --parallel
  • ctest --preset dev --output-on-failure --no-tests=error -LE 'e2e|docker'
  • docker compose -f deploy/compose/docker-compose.arm64.yml --env-file deploy/env/http-server.arm64.env config

Risk

  • Low. Changes are frontend-only and isolated to Turbopack config and the Nominatim autocomplete utility. No backend, database, or API contract changes.
  • The California viewbox filter could silently drop results if Nominatim ever returns a California result with a non-matching address.state string (e.g. abbreviations). Monitored via manual QA on a few known CA addresses.
  • Nominatim has a hard server-side cap of 50 results per request; the limit increase from 5 → 10 is well within that bound.

Rollout and Recovery

  • No feature flags or migrations needed.
  • To revert the Turbopack fix: remove turbopack: { root: __dirname } from next.config.ts and pin next dev with --no-turbopack as a temporary workaround.
  • To revert the California filter: remove the viewbox, bounded, and filter additions from autocompleteAddress in nominatim.ts.

High-Signal PR Checklist

  • The summary states the user-visible or operational outcome, not just file names.
  • The motivation explains why this change is needed now.
  • The change list separates frontend, backend, infrastructure, and documentation work where applicable.
  • Validation includes exact commands run, relevant output, and any checks intentionally skipped.
  • Risks, migrations, feature flags, and rollback steps are called out when relevant.
  • Screenshots or request/response examples are included for UI and API behavior changes.

v7ncentng added 2 commits May 20, 2026 13:05
…addresses

& Increased Nominatim fetch limit from 5 to 10 so more California results survive the state filter. Ran npx --prettier write .
@v7ncentng v7ncentng closed this Jun 28, 2026
@v7ncentng v7ncentng reopened this Jun 28, 2026
@v7ncentng v7ncentng closed this Jun 28, 2026
@v7ncentng v7ncentng reopened this Jun 29, 2026
 Wired up the two-step flow properly:
  a. CSVUploadOverlay now gets onFileSelect, which calls openImportModal(file) from useCSVImport to parse the CSV.
  b. CSVImportModal renders once parsing succeeds, receives csvData and importAddresses to complete the import.
@markboenigk

Copy link
Copy Markdown
Collaborator

The Nominatim changes look solid — the viewbox + bounded=1 + post-filter layering is a reasonable approach and the * 3 fetch multiplier accounts for the filter attrition well. Three small things before merging:

1. app/ui/src/app/edit/components/CSVImportModal.tsx is dead code

This new 745-line file is never imported. Both callers still point at @/app/edit/components/address/CSVImportModal:

  • edit/page.tsx:34
  • upload-save-point/page.tsx:9

If this was meant to be a location refactor, the two import paths need updating. If not, the file should be deleted — it'll otherwise sit in the bundle unexecuted.

2. upload-save-point/page.tsx — duplicate CSS

.upload-parse-error, @keyframes upload-spin, and .upload-spinner are now defined twice in the same <style> block. Browsers silently use the last definition, so no visible breakage, but the first copy of each can be removed.

3. upload-route/page.tsx:373 — missing aria-label on remove button

The PR removes aria-label="Remove file" from the ×-only button. Screen readers have nothing to announce for a bare ×. upload-save-point's equivalent button kept its label — worth restoring this one to match.

…rror

Deletes the unimported CSVImportModal, removes duplicate style rules in upload-save-point, restores the missing aria-label="Remove" button attribute, and excludes playwright.config.ts from the Next.js tsconfig to fix the build type error.
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.

2 participants