← back to La Socrata Ingester

fix(la-data): don't cache a failed catalog fetch; report probe errors honestly

28fcd49f102a186cb9695a6440914b5ea2270833 · 2026-09-11 11:51:36 -0700 · Steve Abrams

Two defects surfaced while running the shipped resolver past a second model. Codex
was spend-capped (HTTP 429) and Kimi's key is expired, so the lens was a local 8B
model whose output was mostly a restatement of the questions — but two of its
prompts were worth answering properly, and one was a real bug.

REAL BUG — fetchCatalog memoised FAILURES. `Promise.resolve(...).catch(() => null)`
cached a null for the life of the process, so one transient blip while resolving the
FIRST drifted source silently downgraded every later source in the same run to the
window-scan path. Not a correctness hole (the window path also demands full proof)
but needlessly narrower. Failures are now forgotten so the next source retries.
Proven by check 11 plus a negative control showing the old shape called the catalog
once where the new one calls it twice.

MESSAGE HONESTY — probeLayer swallowed every error into `null`, which is
indistinguishable from "that layer isn't there". It still fails loud either way
(never ingests), but the error text could mislead a human into reading a flaky
upstream as "the layer genuinely moved away". Probe errors are now counted and the
failure message says "(N probe(s) ERRORED — absence here is not proof of absence)".

Analysed and NOT changed: "what if the catalog and the per-layer plane are BOTH
stale?" A consistently-old upstream is self-consistent — our pinned (new) id
mismatches, the old catalog names the old id, the old per-layer plane re-proves it,
and we repoint to the old id, which in that world IS our layer. Correct. The
dangerous case is SKEW between the two planes, which is exactly what the re-proof
step catches and what check 3 covers. The 'unverifiable keeps a pin but cannot MOVE'
asymmetry is likewise deliberate, not a false-green: it never lets weak evidence
move us onto a new layer, it only declines to break a pin that already matches.

5/5 suites (11 auto-repoint checks). Live re-verified: 9/9 clean on the pinned ids,
9/9 auto-repoint from stale ids. TK-10955. $0 local.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CocwcUw6gFt3tr4CZJmra6

Files touched

• M src/adapters/arcgis.js
• M test/auto-repoint.test.mjs

Diff

commit 28fcd49f102a186cb9695a6440914b5ea2270833
Author: Steve Abrams <steve@designerwallcoverings.com>
Date:   Fri Sep 11 11:51:36 2026 -0700

    fix(la-data): don't cache a failed catalog fetch; report probe errors honestly
    
    Two defects surfaced while running the shipped resolver past a second model. Codex
    was spend-capped (HTTP 429) and Kimi's key is expired, so the lens was a local 8B
    model whose output was mostly a restatement of the questions — but two of its
    prompts were worth answering properly, and one was a real bug.
    
    REAL BUG — fetchCatalog memoised FAILURES. `Promise.resolve(...).catch(() => null)`
    cached a null for the life of the process, so one transient blip while resolving the
    FIRST drifted source silently downgraded every later source in the same run to the
    window-scan path. Not a correctness hole (the window path also demands full proof)
    but needlessly narrower. Failures are now forgotten so the next source retries.
    Proven by check 11 plus a negative control showing the old shape called the catalog
    once where the new one calls it twice.
    
    MESSAGE HONESTY — probeLayer swallowed every error into `null`, which is
    indistinguishable from "that layer isn't there". It still fails loud either way
    (never ingests), but the error text could mislead a human into reading a flaky
    upstream as "the layer genuinely moved away". Probe errors are now counted and the
    failure message says "(N probe(s) ERRORED — absence here is not proof of absence)".
    
    Analysed and NOT changed: "what if the catalog and the per-layer plane are BOTH
    stale?" A consistently-old upstream is self-consistent — our pinned (new) id
    mismatches, the old catalog names the old id, the old per-layer plane re-proves it,
    and we repoint to the old id, which in that world IS our layer. Correct. The
    dangerous case is SKEW between the two planes, which is exactly what the re-proof
    step catches and what check 3 covers. The 'unverifiable keeps a pin but cannot MOVE'
    asymmetry is likewise deliberate, not a false-green: it never lets weak evidence
    move us onto a new layer, it only declines to break a pin that already matches.
    
    5/5 suites (11 auto-repoint checks). Live re-verified: 9/9 clean on the pinned ids,
    9/9 auto-repoint from stale ids. TK-10955. $0 local.
    
    Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_01CocwcUw6gFt3tr4CZJmra6
---
 src/adapters/arcgis.js     | 36 ++++++++++++++++++++++++++++--------
 test/auto-repoint.test.mjs | 29 ++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/src/adapters/arcgis.js b/src/adapters/arcgis.js
index 4e5ee6d..1a4d1df 100644
--- a/src/adapters/arcgis.js
+++ b/src/adapters/arcgis.js
@@ -59,12 +59,18 @@ const LAYER_ENDPOINT_RE = /^(.*)\/(\d+)$/;
 const repointWindow = () => Math.max(0, Number(process.env.LA_REPOINT_WINDOW ?? 10));
 const autoRepointDisabled = () => process.env.LA_NO_AUTO_REPOINT === '1';
 
-const catalogCache = new Map(); // baseUrl -> Promise<catalog|null>, one GET per process
+// One catalog GET per process — but a FAILED fetch is deliberately NOT cached. Caching the
+// failure would let a single transient blip on the first drifted source silently downgrade
+// every later source in the same run to the window-scan path (still safe, since the window
+// path also demands full proof, but needlessly narrower). Re-try it instead.
+const catalogCache = new Map(); // baseUrl -> Promise<catalog>, successful fetches only
 function fetchCatalog(baseUrl, fetcher) {
   if (!catalogCache.has(baseUrl)) {
-    catalogCache.set(baseUrl, Promise.resolve(fetcher(`${baseUrl}?f=json`)).catch(() => null));
+    const p = Promise.resolve(fetcher(`${baseUrl}?f=json`));
+    catalogCache.set(baseUrl, p);
+    p.catch(() => catalogCache.delete(baseUrl)); // forget failures so the next source retries
   }
-  return catalogCache.get(baseUrl);
+  return catalogCache.get(baseUrl).catch(() => null);
 }
 export function _resetCatalogCache() { catalogCache.clear(); } // tests only
 
@@ -85,9 +91,19 @@ function fingerprint(meta, src) {
   return 'ok';
 }
 
-async function probeLayer(baseUrl, id, fetcher) {
+// Probe failures are counted, not just swallowed. A probe that ERRORED is not evidence the
+// layer is absent, and the loud failure message must not imply it was: fetcher already
+// retries 429/5xx five times, so a surviving error is real, and reporting how many probes
+// errored is what tells a human "upstream was flaky" apart from "it genuinely moved away".
+// Either way we fail loud rather than ingest, so this is a message-honesty fix, not a gate.
+async function probeLayer(baseUrl, id, fetcher, stats = null) {
   if (!Number.isInteger(id) || id < 0) return null;
-  try { return await fetcher(`${baseUrl}/${id}?f=json`); } catch { return null; }
+  try {
+    return await fetcher(`${baseUrl}/${id}?f=json`);
+  } catch {
+    if (stats) stats.errors++;
+    return null;
+  }
 }
 
 // A candidate is accepted ONLY on full proof: exact name AND a verified fingerprint.
@@ -97,6 +113,7 @@ const candidateProved = (meta, src) => !!meta && nameMatches(meta, src) && finge
 
 async function findLayerByName(baseUrl, pinnedId, src, fetcher) {
   const tried = [];
+  const stats = { errors: 0 };
   const cat = await fetchCatalog(baseUrl, fetcher);
   const catHits = (cat?.layers ?? []).filter(
     (l) => normName(l?.name) === normName(src.expectName) && normName(l?.type) === 'feature layer'
@@ -104,7 +121,7 @@ async function findLayerByName(baseUrl, pinnedId, src, fetcher) {
   if (catHits.length === 1) {
     const id = catHits[0].id;
     tried.push(`catalog->${id}`);
-    if (candidateProved(await probeLayer(baseUrl, id, fetcher), src)) return { id, via: 'catalog', tried };
+    if (candidateProved(await probeLayer(baseUrl, id, fetcher, stats), src)) return { id, via: 'catalog', tried };
     tried.push(`catalog->${id} FAILED re-proof (stale catalog?)`);
   } else if (catHits.length > 1) {
     tried.push(`catalog ambiguous (${catHits.length} layers named "${src.expectName}")`);
@@ -118,10 +135,13 @@ async function findLayerByName(baseUrl, pinnedId, src, fetcher) {
   const win = repointWindow();
   for (let d = 1; d <= win; d++) {
     for (const id of [pinnedId + d, pinnedId - d]) {
-      if (candidateProved(await probeLayer(baseUrl, id, fetcher), src)) hits.push(id);
+      if (candidateProved(await probeLayer(baseUrl, id, fetcher, stats), src)) hits.push(id);
     }
   }
-  tried.push(`window +/-${win}: ${hits.length} proved match(es)${hits.length ? ` [${hits.join(', ')}]` : ''}`);
+  tried.push(
+    `window +/-${win}: ${hits.length} proved match(es)${hits.length ? ` [${hits.join(', ')}]` : ''}` +
+    (stats.errors ? ` (${stats.errors} probe(s) ERRORED — absence here is not proof of absence)` : '')
+  );
   if (hits.length === 1) return { id: hits[0], via: `window+/-${win}`, tried };
   return { id: null, ambiguous: hits.length > 1, tried };
 }
diff --git a/test/auto-repoint.test.mjs b/test/auto-repoint.test.mjs
index 7481f8a..8d9b7b8 100644
--- a/test/auto-repoint.test.mjs
+++ b/test/auto-repoint.test.mjs
@@ -119,4 +119,31 @@ r = await run({ endpoint: `${NAV}/5` }, async () => { throw new Error('must not
 assert.equal(r.repointed, false);
 assert.equal(r.endpoint, `${NAV}/5`);
 
-console.log('✔ verified auto-repoint: 10/10 checks pass (no network, no database)');
+// 11. A FAILED catalog fetch must NOT be cached. Caching it would let one transient blip on
+//     the first drifted source silently downgrade every LATER source in the same run to the
+//     window-scan path. Here the catalog throws once, then succeeds; the second resolve must
+//     use the catalog path, proving the failure was forgotten rather than memoised.
+{
+  let catalogCalls = 0;
+  const layers = { 419: F('Contract Administration Inspection Districts'), 420: F('Council Districts') };
+  const flaky = async (url) => {
+    if (url === `${NAV}?f=json`) {
+      catalogCalls++;
+      if (catalogCalls === 1) throw new Error('transient catalog blip');
+      return { layers: Object.entries(layers).map(([id, m]) => ({ id: Number(id), name: m.name, type: m.type })) };
+    }
+    const m = /\/(\d+)\?f=json$/.exec(url);
+    const hit = m && layers[m[1]];
+    if (!hit) throw new Error(`no layer ${m && m[1]}`);
+    return hit;
+  };
+  _resetCatalogCache();
+  const first = await resolveLayerIdentity(src(419), flaky);   // catalog throws -> window path
+  assert.equal(first.to, 420, 'window scan must still recover when the catalog is down');
+  assert.match(first.via, /^window/);
+  const second = await resolveLayerIdentity(src(419), flaky);  // catalog retried -> catalog path
+  assert.equal(second.via, 'catalog', 'a failed catalog fetch must not be cached for the run');
+  assert.equal(catalogCalls, 2, 'the catalog must be retried, not memoised as null');
+}
+
+console.log('✔ verified auto-repoint: 11/11 checks pass (no network, no database)');

← 482ed58 feat(la-data): verified auto-repoint for drifting NavigateLA  ·  back to La Socrata Ingester  ·  fix(la-data): tolerate assessor_parcels' audited page-1 400 93a7ac5 →