From 3f2f20a315c67df58ab07f046a57a8e84042f8f7 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 3 Jul 2026 08:31:49 -0700 Subject: [PATCH 1/2] #311: point lite_url at the corrected samples_map_lite_v3; fix latent place_name Array.isArray bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deploys the #311 pipeline fix (SamplingEvent/SamplingSite traversal for place_name/result_time, merged in #318) to production data: rebuilt samples_map_lite from the SAME wide.parquet currently live on R2 (verified byte-identical: exact row counts + per-source min/max pid match against https://data.isamples.org/isamples_202608_wide.parquet before rebuilding), uploaded to R2 as isamples_202608_samples_map_lite_v3.parquet (a NEW filename — the live isamples_202608_samples_map_lite_v2.parquet is served Cache-Control: immutable, max-age=31536000, so overwriting it would leave stale/broken copies at CDN edges and in visitors' browsers for up to a year; same convention _v2 itself already used over the unsuffixed name). Rebuild results (verified before deploying): place_name now resolves for 2,263,648/6,026,242 (37.6%) located samples, result_time for 5,937,692/6,026,242 (98.5%) — both were 0% before this fix. ## A second, latent bug this surfaced Testing the corrected data against the real page (not just raw parquet) found a genuine frontend bug that was invisible until now: the four places in explorer.qmd that render place_name (`Array.isArray(placeParts) && placeParts.length > 0 ? placeParts.filter(Boolean).join(' › ') : ''`) all checked `Array.isArray()`. Observable's DuckDBClient returns Arrow LIST columns (place_name is VARCHAR[]) as an Arrow `Vector` — iterable, has `.length`, but is NOT a plain JS Array, so `Array.isArray()` is FALSE on it. Every non-null place_name was silently rendering as blank. This was masked in every deploy up to today because place_name was 100% NULL in production before the samples_map_lite rebuild above — the bug had no observable symptom until real data started flowing. Extracted the shared logic to `formatPlaceName()` in assets/js/explorer-utils.js (matching this file's existing extracted- pure-helper convention, unit-tested under Node) and replaced all 4 call sites. The new unit test reproduces the bug directly with a fake non-Array iterable shaped like an Arrow Vector, asserting `Array.isArray(vector) === false` before asserting `formatPlaceName` still handles it correctly — so a regression back to a bare `Array.isArray` check would be caught even without live parquet data. Verified end-to-end against the real corrected R2 file (fresh browser origin, to rule out ES-module cache artifacts from repeated same-session testing): table Place column shows real values ("Axial Seamount summit caldera" for IGSN:321000001), Date column shows real values ("2013-12-20"), CSV export matches. 0 console errors. 48/48 JS unit tests and 40/40 pipeline tests pass. --- assets/js/explorer-utils.js | 17 ++++++++++++++++ explorer.qmd | 31 +++++++++++++++--------------- tests/unit/explorer-utils.test.mjs | 25 +++++++++++++++++++++++- 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/assets/js/explorer-utils.js b/assets/js/explorer-utils.js index 09f0323..fd6da63 100644 --- a/assets/js/explorer-utils.js +++ b/assets/js/explorer-utils.js @@ -17,6 +17,23 @@ export function searchTerms(value) { return String(value || '').trim().split(/\s+/).filter(Boolean); } +// Format a place_name VARCHAR[] column value (from DuckDB-WASM) into a +// display string, e.g. ['Country', 'Region', 'Site'] -> 'Country › Region › +// Site'. #311 (Codex-adjacent catch, discovered once place_name started +// carrying real data): Observable's DuckDBClient returns Arrow LIST columns +// as an Arrow `Vector` (iterable, has .length), NOT a plain JS Array — +// `Array.isArray(vector)` is FALSE, so the four call sites in explorer.qmd +// that used to check `Array.isArray(placeParts)` silently rendered every +// non-null place as blank. This was invisible until now because place_name +// was 100% NULL in production before the #311 pipeline fix landed. Array.from +// works on both a plain Array and an Arrow Vector (both are iterable); the +// null/undefined guard is required because Array.from(null) throws. +export function formatPlaceName(placeParts) { + if (placeParts == null) return ''; + const arr = Array.from(placeParts); + return arr.length > 0 ? arr.filter(Boolean).join(' › ') : ''; +} + // Parse a numeric URL param with a default and optional clamping. export function parseNum(val, def, min, max) { if (val == null) return def; diff --git a/explorer.qmd b/explorer.qmd index 5057ff2..4d84142 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -806,7 +806,16 @@ h3_res8_url = `${R2_BASE}/isamples_202608_h3_summary_res8.parquet` // immutable/1-yr) — every visitor fetches fresh data, no cache purge. The next // generation builds res4/res6 into the canonical name natively (build change), // so this _v2 suffix is a one-off retrofit for 202608. -lite_url = `${R2_BASE}/isamples_202608_samples_map_lite_v2.parquet` +// +// #311: _v3 rebuilds place_name/result_time via the SamplingEvent/ +// SamplingSite graph traversal (the fix in build_frontend_derived.py) — _v2's +// place_name/result_time were 100% NULL, a dead read off MaterialSampleRecord +// directly (see build_frontend_derived.py's `samp` CTE for the traversal). +// Rebuilt from the SAME wide.parquet as _v2 (row-count and per-source +// min/max-pid verified identical against the live wide before rebuilding), +// so this is a pure column-content fix, not a data-vintage change. Same +// immutable-cache reasoning as _v2: new filename, never overwrite. +lite_url = `${R2_BASE}/isamples_202608_samples_map_lite_v3.parquet` // Explicit versioned wide (#272: OC concept-enriched — popups read material/ // object-type from this file). The stable alias `current/wide.parquet` still // points at the previous wide until the production cutover flips the manifest; @@ -902,6 +911,7 @@ parseNum = _explorerUtils.parseNum csvParamValues = _explorerUtils.csvParamValues sourceUrl = _explorerUtils.sourceUrl readHash = _explorerUtils.readHash +formatPlaceName = _explorerUtils.formatPlaceName // === Source Filter: get active sources and build SQL clause === function getActiveSources() { @@ -1617,10 +1627,7 @@ function updateSampleCard(sample) { if (!el) return; const color = SOURCE_COLORS[sample.source] || '#666'; const name = SOURCE_NAMES[sample.source] || sample.source; - const placeParts = sample.place_name; - const placeStr = Array.isArray(placeParts) && placeParts.length > 0 - ? placeParts.filter(Boolean).join(' › ') - : ''; + const placeStr = formatPlaceName(sample.place_name); const srcUrl = sourceUrl(sample.pid); el.innerHTML = `

Sample

@@ -1802,10 +1809,7 @@ function updateSamples(samples) { for (const s of samples) { const color = SOURCE_COLORS[s.source] || '#666'; const name = SOURCE_NAMES[s.source] || s.source; - const placeParts = s.place_name; - const desc = Array.isArray(placeParts) && placeParts.length > 0 - ? placeParts.filter(Boolean).join(' › ') - : ''; + const desc = formatPlaceName(s.place_name); const sUrl = sourceUrl(s.pid); h += `
@@ -2633,10 +2637,7 @@ tableView = { tableEl.innerHTML = '
No samples match the current filters.
'; } else if (pageRows.length > 0) { const body = pageRows.map(r => { - const placeParts = r.place_name; - const place = Array.isArray(placeParts) && placeParts.length > 0 - ? placeParts.filter(Boolean).join(' › ') - : ''; + const place = formatPlaceName(r.place_name); const lat = r.latitude != null ? Number(r.latitude).toFixed(5) : ''; const lng = r.longitude != null ? Number(r.longitude).toFixed(5) : ''; const label = r.label || r.pid || ''; @@ -3016,9 +3017,7 @@ tableView = { } function csvRow(r) { - const placeParts = r.place_name; - const place = Array.isArray(placeParts) && placeParts.length > 0 - ? placeParts.filter(Boolean).join(' › ') : ''; + const place = formatPlaceName(r.place_name); const labelFor = (uri) => uri ? ((typeof window !== 'undefined' && window.conceptLabelForUri) ? window.conceptLabelForUri(uri) : uri) : ''; diff --git a/tests/unit/explorer-utils.test.mjs b/tests/unit/explorer-utils.test.mjs index 431b4d5..47aa175 100644 --- a/tests/unit/explorer-utils.test.mjs +++ b/tests/unit/explorer-utils.test.mjs @@ -4,7 +4,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { escapeHtml, searchTerms, parseNum, csvParamValues, sourceUrl, readHash, - facetCountsDisplayState, + facetCountsDisplayState, formatPlaceName, } from '../../assets/js/explorer-utils.js'; test('escapeHtml escapes the five HTML-significant chars; nullish -> ""', () => { @@ -17,6 +17,29 @@ test('escapeHtml escapes the five HTML-significant chars; nullish -> ""', () => assert.equal(escapeHtml(0), '0'); }); +test('formatPlaceName: array -> joined string; null/empty -> ""', () => { + assert.equal(formatPlaceName(['Country', 'Region', 'Site']), 'Country › Region › Site'); + assert.equal(formatPlaceName(['Only']), 'Only'); + assert.equal(formatPlaceName([]), ''); + assert.equal(formatPlaceName(null), ''); + assert.equal(formatPlaceName(undefined), ''); + assert.equal(formatPlaceName(['A', null, 'B']), 'A › B'); // filter(Boolean) drops null entries +}); + +test('formatPlaceName: works on a non-Array iterable (Arrow Vector shape) — #311', () => { + // Reproduces the actual bug: DuckDB-WASM/Arrow LIST columns come back as + // an iterable, .length-bearing object that is NOT a plain JS Array — + // Array.isArray() on this returns false, which is exactly what silently + // blanked every Place cell once place_name started carrying real data. + class FakeArrowVector { + constructor(items) { this._items = items; this.length = items.length; } + [Symbol.iterator]() { return this._items[Symbol.iterator](); } + } + const vector = new FakeArrowVector(['Axial Seamount summit caldera']); + assert.equal(Array.isArray(vector), false, 'sanity: the fake vector must NOT be a plain Array'); + assert.equal(formatPlaceName(vector), 'Axial Seamount summit caldera'); +}); + test('searchTerms splits on whitespace, drops empties', () => { assert.deepEqual(searchTerms(' hello world '), ['hello', 'world']); assert.deepEqual(searchTerms(''), []); From 3bfde1c63ff0a94c11f48339d1ed472e0f60b394 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 3 Jul 2026 08:37:09 -0700 Subject: [PATCH 2/2] address Codex review: escape place_name/result_time in sample card + nearby list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review of the previous commit: the samples-table and CSV render paths for place_name already went through escapeHtml()/CSV-quoting, but two other call sites — the in-map sample detail card (updateSampleCard) and the cluster-click "Nearby Samples" list — interpolate placeStr/desc directly into innerHTML with no escaping. Not a new defect (this code predates today), but my #311 fix is what makes it newly exploitable: place_name (and result_time, same reasoning) went from 100%-NULL in every prior deploy to carrying real, externally-sourced text (SESAR/OpenContext/ GEOME/Smithsonian aren't sanitized inputs) for the first time today. Wrapped placeStr/desc/sample.result_time in escapeHtml() at both sites. Left sample.label/pid unescaped in these same functions — that's a separate, pre-existing gap (labels have carried real data in every past deploy, not newly activated by this change) and out of scope for this commit; worth a follow-up issue. Verified: clicked a sample with real place_name data (IGSN:321000001, "Axial Seamount summit caldera") — card and nearby-list both render correctly, 0 console errors. 48/48 unit tests still pass. --- explorer.qmd | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/explorer.qmd b/explorer.qmd index 4d84142..bfa6390 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -1640,8 +1640,8 @@ function updateSampleCard(sample) {
${sample.lat.toFixed(5)}, ${sample.lng.toFixed(5)}
- ${placeStr ? `
${placeStr}
` : ''} - ${sample.result_time ? `
Date: ${sample.result_time}
` : ''} + ${placeStr ? `
${escapeHtml(placeStr)}
` : ''} + ${sample.result_time ? `
Date: ${escapeHtml(sample.result_time)}
` : ''} ${srcUrl ? `` : ''}
Loading full details...
`; @@ -1816,7 +1816,7 @@ function updateSamples(samples) { ${sUrl ? `${s.label || s.pid}` : `${s.label || s.pid}`} ${name}
- ${desc ? `
${desc}
` : ''} + ${desc ? `
${escapeHtml(desc)}
` : ''}
`; } el.innerHTML = h;