Add more spatial extras (length, area, distance_to_filter and intersection_area)

[LionelZoubritzky-IGN]

This PR introduces a number of small refactors (each in their own commit) and finally adds four new spatial_extras targets: length, area, distance_to_filter and intersection_area. The latter two can only be used when there is a spatial filter.

Requests error when they are absurd, for example asking for area on a geometry which is stored as format: "geometry-point" in the schema. When they are not absurd but still impossible in practice (the same but with "geometry-any" that actually returns a Point for example), the required field is set to null.

Close #136, although we may want to add even more spatial_extra in the future. For the time being, these new spatial_extras allow answering new questions like:

• how long is Saône river exactly ? (length) -> 515,08 km
• give me the 5 largest parks in Tours (area) -> La Cousinerie (24ha), Sainte-Radegonde (16ha), ...
• what is the swimming pool closest to the "Lux" cinema in Caen? (distance_to_filter) -> Piscine du Sivom des Trois Vallées de Mondeville (1.37 km)
• what is the surface of forest within 5km of Orléans? (intersection_area) -> 1 039,3 ha

[esgn]

Better to isolate refactors in single PRs than to mix it with a thematic PR. See the new pull_request_template.md

[esgn]

intersect() is re-run against the full reference geometry for every returned feature, and spatialFilterToGeometry + geometryToPolygons (lines 126-127) are recomputed on every iteration.

Measured with a real reference (département 35, a MultiPolygon with 60,697 vertices) against real communes: 431 ms per feature, i.e. 43 s of blocked event loop at limit=100 — the default — and ~36 min at limit=5000. That is exactly the scenario of the integration test this branch adds (communes × département). Nothing bounds it upstream: the CQL goes out in a POST body, so no URL length limit caps the size of the reference geometry.

Under http-stream transport this freezes every session, and the AbortControllers armed in helpers/http.ts:157 during the freeze all fire at once when it lifts → cascading 504s for unrelated callers.

Measured fix (×217, areas bit-identical — bboxClip, bbox and dropEmptyRings are already available in this file):

case "intersects_feature":
case "travel_time": {
  const spatialFilterGeometry = spatialFilterToGeometry(spatialFilter, resolvedGeometryRef);
  const filterPolygons = geometryToPolygons(spatialFilterGeometry);
  if (!filterPolygons) return null;
  // Whatever lies outside the feature's bbox cannot intersect it, so clipping the
  // reference first leaves the result unchanged while polyclip only ever processes
  // the neighbouring vertices.
  const clippedFilter = dropEmptyRings(bboxClip(filterPolygons, bbox(geo)).geometry);
  if (!clippedFilter) return null;
  const inter = intersect(featureCollection([feature(clippedFilter), feature(geo)]));
  return inter === null ? null : inter.geometry;
}

⚠️ The only sound bbox shortcut is "disjoint bboxes → 0". The symmetric one — "feature bbox contained in the filter bbox → area(geo)" — is wrong: a département is concave.

[LionelZoubritzky-IGN]

8f8f5c5

[esgn]

=> The filter geometry and its centroid are recomputed for every feature.

spatialFilterToCentroid / spatialFilterToGeometry depend only on input and resolvedGeometryRef, both constant across the whole collection — yet they are called from the per-feature loop in transformFeatureCollectionResponse (response.ts:94-100).

Measured: centroid(département 35) costs 2.79 ms per call, i.e. ~93% of the 3.0 ms per feature spent on distance_to_filter → 15 s of pure waste at limit=5000 for an identical result. Same pattern for the circle(64) of dwithin_point and the bboxPolygon of bbox_filter.

Suggestion: a prepareSpatialContext(input, resolvedGeometryRef) computed once in transformFeatureCollectionResponse (filter geometry, its polygons, its centroid) and passed down to deriveFromGeometry. It is also the natural home for the intersection_area performance fix.

[LionelZoubritzky-IGN]

8f8f5c5

[esgn]

case "travel_time" falls into the centroid(spatialFilterToGeometry(...)) branch, so it measures from the centroid of the isochrone even though travelTimeFilterSchema already carries lon/lat, described as "Longitude / Latitude du point de départ" (schema.ts:95-96). That is exact, free, and it is what a user means by "distance to the travel-time filter".

Suggestion : move case "travel_time" into the dwithin_point / intersects_point branch and return { type: "Point", coordinates: [spatialFilter.lon, spatialFilter.lat] }. Then align the description at schema.ts:180, which currently says "le centroïde du filtre" without distinguishing the cases.

[LionelZoubritzky-IGN]

8f8f5c5 (I forgot to commit it separately)

[esgn]

=< intersection_area returns 0 where the published contract promises null.

intersectionAreaWithSpatialFilter returns null for three distinct situations — "nothing areal"
(l. 113-115), "point filter" (l. 119) and "empty intersection" and this line collapses all three
into 0.

The entry point is real: getGeometryTypeDimension("GeometryCollection") returns "?" (properties.ts:81), which short-circuits the validation loop (properties.ts:153), so the 29 geometry-any collections in the catalog (wfs_du, wfs_sup, wfs_scot) pass with no dimension check at all.

Suggestion: have intersectionAreaWithSpatialFilter return number | null, so the distinction is carried by the callee instead of being reconstructed by the caller:

// `null` = not computable (nothing areal in the geometry)
// `0`    = computed, no overlap (including a point filter: a point has an empty interior)

The intersects_point → 0 semantics are preserved: that value is geometrically correct.

[LionelZoubritzky-IGN]

I'm not completely clear on what we actually want in the end. What makes most sense to me is the following:

1. If, for any reason, we cannot compute what is asked (for example, the centroid of a feature whose WFS did not return a geometry), return null for that specific spatial_extra.
2. if the LLM already has the information at query-time that should let it know that the query itself is asking for something trivial or absurd (for example, the intersection area with a point filter), error immediately with an explicit message stating why you should not ask for that.

In the case of area intersection here, I see the following possibilities:

• The filter is not 2D and the LLM should know that: throw an error. This should the behavior for intersects_point_filter only. It's not the case currently.
• The filter is not 2D but it is not trivial to see (the LLM does not necessarily have this information): return intersection_area=0: simple and correct. return intersection_area=null. This should be the behavior for a non-2D intersects_feature_filter. see next comment
• The filter is 2D and the intersection with the geometry is empty: return intersection_area=0. This can happen with bbox_filter because of Document how bbox_filter works #196 for example.

@esgn what do you think?

[LionelZoubritzky-IGN]

Actually, after writing #187 (comment), I think it's just easier to put intersection_area to 0 in the second case, instead of intersection_area=null. We should reserve null for error cases (for instance, there was an internal error when computing the intersection).

[LionelZoubritzky-IGN]

Tentative in c972383, let me know

[esgn]

First in schema.ts it is written

    "Si l'élément à calculer est trivial (bbox d'un point, aire d'une géométrie linéaire), une erreur indiquera comment corriger la requête.\n"+
    "Si une valeur n'est pas calculable pour une autre raison, elle sera remplacée par `null` dans la réponse.

So :

• case 1 : the mismatch is knowable before the query, from the catalog => error, the whole request fails
• case 2 : the mismatch only shows up on that one feature => null
• case 3 : the computation ran => a number, 0 included

Examining the current version of the code :

Extra	Returns	null should mean	State
centroid	{lon, lat}	geometry missing or unreadable	ok
bbox	4-value array	geometry missing, unreadable or empty	off: an empty geometry yields [null,null,null,null], a third shape that is neither a bbox nor null
length	a number	non-linear geometry, so 0 only ever means a line of zero length	off: returns null for any GeometryCollection, even one made only of LineStrings, where the length is computable and area does walk collections
area	a number	non-areal geometry, so 0 only ever means a polygon of zero area	off: an empty Polygon yields 0
distance_to_filter	a number	geometry or filter unreadable, so 0 only ever means the feature covers the reference point	ok
intersection_area	a number	feature or filter not areal, so 0 only ever means genuinely no overlap	off: five cases out of six yield 0

[esgn]

does the brief in #136 makes things clearer ?

[esgn]

.required() is a no-op here: it only unwraps ZodOptional, and this is a ZodEffects(ZodDefault(ZodArray)). Verified on the published schema required === ["typename"].

So the comment credits the wrong construct: .default([]) alone guarantees the field.

Drop both .required() calls (here and l. 405), move the comment, and pin the invariant with expect(gpfGetFeaturesInputSchema.parse({ typename: "X" })).toHaveProperty("spatial_extras").

[LionelZoubritzky-IGN]

bc2e07f

[esgn]

This promises "…elle sera remplacée par null dans la réponse", but validateSpatialExtras (properties.ts:134-160) now fails the whole request instead. And the bbox-on-geometry-point rejection (70 collections) is documented nowhere, unlike length/area at l. 154-155.

Restrict the null sentence to what is not computable per feature, and state that a dimension mismatch is a hard error.

[LionelZoubritzky-IGN]

It's like for #187 (comment): I think we need to distinguish trivial queries (like bbox on a geometry-point, or a dimension mismatch), which should error, and failures to compute, which should be marked with a null in the answer, so the LLM knows that something went wrong.

The edge-case is when we detect a "trivial" case only after running a WFS query, typically for a intersects_feature_filter where the feature ends up not having the correct dimension. I think null is the correct answer here.

If you agree with this, I can document it.

[LionelZoubritzky-IGN]

See c972383

[esgn]

I'm starting a review commit by commit to try and not miss anything.

Regarding bbc29e7

In src/tools/GpfGetFeatureByIdTool.ts:89

• Now that executeGetFeatureById takes a GpfGetFeatureByIdInput, and validatedInput is one, this field-by-field projection only existed to feed the intermediate type this commit removes => return executeGetFeatureById(validatedInput);
• The schema is .strict(), so keeping the projection just adds drift risk a field added to the schema later would be silently dropped here, with no tsc error.

[esgn]

Now commit 2 : 026c561

1. I get the idea. Dealing with // TODO: use a more solid guard that will not break at the first contract change in queryPreparation.ts. But I tend to think limit would be a better choice than spatial_extras. What will happen in gpf_get_features_layer if we use spatial_extras as a discriminant ?
2. .merge(gpfGeometryExtraInputSchema.required()) Are you sure this required() is doing the job ? Is it not this part that guarantee spatial_extras presence

const gpfGeometryExtraInputSchema = z.object({
  spatial_extras: z
    .array(z.enum(GPF_GET_FEATURES_SPATIAL_EXTRAS))
    .default([]) <= in here

[esgn]

The wrapper is redundant now (and being a catch-all, it also staples the hint onto network failures and overwrites error.name) => suggest dropping it.

Nit: these two new messages are the only Utilise in src/ (2 vs 15 Utiliser).

[LionelZoubritzky-IGN]

I don't see what wrapper you mean to drop here?

[esgn]

Commit 3 : bdeadda

Done in previous comments.

[esgn]

Nice cleanup => I checked the derived SpatialFilter against the hand-written union it replaces
(discriminated field access, literal operator, never exhaustiveness) and it is strictly equivalent,
with key order unchanged.

One thing it makes load-bearing: key.replace(/_filter$/, "") assumes every filter key ends in _filter. If one ever doesn't, the operator silently becomes the whole key, SpatialFilterEntry yields never so that member drops out of the union, and the default-less switch in queryPreparation.ts:198 returns cqlFilter: undefined => a query with no spatial filter at all, and no error.

Cheap guard: default: { const _: never = spatialFilter; } in that switch.

[LionelZoubritzky-IGN]

f3b630b

I think it's a bit redundant with the identical guard already present in wfs/spatialExtras.ts, but it doesn't hurt either.

[esgn]

Commit 4 : 0892147

Done in previous comment

[esgn]

Good deduplication (the two copies were byte-identical).
value is Exclude<Geometry, GeometryCollection> is exactly what the body checks.

But the return type is new, and value is Geometry is not what the body checks. It requires coordinates, so it rejects a valid GeometryCollection (which carries geometries instead), and it never looks at what coordinates holds, so { type: "CustomType", coordinates: null } passes and gets narrowed to Geometry. value is Exclude<Geometry, GeometryCollection> would describe the body exactly.

➡️ Related: test/wfs/geometry.test.ts drops "should throw for a completely unknown type" because "CustomType" no longer typechecks, but that is the very case this guard still lets through from remote data. Worth keeping with a cast.

[LionelZoubritzky-IGN]

I narrowed the return type to Exclude<Geometry, GeometryCollection>, which has no downstream effect since callers widen it back to Geometry, and I don't see the point in threading the precise GeometryCollection exclusion everywhere. But the test is relevant, so I added it back in b56ddaf

[esgn]

Commit 5 : fba3ca3

Done in previous comments

[esgn]

The summary line is now wrong: this function is what serializes the geometry, so it is no longer "already serialized" when it arrives. The @param below was updated, this line was not.

 * Compiles an `intersects_feature` spatial filter from the resolved reference geometry.

[LionelZoubritzky-IGN]

4a9deee

[esgn]

Commit 6 : 8b1d2d5

Done in previous comments