diff --git a/ui-react/src/graphql/generated/modeling.ts b/ui-react/src/graphql/generated/modeling.ts index b989d77..a0fc7d4 100644 --- a/ui-react/src/graphql/generated/modeling.ts +++ b/ui-react/src/graphql/generated/modeling.ts @@ -127,11 +127,25 @@ export type Thread = { response_variable_id?: Maybe; driving_variable?: Maybe; response_variable?: Maybe; + /** + * The thread's region, with the geometries the Datasets step narrows on. + * `region_id` alone cannot do that job — it names the region without saying + * where it is. + */ + region?: Maybe; events: ThreadProvenance[]; permissions: ThreadPermission[]; thread_models?: ThreadModel[]; }; +export type ThreadRegion = { + __typename?: 'region'; + id: string; + name?: Maybe; + /** Hasura returns the jsonb `geometry` column already parsed. */ + geometries: Array<{ id: number; geometry?: Maybe }>; +}; + export type Task = { __typename?: 'task'; id: string; @@ -290,6 +304,14 @@ const THREAD_INFO = gql` id name } + region { + id + name + geometries { + id + geometry + } + } thread_models { id thread_id diff --git a/ui-react/src/lib/__tests__/data-catalog.test.ts b/ui-react/src/lib/__tests__/data-catalog.test.ts index 14c4211..2cb87e6 100644 --- a/ui-react/src/lib/__tests__/data-catalog.test.ts +++ b/ui-react/src/lib/__tests__/data-catalog.test.ts @@ -59,6 +59,92 @@ const PROSE_MATCH: CkanPackage = { resources: [{ id: 'r4', format: 'pdf', mint_standard_variables: '' }], }; +// ─── Region fixtures (issue #97) ────────────────────────────────────────────── +// +// Every geometry shape below is one TACC actually holds in `spatial`. The old +// extractor read `coordinates[0]` and understood a bare Polygon only, so a +// client-side region filter built on it would have dropped the rest in silence. + +/** Texas, near enough — the bounding box measured off the region's geometry. */ +const TEXAS = { xmin: -106.64, xmax: -93.52, ymin: 25.84, ymax: 36.5 }; + +const TEXAS_POLYGON = JSON.stringify({ + type: 'Polygon', + coordinates: [ + [ + [-106.64, 25.84], + [-93.52, 25.84], + [-93.52, 36.5], + [-106.64, 36.5], + [-106.64, 25.84], + ], + ], +}); + +function annotated(name: string, spatial?: string): CkanPackage { + return { + id: `uuid-${name}`, + name, + title: name, + ...(spatial ? { spatial } : {}), + resources: [{ id: `r-${name}`, format: 'csv', mint_standard_variables: 'a' }], + }; +} + +const IN_TEXAS = annotated('in-texas', TEXAS_POLYGON); +const IN_ALASKA = annotated( + 'in-alaska', + JSON.stringify({ + type: 'Polygon', + coordinates: [ + [ + [-150, 60], + [-149, 60], + [-149, 61], + [-150, 61], + [-150, 60], + ], + ], + }), +); +const NO_LOCATION = annotated('no-location'); +const FEATURE_IN_TEXAS = annotated( + 'feature-in-texas', + JSON.stringify({ + type: 'Feature', + properties: {}, + geometry: JSON.parse(TEXAS_POLYGON) as unknown, + }), +); +const FEATURE_COLLECTION_IN_TEXAS = annotated( + 'feature-collection-in-texas', + JSON.stringify({ + type: 'FeatureCollection', + features: [{ type: 'Feature', properties: {}, geometry: JSON.parse(TEXAS_POLYGON) as unknown }], + }), +); +const POINT_IN_TEXAS = annotated( + 'point-in-texas', + JSON.stringify({ type: 'Point', coordinates: [-97.74, 30.27] }), +); +const MULTI_IN_ALASKA = annotated( + 'multipolygon-in-alaska', + JSON.stringify({ + type: 'MultiPolygon', + coordinates: [ + [ + [ + [-150, 60], + [-149, 60], + [-149, 61], + [-150, 61], + [-150, 60], + ], + ], + ], + }), +); + beforeEach(() => { searchRequests = []; window.__MINT_CONFIG__ = { DATA_CATALOG_API: CKAN_HOST } as never; @@ -120,13 +206,13 @@ describe('findDatasets', () => { expect(found).toHaveLength(1); }); - it('keeps the bounding box on the request', async () => { + it('never sends ext_bbox: the region is applied here, so nothing is dropped for it', async () => { stubSearch([]); await findDatasets({ standard_variable_names__in: ['a'], spatial_coverage__intersects: { xmin: -100, xmax: -97, ymin: 29, ymax: 31 }, }); - expect(searchRequests[0]?.searchParams.get('ext_bbox')).toBe('-100,29,-97,31'); + expect(searchRequests[0]?.searchParams.has('ext_bbox')).toBe(false); }); it('still narrows by date range', async () => { @@ -148,6 +234,52 @@ describe('findDatasets', () => { expect(found).toHaveLength(2); }); + it('labels a dataset inside the region, and one outside it, without dropping either', async () => { + stubSearch([IN_TEXAS, IN_ALASKA]); + const found = await findDatasets({ spatial_coverage__intersects: TEXAS }); + expect(found.map((d) => [d.id, d.region_match])).toEqual([ + [IN_TEXAS.name, 'inside'], + [IN_ALASKA.name, 'outside'], + ]); + }); + + it('labels a dataset with no spatial field unknown, not outside', async () => { + // The defect this guards: ext_bbox filtered on *having* a location, so + // these never reached the client — 11 of TACC's 33 annotated packages. + stubSearch([NO_LOCATION]); + const found = await findDatasets({ spatial_coverage__intersects: TEXAS }); + expect(found).toHaveLength(1); + expect(found[0]?.region_match).toBe('unknown'); + }); + + it('places a Feature and a FeatureCollection, which ext_bbox drops entirely', async () => { + stubSearch([FEATURE_IN_TEXAS, FEATURE_COLLECTION_IN_TEXAS, POINT_IN_TEXAS, MULTI_IN_ALASKA]); + const found = await findDatasets({ spatial_coverage__intersects: TEXAS }); + expect(found.map((d) => d.region_match)).toEqual(['inside', 'inside', 'inside', 'outside']); + }); + + it('calls everything inside when no region is asked for', async () => { + stubSearch([IN_TEXAS, IN_ALASKA, NO_LOCATION]); + const found = await findDatasets({}); + expect(found.every((d) => d.region_match === 'inside')).toBe(true); + }); + + it('accepts a regions list of geometries as the region', async () => { + stubSearch([IN_TEXAS, IN_ALASKA]); + const found = await findDatasets({ + spatial_coverage__intersects: [JSON.parse(TEXAS_POLYGON) as unknown], + }); + expect(found.map((d) => d.region_match)).toEqual(['inside', 'outside']); + }); + + it('applies no region filter when the region carries no usable extent', async () => { + // An empty geometry list must not read as an empty box, which would call + // the whole catalog "elsewhere". + stubSearch([IN_TEXAS, IN_ALASKA]); + const found = await findDatasets({ spatial_coverage__intersects: [] }); + expect(found.map((d) => d.region_match)).toEqual(['inside', 'inside']); + }); + it('caps the datasets returned by limit, without capping what it fetches', async () => { stubSearch([CARRIER, { ...CARRIER, id: 'uuid-3', name: 'second-carrier' }]); const found = await findDatasets({ diff --git a/ui-react/src/lib/data-catalog.ts b/ui-react/src/lib/data-catalog.ts index 85c4f04..5c41427 100644 --- a/ui-react/src/lib/data-catalog.ts +++ b/ui-react/src/lib/data-catalog.ts @@ -14,6 +14,7 @@ import { cleanString, overlapsDateRange, + packageRegionMatch, packagesMatchingVariables, packageTags, packageTimePeriod, @@ -23,7 +24,9 @@ import { showPackage, type CkanPackage, type CkanResource, + type RegionMatch, } from './datasets/ckan'; +import { geoJsonBoundingBox, unionBoundingBox, type BoundingBox } from './geo/bbox'; // ─── Domain types ───────────────────────────────────────────────────────────── @@ -50,6 +53,12 @@ export interface DataCatalogDataset { id: string; name: string; region: string; + /** + * Where this dataset sits relative to the region asked for. `unknown` means + * the dataset declares no location at all — it is not a claim that the + * dataset is elsewhere, and the UI must not hide it as though it were. + */ + region_match: RegionMatch; variables: string[]; datatype: string; time_period: DataCatalogTimePeriod | null; @@ -84,13 +93,18 @@ export interface DatasetQueryParams { // ─── Internal shape helpers ─────────────────────────────────────────────────── -function datasetFromCkanPackage(pkg: CkanPackage, variables: string[]): DataCatalogDataset { +function datasetFromCkanPackage( + pkg: CkanPackage, + variables: string[], + regionMatch: RegionMatch, +): DataCatalogDataset { const resources = pkg.resources ?? []; return { // Prefer the name slug: it is what CKAN URLs use and package_show accepts. id: cleanString(pkg.name) || cleanString(pkg.id), name: cleanString(pkg.title) || cleanString(pkg.name), region: '', + region_match: regionMatch, variables, // CKAN has no datatype field; resource formats are the closest analogue. datatype: cleanString(resources[0]?.format), @@ -134,16 +148,21 @@ function resourceFromCkanResource(row: CkanResource, parent: CkanPackage): DataC * * The variable names are NOT sent as a CKAN `q`. They live on each resource in * `mint_standard_variables`, which Solr neither indexes nor tokenises usefully, - * so a free-text query matches prose instead of the annotation. Fetch the - * bbox-filtered catalog and match the field here, as the legacy Lit client does. + * so a free-text query matches prose instead of the annotation. Fetch the whole + * catalog and match the field here, as the legacy Lit client does. + * + * The region is applied here too, and **nothing is dropped for it**. Every + * dataset comes back carrying `region_match`, so the caller can show what has + * no location and count what is elsewhere. `ext_bbox` cannot express that: it + * filters on *having* a location, so a dataset with no `spatial` field never + * reaches the client to be reasoned about — 11 of TACC's 33 annotated packages, + * for every region. */ export async function findDatasets(params: DatasetQueryParams): Promise { const variables = params.standard_variable_names__in ?? []; - const boundingBox = toBoundingBox(params.spatial_coverage__intersects); + const region = toBoundingBox(params.spatial_coverage__intersects); - const packages = await searchAllPackages({ - ...(boundingBox ? { boundingBox } : {}), - }); + const packages = await searchAllPackages({}); // CKAN cannot filter reliably on temporal extras, so narrow the window here. const start = params.end_time__lte ? new Date(params.end_time__lte) : null; @@ -157,27 +176,38 @@ export async function findDatasets(params: DatasetQueryParams): Promise datasetFromCkanPackage(pkg, variables)); + return capped.map((pkg) => + datasetFromCkanPackage(pkg, variables, packageRegionMatch(pkg, region)), + ); } /** - * Coax the legacy `spatial_coverage__intersects` payload into a bounding box. - * Callers pass either a bare {xmin,xmax,ymin,ymax} or the MINT - * `{ type: 'BoundingBox', value: {...} }` envelope; anything else is ignored. + * Coax the `spatial_coverage__intersects` payload into a bounding box. + * + * Callers pass a region's GeoJSON geometries, a bare {xmin,xmax,ymin,ymax}, or + * the MINT `{ type: 'BoundingBox', value: {...} }` envelope. Anything with no + * usable extent yields undefined, which means "no region filter" — never an + * empty box, which would classify the whole catalog as elsewhere. */ -function toBoundingBox( - raw: unknown, -): { xmin: number; xmax: number; ymin: number; ymax: number } | undefined { +function toBoundingBox(raw: unknown): BoundingBox | undefined { if (!raw || typeof raw !== 'object') return undefined; + + // A region carries a list of geometries; take the box that encloses them all. + if (Array.isArray(raw)) return unionBoundingBox(raw) ?? undefined; + const obj = raw as Record; const candidate = ( obj['value'] && typeof obj['value'] === 'object' ? obj['value'] : obj ) as Record; const coords = ['xmin', 'xmax', 'ymin', 'ymax'].map((k) => Number(candidate[k])); - if (coords.some((n) => !isFinite(n))) return undefined; - const [xmin, xmax, ymin, ymax] = coords as [number, number, number, number]; - return { xmin, xmax, ymin, ymax }; + if (!coords.some((n) => !isFinite(n))) { + const [xmin, xmax, ymin, ymax] = coords as [number, number, number, number]; + return { xmin, xmax, ymin, ymax }; + } + + // Not a bounding box — try to read it as GeoJSON. + return geoJsonBoundingBox(raw) ?? undefined; } /** diff --git a/ui-react/src/lib/datasets/ckan.ts b/ui-react/src/lib/datasets/ckan.ts index 0f8b1e9..43eb2d5 100644 --- a/ui-react/src/lib/datasets/ckan.ts +++ b/ui-react/src/lib/datasets/ckan.ts @@ -25,6 +25,7 @@ */ import { getDataCatalogApiUrl } from '../config'; +import { boundingBoxesOverlap, geoJsonBoundingBox, type BoundingBox } from '../geo/bbox'; import type { DateRange, SpatialCoverage } from './types'; @@ -78,13 +79,7 @@ interface CkanSearchResult { results?: CkanPackage[]; } -/** Bounding box in the order CKAN's `ext_bbox` expects. */ -export interface BoundingBox { - xmin: number; - xmax: number; - ymin: number; - ymax: number; -} +export type { BoundingBox }; // ─── Transport ──────────────────────────────────────────────────────────────── @@ -245,6 +240,45 @@ export function packageSpatialCoverage(pkg: CkanPackage): SpatialCoverage | unde } } +/** + * Where a package sits relative to the region asked for. + * + * Three answers, not two: "no location declared" and "a location, elsewhere" + * are different claims about the data and the Datasets step treats them + * differently — the first is shown and badged, the second hidden and counted. + * Collapsing them is what made a third of TACC's annotated catalog invisible. + */ +export type RegionMatch = 'inside' | 'outside' | 'unknown'; + +/** + * A package's bounding box, from ckanext-spatial's stringified GeoJSON. + * + * Handles every shape TACC actually holds — Polygon, MultiPolygon, Point, + * Feature and FeatureCollection — unlike `packageSpatialCoverage`, which reads + * `coordinates[0]` and understands a bare Polygon only. + */ +export function packageBoundingBox(pkg: CkanPackage): BoundingBox | null { + return geoJsonBoundingBox(pkg.spatial); +} + +/** + * Classify a package against a region's bounding box. + * + * With no region asked for, everything is `inside`: there is no claim to fail. + * + * This is deliberately more forgiving than CKAN's own `ext_bbox`, which is not + * merely a server-side version of the same test. ckanext-spatial indexes bare + * geometries only, so `ext_bbox` also drops packages whose `spatial` is a + * `Feature` or a `FeatureCollection` — at TACC, two annotated packages that are + * squarely inside Texas. + */ +export function packageRegionMatch(pkg: CkanPackage, region?: BoundingBox | null): RegionMatch { + if (!region) return 'inside'; + const box = packageBoundingBox(pkg); + if (!box) return 'unknown'; + return boundingBoxesOverlap(box, region) ? 'inside' : 'outside'; +} + /** CKAN's `version` is often the literal string "None". */ export function cleanString(val: unknown): string { if (val === undefined || val === null || val === 'None') return ''; diff --git a/ui-react/src/lib/geo/__tests__/bbox.test.ts b/ui-react/src/lib/geo/__tests__/bbox.test.ts new file mode 100644 index 0000000..6b65177 --- /dev/null +++ b/ui-react/src/lib/geo/__tests__/bbox.test.ts @@ -0,0 +1,136 @@ +/** + * Tests for the shared GeoJSON bounding-box extractor (issue #97). + * + * The defect these guard against: the two extractors this replaces each + * understood a subset of GeoJSON and returned nothing for the rest. Building a + * spatial filter on either would have hidden real datasets without a word. + * + * `packageSpatialCoverage` read `coordinates[0]`, so it understood a bare + * Polygon only. `calculateBoundingBox` recursed through `coordinates` and + * `geometries` but not `Feature.geometry` or `FeatureCollection.features`. + * TACC's CKAN holds Polygon ×103, FeatureCollection ×5, MultiPolygon, Point + * and Feature — so 8 packages sat outside the union of the two. + */ +import { describe, expect, it } from 'vitest'; + +import { boundingBoxesOverlap, geoJsonBoundingBox, unionBoundingBox } from '../bbox'; + +const SQUARE = [ + [-2, -1], + [2, -1], + [2, 1], + [-2, 1], + [-2, -1], +]; +const POLYGON = { type: 'Polygon', coordinates: [SQUARE] }; +const EXPECTED = { xmin: -2, xmax: 2, ymin: -1, ymax: 1 }; + +describe('geoJsonBoundingBox', () => { + it('reads a Polygon', () => { + expect(geoJsonBoundingBox(POLYGON)).toEqual(EXPECTED); + }); + + it('reads a MultiPolygon, which nests one ring deeper', () => { + expect(geoJsonBoundingBox({ type: 'MultiPolygon', coordinates: [[SQUARE]] })).toEqual(EXPECTED); + }); + + it('reads a Point, whose coordinates are a bare position', () => { + expect(geoJsonBoundingBox({ type: 'Point', coordinates: [-97.74, 30.27] })).toEqual({ + xmin: -97.74, + xmax: -97.74, + ymin: 30.27, + ymax: 30.27, + }); + }); + + it('reads a position carrying an elevation', () => { + expect(geoJsonBoundingBox({ type: 'Point', coordinates: [1, 2, 300] })).toEqual({ + xmin: 1, + xmax: 1, + ymin: 2, + ymax: 2, + }); + }); + + it('reads a Feature, which holds its geometry one level down', () => { + expect(geoJsonBoundingBox({ type: 'Feature', properties: {}, geometry: POLYGON })).toEqual( + EXPECTED, + ); + }); + + it('reads a FeatureCollection, which holds a list of features', () => { + expect( + geoJsonBoundingBox({ + type: 'FeatureCollection', + features: [{ type: 'Feature', properties: {}, geometry: POLYGON }], + }), + ).toEqual(EXPECTED); + }); + + it('reads a GeometryCollection', () => { + expect(geoJsonBoundingBox({ type: 'GeometryCollection', geometries: [POLYGON] })).toEqual( + EXPECTED, + ); + }); + + it('spans every member of a collection, not just the first', () => { + expect( + geoJsonBoundingBox({ + type: 'GeometryCollection', + geometries: [POLYGON, { type: 'Point', coordinates: [10, 10] }], + }), + ).toEqual({ xmin: -2, xmax: 10, ymin: -1, ymax: 10 }); + }); + + it('parses a JSON string, which is how CKAN stores it', () => { + expect(geoJsonBoundingBox(JSON.stringify(POLYGON))).toEqual(EXPECTED); + }); + + it('returns null for absent, unparseable or coordinate-free input', () => { + expect(geoJsonBoundingBox(undefined)).toBeNull(); + expect(geoJsonBoundingBox('{not json')).toBeNull(); + expect(geoJsonBoundingBox({ type: 'Polygon', coordinates: [] })).toBeNull(); + expect(geoJsonBoundingBox({ type: 'FeatureCollection', features: [] })).toBeNull(); + }); +}); + +describe('unionBoundingBox', () => { + it('encloses every value that has an extent', () => { + expect(unionBoundingBox([POLYGON, { type: 'Point', coordinates: [10, -10] }])).toEqual({ + xmin: -2, + xmax: 10, + ymin: -10, + ymax: 1, + }); + }); + + it('ignores the values that have none', () => { + expect(unionBoundingBox([null, 'not json', POLYGON])).toEqual(EXPECTED); + }); + + it('is null when nothing has an extent', () => { + expect(unionBoundingBox([])).toBeNull(); + expect(unionBoundingBox([null, undefined])).toBeNull(); + }); +}); + +describe('boundingBoxesOverlap', () => { + const a = { xmin: 0, xmax: 10, ymin: 0, ymax: 10 }; + + it('is true when the boxes intersect', () => { + expect(boundingBoxesOverlap(a, { xmin: 5, xmax: 15, ymin: 5, ymax: 15 })).toBe(true); + }); + + it('is true when one contains the other', () => { + expect(boundingBoxesOverlap(a, { xmin: 1, xmax: 2, ymin: 1, ymax: 2 })).toBe(true); + }); + + it('is true when they only touch: a dataset on the border is not elsewhere', () => { + expect(boundingBoxesOverlap(a, { xmin: 10, xmax: 20, ymin: 0, ymax: 10 })).toBe(true); + }); + + it('is false when they are apart on either axis', () => { + expect(boundingBoxesOverlap(a, { xmin: 11, xmax: 20, ymin: 0, ymax: 10 })).toBe(false); + expect(boundingBoxesOverlap(a, { xmin: 0, xmax: 10, ymin: 11, ymax: 20 })).toBe(false); + }); +}); diff --git a/ui-react/src/lib/geo/bbox.ts b/ui-react/src/lib/geo/bbox.ts new file mode 100644 index 0000000..56e0358 --- /dev/null +++ b/ui-react/src/lib/geo/bbox.ts @@ -0,0 +1,112 @@ +/** + * GeoJSON bounding boxes. + * + * One extractor, shared by everything that has to reduce a GeoJSON value to a + * rectangle: region geometries from Hasura, and CKAN's `spatial` field. + * + * It exists because the partial versions it replaces dropped data silently. + * `packageSpatialCoverage` read `coordinates[0]` and so understood a bare + * Polygon only, while TACC's catalog also holds `FeatureCollection`, `Feature`, + * `Point` and `MultiPolygon`; `calculateBoundingBox` recursed through + * `coordinates` and `geometries` but not through `Feature.geometry` or + * `FeatureCollection.features`. A spatial filter built on either would have + * hidden real datasets without a word — the #94 fault again. + * + * The walk is deliberately structural rather than type-driven: it follows + * whichever of `coordinates` / `geometries` / `geometry` / `features` a node + * carries, so an unfamiliar GeoJSON shape still yields its coordinates instead + * of yielding nothing. + */ + +export interface BoundingBox { + xmin: number; + xmax: number; + ymin: number; + ymax: number; +} + +/** + * Every `[lon, lat]` pair reachable from a GeoJSON value, at any nesting depth. + * + * A position is recognised by shape — an array whose first two entries are + * numbers — which covers `[lon, lat]` and the `[lon, lat, elevation]` GeoJSON + * also allows. + */ +function collectPositions(node: unknown, out: Array<[number, number]>): void { + if (Array.isArray(node)) { + if (node.length >= 2 && typeof node[0] === 'number' && typeof node[1] === 'number') { + out.push([node[0], node[1]]); + return; + } + for (const child of node) collectPositions(child, out); + return; + } + if (!node || typeof node !== 'object') return; + + const obj = node as Record; + // A node may legitimately carry more than one of these (a Feature holding a + // GeometryCollection, say), so every branch is followed rather than the first. + if ('coordinates' in obj) collectPositions(obj['coordinates'], out); + if ('geometries' in obj) collectPositions(obj['geometries'], out); + if ('geometry' in obj) collectPositions(obj['geometry'], out); + if ('features' in obj) collectPositions(obj['features'], out); +} + +/** + * The bounding box of any GeoJSON value — geometry, Feature, FeatureCollection + * or GeometryCollection — accepted as an object or as a JSON string. + * + * Returns null when the value is unparseable or holds no position at all. That + * is "no location", which callers must distinguish from "a location elsewhere": + * they are different claims and the Datasets step answers them differently. + */ +export function geoJsonBoundingBox(geo: unknown): BoundingBox | null { + let value = geo; + if (typeof value === 'string') { + try { + value = JSON.parse(value); + } catch { + return null; + } + } + if (!value || typeof value !== 'object') return null; + + const positions: Array<[number, number]> = []; + collectPositions(value, positions); + if (!positions.length) return null; + + let xmin = Infinity; + let xmax = -Infinity; + let ymin = Infinity; + let ymax = -Infinity; + for (const [lon, lat] of positions) { + if (lon < xmin) xmin = lon; + if (lon > xmax) xmax = lon; + if (lat < ymin) ymin = lat; + if (lat > ymax) ymax = lat; + } + return { xmin, xmax, ymin, ymax }; +} + +/** The box enclosing several GeoJSON values, or null when none carries one. */ +export function unionBoundingBox(values: unknown[]): BoundingBox | null { + let box: BoundingBox | null = null; + for (const value of values) { + const next = geoJsonBoundingBox(value); + if (!next) continue; + box = box + ? { + xmin: Math.min(box.xmin, next.xmin), + xmax: Math.max(box.xmax, next.xmax), + ymin: Math.min(box.ymin, next.ymin), + ymax: Math.max(box.ymax, next.ymax), + } + : next; + } + return box; +} + +/** True when two boxes share any area. Touching edges count as overlapping. */ +export function boundingBoxesOverlap(a: BoundingBox, b: BoundingBox): boolean { + return !(a.xmax < b.xmin || a.xmin > b.xmax || a.ymax < b.ymin || a.ymin > b.ymax); +} diff --git a/ui-react/src/pages/modeling/MintThread.tsx b/ui-react/src/pages/modeling/MintThread.tsx index e4e6aa2..a14d66b 100644 --- a/ui-react/src/pages/modeling/MintThread.tsx +++ b/ui-react/src/pages/modeling/MintThread.tsx @@ -118,6 +118,13 @@ export function MintThread({ threadId: threadIdProp }: MintThreadProps = {}) { [execRaw], ); + // The geometries the Datasets step narrows on. A thread with no region hands + // down undefined, which means "no region filter" rather than an empty extent. + const regionGeometry = useMemo( + () => (thread?.region?.geometries ?? []).map((g) => g.geometry).filter(Boolean), + [thread?.region], + ); + // The execution engine writes the run counters; nothing pushes them back, so // poll while a submitted run is still unfinished and stop as soon as it is. const runsInFlight = hasUnfinishedRuns(threadExecutionData?.execution_summary ?? {}); @@ -316,6 +323,7 @@ export function MintThread({ threadId: threadIdProp }: MintThreadProps = {}) { models={execData.models} ensembles={execData.model_ensembles} persistedData={execData.data} + regionGeometry={regionGeometry} onUpdated={handleThreadUpdated} onContinue={goNext} onBack={goBack} diff --git a/ui-react/src/pages/modeling/thread/MintDatasets.tsx b/ui-react/src/pages/modeling/thread/MintDatasets.tsx index 02cef38..55d8adb 100644 --- a/ui-react/src/pages/modeling/thread/MintDatasets.tsx +++ b/ui-react/src/pages/modeling/thread/MintDatasets.tsx @@ -12,7 +12,7 @@ * - Allow filtering/selecting individual resources for a dataset. * - On "Select & Continue", write selections to Hasura via UpdateThreadData mutation. */ -import React, { useEffect, useRef, useState } from 'react'; +import React, { useEffect, useMemo, useRef, useState } from 'react'; import { Thread, @@ -342,7 +342,7 @@ function InputDatasetPicker({ onChange, }: InputDatasetPickerProps) { const skip = !editMode && existingBindings.length > 0; - const { datasets, loading } = useDataCatalogDatasets({ + const { datasets: allDatasets, loading } = useDataCatalogDatasets({ variableNames: input.variables, regionGeometry, startDate: thread.start_date ? new Date(thread.start_date) : null, @@ -363,6 +363,16 @@ function InputDatasetPicker({ } | null>(null); const [compareDialog, setCompareDialog] = useState(null); + // `findDatasets` no longer drops datasets for the region — it labels them, so + // the wizard's Datasets step can show what has no location and count what is + // elsewhere (#97). This panel has no such affordance, so it keeps the older + // behaviour and hides the ones the region positively rules out. Datasets with + // no declared location are kept, as they always were. + const datasets = useMemo( + () => allDatasets.filter((ds) => ds.region_match !== 'outside'), + [allDatasets], + ); + // Pre-select existing bindings useEffect(() => { if (existingBindings.length > 0 && !editMode) { diff --git a/ui-react/src/pages/modeling/thread/wizard/DatasetsStep.tsx b/ui-react/src/pages/modeling/thread/wizard/DatasetsStep.tsx index 2ea0b9e..3703e2a 100644 --- a/ui-react/src/pages/modeling/thread/wizard/DatasetsStep.tsx +++ b/ui-react/src/pages/modeling/thread/wizard/DatasetsStep.tsx @@ -107,6 +107,48 @@ interface DatasetsStepProps { onBack?: () => void; } +/** + * How a dataset reads in the picker: its name, plus what is *missing* from it. + * + * Missing metadata is stated rather than hidden. A dataset with no declared + * extent is a candidate — it makes no claim to be elsewhere — but the person + * choosing it should know the region and date filters could not speak for it. + */ +export function datasetOptionLabel( + ds: Pick, + requested: RequestedRange | null, +): string { + const notes: string[] = []; + if (ds.region_match === 'unknown') notes.push('! no location'); + if (!ds.time_period) notes.push('! no dates'); + else { + const cov = dateCoverage(requested, toPeriod(ds.time_period)); + if (cov === 'full') notes.push('dates full'); + else if (cov === 'partial') notes.push('dates partial'); + } + return notes.length ? `${ds.name} · ${notes.join(' · ')}` : ds.name; +} + +/** + * Split the candidates by what the region filter can say about them. + * + * Three buckets, because there are three answers. `outside` is a positive claim + * — the dataset declares a location and it is not here — and is the only one + * worth hiding. `unknown` declares nothing, so hiding it would be an assertion + * the data does not support. + */ +export function splitByRegion(datasets: DataCatalogDataset[]): { + inRegion: DataCatalogDataset[]; + noLocation: DataCatalogDataset[]; + outside: DataCatalogDataset[]; +} { + return { + inRegion: datasets.filter((d) => d.region_match === 'inside'), + noLocation: datasets.filter((d) => d.region_match === 'unknown'), + outside: datasets.filter((d) => d.region_match === 'outside'), + }; +} + /** Per-input dataset picker — lists candidates and assigns one dataset id. Isolated per model. */ function InputPicker({ thread, @@ -123,6 +165,7 @@ function InputPicker({ assignedId: string | null; onAssign: (datasetId: string | null, dataset?: DataCatalogDataset) => void; }) { + const [showOutside, setShowOutside] = useState(false); const { datasets, loading } = useDataCatalogDatasets({ variableNames: variables, regionGeometry, @@ -131,38 +174,62 @@ function InputPicker({ skip: false, }); + const { inRegion, noLocation, outside } = useMemo(() => splitByRegion(datasets), [datasets]); + const offered = showOutside ? datasets : [...inRegion, ...noLocation]; + + // An already-bound dataset stays selectable even when the region now rules it + // out, so the control keeps showing what the thread actually holds. + const options = + assignedId && !offered.some((d) => d.id === assignedId) + ? [...offered, ...datasets.filter((d) => d.id === assignedId)] + : offered; + + const outsideToggle = outside.length > 0 && ( + + ); + if (loading) { return Loading datasets…; } - if (datasets.length === 0) { - return No matching datasets.; + if (options.length === 0) { + return ( + + No matching datasets in this region. + {outsideToggle} + + ); } return ( - { + const id = e.target.value || null; + onAssign( + id, + datasets.find((d) => d.id === id), + ); + }} + className="rounded border border-gray-300 px-2 py-1 text-xs" + > + + {options.map((ds) => ( - ); - })} - + ))} + + {outsideToggle} + ); } @@ -335,10 +402,22 @@ export function DatasetsStep({ ); } + // A region with no geometry cannot narrow anything; say so rather than + // implying a filter that never ran. + const regionHasExtent = Array.isArray(regionGeometry) + ? regionGeometry.length > 0 + : Boolean(regionGeometry); const chips = [ { icon: '📦', label: 'Input', value: 'per model input' }, ...(thread.region_id - ? [{ icon: '⌖', label: 'Region', value: thread.region_id, source: 'from Framing' }] + ? [ + { + icon: '⌖', + label: 'Region', + value: thread.region?.name ?? thread.region_id, + source: regionHasExtent ? 'from Framing' : 'from Framing · no extent, not applied', + }, + ] : []), ...(requested ? [ diff --git a/ui-react/src/pages/modeling/thread/wizard/__tests__/DatasetsStep.test.tsx b/ui-react/src/pages/modeling/thread/wizard/__tests__/DatasetsStep.test.tsx index 0c4038e..9074237 100644 --- a/ui-react/src/pages/modeling/thread/wizard/__tests__/DatasetsStep.test.tsx +++ b/ui-react/src/pages/modeling/thread/wizard/__tests__/DatasetsStep.test.tsx @@ -1,8 +1,16 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import userEvent from '@testing-library/user-event'; import { renderWithProviders, screen } from '@/test/utils/render'; import type { Thread } from '@/graphql/generated/modeling'; import type { ModelEnsembleMap, ThreadModel } from '@/graphql/generated/execution'; -import { DatasetsStep, assignmentsFromBindings, dateCoverage } from '../DatasetsStep'; +import type { DataCatalogDataset } from '@/lib/data-catalog'; +import { + DatasetsStep, + assignmentsFromBindings, + datasetOptionLabel, + dateCoverage, + splitByRegion, +} from '../DatasetsStep'; beforeEach(() => { vi.stubGlobal( @@ -101,6 +109,170 @@ describe('assignmentsFromBindings', () => { }); }); +// ─── Region handling (issue #97) ────────────────────────────────────────────── + +function dataset( + id: string, + region_match: DataCatalogDataset['region_match'], + time_period: DataCatalogDataset['time_period'] = null, +): DataCatalogDataset { + return { + id, + name: id, + region: '', + region_match, + variables: [], + datatype: 'csv', + time_period, + description: '', + version: '', + limitations: '', + source: { name: '', url: '', type: '' }, + resources: [], + }; +} + +describe('splitByRegion', () => { + it('keeps "no location" apart from "elsewhere": they are different claims', () => { + const { inRegion, noLocation, outside } = splitByRegion([ + dataset('here', 'inside'), + dataset('nowhere', 'unknown'), + dataset('elsewhere', 'outside'), + ]); + expect(inRegion.map((d) => d.id)).toEqual(['here']); + expect(noLocation.map((d) => d.id)).toEqual(['nowhere']); + expect(outside.map((d) => d.id)).toEqual(['elsewhere']); + }); +}); + +describe('datasetOptionLabel', () => { + const requested = { start: new Date('2000-01-01'), end: new Date('2026-01-01') }; + + it('badges a dataset that declares no location', () => { + expect(datasetOptionLabel(dataset('x', 'unknown'), null)).toContain('! no location'); + }); + + it('badges a dataset that declares no dates, as the date rule already implied', () => { + expect(datasetOptionLabel(dataset('x', 'inside'), requested)).toContain('! no dates'); + }); + + it('says nothing about location for a dataset inside the region', () => { + const label = datasetOptionLabel( + dataset('x', 'inside', { + start_date: new Date('1999-01-01'), + end_date: new Date('2027-01-01'), + }), + requested, + ); + expect(label).not.toContain('no location'); + expect(label).toContain('dates full'); + }); +}); + +describe('DatasetsStep region filter', () => { + /** Two annotated packages: one in the box, one far outside, one with no `spatial`. */ + const TEXAS_POLYGON = { + type: 'Polygon', + coordinates: [ + [ + [-106, 26], + [-94, 26], + [-94, 36], + [-106, 36], + [-106, 26], + ], + ], + }; + + function ckanPackage(name: string, spatial?: unknown) { + return { + id: `uuid-${name}`, + name, + title: name, + ...(spatial ? { spatial: JSON.stringify(spatial) } : {}), + resources: [{ id: `r-${name}`, format: 'csv', mint_standard_variables: 'sv-precip' }], + }; + } + + beforeEach(() => { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ + success: true, + result: { + count: 3, + results: [ + ckanPackage('austin-rain', TEXAS_POLYGON), + ckanPackage('bethel-elevation', { type: 'Point', coordinates: [-161.7, 60.79] }), + ckanPackage('gam-model-files'), + ], + }, + }), + }), + ); + }); + + function renderStep() { + return renderWithProviders( + , + ); + } + + it('offers the in-region dataset and the one with no location, and badges the latter', async () => { + renderStep(); + const picker = await screen.findByLabelText('Choose dataset'); + expect(picker).toHaveTextContent('austin-rain'); + expect(picker).toHaveTextContent('gam-model-files · ! no location'); + // 11 of TACC's 33 annotated packages have no extent; ext_bbox hid all of them. + expect(picker).toHaveTextContent('Choose · 2 options'); + }); + + it('hides the dataset that is positively elsewhere, behind a counted link', async () => { + renderStep(); + const picker = await screen.findByLabelText('Choose dataset'); + expect(picker).not.toHaveTextContent('bethel-elevation'); + expect( + screen.getByRole('button', { name: /Show 1 dataset outside this region/ }), + ).toBeInTheDocument(); + }); + + it('reveals the outside dataset when the link is used', async () => { + renderStep(); + await screen.findByLabelText('Choose dataset'); + await userEvent.click(screen.getByRole('button', { name: /Show 1 dataset/ })); + expect(screen.getByLabelText('Choose dataset')).toHaveTextContent('bethel-elevation'); + }); + + it('applies no region filter when the thread region carries no geometry', async () => { + renderWithProviders( + , + ); + const picker = await screen.findByLabelText('Choose dataset'); + expect(picker).toHaveTextContent('Choose · 3 options'); + expect(screen.getByTestId('filtered-by-banner')).toHaveTextContent(/no extent, not applied/); + }); +}); + describe('DatasetsStep', () => { it('counts a binding already written to the database', async () => { renderWithProviders( diff --git a/ui-react/src/pages/regions/regionUtils.ts b/ui-react/src/pages/regions/regionUtils.ts index f8be8d1..97d58af 100644 --- a/ui-react/src/pages/regions/regionUtils.ts +++ b/ui-react/src/pages/regions/regionUtils.ts @@ -2,12 +2,9 @@ * Shared types and utilities for region-related pages. */ -export interface BoundingBox { - xmin: number; - xmax: number; - ymin: number; - ymax: number; -} +import { unionBoundingBox, type BoundingBox } from '@/lib/geo/bbox'; + +export type { BoundingBox }; export interface RegionGeometryData { id: number; @@ -54,29 +51,15 @@ export function parseGeometry( } } -/** Calculate a bounding box from an array of geometry strings. */ +/** + * Calculate a bounding box from an array of region geometries. + * + * The walk lives in lib/geo/bbox so the Datasets step and the region map read + * the same GeoJSON the same way; this used to miss `Feature` and + * `FeatureCollection`, which CKAN and uploaded region files both produce. + */ export function calculateBoundingBox(geometries: RegionGeometryData[]): BoundingBox | null { - let xmin = 99999; - let ymin = 99999; - let xmax = -99999; - let ymax = -99999; - let hasCoords = false; - - geometries.forEach((geomObj) => { - const geom = parseGeometry(geomObj.geometry); - if (!geom) return; - const coords = extractCoordinates(geom); - coords.forEach(([lon, lat]) => { - if (lon < xmin) xmin = lon; - if (lon > xmax) xmax = lon; - if (lat < ymin) ymin = lat; - if (lat > ymax) ymax = lat; - hasCoords = true; - }); - }); - - if (!hasCoords) return null; - return { xmin, xmax, ymin, ymax }; + return unionBoundingBox(geometries.map((g) => g.geometry)); } /** A map viewport expressed as geographic edges (degrees). */ @@ -95,27 +78,6 @@ export function boundingBoxInViewport(bb: BoundingBox, vp: ViewportBounds): bool return !(bb.xmax < vp.west || bb.xmin > vp.east || bb.ymax < vp.south || bb.ymin > vp.north); } -function extractCoordinates(geom: GeoJSON.Geometry): Array<[number, number]> { - const coords: Array<[number, number]> = []; - - function recurse(obj: unknown) { - if (Array.isArray(obj)) { - if (obj.length >= 2 && typeof obj[0] === 'number' && typeof obj[1] === 'number') { - coords.push([obj[0], obj[1]]); - } else { - obj.forEach(recurse); - } - } else if (obj && typeof obj === 'object' && 'coordinates' in obj) { - recurse((obj as GeoJSON.Geometry & { coordinates: unknown }).coordinates); - } else if (obj && typeof obj === 'object' && 'geometries' in obj) { - (obj as GeoJSON.GeometryCollection).geometries.forEach(recurse); - } - } - - recurse(geom); - return coords; -} - /** Generate a unique region ID from the parent region ID. */ export function generateRegionId(parentRegionId: string, name: string): string { const slug = name