Skip to content

test(orm): document PostGIS spatial support on memory.location_geo#23

Merged
pyramation merged 2 commits intomainfrom
feat/orm-spatial-filter-test
Apr 17, 2026
Merged

test(orm): document PostGIS spatial support on memory.location_geo#23
pyramation merged 2 commits intomainfrom
feat/orm-spatial-filter-test

Conversation

@pyramation
Copy link
Copy Markdown
Contributor

@pyramation pyramation commented Apr 17, 2026

Summary

Adds integration test coverage for PostGIS on memories.location_geo and, while writing the tests, discovers that the generated spatial filter operators are broken upstream. Three memories are seeded with geography(Point, 4326) locations — SF, Oakland, and NYC — and a new describe block covers what actually works today vs. what doesn't.

What the tests assert (all passing in CI):

  • Geography columns round-trip correctly: the stored Point comes back as GeoJSON + srid: 4326 through the generated locationGeo { geojson srid } output.
  • locationGeo: { isNull: false } returns all three geo-tagged rows.
  • Regression guard: locationGeo: { bboxIntersects2D: <polygon> } currently fails with parse error - invalid geometry. The test pins this broken state so that if a future graphile-postgis release fixes it, the test will flip and we'll know to promote the operators to "supported".

Why the regression guard exists. The generated GeographyInterfaceFilter exposes bboxIntersects2D, coveredBy, covers, exactlyEquals, and intersects, but in graphile-postgis@2.9.7 their SQL resolvers pass the GeoJSON value straight through to PostgreSQL instead of wrapping it with ST_GeomFromGeoJSON(...)::geography. Only the withinDistance operator has the correct wrapping, and it is not wired into the generated filter at all. Net effect: "find memories within 5km of here" and "find memories inside this polygon" are not possible through the typed ORM today — they require raw SQL (ST_DWithin / ST_Covers). The describe block documents this in code comments.

Files touched: <ref_file file="/home/ubuntu/repos/agentic-db/packages/integration-tests/fixtures/seed/test-data.sql" /> (3-row seed append) and <ref_file file="/home/ubuntu/repos/agentic-db/packages/integration-tests/tests/orm.test.ts" /> (one new describe block).

Updates since last revision

The first pass of this PR tried to assert that bboxIntersects2D, coveredBy, and intersects returned the expected Bay-Area rows. CI surfaced parse error - invalid geometry on all three. After tracing that to graphile-postgis@2.9.7's operator SQL (no ST_GeomFromGeoJSON wrap), the tests were rewritten to (a) cover the parts that do work and (b) pin the broken state as a regression guard rather than asserting a wrong claim.

Review & Testing Checklist for Human

  • Confirm the upstream-bug analysis. The regression-guard test encodes my read of graphile-postgis@2.9.7/plugins/connection-filter-operators.js vs. within-distance-operator.js. If my reading is wrong (e.g., the "parse error" is actually caused by the input scalar, not the operator SQL), the comment in the describe block overstates the fault. Worth a skim of those two plugin files before merging.
  • Decide on the README. The README still advertises "find memories within 5km of here works out of the box" — this PR does not change that line but now has a reproducible test showing it's not true via the typed ORM. Let me know if you want a follow-up PR to soften the wording, or to wire withinDistance / dwithin into the filter schema so the claim becomes true.
  • Seed collision check. The new seed inserts 3 memories with eeeeeeee-…0001/2/3 IDs. Nothing else in orm.test.ts queries memories without filtering by its own created id, so this should be safe, but worth a once-over.

Notes

  • All 8 CI checks pass on the latest commit.
  • No changes to schema, ORM, CLI, or README. Additive: test + seed only.

Link to Devin session: https://app.devin.ai/sessions/f7a6bf96d0784c7d90012f65f0fb50c3
Requested by: @pyramation

Seeds three memories with stable IDs and geography(Point, 4326) locations
(SF, Oakland, NYC) and exercises the generated GeographyInterfaceFilter
operators against memory.location_geo:

- bboxIntersects2D with a Bay Area polygon returns SF + Oakland, not NYC
- coveredBy with the same polygon (point-in-polygon containment)
- intersects with the same polygon
- isNull: false returns all geo-tagged rows and their geojson payload

Also documents in the describe block that there is currently NO
`dwithin` / `distance` operator on GeographyInterfaceFilter, so
'find memories within 5km of here' — as advertised in the README — is
not expressible through the typed filter today and requires raw SQL
(ST_DWithin) or a client-side bounding box. The four passing assertions
narrow the README claim to what actually works (polygon/bbox
containment), rather than removing it.
@devin-ai-integration
Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

…on guard

- bboxIntersects2D/coveredBy/intersects fail upstream with 'parse error -
  invalid geometry' (graphile-postgis@2.9.7 doesnt wrap GeoJSON with
  ST_GeomFromGeoJSON for these operators)
- Keep a geography round-trip test (GeoJSON + srid output) and an
  isNull:false filter test covering whats actually supported today
- Add a regression guard that asserts the broken state so a future
  upstream fix will flip the test and alert us to promote the operators
@devin-ai-integration devin-ai-integration Bot changed the title test(orm): add PostGIS spatial filter tests on memory.location_geo test(orm): document PostGIS spatial support on memory.location_geo Apr 17, 2026
@pyramation pyramation merged commit 89e7e1a into main Apr 17, 2026
8 checks passed
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