From a98cbe8055b1e4651e66cf725e1a5e208003fe60 Mon Sep 17 00:00:00 2001 From: Doxa Maps Date: Mon, 28 Sep 2026 09:07:50 +0000 Subject: [PATCH] =?UTF-8?q?check:facet-changes-map=20=E2=80=94=20measure?= =?UTF-8?q?=20MapLibre=20too,=20not=20just=20Mapbox?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BUG-FACET-GATE-CANNOT-MEASURE-MAPLIBRE. The gate read source._data four times, and the two engines store it differently -- cited from the bundles this repo loads, the same constructor in each: mapbox-gl v3.24.0 this._data = i.data the raw GeoJSON maplibre-gl this._data = "string"==typeof i.data ? {url:…} : {geojson: i.data} A GATE blind to MapLibre passes a map that is broken there, which is worse than a plugin carrying the same bug. ★ ALL FOUR READS WERE BROKEN, NOT THREE. The census called :145 "guarded" because it had `s._data || (s.serialize && s.serialize().data)` -- but `||` fires only when the left side is FALSY, and {geojson: …} is TRUTHY, so the fallback never ran. The wrapper has to be UNWRAPPED, not fallen back from. That correction is the reason this is a shared helper rather than a copied one-liner. __srcFeatures() goes in the WALK bundle -- the in-page script the gate already injects -- NOT as an import of library/maps/pin-selection.js: all four call sites run inside page.evaluate / page.waitForFunction bodies, which are serialised to the browser, where a Node-side import does not exist. Same logic as readSourceFeatures(), duplicated across a process boundary, which is the one place duplication is not a smell. A remote (url) source returns null, not 0: null means "could not measure", while 0 would mean "the map drew nothing", which is the finding this gate reports. THE MAPLIBRE SKIP IS RETIRED. It exited 0 while announcing SKIP -- NOT MEASURED, correct for the limitation but a gate that never looked at half the product. A source present with an unreadable payload is now a real failure with a diagnostic, because the known cause of that state is fixed. check:facet-shape negative-tests the helper headlessly against both engine shapes, in sr's check-pin-selection style, evaluating it OUT OF THE WALK STRING so the thing tested is the thing injected. 5/5. Poison-tested: with the unwrap removed, ONLY the maplibre case fails (4/5), so it catches the regression and is not indiscriminate. internal/ is the build owner's under rule 5B -- source sha to studio. Running the gate itself launches a browser and is rega's. --- internal/check-facet-changes-map.mjs | 98 ++++++++++++++++++++++++---- package.json | 1 + 2 files changed, 86 insertions(+), 13 deletions(-) diff --git a/internal/check-facet-changes-map.mjs b/internal/check-facet-changes-map.mjs index bcbbb98ba..29ad26b15 100644 --- a/internal/check-facet-changes-map.mjs +++ b/internal/check-facet-changes-map.mjs @@ -132,6 +132,39 @@ function __walk(sel) { // Measured: one run in three failed that way on the dev surface while the product was fine. // A gate that reports a product bug when its own handle went stale is a gate that gets ignored, so // __live() proves the handle still works before any assertion reads through it. +// ★ READ A GEOJSON SOURCE'S FEATURES WITHOUT ASSUMING ONE ENGINE'S PRIVATE SHAPE. +// This gate could only ever measure Mapbox (BUG-FACET-GATE-CANNOT-MEASURE-MAPLIBRE), and the cause +// is four reads of 'source._data'. The two engines store it differently — cited from the bundles +// this repo actually loads, same constructor in each: +// mapbox-gl v3.24.0 this._data = i.data (the raw GeoJSON) +// maplibre-gl this._data = "string"==typeof i.data ? {url:…} : {geojson: i.data} +// So '_data.features' is undefined under MapLibre and every read fell to an empty list. A GATE that +// cannot see MapLibre's features PASSES A MAP THAT IS BROKEN THERE, which is worse than a plugin +// having the same bug. +// +// ⚠ THE 's._data || s.serialize().data' FALLBACK DOES NOT FIX IT, and that is why this is a helper +// rather than a copied one-liner: '||' only fires when the left side is FALSY, and '{geojson: …}' +// is TRUTHY. The old __state() carried that form and was broken under MapLibre exactly like the +// three unguarded reads. The wrapper has to be UNWRAPPED, not fallen back from. +// +// ⚠ AND IT LIVES HERE, IN THE PAGE, not as an import of library/maps/pin-selection.js. All four +// call sites run inside page.evaluate / page.waitForFunction bodies, which are serialised to the +// browser — a Node-side import does not exist there. This is the same logic as readSourceFeatures(), +// deliberately duplicated across the process boundary, which is the one place duplication is not a +// smell. If that helper's rules change, this must follow. +function __srcFeatures(m, id) { + let s; try { s = m.getSource(id) } catch (e) { return null } + if (!s) return null; + let data = s._data !== undefined ? s._data : (s.serialize && s.serialize().data); + if (data && typeof data === 'object' && !Array.isArray(data)) { + if (data.geojson !== undefined) data = data.geojson; + // A REMOTE SOURCE IS NOT AN EMPTY ONE. Its features live on a server and cannot be enumerated + // here; returning [] would read as "the map drew nothing", which is the bug this gate exists to + // catch. null means "could not measure", and the caller must not treat it as zero. + else if (data.url !== undefined) return null; + } + return (data && data.features) || null; +} function __live() { const m = window.__rdashMap; if (!m) return null; @@ -142,8 +175,7 @@ function __state() { if (!m) return { stale: true, err: 'map handle is stale (torn down and re-created)' }; let s; try { s = m.getSource('rd-bubbles') } catch (e) { return { stale: true, err: 'map torn down: ' + e.message } } if (!s) return { err: 'no rd-bubbles source' }; - const d = s._data || (s.serialize && s.serialize().data); - const feats = (d && d.features) || []; + const feats = __srcFeatures(m, 'rd-bubbles') || []; let color = null; try { color = m.getPaintProperty('rd-bubble-layer', 'circle-color') } catch (e) {} return { n: feats.length, @@ -193,6 +225,40 @@ const LEVELS = [ { label: 'Needs Prayer', holds: (n) => !(Number(n) > 0) }, ] +// ── SHAPE TEST (headless, no browser) ──────────────────────────────────────────────────────────── +// ★ NEGATIVE-TESTS __srcFeatures AGAINST BOTH ENGINE SHAPES, in sr's check-pin-selection style. +// ⚠ NOT `--self-test`: that already means "run the browser gate with the bug put back, it MUST +// fail". This one never opens a browser, so it can run anywhere and in gates:pre. +// It evaluates the helper OUT OF THE WALK STRING ITSELF, so the thing tested is the thing injected +// into the page — a copy pasted here would pass while the shipped helper rotted. +if (process.argv.includes('--shape-test')) { + const FC = { type: 'FeatureCollection', features: [{ type: 'Feature', properties: { slug: 'a' } }, + { type: 'Feature', properties: { slug: 'b' } }] } + const srcFeatures = new Function(`${WALK}\nreturn __srcFeatures`)() + const mapWith = (src) => ({ getSource: (id) => (id === 'rd-bubbles' ? src : null) }) + const cases = [ + ['mapbox shape (_data = FeatureCollection)', () => srcFeatures(mapWith({ _data: FC }), 'rd-bubbles'), 2], + ['maplibre shape (_data = { geojson })', () => srcFeatures(mapWith({ _data: { geojson: FC } }), 'rd-bubbles'), 2], + ['public fallback (no _data, serialize())', () => srcFeatures(mapWith({ serialize: () => ({ data: FC }) }), 'rd-bubbles'), 2], + // A REMOTE SOURCE MUST NOT READ AS EMPTY. null means "could not measure"; 0 would mean "the map + // drew nothing", which is the finding this gate exists to report. + ['remote (_data = { url }) -> null, not 0', () => srcFeatures(mapWith({ _data: { url: 'https://x.invalid/a.json' } }), 'rd-bubbles'), null], + ['absent source -> null', () => srcFeatures(mapWith(null), 'rd-bubbles'), null], + ] + let ok = 0 + for (const [name, run, want] of cases) { + const got = run() + const n = got === null || got === undefined ? null : got.length + const pass = n === want + console.log(` ${pass ? '✓' : '✗'} ${name} -> ${n === null ? 'null' : n + ' feature(s)'}`) + if (pass) ok++ + } + console.log(ok === cases.length + ? `FACET-SHAPE-TEST: PASS — ${ok}/${cases.length}, both engine shapes measured` + : `FACET-SHAPE-TEST: FAIL — ${ok}/${cases.length}`) + process.exit(ok === cases.length ? 0 : 1) +} + async function reinject(page) { await page.addScriptTag({ content: WALK }) } // Read __state(), tolerating a STALE map handle: if the panel re-created its Map mid-interaction @@ -207,7 +273,7 @@ async function readState(page, label) { try { await page.waitForFunction(`(() => { const m = window.__rdashMap; if (!m) return false; - try { const s = m.getSource('rd-bubbles'); return !!(s && s._data && s._data.features && s._data.features.length > 0) } + try { const f = __srcFeatures(m, 'rd-bubbles'); return !!(f && f.length > 0) } catch (e) { return false } })()`, { timeout: 30000 }) } catch (e) { /* fall through to a final attempt */ } @@ -249,7 +315,7 @@ const run = async () => { const m = window.__rdashMap; if (!m) return false; let s; try { s = m.getSource('rd-bubbles') } catch (e) { return false } if (!s) return false; - const d = s._data; return !!(d && d.features && d.features.length > 0); + const f = __srcFeatures(m, 'rd-bubbles'); return !!(f && f.length > 0); })()`, { timeout: 90000 }) } catch (e) { // ★ DISTINGUISH "I CANNOT MEASURE THIS ENGINE" FROM "THE PRODUCT IS BROKEN". @@ -271,18 +337,24 @@ const run = async () => { let s = null; try { s = m.getSource('rd-bubbles') } catch (e) {} let engine = 'unknown'; try { engine = m.getContainer && m.getContainer().classList.contains('maplibregl-map') ? 'maplibre' : 'mapbox' } catch (e) {} - const d = s && s._data; - return { engine, hasSource: !!s, payloadReadable: !!(d && d.features), layer: !!m.getLayer('rd-bubble-layer') }; + const f = __srcFeatures(m, 'rd-bubbles'); + return { engine, hasSource: !!s, payloadReadable: !!f, layer: !!m.getLayer('rd-bubble-layer') }; })()`) } catch (_) { /* page gone — fall through to the original failure */ } - if (diag && diag.engine === 'maplibre' && diag.hasSource && !diag.payloadReadable) { + // ★ THE MAPLIBRE SKIP IS GONE, because the reason for it is. This branch used to bail out with + // "SKIP — this gate reads the GeoJSON source through a private Mapbox field (_data) that + // MapLibre does not keep live", exiting 0 while announcing SKIP — NOT MEASURED, correct for the + // limitation but a gate that never looks at half the product. __srcFeatures() unwraps + // MapLibre's {geojson} wrapper, so the features ARE readable on both engines now and a failure + // here is a real one. + // If payloadReadable is still false with a source present, that is a NEW condition (a url-backed + // source, or an engine shape neither bundle documents) and it should be investigated, not + // skipped past — so it now falls through to the ordinary failure below, with the diagnostic. + if (diag && diag.hasSource && !diag.payloadReadable) { await browser.close() - console.log('FACET-CHANGES-MAP: SKIP — this gate reads the GeoJSON source through a private') - console.log(' Mapbox field (_data) that MapLibre does not keep live, so the dots cannot be') - console.log(' counted on this engine. The source and layer ARE present; nothing here is a') - console.log(' product finding. Run it without ?provider=maplibre, or re-express the assertion') - console.log(' on the public querySourceFeatures() — see BUG-FACET-GATE-CANNOT-MEASURE-MAPLIBRE.') - process.exit(0) + fail(`the ${diag.engine} source 'rd-bubbles' exists but its features could not be read ` + + `(payload unreadable — a remote/url source, or a shape neither engine bundle documents). ` + + `This is NOT the old MapLibre _data limitation: that is fixed.`) } await browser.close(); fail('the map never populated (no rd-bubbles features after 90s)') } diff --git a/package.json b/package.json index 5920ceff4..3b4282e43 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,7 @@ "check:facet-cascade": "node internal/check-facet-cascade.mjs", "check:facet-cascade:offline": "node internal/check-facet-cascade.mjs --offline", "check:facet-cascade:self-test": "node internal/check-facet-cascade.mjs --self-test", + "check:facet-shape": "node internal/check-facet-changes-map.mjs --shape-test", "check:facet-changes-map": "node internal/check-facet-changes-map.mjs", "check:facet-changes-map:built": "sh -c 'BASE_URL=${BASE_URL:-http://127.0.0.1:4173} PROBE_PATH=${PROBE_PATH:-/doxa-maps/research-dashboard/research-dashboard/index.html} exec node internal/check-facet-changes-map.mjs'", "check:facet-changes-map:self-test": "node internal/check-facet-changes-map.mjs --self-test",