From 88fe7509d0d64b06ccf7778bbaf1c560a292deb2 Mon Sep 17 00:00:00 2001 From: Doxa Maps Date: Mon, 28 Sep 2026 13:55:40 +0000 Subject: [PATCH] The packet was praying over the numbers the page loaded with, not the live ones rega RQ3(b): the packet's "Has Prayer" selection showed 50 of 997 where the legend row said 699. The legend row is right and the packet was wrong. A pin's prayer count arrives TWICE. The GeoJSON property is whatever was true when the page loaded; from then on polling writes the current count with setFeatureState, and prayer-progress.js:94 colours from ['feature-state','peoplePraying'] once polling is on. So the lit pins a reader counts are feature-state, and pin-selection.js had no 'feature-state' case at all -- it resolved to undefined, to-number defaulted it to 0, and the comparison selected only the pins whose FROZEN property happened to be above zero. 50 of them. The other 649 people groups had been prayed for and the packet left them out, which is the one thing a prayer packet must not get wrong. resolveExpr and matchesExpression now take the state alongside the properties, and selectedPinIds reads it per pin off the MAP rather than off the feature -- the source still holds the GeoJSON it loaded, setFeatureState never touches it. The source is added with promoteId:'uniqueId', so the id feature-state is keyed by is the same props[idKey] the selection already uses. Degrading is deliberate in both directions: before the first poll there is no state and property expressions still select, so an early packet is thinner and never empty; getFeatureState throws on an unknown source, so it is guarded and a bad source loses the selection rather than aborting the print. ALSO, rega RQ3(a), a candidate and not yet a confirmed cause. prewarmRegionPaint wrapped any non-feature-reading paint value in a case to make it data-driven, including a zoom interpolate -- I argued for that explicitly. useMapLayers.js:319 already held the rule and I had not read it: zoom interpolation is kept by putting the case in the interpolate OUTPUTS, and "the reverse (zoom nested inside a 'case') is what breaks the layer, so don't do that." That reverse is the shape Mapbox classifies as composite before reaching for an interpolation factor the expression cannot supply, which is the class of the interpolationFactor page error. The wrap now skips zoom expressions too. A missed pre-warm costs a frame; an illegal paint costs the layer. None of the three properties it touches is a zoom interpolate at boot, so this is not proof -- it removes the only writer that can build that shape from a runtime value. check-pin-selection asserts the live count wins over the stale property, that the stale property alone selects nothing, that a map with no feature-state still selects on properties, and that a throwing getFeatureState degrades. Negative-tested: removing the case, not reading the state, and dropping the throw-guard each turn it red; restored, green. --- internal/check-pin-selection.mjs | 35 +++++++++- library/maps/pin-selection.js | 65 ++++++++++++------- .../doxa-research-map/research-map.vue | 17 ++++- 3 files changed, 92 insertions(+), 25 deletions(-) diff --git a/internal/check-pin-selection.mjs b/internal/check-pin-selection.mjs index 27a068ed6..2985faec1 100644 --- a/internal/check-pin-selection.mjs +++ b/internal/check-pin-selection.mjs @@ -78,9 +78,42 @@ for (const [label, src] of [['mapbox', { _data: COLLECTION }], ['maplibre', { _d } eq('expressions are ANDed', matchesExpression(['all', ['==', ['get', 'uniqueId'], 'a'], ['==', ['get', 'languageFamily'], 'Indo-European']], FEATURES[0].properties), false) +// ── PACKETSEL: the prayer count is LIVE, so the packet must read feature-state ────────────────── +// rega, RQ3(b): the packet's "Has Prayer" selection showed 50 of 997 where the legend row said 699. +// The legend is right. Polling writes each pin's current count with setFeatureState() and +// prayer-progress.js:94 colours from ['feature-state','peoplePraying']; the GeoJSON property is +// whatever was true at page load. Reading the property selects the wrong people to pray for, which is +// the one thing this packet exists to get right. +const HAS_PRAYER = ['>', ['to-number', ['feature-state', 'peoplePraying'], 0], 0] +// 'a' has prayer only in the live state, 'b' only in the stale property, 'c' in neither. +const PRAY_FEATURES = [ + { type: 'Feature', properties: { uniqueId: 'a', peoplePraying: 0 }, geometry: { type: 'Point', coordinates: [1, 1] } }, + { type: 'Feature', properties: { uniqueId: 'b', peoplePraying: 12 }, geometry: { type: 'Point', coordinates: [2, 2] } }, + { type: 'Feature', properties: { uniqueId: 'c', peoplePraying: 0 }, geometry: { type: 'Point', coordinates: [3, 3] } }, +] +const STATE = { a: { peoplePraying: 7 } } +const prayMap = { + getSource: (id) => (id === 'pins' ? { _data: { type: 'FeatureCollection', features: PRAY_FEATURES } } : null), + getFeatureState: ({ id }) => STATE[id] || {}, +} +eq('feature-state selects the LIVE prayer count, not the loaded one', + [...selectedPinIds(prayMap, { sourceId: 'pins', expressions: [HAS_PRAYER] })].sort().join(','), 'a') +eq('the stale property alone does NOT select a pin', + selectedPinIds(prayMap, { sourceId: 'pins', expressions: [HAS_PRAYER] }).has('b'), false) +// A map with no feature-state at all (no polling yet) must still select on the loaded property rather +// than collapsing to nothing — a packet printed before the first poll is thinner, never empty. +const noState = { getSource: prayMap.getSource } +eq('no getFeatureState -> property expression still works', + [...selectedPinIds(noState, { sourceId: 'pins', expressions: [['>', ['to-number', ['get', 'peoplePraying'], 0], 0]] })].join(','), 'b') +eq('feature-state on a map that has none is undefined, not a throw', + matchesExpression(HAS_PRAYER, PRAY_FEATURES[1].properties, undefined), false) +eq('a source that throws on getFeatureState degrades instead of aborting the print', + selectedPinIds({ getSource: prayMap.getSource, getFeatureState: () => { throw new Error('no such source') } }, + { sourceId: 'pins', expressions: [HAS_PRAYER] }).size, 0) + 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.`) +console.log(`${TAG}: PASS — both engine shapes read the same features, a remote source says so rather than looking empty, selectedPinIds agrees with the reader, and the prayer count is read LIVE from feature-state rather than frozen at load.`) diff --git a/library/maps/pin-selection.js b/library/maps/pin-selection.js index 3d6a5f18e..bbc853514 100644 --- a/library/maps/pin-selection.js +++ b/library/maps/pin-selection.js @@ -19,7 +19,7 @@ * is in library/ rather than in one plugin. * * The evaluator covers the expression subset this product's filters use — all/any/!/has/comparisons/ - * in/match, plus get/literal/coalesce/to-number. Anything else evaluates false rather than throwing: + * in/match, plus get/literal/feature-state/coalesce/to-number. Anything else evaluates false rather than throwing: * a packet that prints fewer rows is recoverable, a map that throws mid-print is not. */ @@ -28,65 +28,72 @@ // regions silently no-op when the user is zoomed in. We evaluate the Mapbox // filter expression against the full source data instead, so fitBounds can // zoom OUT to a different region or IN to a tight cluster uniformly. -export function resolveExpr(expr, props) { +export function resolveExpr(expr, props, state) { if (!Array.isArray(expr)) return expr; const op = expr[0]; switch (op) { case 'get': return props?.[expr[1]]; case 'literal': return expr[1]; + // ★ THE LIVE VALUE, NOT THE ONE FROZEN AT LOAD. A pin's prayer count arrives twice: as a GeoJSON + // property at initial load, and thereafter through setFeatureState() as polling updates it. + // prayer-progress.js:94 colours from ['feature-state','peoplePraying'] once polling is on, so a + // reader looking at 699 lit "Has Prayer" pins is looking at feature-state. Without this case the + // expression resolved to undefined, to-number defaulted it to 0, and the packet selected only + // the pins whose FROZEN property happened to be > 0 — 50 of 997 where the legend said 699. + case 'feature-state': return state == null ? undefined : state[expr[1]]; case 'has': return props != null && expr[1] in props; case '!has': return props == null || !(expr[1] in props); - case '!': return !resolveExpr(expr[1], props); + case '!': return !resolveExpr(expr[1], props, state); case 'coalesce': { for (let i = 1; i < expr.length; i++) { - const v = resolveExpr(expr[i], props); + const v = resolveExpr(expr[i], props, state); if (v != null) return v; } return null; } case 'to-number': { - const v = resolveExpr(expr[1], props); + const v = resolveExpr(expr[1], props, state); const n = Number(v); if (Number.isFinite(n)) return n; if (expr.length > 2) { - const fb = Number(resolveExpr(expr[2], props)); + const fb = Number(resolveExpr(expr[2], props, state)); return Number.isFinite(fb) ? fb : 0; } return NaN; } case 'to-string': { - const v = resolveExpr(expr[1], props); + const v = resolveExpr(expr[1], props, state); return v == null ? '' : String(v); } - case 'to-boolean': return Boolean(resolveExpr(expr[1], props)); + case 'to-boolean': return Boolean(resolveExpr(expr[1], props, state)); case 'slice': { - const s = resolveExpr(expr[1], props); - const i = resolveExpr(expr[2], props); - const j = expr.length > 3 ? resolveExpr(expr[3], props) : undefined; + const s = resolveExpr(expr[1], props, state); + const i = resolveExpr(expr[2], props, state); + const j = expr.length > 3 ? resolveExpr(expr[3], props, state) : undefined; if (typeof s !== 'string') return ''; return j != null ? s.slice(i, j) : s.slice(i); } default: return expr; } } -export function matchesExpression(filter, props) { +export function matchesExpression(filter, props, state) { if (filter === true) return true; if (filter === false) return false; if (!Array.isArray(filter)) return Boolean(filter); const op = filter[0]; if (op === 'all') { for (let i = 1; i < filter.length; i++) { - if (!matchesExpression(filter[i], props)) return false; + if (!matchesExpression(filter[i], props, state)) return false; } return true; } if (op === 'any') { for (let i = 1; i < filter.length; i++) { - if (matchesExpression(filter[i], props)) return true; + if (matchesExpression(filter[i], props, state)) return true; } return false; } - if (op === '!') return !matchesExpression(filter[1], props); + if (op === '!') return !matchesExpression(filter[1], props, state); if (op === 'has') return props != null && filter[1] in props; if (op === '!has') return props == null || !(filter[1] in props); @@ -95,8 +102,8 @@ export function matchesExpression(filter, props) { // Legacy form: ['==', 'propName', value] (no ['get', ...]) const left = (typeof filter[1] === 'string') ? props?.[filter[1]] - : resolveExpr(filter[1], props); - const right = resolveExpr(filter[2], props); + : resolveExpr(filter[1], props, state); + const right = resolveExpr(filter[2], props, state); switch (op) { case '==': return left === right || (left == null && right == null); case '!=': return !(left === right || (left == null && right == null)); @@ -107,21 +114,21 @@ export function matchesExpression(filter, props) { } } if (op === 'in') { - const v = resolveExpr(filter[1], props); - const haystack = resolveExpr(filter[2], props); + const v = resolveExpr(filter[1], props, state); + const haystack = resolveExpr(filter[2], props, state); if (Array.isArray(haystack)) return haystack.includes(v); if (typeof haystack === 'string') return haystack.includes(v); return false; } if (op === 'match') { - const input = resolveExpr(filter[1], props); + const input = resolveExpr(filter[1], props, state); const lastIdx = filter.length - 1; for (let i = 2; i < lastIdx; i += 2) { const labelRaw = filter[i]; const labels = Array.isArray(labelRaw) ? labelRaw : [labelRaw]; - if (labels.includes(input)) return Boolean(resolveExpr(filter[i + 1], props)); + if (labels.includes(input)) return Boolean(resolveExpr(filter[i + 1], props, state)); } - return Boolean(resolveExpr(filter[lastIdx], props)); + return Boolean(resolveExpr(filter[lastIdx], props, state)); } return false; } @@ -172,11 +179,23 @@ export function readSourceFeatures(map, sourceId) { export function selectedPinIds(map, { sourceId = 'language-families', expressions = [], idKey = 'uniqueId' } = {}) { const ids = new Set() const exprs = (expressions || []).filter((e) => e != null && e !== true) + // ★ FEATURE-STATE IS READ PER PIN, and it has to come off the MAP rather than off the feature: the + // GeoJSON the source holds is what was loaded, and setFeatureState writes to a separate per-feature + // store that never touches it. useMapLayers adds this source with promoteId:'uniqueId', so the id + // feature-state is keyed by IS props[idKey] -- if a caller ever passes a different idKey the lookup + // yields {} and the evaluation falls back to the loaded properties, which is the old behaviour and + // not a new failure. getFeatureState throws on an unknown source, so it is guarded once, not per pin. + const canReadState = typeof map?.getFeatureState === 'function' + const stateOf = (id) => { + if (!canReadState || id == null) return undefined + try { return map.getFeatureState({ source: sourceId, id }) } catch (_) { return undefined } + } for (const f of readSourceFeatures(map, sourceId)) { const props = f && f.properties if (!props) continue - if (exprs.length && !exprs.every((e) => matchesExpression(e, props))) continue const id = props[idKey] + const state = stateOf(id) + if (exprs.length && !exprs.every((e) => matchesExpression(e, props, state))) continue if (id != null) ids.add(String(id)) } return ids diff --git a/plugin-designs/doxa-research-map/research-map.vue b/plugin-designs/doxa-research-map/research-map.vue index 55036cd42..4adf05385 100644 --- a/plugin-designs/doxa-research-map/research-map.vue +++ b/plugin-designs/doxa-research-map/research-map.vue @@ -1136,11 +1136,26 @@ function prewarmRegionPaint(m) { // what my first version did. const readsFeature = (e) => Array.isArray(e) && (e[0] === 'get' || e[0] === 'has' || e[0] === 'feature-state' || e.some(readsFeature)) + // ★ AND A ZOOM EXPRESSION MUST NOT BE WRAPPED, WHICH IS THE OPPOSITE OF WHAT THE NOTE ABOVE ARGUED. + // I reasoned that a zoom interpolate is an array but not data-driven, so skipping it would leave a + // variant to compile on the first gesture — true, and beside the point, because the wrap is not legal. + // useMapLayers.js:319 already carries the rule, written for the pin ring: zoom interpolation is kept + // by putting the 'case' in the interpolate OUTPUTS; "the reverse (zoom nested inside a 'case') is what + // breaks the layer, so don't do that." ['case', cond, ['interpolate',['linear'],['zoom'],…], …] is + // exactly that reverse. Mapbox then classifies the property as composite and reaches for an + // interpolation factor the expression cannot supply — the class of + // this.expression.interpolationFactor is not a function + // rega saw once on Mapbox at L54, on the clear → other-row path. A missed pre-warm costs a frame; an + // illegal paint costs the layer, so when the two rules disagree this one wins. + // + // exp-shepherd's port skips ANY array and is immune to this by accident; the difference between us + // was never about zoom expressions being data-driven, it was about which mistake is recoverable. + const readsZoom = (e) => Array.isArray(e) && (e[0] === 'zoom' || e.some(readsZoom)) const warm = (layer, prop) => { if (!m.getLayer(layer)) return const cur = m.getPaintProperty(layer, prop) if (cur === undefined || cur === null) return - if (readsFeature(cur)) return + if (readsFeature(cur) || readsZoom(cur)) return m.setPaintProperty(layer, prop, wrap(cur)) } try {