pal-e-docs

Review: Properties tab: proximity-based default sort order

review-1451-2026-06-13 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- present
  • [x] Repo -- ldraney/landscaping-assistant
  • [x] User Story -- present, well-formed
  • [x] Context -- present, explains paper schedule mental model
  • [x] File Targets -- present, 3 modify targets + 2 do-not-touch
  • [x] Feature Flag -- present, set to "none" with rationale
  • [x] Acceptance Criteria -- present, 5 criteria
  • [x] Test Expectations -- present, 3 test cases + run command
  • [x] Constraints -- present, 4 constraints
  • [x] Checklist -- present
  • [x] Related -- present

Traceability

  • [x] story:property-crud label -- "Add/edit/delete client properties"
  • [x] story note verified -- found in project-landscaping-assistant user-stories section
  • [x] arch:rails-app label -- Rails application component
  • [ ] arch note MISSING -- [SCOPE] Create architecture note arch-rails-app for the Rails application component
  • [x] Forgejo issue -- https://forgejo.tail5b443a.ts.net/ldraney/landscaping-assistant/issues/205, open

File Targets

  • [x] app/controllers/properties_controller.rb -- verified: line 9 sorts by LOWER(client_name), :address_line in the manage action
  • [x] app/controllers/weeks_controller.rb -- verified: line 10 sorts by LOWER(client_name) in the index action
  • [x] app/models/property.rb -- verified: no by_proximity scope exists yet, latitude and longitude attributes confirmed in schema (float columns)
  • [x] Do-not-touch: db/schema.rb -- correct, lat/lng columns already exist
  • [x] Do-not-touch: app/models/work_queue_item.rb -- correct, queue ordering is user-defined

Repo Placement

OK -- issue is filed on ldraney/landscaping-assistant and all file targets are in the same repo. Single-repo change.

Dependencies

No blocking dependencies found on the board. The latitude and longitude columns already exist in the schema. The Add Location feature (already shipped) populates these fields. No other board items block or are blocked by this ticket.

Acceptance Criteria

All 5 criteria are testable and well-scoped. The "deterministic" criterion is important -- nearest-neighbor chains can produce different results depending on the starting point, so the base point must be fixed. The test expectations provide clear unit and request test guidance with a concrete run command.

Blast Radius

Minor concern: app/controllers/work_queue_items_controller.rb line 16 also loads and sorts properties: Property.active.includes(:services, :work_queue_items).order(updated_at: :desc). This is the Today view's "add to queue" dropdown, which intentionally sorts by recency (most recently updated first for quick-add). This sort should NOT change, but the issue's "Files the agent should NOT touch" section only lists app/models/work_queue_item.rb -- it does not mention app/controllers/work_queue_items_controller.rb. An agent could mistakenly apply the proximity sort there too. Recommend adding it to the do-not-touch list to prevent agent confusion.
The PropertiesController#index action (line 4-6) only loads services, not properties -- the property list is loaded in the manage action. This is correct in the ticket.

Decomposition Assessment

3 file targets in 1 repo, 5 acceptance criteria, straightforward greedy nearest-neighbor algorithm. Estimated agent work well under 5 minutes. No decomposition needed.

Recommendation

  • [BODY] Add app/controllers/work_queue_items_controller.rb to the "Files the agent should NOT touch" list with note: "Today view dropdown sorts by recency intentionally, not proximity"
  • [SCOPE] Create architecture note arch-rails-app for the Rails application component (shared across many board items, not blocking for this ticket)