From 9f56e9df981eeb6f090c757d36bd081415edb34d Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Sun, 11 Oct 2026 09:47:56 -0400 Subject: [PATCH 1/4] fix(terrain): refresh cache recency when resolving warm batches --- CHANGELOG.md | 1 + docs/CURRENT-STATE.md | 5 ++++ src/services/terrain-warm-batch-lru.test.mjs | 24 ++++++++++++++++++++ src/services/terrainHeights.js | 2 +- 4 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 src/services/terrain-warm-batch-lru.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index a770fbd35..e3075234c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ - Tool string length validation counts Unicode characters correctly, including characters outside the basic multilingual plane. - Director timeline seeks cross the dateline along the short longitude arc. +- Frequently reused terrain batches retain their cached heights ahead of unused cells. - Treat explicitly current, complete empty vessel coverage as a successful update, clearing obsolete contacts while retaining stale or incomplete snapshots. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 1f794a5b5..246f3067c 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -5326,3 +5326,8 @@ datacenters, dams and the Nepal locator retain that shared credit while displaye Military fragments stitch by source identity. Different mapped parcels merge only when their footprints touch or contain one another, never across a seam tolerance gap. + + +### fix(terrain): refresh cache recency when resolving warm batches + +Use the existing LRU read path for batch lookups; provider requests, fallback cooldowns and bounded residency retain their current contracts. diff --git a/src/services/terrain-warm-batch-lru.test.mjs b/src/services/terrain-warm-batch-lru.test.mjs new file mode 100644 index 000000000..ac718b998 --- /dev/null +++ b/src/services/terrain-warm-batch-lru.test.mjs @@ -0,0 +1,24 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createTerrainHeights } from './terrainHeights.js'; +test('warm batch consumers retain their terrain cells ahead of unused cells', async () => { + let calls = 0; + const terrain = createTerrainHeights({ + maxCacheEntries: 2, + source: { + async getHeights(points) { + calls++; + return points.map((p) => ({ ellipsoid: 100 + p.lon })); + }, + }, + }); + const p = (lon) => ({ lat: 0, lon }); + await terrain.resolveEllipsoidalGround([p(1)]); + await terrain.resolveEllipsoidalGround([p(2)]); + await terrain.resolveEllipsoidalGround([p(1)]); + assert.equal(calls, 2); + await terrain.resolveEllipsoidalGround([p(3)]); + assert.equal(terrain.cachedEllipsoidalGround(0, 1), 101); + assert.equal(terrain.cachedEllipsoidalGround(0, 2), null); + assert.equal(calls, 3); +}); diff --git a/src/services/terrainHeights.js b/src/services/terrainHeights.js index a6502aeb5..124777150 100644 --- a/src/services/terrainHeights.js +++ b/src/services/terrainHeights.js @@ -273,7 +273,7 @@ export function createTerrainHeights({ const seenKeys = new Set(); const now = Date.now(); for (const item of work) { - const entry = cache.get(item.key); + const entry = cacheTouch(item.key); const fallbackCooling = entry?.source === 'geoid-fallback' && Number.isFinite(entry.retryAt) && From 07757a3cae25956f362525432bf2b45bb1908bd3 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 12 Oct 2026 11:14:26 -0400 Subject: [PATCH 2/4] docs: address maintainer review for PR #1040 --- docs/CURRENT-STATE.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 246f3067c..fcb2f3b3f 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1192,6 +1192,8 @@ The browser layers and their rendering remain in their existing modules. ## Terrain height cache bound +Warm batch lookups also promote their cached entries in the least-recently-used eviction order. + The client terrain-height resolver's in-memory cache is bounded at 20 000 entries (`DEFAULT_MAX_CACHE_ENTRIES`, overridable per instance through `createTerrainHeights({ maxCacheEntries })`). Coordinate rounding makes repeated @@ -5326,8 +5328,3 @@ datacenters, dams and the Nepal locator retain that shared credit while displaye Military fragments stitch by source identity. Different mapped parcels merge only when their footprints touch or contain one another, never across a seam tolerance gap. - - -### fix(terrain): refresh cache recency when resolving warm batches - -Use the existing LRU read path for batch lookups; provider requests, fallback cooldowns and bounded residency retain their current contracts. From b4aa61d6ef53982ed2cf34f0eea65cabf181c86d Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Wed, 14 Oct 2026 16:37:30 -0700 Subject: [PATCH 3/4] fix(terrain): keep a batch's warm points when its own fetches evict them A warm entry could be evicted by the same call's later fetches and then reported unresolved. The call now remembers warm hits as it does fetched ones. Moves the batch LRU test beside the existing cache tests and folds the docs note into the section's eviction paragraph. --- docs/CURRENT-STATE.md | 10 ++--- src/services/terrain-warm-batch-lru.test.mjs | 24 ------------ src/services/terrainHeights.js | 20 +++++----- src/services/terrainHeights.test.mjs | 41 ++++++++++++++++++++ 4 files changed, 54 insertions(+), 41 deletions(-) delete mode 100644 src/services/terrain-warm-batch-lru.test.mjs diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index fcb2f3b3f..d28fee7e1 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1192,19 +1192,17 @@ The browser layers and their rendering remain in their existing modules. ## Terrain height cache bound -Warm batch lookups also promote their cached entries in the least-recently-used eviction order. - The client terrain-height resolver's in-memory cache is bounded at 20 000 entries (`DEFAULT_MAX_CACHE_ENTRIES`, overridable per instance through `createTerrainHeights({ maxCacheEntries })`). Coordinate rounding makes repeated visits to one place share a key but does not bound how many distinct places a session visits, and the cache previously had no eviction, so a long session retained every coordinate it ever resolved. Eviction is least-recently-used: -a consumer read promotes its entry, so the cell being watched is not dropped for -having been resolved early. The bound sits far above a city-scale session, so +a consumer read, including a warm hit in a batch lookup, promotes its entry, so +the cell being watched is not dropped for having been resolved early. The bound sits far above a city-scale session, so normal use never evicts and issues no additional proxy requests. A batch's -results are assembled from what that call resolved, so eviction during a large -batch cannot report a just-resolved point as unresolved. Re:Earth and +results are assembled from what that call resolved or found warm, so eviction +during a large batch cannot report one of its points as unresolved. Re:Earth and geoid-fallback entry semantics, the fallback cooldown and the abort-time flush are unchanged. diff --git a/src/services/terrain-warm-batch-lru.test.mjs b/src/services/terrain-warm-batch-lru.test.mjs deleted file mode 100644 index ac718b998..000000000 --- a/src/services/terrain-warm-batch-lru.test.mjs +++ /dev/null @@ -1,24 +0,0 @@ -import test from 'node:test'; -import assert from 'node:assert/strict'; -import { createTerrainHeights } from './terrainHeights.js'; -test('warm batch consumers retain their terrain cells ahead of unused cells', async () => { - let calls = 0; - const terrain = createTerrainHeights({ - maxCacheEntries: 2, - source: { - async getHeights(points) { - calls++; - return points.map((p) => ({ ellipsoid: 100 + p.lon })); - }, - }, - }); - const p = (lon) => ({ lat: 0, lon }); - await terrain.resolveEllipsoidalGround([p(1)]); - await terrain.resolveEllipsoidalGround([p(2)]); - await terrain.resolveEllipsoidalGround([p(1)]); - assert.equal(calls, 2); - await terrain.resolveEllipsoidalGround([p(3)]); - assert.equal(terrain.cachedEllipsoidalGround(0, 1), 101); - assert.equal(terrain.cachedEllipsoidalGround(0, 2), null); - assert.equal(calls, 3); -}); diff --git a/src/services/terrainHeights.js b/src/services/terrainHeights.js index 124777150..f84a4413f 100644 --- a/src/services/terrainHeights.js +++ b/src/services/terrainHeights.js @@ -272,28 +272,26 @@ export function createTerrainHeights({ const uncached = []; const seenKeys = new Set(); const now = Date.now(); + // What this call resolved or found warm, so the assembly below can report a + // point even if a later chunk's writes pushed it past CACHE_MAX_ENTRIES. + // Without it a batch big enough to evict its own entries would report + // points as unresolved that it had in fact resolved. + const resolvedThisCall = new Map(); for (const item of work) { const entry = cacheTouch(item.key); const fallbackCooling = entry?.source === 'geoid-fallback' && Number.isFinite(entry.retryAt) && now < entry.retryAt; - if ( - (entry && entry.source === 'reearth') || - fallbackCooling || - seenKeys.has(item.key) - ) + if ((entry && entry.source === 'reearth') || fallbackCooling) { + resolvedThisCall.set(item.key, entry); continue; + } + if (seenKeys.has(item.key)) continue; seenKeys.add(item.key); uncached.push(item); } - // What this call resolved, so the assembly below can report a point even - // if a later chunk's writes pushed it past CACHE_MAX_ENTRIES. Without it a - // batch big enough to evict its own earlier chunks would report points as - // unresolved that it had in fact just resolved. - const resolvedThisCall = new Map(); - // Resolve the network path in sequential <=CHUNK_SIZE chunks. Each chunk's // failure is isolated to that chunk's points (geoid fallback), so a single // bad chunk doesn't lose results for the rest of a large batch. diff --git a/src/services/terrainHeights.test.mjs b/src/services/terrainHeights.test.mjs index c9884e89c..5e80b9be2 100644 --- a/src/services/terrainHeights.test.mjs +++ b/src/services/terrainHeights.test.mjs @@ -114,6 +114,47 @@ test('a batch larger than the cache bound still reports every point it resolved' assert.equal(warm.length, 4); }); +test('warm batch consumers retain their terrain cells ahead of unused cells', async () => { + let calls = 0; + const terrain = createTerrainHeights({ + maxCacheEntries: 2, + source: { + async getHeights(points) { + calls++; + return points.map((p) => ({ ellipsoid: 100 + p.lon })); + }, + }, + }); + const p = (lon) => ({ lat: 0, lon }); + await terrain.resolveEllipsoidalGround([p(1)]); + await terrain.resolveEllipsoidalGround([p(2)]); + await terrain.resolveEllipsoidalGround([p(1)]); + assert.equal(calls, 2); + await terrain.resolveEllipsoidalGround([p(3)]); + assert.equal(terrain.cachedEllipsoidalGround(0, 1), 101); + assert.equal(terrain.cachedEllipsoidalGround(0, 2), null); + assert.equal(calls, 3); +}); + +test('a batch still reports its warm points after its own fetches evict them', async () => { + const terrain = createTerrainHeights({ + source: stubSource(), + maxCacheEntries: 2, + }); + await terrain.resolveEllipsoidalGround([pointAt(0)]); + + const results = await terrain.resolveEllipsoidalGround([ + pointAt(0), + pointAt(1), + pointAt(2), + ]); + + results.forEach((result, i) => { + assert.equal(result.source, 'reearth', `point ${i} reported resolved`); + assert.equal(result.ellipsoid, heightAt(lonAt(i))); + }); +}); + test('an out-of-range cache bound falls back to the default instead of disabling the cache', async () => { for (const maxCacheEntries of [0, -1, Number.NaN, undefined]) { const terrain = createTerrainHeights({ From d861f24d3e4456de567590ba253cd6a5b84a68f3 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Wed, 14 Oct 2026 20:11:57 -0400 Subject: [PATCH 4/4] docs: separate CHANGELOG bullets after rebase Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3075234c..80d2a4544 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ - Tool string length validation counts Unicode characters correctly, including characters outside the basic multilingual plane. - Director timeline seeks cross the dateline along the short longitude arc. + - Frequently reused terrain batches retain their cached heights ahead of unused cells. - Treat explicitly current, complete empty vessel coverage as a successful update,