Normalised both geom and point to 4326 in the containsPoint fn

[RitoBose]

Type of Change

• Bugfix
• New feature
• [x ] Enhancement
• Refactoring
• Dependency updates

Description

Normalised both geom and point to 4326 in the containsPoint fn

Additional Changes

• This PR modifies the database schema (database migration added)
• This PR modifies dhall configs/environment variables

Motivation and Context

How did you test it?

Checklist

• I formatted the code and addressed linter errors ./dev/format-all-files.sh
• I reviewed submitted code
• I added unit tests for my changes where possible
• I added a CHANGELOG entry if applicable

Summary by CodeRabbit

• Refactor
  • Enhanced internal geometry processing for spatial operations to improve code consistency and maintainability. No user-facing changes or functional impacts.

[coderabbitai]

Walkthrough

The containsPoint function in the Esqueleto storage layer was refactored to construct geometries using ST_SetSRID SQL function directly instead of parsing WKT strings, and to generate test points via getPoint rather than text construction. Internal helper bindings were removed and replaced with a new setSRID binding, while preserving the function's external behavior.

Changes

Cohort / File(s)	Summary
Geometry Construction Refactoring lib/mobility-core/src/Kernel/Storage/Esqueleto/Functions.hs	Refactored containsPoint to use ST_SetSRID for geometry SRID configuration and getPoint for test point generation; removed geomFromText and pointText helper bindings; added setSRID binding wrapping unsafeSqlFunction "ST_SetSRID".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A geometry dance, refined with care,
No WKT strings floating through the air,
SQL functions now lead the way,
SRID-set shapes brightening the day! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: normalizing both geometry and point to SRID 4326 in the containsPoint function, which directly corresponds to the refactoring shown in the summary.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/containsPoint

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/mobility-core/src/Kernel/Storage/Esqueleto/Functions.hs`:
- Around line 87-88: The containsPoint implementation currently uses setSRID
(ST_SetSRID(geom, 4326)) which only relabels metadata and can silently
misclassify coordinates; mirror the safer containsPointGeom pattern instead:
replace the unsafeSqlValue "geom" SRID relabeling with constructing the query
geometry explicitly (use ST_GeomFromText/POINT with lon/lat and SRID 4326 or
delegate to the containsPointGeom helper), updating the args construction and
any references to setSRID so the point is created with explicit SRID rather than
just relabeling the existing geom metadata.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: da16042f-9f48-433a-9cd6-97a43fcdb02e

📥 Commits

Reviewing files that changed from the base of the PR and between c05a1e0 and fffc346.

📒 Files selected for processing (1)

• lib/mobility-core/src/Kernel/Storage/Esqueleto/Functions.hs

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether geometry storage/usage guarantees 4326 coordinates.

set -euo pipefail

echo "1) Find geom column definitions and SRID constraints in migrations/schema files"
fd -HI . | xargs rg -n -C2 -P 'geom|geometry\(|geography\(|SRID|4326|AddGeometryColumn|ST_SetSRID|ST_Transform' || true

echo
echo "2) Find writes/builders of geom to detect source SRID assumptions"
rg -n -C2 -P 'ST_SetSRID\s*\(|ST_Transform\s*\(|ST_GeomFrom(Text|GeoJSON)\s*\(|ST_Point\s*\(' || true

echo
echo "3) Find all containsPoint implementations/callers for consistent CRS handling"
rg -n -C2 -P '\bcontainsPoint\b|\bst_contains\b' || true

Repository: nammayatri/shared-kernel

Length of output: 50381

---

Inconsistent SRID handling in containsPoint — consider containsPointGeom pattern

On Lines 87-88, ST_SetSRID(geom, 4326) only relabels SRID metadata; it does not reproject coordinates. While this works if geom is guaranteed to hold EPSG:4326 coordinates with missing metadata, the codebase has an alternative implementation containsPointGeom (lines 90–96) that constructs the point explicitly with SRID using ST_GeomFromText. For consistency and safety, either:

• Confirm via schema/migrations that geom column always stores 4326 coordinates and document this assumption, or
• Use the containsPointGeom pattern throughout to avoid silent coordinate misclassification.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/mobility-core/src/Kernel/Storage/Esqueleto/Functions.hs` around lines 87
- 88, The containsPoint implementation currently uses setSRID (ST_SetSRID(geom,
4326)) which only relabels metadata and can silently misclassify coordinates;
mirror the safer containsPointGeom pattern instead: replace the unsafeSqlValue
"geom" SRID relabeling with constructing the query geometry explicitly (use
ST_GeomFromText/POINT with lon/lat and SRID 4326 or delegate to the
containsPointGeom helper), updating the args construction and any references to
setSRID so the point is created with explicit SRID rather than just relabeling
the existing geom metadata.