diff --git a/changelog.d/1093-multipart-cable-feature-ownership.md b/changelog.d/1093-multipart-cable-feature-ownership.md new file mode 100644 index 000000000..777a159d0 --- /dev/null +++ b/changelog.d/1093-multipart-cable-feature-ownership.md @@ -0,0 +1,4 @@ +- Keep every submarine-cable segment associated with its source feature after + Cesium expands multipart geometry, correcting segment colors, click destinations + and horizon culling while retaining one reference stem per feature + (@Aliciawque, #1093). diff --git a/changelog.d/1447-bikeshare-derived-capacity.md b/changelog.d/1447-bikeshare-derived-capacity.md new file mode 100644 index 000000000..c241b58f7 --- /dev/null +++ b/changelog.d/1447-bikeshare-derived-capacity.md @@ -0,0 +1,3 @@ +- Bike Share stations whose feed publishes no capacity now take it from each + status poll's bikes and docks instead of keeping the first poll's value or + the placeholder 15 (Aditya, @Rohilalala, #1447). diff --git a/changelog.d/1462-voice-alias-hint-budget.md b/changelog.d/1462-voice-alias-hint-budget.md new file mode 100644 index 000000000..ae0b1be38 --- /dev/null +++ b/changelog.d/1462-voice-alias-hint-budget.md @@ -0,0 +1,5 @@ +- Leave layer aliases the layer id already spells, such as "dams" for + `local-dams`, out of the voice layer hint. The Realtime prompt is 305 + characters shorter, leaving room under the 52,000 gate for new + voice-toggleable layers; every alias is still accepted (@quanticsoul4772, + #1462). diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 84d42cb7b..8a8a5aa1d 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1898,6 +1898,8 @@ limit, as well as when a tile decodes or the source resets. GBFS null or empty availability counts retain an unknown value through station parsing and rendering; they use the neutral station color rather than reporting an empty dock. Explicit zero and caller-supplied numeric fallbacks remain valid. +A station whose `station_information` publishes no capacity takes bikes plus +docks from each status poll (15 until its status arrives), not from the first. ## Terrain height cache bound @@ -6371,6 +6373,12 @@ three Cesium data sources; enabling rebuilds from the accepted parsed cache, and destroying clears it. Load ownership prevents cancelled work from adding entities after teardown. +Every Cesium entity from multipart cable or landing-point geometry stays tied +to its source feature, including generated IDs and IDs that collide with +Cesium's multipart suffixes. Each segment gets its own feature's styling, click +destination and horizon-culling reference, and each source feature has one +reference stem. The source collections are not modified. + Natural Earth regions and neighborhood polygon lookup are package exports. Their existing lazy loaders, retry behavior, bundled datasets and attribution are unchanged. The cable dataset remains CC BY-NC-SA 3.0 and is not covered by diff --git a/src/data/telegeographySubmarineCables.test.mjs b/src/data/telegeographySubmarineCables.test.mjs index 3850748dc..110f65f18 100644 --- a/src/data/telegeographySubmarineCables.test.mjs +++ b/src/data/telegeographySubmarineCables.test.mjs @@ -1754,3 +1754,252 @@ test('another GeoJSON source renders with its own label and rebuilds from accept env.layer.destroy(env.viewer); } }); + +function multipartCableFixture() { + return [ + { + type: 'Feature', + properties: { + id: 'alpha', + name: 'Alpha cable', + color: '#ff0000', + coordinates: [-40, 35], + }, + geometry: { + type: 'MultiLineString', + coordinates: [ + [ + [-40, 35], + [-39, 36], + ], + [ + [-38, 37], + [-37, 38], + ], + [ + [-36, 39], + [-35, 40], + ], + ], + }, + }, + { + type: 'Feature', + properties: { + id: 'beta', + name: 'Beta cable', + color: '#0000ff', + coordinates: [140, -35], + }, + geometry: { + type: 'LineString', + coordinates: [ + [140, -35], + [141, -34], + ], + }, + }, + ]; +} + +function geometrySource(cables, landingPoints = []) { + return { + label: 'Multipart test source', + async fetch() { + return { + cables: { type: 'FeatureCollection', features: cables }, + landingPoints: { type: 'FeatureCollection', features: landingPoints }, + }; + }, + }; +} + +test('multipart association accepts frozen provider collections without modifying properties', async () => { + const payload = { + cables: { type: 'FeatureCollection', features: multipartCableFixture() }, + landingPoints: { + type: 'FeatureCollection', + features: multipartCableFixture(), + }, + }; + payload.cables.features[0].properties.__gevSourceFeatureIndex = + 'provider value'; + const before = structuredClone(payload); + const freeze = (value) => { + if (value && typeof value === 'object') { + for (const child of Object.values(value)) freeze(child); + Object.freeze(value); + } + return value; + }; + freeze(payload); + const env = await createRealCableLayerHarness({ + source: { label: 'Frozen multipart source', fetch: async () => payload }, + }); + try { + for (const dataSource of env.dataSources.slice(0, 2)) { + assert.deepEqual( + dataSource.entities.values.map( + (entity) => entity.__gevTeleGeography?.label, + ), + ['Alpha cable', 'Alpha cable', 'Alpha cable', 'Beta cable'], + ); + } + assert.equal(env.dataSources[2].entities.values.length, 4); + assert.deepEqual(payload, before); + const entity = env.dataSources[0].entities.values[0]; + assert.equal(entity.properties.__gevSourceFeatureIndex.getValue(), 0); + assert.match(entity.description.getValue(), /__gevSourceFeatureIndex/); + } finally { + env.layer.destroy(env.viewer); + } +}); + +test('multipart cables retain their own styling, picking and culling on every segment', async () => { + const features = multipartCableFixture(); + const before = JSON.stringify(features); + const env = await createRealCableLayerHarness({ + source: geometrySource(features), + }); + try { + const entities = env.dataSources[0].entities.values; + assert.equal(entities.length, 4); + for (const [index, entity] of entities.entries()) { + const feature = features[index < 3 ? 0 : 1]; + const info = entity.__gevTeleGeography; + assert.equal(info?.label, feature.properties.name, entity.id); + assert.deepEqual(info.reference, { + lon: feature.properties.coordinates[0], + lat: feature.properties.coordinates[1], + }); + assert.deepEqual( + entity.polyline.material.color.getValue(), + Cesium.Color.fromCssColorString(feature.properties.color).withAlpha( + 0.92, + ), + ); + assert.equal(entity.polyline.width.getValue(), 2.5); + } + env.raiseSweep(); + assert.deepEqual( + entities.map((entity) => entity.show), + [true, true, true, false], + ); + assert.equal( + env.dataSources[2].entities.values.length, + 2, + 'one reference per feature', + ); + assert.equal( + JSON.stringify(features), + before, + 'source features stay unchanged', + ); + } finally { + env.layer.destroy(env.viewer); + } +}); + +test('multipart association supports missing and repeated IDs without guessing Cesium suffixes', async () => { + const features = multipartCableFixture(); + features[1].id = 'alpha_2'; + delete features[1].properties.id; + const generated = structuredClone(features[1]); + delete generated.id; + generated.properties.name = 'Generated cable'; + features.push(generated); + const repeated = structuredClone(features[1]); + repeated.properties.id = 'alpha'; + repeated.properties.name = 'Repeated ID cable'; + features.push(repeated); + const env = await createRealCableLayerHarness({ + source: geometrySource(features), + }); + try { + const entities = env.dataSources[0].entities.values; + assert.deepEqual( + entities.map((entity) => entity.id), + ['alpha', 'alpha_2', 'alpha_3', 'alpha_2_2', 'cable-2', 'alpha_4'], + ); + assert.deepEqual( + entities.map((entity) => entity.__gevTeleGeography?.label), + [ + 'Alpha cable', + 'Alpha cable', + 'Alpha cable', + 'Beta cable', + 'Generated cable', + 'Repeated ID cable', + ], + ); + assert.deepEqual( + env.dataSources[2].entities.values.map( + (entity) => entity.__gevTeleGeography.featureId, + ), + ['alpha', 'alpha_2', 'cable-2', 'alpha'], + ); + } finally { + env.layer.destroy(env.viewer); + } +}); + +test('multipart landing geometry keeps every pick with one reference per feature', async () => { + const features = multipartCableFixture(); + const env = await createRealCableLayerHarness({ + source: geometrySource([], features), + }); + try { + const entities = env.dataSources[1].entities.values; + assert.deepEqual( + entities.map((entity) => entity.__gevTeleGeography?.label), + ['Alpha cable', 'Alpha cable', 'Alpha cable', 'Beta cable'], + ); + assert.ok( + entities.every( + (entity) => entity.__gevTeleGeography.kind === 'landing-point', + ), + ); + assert.equal(env.dataSources[2].entities.values.length, 2); + } finally { + env.layer.destroy(env.viewer); + } +}); + +test('every bundled cable segment is associated with its source feature', async () => { + const { features } = JSON.parse( + readFileSync( + new URL( + './local_data/telegeography_submarine_cables/cable-geo.json', + import.meta.url, + ), + 'utf8', + ), + ); + // Some bundled features share a cable ID but have different references. + const expandedFeatures = features.flatMap((feature) => + feature.geometry.coordinates.map(() => feature), + ); + assert.ok(expandedFeatures.length > features.length); + const env = await createRealCableLayerHarness({ + source: geometrySource(features), + }); + try { + const entities = env.dataSources[0].entities.values; + assert.equal(entities.length, expandedFeatures.length); + for (const [index, entity] of entities.entries()) { + const feature = expandedFeatures[index]; + assert.equal( + entity.__gevTeleGeography?.label, + feature.properties.name, + entity.id, + ); + assert.deepEqual(entity.__gevTeleGeography.reference, { + lon: feature.properties.coordinates[0], + lat: feature.properties.coordinates[1], + }); + } + assert.equal(env.dataSources[2].entities.values.length, features.length); + } finally { + env.layer.destroy(env.viewer); + } +}); diff --git a/src/firstRunExperience.test.mjs b/src/firstRunExperience.test.mjs index 56548c1da..81c1cfda5 100644 --- a/src/firstRunExperience.test.mjs +++ b/src/firstRunExperience.test.mjs @@ -1029,10 +1029,11 @@ test('the voice TOOL SCHEMA matches the pinned contract — the mission mapping // Panel adds timeout and named voice/provider targets; consolidated guidance // preserves the mission mapping and globe-rotation arguments. // S2 GhostMaps extends the generated layer enums; both feature contracts remain. - assert.equal(block.length, 31206, 'serialized tool schema length drifted'); + // The layer alias hint leaves out aliases the layer id already spells. + assert.equal(block.length, 30901, 'serialized tool schema length drifted'); assert.equal( crypto.createHash('sha256').update(block).digest('hex'), - 'f772df95404494bf492e02fde1beabffdfe61986940bffd0cf9ad5a38d768746', + '9e1bd8fde648953c47718ba248758415a95c1484b0da6369df6b89b0505c0de2', 'the first-run missions must ride EXISTING tools: no schema edit, no cache bust', ); const instructions = fs.readFileSync( diff --git a/src/layers/bikeshare/proxyRoute.test.mjs b/src/layers/bikeshare/proxyRoute.test.mjs index e4153a203..718dc5034 100644 --- a/src/layers/bikeshare/proxyRoute.test.mjs +++ b/src/layers/bikeshare/proxyRoute.test.mjs @@ -446,3 +446,62 @@ test('unknown GBFS counts stay unknown through the source, model and station col assert.equal(model.toNonNegativeInteger('3'), 3); } }); + +test('a station without a published capacity re-derives it from every status poll', async () => { + const { createRendering } = await import('./rendering.js'); + const { COLOR_YELLOW } = await import('./policy.js'); + const model = createModel({}); + const state = { + _cityRuntime: new Map(), + _stationRenderMap: new Map(), + _pointCollection: { add: (options) => ({ ...options }) }, + }; + const rendering = createRendering({ + state, + services: { render: { governorRequestRender() {} } }, + parts: { model }, + }); + const info = model.parseStationInformation({ + data: { + stations: [ + { station_id: 'open', name: 'Open', lat: 30.26, lon: -97.74 }, + { + station_id: 'fixed', + name: 'Fixed', + lat: 30.27, + lon: -97.75, + capacity: 13, + }, + ], + }, + }); + rendering.ensureCityPoints('austin', info); + const poll = (stations) => + rendering.applyStatusToPoints( + 'austin', + model.parseStationStatus({ data: { stations } }), + ); + const open = state._stationRenderMap.get('austin:open'); + const fixed = state._stationRenderMap.get('austin:fixed'); + + // The first poll has no row for the station: capacity is a placeholder. + poll([ + { + station_id: 'fixed', + num_bikes_available: 4, + num_docks_available: 9, + }, + ]); + poll([ + { station_id: 'open', num_bikes_available: 10, num_docks_available: 20 }, + { station_id: 'fixed', num_bikes_available: 2, num_docks_available: 3 }, + ]); + assert.equal(open.capacity, 30, 'the first poll fixed the capacity'); + assert.equal(open.point.color, COLOR_YELLOW, '10 of 30 is not plenty'); + assert.equal(fixed.capacity, 13, 'a published capacity is kept'); + + poll([ + { station_id: 'open', num_bikes_available: 4, num_docks_available: 2 }, + ]); + assert.equal(open.capacity, 6); +}); diff --git a/src/layers/bikeshare/rendering.js b/src/layers/bikeshare/rendering.js index f0c737d1d..c75189d4a 100644 --- a/src/layers/bikeshare/rendering.js +++ b/src/layers/bikeshare/rendering.js @@ -185,6 +185,7 @@ export function createRendering({ stationName: station.name, point, capacity: parts.model.toNonNegativeInteger(station.capacity), + publishedCapacity: parts.model.toNonNegativeInteger(station.capacity), bikesAvailable: null, docksAvailable: null, isInstalled: station.isInstalled, @@ -245,7 +246,8 @@ export function createRendering({ if (!record) continue; const status = statusMap.get(record.stationId) || null; - const capacity = resolveCapacity(record.capacity, status); + // Resolve from the published capacity, not last poll's derived value. + const capacity = resolveCapacity(record.publishedCapacity, status); record.capacity = capacity; record.bikesAvailable = parts.model.toNonNegativeInteger( diff --git a/src/layers/submarineCables/ingestion.js b/src/layers/submarineCables/ingestion.js index 5468996d5..714b8753b 100644 --- a/src/layers/submarineCables/ingestion.js +++ b/src/layers/submarineCables/ingestion.js @@ -6,6 +6,21 @@ import { } from './geometry.js'; import * as Cesium from 'cesium'; +const SOURCE_FEATURE_INDEX = '__gevSourceFeatureIndex'; + +function indexedCollection(features) { + return { + type: 'FeatureCollection', + features: features.map((feature, index) => ({ + ...feature, + properties: { + ...feature.properties, + [SOURCE_FEATURE_INDEX]: index, + }, + })), + }; +} + export function createIngestion({ state, parts, source }) { async function load(viewer) { if (state._loading || state._loaded) return; @@ -49,7 +64,7 @@ export function createIngestion({ state, parts, source }) { const landingFeatures = normalizeFeatures(landingJson, 'landing'); const cableDataSource = await Cesium.GeoJsonDataSource.load( - { type: 'FeatureCollection', features: cableFeatures }, + indexedCollection(cableFeatures), { clampToGround: true, stroke: state.cableColor.withAlpha(0.95), @@ -61,7 +76,7 @@ export function createIngestion({ state, parts, source }) { ); if (!owns()) return; const landingDataSource = await Cesium.GeoJsonDataSource.load( - { type: 'FeatureCollection', features: landingFeatures }, + indexedCollection(landingFeatures), { clampToGround: true, stroke: state.landingColor.withAlpha(0.9), @@ -131,7 +146,12 @@ export function createIngestion({ state, parts, source }) { state._referenceRecords = []; state._surfaceRecords = []; - cableEntities.forEach((entity, index) => { + // Cesium expands multipart geometry into several entities and suffixes + // colliding IDs. Its copied properties retain this source index even + // when a provider has no ID, so every segment keeps the right owner. + const cableReferences = new Set(); + cableEntities.forEach((entity) => { + const index = entity.properties?.[SOURCE_FEATURE_INDEX]?.getValue(); const feature = cableFeatures[index]; const reference = featureReference(feature); if (!reference) return; @@ -146,6 +166,8 @@ export function createIngestion({ state, parts, source }) { entity, base: Cesium.Cartesian3.fromDegrees(reference.lon, reference.lat, 0), }); + if (cableReferences.has(index)) return; + cableReferences.add(index); parts.rendering.addReferenceStem({ reference, label: featureLabel(feature), @@ -155,7 +177,9 @@ export function createIngestion({ state, parts, source }) { }); }); - landingEntities.forEach((entity, index) => { + const landingReferences = new Set(); + landingEntities.forEach((entity) => { + const index = entity.properties?.[SOURCE_FEATURE_INDEX]?.getValue(); const feature = landingFeatures[index]; const reference = featureReference(feature); if (!reference) return; @@ -170,6 +194,8 @@ export function createIngestion({ state, parts, source }) { entity, base: Cesium.Cartesian3.fromDegrees(reference.lon, reference.lat, 0), }); + if (landingReferences.has(index)) return; + landingReferences.add(index); parts.rendering.addReferenceStem({ reference, label: featureLabel(feature), diff --git a/src/voice/actionSchemas.test.mjs b/src/voice/actionSchemas.test.mjs index 4e6586729..6f16f7606 100644 --- a/src/voice/actionSchemas.test.mjs +++ b/src/voice/actionSchemas.test.mjs @@ -47,7 +47,8 @@ test('the complete Realtime tool payload pins the manifest-generated layer relea // is consolidated without changing either feature's arguments. // Snow leaves the set_visual_style enum. // The Operator HUD layout leaves set_hud's layout enum. - 'f47a7f78f13b1f79978fa524fad2618e32bd813e1a5dfd746bf35d29909fc91b', + // The layer alias hint leaves out aliases the layer id already spells. + 'e65812db60a2702cebb496fca257cb04a28f1bbbf250cb9ab65a3db33005e0c8', ); }); diff --git a/src/voice/layerManifest.js b/src/voice/layerManifest.js index 267399e58..962bf6949 100644 --- a/src/voice/layerManifest.js +++ b/src/voice/layerManifest.js @@ -525,22 +525,31 @@ const SELF_EVIDENT_IDS = new Set([ 'radio', 'wind', 'directions', - // The brand is already the layer id; all spoken aliases remain accepted. - 's2-ghostmaps', ]); +/** Lower case with spaces and hyphens removed: "Data centers" → "datacenters". */ +const spelled = (text) => text.toLowerCase().replace(/[\s-]+/g, ''); + /** * Compact model-facing alias hint: "alias/alias→id; …" for toggleable layers * whose id is not already the spoken word. At most two aliases per layer keep - * the tool JSON small; the runner accepts the full alias list. + * the tool JSON small; the runner accepts the full alias list. Of those two, + * an alias the id already spells ("dams"→local-dams, "weather radar"→ + * weather-radar) is left out, and a layer with neither left gets no hint. * @returns {string} One line for the layerId description. */ export function voiceLayerAliasHint() { return VOICE_LAYER_MANIFEST.filter( - (entry) => - entry.toggle && entry.aliases.length && !SELF_EVIDENT_IDS.has(entry.id), + (entry) => entry.toggle && !SELF_EVIDENT_IDS.has(entry.id), ) - .map((entry) => `${entry.aliases.slice(0, 2).join('/')}→${entry.id}`) + .map((entry) => ({ + id: entry.id, + aliases: entry.aliases + .slice(0, 2) + .filter((alias) => !spelled(entry.id).includes(spelled(alias))), + })) + .filter((entry) => entry.aliases.length) + .map((entry) => `${entry.aliases.join('/')}→${entry.id}`) .join(';'); } diff --git a/src/voice/layerManifest.test.mjs b/src/voice/layerManifest.test.mjs index d0222b934..ab382c460 100644 --- a/src/voice/layerManifest.test.mjs +++ b/src/voice/layerManifest.test.mjs @@ -9,6 +9,7 @@ import { VOICE_TOGGLE_LAYER_IDS, VOICE_CONTEXT_LAYER_IDS, VOICE_QUERY_LAYER_IDS, + voiceLayerAliasHint, voiceLayerAliasMap, } from './layerManifest.js'; @@ -78,6 +79,23 @@ test('tool enums are generated from the manifest', () => { assert.ok(panels.includes('weather-panel') && panels.includes('recent-imagery-panel')); }); +test('the alias hint leaves out names the layer id already spells', () => { + const hint = voiceLayerAliasHint(); + const entries = hint.split(';').map((entry) => entry.split('→')); + const flat = (text) => text.toLowerCase().replace(/[\s-]+/g, ''); + for (const [aliases, id] of entries) { + for (const alias of aliases.split('/')) { + assert.ok(!flat(id).includes(flat(alias)), `"${alias}" is already spelled by ${id}`); + } + } + const ids = entries.map(([, id]) => id); + assert.ok(!ids.includes('local-dams'), 'a layer left with no alias gets no entry'); + assert.ok(!ids.includes('weather-radar')); + assert.match(hint, /(^|;)aqhi→air-quality(;|$)/, 'an alias the id does not spell stays'); + assert.match(hint, /(^|;)hurricanes→weather-cyclones(;|$)/); + assert.match(hint, /(^|;)fires\/wildfires→local-firms(;|$)/); +}); + test('spoken aliases resolve to real layers, keep every legacy phrase and never collide', () => { const aliases = voiceLayerAliasMap(); for (const id of aliases.values()) assert.ok(manifestIds.includes(id), id);