A test for the source reader, because a no-op cannot be seen any other way
dx-shepherd asked for this with a7b12b44 and the reason is exact: every caller of readSourceFeatures is
written to do NOTHING when it finds no features, deliberately, so that a legend row for an off-screen
region degrades quietly instead of throwing. That is why the MapLibre bug produced no error, no console
line and no visual difference except a camera that did not move — and why it survived until a packet
printed the wrong 50 people.
check:pin-selection drives the reader with fixtures for both engines and no map at all:
· the MAPBOX shape (_data IS the FeatureCollection) reads its features;
· the MAPLIBRE shape (_data is { geojson }) reads the SAME features — the regression that shipped;
· a REMOTE source ({ url }) returns [] AND WARNS;
· the serialize() fallback works, a missing source is [] rather than a throw, a bare Feature is one;
· selectedPinIds ANDs its expressions, and with none returns every pin rather than none.
★ NEGATIVE-TESTED, which is the only thing that makes it worth having: run against the reader as it
shipped, it fails with exactly four findings — the maplibre shape reads 0 instead of 3, selectedPinIds
narrows to nothing, "no expressions" returns nothing, and the remote case returns [] silently. Against
the fixed reader it passes. A test that cannot fail on the code it was written for proves nothing.
★ AND THE REMOTE CASE IS NOW A DECISION, not an accident. dx-shepherd asked what { url } should do. It
returns [] and warns once per source: the features live on a server and cannot be enumerated locally, so
"I cannot read this" is a different fact from "there is nothing here". Collapsing those two is precisely
how the MapLibre bug hid for as long as it did, and no caller can tell them apart from an empty array.
Verified: check:pin-selection, check:mjs-parse, check:doc-scripts, check:package-json-unique-keys all
PASS. gates:pre was NOT run clean here — my main clone sits three landings behind origin/main, so a
newer package.json lists check:targets while that file is absent from the working tree; that failure is
my verification setup, not this change, and I checked it on an untouched tree before reporting.
This commit is contained in:
parent
8ce5b9fdd1
commit
f44da45258
3 changed files with 104 additions and 5 deletions
86
internal/check-pin-selection.mjs
Normal file
86
internal/check-pin-selection.mjs
Normal file
|
|
@ -0,0 +1,86 @@
|
|||
#!/usr/bin/env node
|
||||
/**
|
||||
* check-pin-selection — the shared source reader understands BOTH engines, and says so when it cannot read.
|
||||
*
|
||||
* ★ WHY THIS EXISTS, and it is a specific bug rather than a principle. library/maps/pin-selection.js
|
||||
* reads a GeoJSON source's features to answer "which pins does this expression select" — it backs
|
||||
* zoomToLegendRow (the camera fit on a legend row click) for research-map, country-posters and
|
||||
* doxa-simple-map, and the research map's prayer packet. It was written against Mapbox, where
|
||||
* `source._data` IS the GeoJSON. maplibre-gl wraps it:
|
||||
*
|
||||
* this._data = typeof options.data === 'string' ? { url: options.data } : { geojson: options.data }
|
||||
*
|
||||
* so `data.features` was undefined and the reader returned an EMPTY LIST on MapLibre — the keyless
|
||||
* engine the public builds use. Measured by rega on 2026-09-28: a language-family selection and a
|
||||
* prayer selection both logged "0 pin(s) selected" with the pins plainly on screen.
|
||||
*
|
||||
* ★ THE FAILURE MODE IS WHY A TEST IS THE ONLY GUARD. Every caller is written to NO-OP when it finds no
|
||||
* features — deliberately, so a legend row for an off-screen region degrades quietly rather than
|
||||
* throwing. So the engine bug produced no error, no console line and no visual difference except a
|
||||
* camera that did not move. Nothing but an assertion on both shapes can see it.
|
||||
*
|
||||
* WHAT IT ASSERTS, with no browser and no map:
|
||||
* 1. the MAPBOX shape (raw FeatureCollection on _data) reads its features;
|
||||
* 2. the MAPLIBRE shape ({ geojson }) reads the SAME features — the regression that shipped;
|
||||
* 3. a REMOTE source ({ url }) returns [] and WARNS — "cannot read" is a different fact from "found
|
||||
* none", and collapsing them is how the first bug hid;
|
||||
* 4. the serialize() fallback still works, and a missing source is [] rather than a throw;
|
||||
* 5. selectedPinIds ANDs its expressions and returns ids, so the reader and the evaluator agree.
|
||||
*
|
||||
* Headless, no network. Exit 0 pass · 1 fail.
|
||||
*/
|
||||
import { resolve, dirname } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
|
||||
const TAG = 'PIN-SELECTION'
|
||||
const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..')
|
||||
const { readSourceFeatures, selectedPinIds, matchesExpression } =
|
||||
await import(resolve(ROOT, 'library/maps/pin-selection.js'))
|
||||
|
||||
const FEATURES = [
|
||||
{ type: 'Feature', properties: { uniqueId: 'a', languageFamily: 'Afro-Asiatic' }, geometry: { type: 'Point', coordinates: [1, 1] } },
|
||||
{ type: 'Feature', properties: { uniqueId: 'b', languageFamily: 'Indo-European' }, geometry: { type: 'Point', coordinates: [2, 2] } },
|
||||
{ type: 'Feature', properties: { uniqueId: 'c', languageFamily: 'Afro-Asiatic' }, geometry: { type: 'Point', coordinates: [3, 3] } },
|
||||
]
|
||||
const COLLECTION = { type: 'FeatureCollection', features: FEATURES }
|
||||
const mapWith = (src) => ({ getSource: (id) => (id === 'pins' ? src : null) })
|
||||
|
||||
const fails = []
|
||||
const eq = (what, got, want) => { if (got !== want) fails.push(`${what}: got ${got}, expected ${want}`) }
|
||||
|
||||
// 1 + 2 — the two engines must agree, which is the whole point
|
||||
eq('mapbox shape (_data = FeatureCollection)', readSourceFeatures(mapWith({ _data: COLLECTION }), 'pins').length, 3)
|
||||
eq('maplibre shape (_data = { geojson })', readSourceFeatures(mapWith({ _data: { geojson: COLLECTION } }), 'pins').length, 3)
|
||||
|
||||
// 3 — a remote source: empty AND loud
|
||||
const warned = []
|
||||
const realWarn = console.warn
|
||||
console.warn = (...a) => warned.push(a.join(' '))
|
||||
const remote = readSourceFeatures(mapWith({ _data: { url: 'https://example.invalid/pins.json' } }), 'pins')
|
||||
console.warn = realWarn
|
||||
eq('remote ({ url }) returns no features', remote.length, 0)
|
||||
if (!warned.some((w) => w.includes('remote'))) fails.push('remote ({ url }): returned [] SILENTLY — "cannot read" must not look like "found none"')
|
||||
|
||||
// 4 — fallbacks
|
||||
eq('serialize() fallback', readSourceFeatures(mapWith({ serialize: () => ({ data: COLLECTION }) }), 'pins').length, 3)
|
||||
eq('missing source', readSourceFeatures(mapWith(null), 'pins').length, 0)
|
||||
eq('single Feature (not a collection)', readSourceFeatures(mapWith({ _data: FEATURES[0] }), 'pins').length, 1)
|
||||
|
||||
// 5 — the reader and the evaluator agree, on both shapes
|
||||
for (const [label, src] of [['mapbox', { _data: COLLECTION }], ['maplibre', { _data: { geojson: COLLECTION } }]]) {
|
||||
const ids = selectedPinIds(mapWith(src), {
|
||||
sourceId: 'pins',
|
||||
expressions: [['==', ['get', 'languageFamily'], 'Afro-Asiatic']],
|
||||
})
|
||||
eq(`${label}: selectedPinIds narrows to the matching pins`, [...ids].sort().join(','), 'a,c')
|
||||
const all = selectedPinIds(mapWith(src), { sourceId: 'pins', expressions: [] })
|
||||
eq(`${label}: no expressions means every pin, not none`, all.size, 3)
|
||||
}
|
||||
eq('expressions are ANDed', matchesExpression(['all', ['==', ['get', 'uniqueId'], 'a'], ['==', ['get', 'languageFamily'], 'Indo-European']], FEATURES[0].properties), false)
|
||||
|
||||
if (fails.length) {
|
||||
console.error(`${TAG}: FAIL`)
|
||||
for (const f of fails) console.error(` ✗ ${f}`)
|
||||
process.exit(1)
|
||||
}
|
||||
console.log(`${TAG}: PASS — both engine shapes read the same features, a remote source says so rather than looking empty, and selectedPinIds agrees with the reader.`)
|
||||
|
|
@ -125,6 +125,8 @@ export function matchesExpression(filter, props) {
|
|||
}
|
||||
return false;
|
||||
}
|
||||
const _warnedUrlSources = new Set();
|
||||
|
||||
export function readSourceFeatures(map, sourceId) {
|
||||
const src = map.getSource(sourceId);
|
||||
if (!src) return [];
|
||||
|
|
@ -134,13 +136,22 @@ export function readSourceFeatures(map, sourceId) {
|
|||
// this._data = typeof options.data === 'string' ? { url: options.data } : { geojson: options.data }
|
||||
// — so `data.features` is undefined there and every caller silently got an empty list. Mapbox stores
|
||||
// the GeoJSON directly, which is the engine this was written against, so it worked where it was
|
||||
// tested and failed where it was not.
|
||||
// Measured: rega's packet probe on MapLibre logged "0 pin(s) selected" for both a language-family
|
||||
// and a prayer selection, with the pins plainly on screen.
|
||||
// A URL source has no local features to read, and saying so is not the same as finding none.
|
||||
// tested and failed where it was not. Measured: rega's packet probe on MapLibre logged "0 pin(s)
|
||||
// selected" for both a language-family and a prayer selection, with the pins plainly on screen.
|
||||
if (data && typeof data === 'object' && !Array.isArray(data)) {
|
||||
if (data.geojson !== undefined) data = data.geojson;
|
||||
else if (data.url !== undefined) return [];
|
||||
else if (data.url !== undefined) {
|
||||
// ★ A REMOTE SOURCE IS NOT AN EMPTY ONE, and saying so out loud is the whole point. The
|
||||
// features live on a server; this cannot enumerate them. Returning [] silently is exactly
|
||||
// the shape of the bug above — a caller would no-op and look merely unlucky. Warned ONCE
|
||||
// per source so a map that legitimately streams its pins does not spam a console.
|
||||
if (!_warnedUrlSources.has(sourceId)) {
|
||||
_warnedUrlSources.add(sourceId);
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn(`[pin-selection] source "${sourceId}" is remote (url); its features cannot be read locally, so nothing can be selected from it.`);
|
||||
}
|
||||
return [];
|
||||
}
|
||||
}
|
||||
if (!data) return [];
|
||||
if (Array.isArray(data?.features)) return data.features;
|
||||
|
|
|
|||
|
|
@ -182,6 +182,7 @@
|
|||
"check:parse": "node internal/inline-parse-check.mjs",
|
||||
"check:pinned-umd": "node internal/check-pinned-umd.mjs",
|
||||
"check:popup-escaping": "node internal/check-popup-escaping.mjs",
|
||||
"check:pin-selection": "node internal/check-pin-selection.mjs",
|
||||
"check:poster-action-print": "node internal/check-poster-action-print.mjs",
|
||||
"check:poster-card-fits": "node internal/check-poster-card-fits.mjs",
|
||||
"check:poster-degrades": "node internal/check-poster-degrades-honestly.mjs",
|
||||
|
|
@ -416,6 +417,7 @@
|
|||
"check:pin-dim-slider": "the side menu's pin-dim slider and the legend's dimming must COMPOSE, not compete. Both want the pin layer's circle-opacity, so there is ONE writer and the value is the PRODUCT: slider x (1 matched, 0 dimmed). Asserts the LIVE paint property, not the store: flat 1 with clustering off, flat 0.5 with clustering on, ['case', expr, 0.5, 0] once something is dimmed, and ['case', expr, 1, 0] when the slider returns to 1. Dims by RELIGION deliberately - a region selection sets a layer FILTER rather than an opacity expression, so testing that path would report a failure about code that never writes opacity.",
|
||||
"check:pinned-umd": "library/utils/loadPinnedUmd.js (d3 for PPLR and the Wall) loads a pinned UMD with SRI and never touches the host page's global of the same name: a host's own d3 gains no keys, a failed load leaves it in place. Headless.",
|
||||
"check:popup-escaping": "the library's pin and region popups render hostile data (an <img onerror>, a <script>, a javascript: URL) as text, never markup: every field goes through escapeHtml/safeUrl (library/utils/html.js). Headless.",
|
||||
"check:pin-selection": "the shared source reader understands BOTH engines: mapbox-gl stores a GeoJSON source _data as the FeatureCollection, maplibre-gl wraps it as { geojson } (or { url } when remote). Reading only the Mapbox shape returned an EMPTY list on MapLibre — the keyless engine public builds use — which silently killed zoomToLegendRow camera fits on three maps and the research packet row selection, with no error because every caller no-ops on no features. Asserts both shapes, that a remote source returns [] AND warns, the serialize fallback, and that selectedPinIds agrees with the reader. Headless, no map. Exit 0 pass, 1 fail.",
|
||||
"check:poster-action-print": "a poster button opens the print dialog (Driver rule 2026-09-28: never a direct download). PosterWall's posterAction defaults to 'print', and any consumer that opts into 'download' carries a posterAction-download: comment saying why. Source-only, headless.",
|
||||
"check:poster-card-fits": "a poster card's print-options row must stay inside its own card. Reported by the repo agent off a 1440px screenshot: 27/27 cards overflowing by 110px, 36 neighbour intrusions. .pfs already had width:82%, so SOURCE READING SAID IT WAS FINE — flex items default to min-width:auto and refuse to shrink. Asserts BOTH no-overlap AND that the controls keep a usable box, because a row whose buttons collapsed to nothing would also \"not overflow\". Sweeps 390/768/1024/1440/1920px and asserts the row sits BELOW the preview too — measuring only left/right edges is how it passed a driver-visible regression. CARD_FIT_WIDTH overrides the list. SKIPs when nothing is served.",
|
||||
"check:poster-degrades": "with NEITHER poster nor source — the DEFAULT drop — nothing must print and the reader must be told why. check:poster-print-fallback SKIPs that case, so the behaviour a reader most often meets had no gate. MUST be served from the TREE ROOT, not public/, or assetsBase resolves outside the tree and the poster loads when it should 404.",
|
||||
|
|
|
|||
Loading…
Reference in a new issue