mirror of
https://github.com/koala73/worldmonitor.git
synced 2026-04-25 17:14:57 +02:00
feat(supply-chain): Sprint E — scenario visual completion + service parity (#2910)
* feat(supply-chain): Sprint E — scenario visual completion + service parity - E1: fetchSectorDependency exported from supply-chain service index - E2: PRO gate + all-renderer dispatch in MapContainer.activateScenario - E3: scenario summary banner in SupplyChainPanel (dismiss wired) - E4: "Simulate Closure" trigger button in expanded chokepoint cards - E5: affectedIso2s heat layer in DeckGLMap (GeoJsonLayer, red tint) - E6: SVG renderer setScenarioState (best-effort iso2 fill) - E7: Globe renderer scenario polygons via flushPolygons - E8: integration tests for scenario run/status endpoints * fix(supply-chain): address PR #2910 review findings (P1 + P2 + P3) - Wire setOnScenarioActivate + setOnDismissScenario in panel-layout.ts (todo #155) - Rename shadow variable t→tmpl in SCENARIO_TEMPLATES.find (todo #152) - Add statusResp.ok guard in scenario polling loop (todo #153) - Replace status.result! non-null assertion with shape guard (todo #154) - Add AbortController to prevent concurrent polling races (todo #162) - Add polygonStrokeColor scenario branch (transparent) in GlobeMap (todo #156) - Re-export SCENARIO_TEMPLATES via src/config/scenario-templates.ts (todo #157) - Cache affectedIso2Set in DeckGLMap.setScenarioState (todo #158) - Add scenario paths to PREMIUM_RPC_PATHS for auth injection (todo #160) - Show template name in scenario banner instead of raw ID (todo #163) * fix(supply-chain): address PR #2910 review findings - Add auth headers to scenario fetch calls in SupplyChainPanel - Reset button state on scenario dismiss - Poll status immediately on first iteration (no 2s delay) - Pre-compute scenario polygons in GlobeMap.setScenarioState - Use scenarioId for DeckGL updateTriggers precision * fix(supply-chain): wire panel instance to MapContainer, stop button click propagation - Call setSupplyChainPanel() in panel-layout.ts so scenario banner renders - Add stopPropagation() to Simulate Closure button to prevent card collapse
This commit is contained in:
@@ -780,7 +780,16 @@ export class PanelLayoutManager implements AppModule {
|
|||||||
|
|
||||||
this.createPanel('trade-policy', () => new TradePolicyPanel());
|
this.createPanel('trade-policy', () => new TradePolicyPanel());
|
||||||
this.createPanel('sanctions-pressure', () => new SanctionsPressurePanel());
|
this.createPanel('sanctions-pressure', () => new SanctionsPressurePanel());
|
||||||
this.createPanel('supply-chain', () => new SupplyChainPanel());
|
const supplyChainPanel = this.createPanel('supply-chain', () => new SupplyChainPanel());
|
||||||
|
if (supplyChainPanel) {
|
||||||
|
supplyChainPanel.setOnScenarioActivate((id, result) => {
|
||||||
|
this.ctx.map?.activateScenario(id, result);
|
||||||
|
});
|
||||||
|
supplyChainPanel.setOnDismissScenario(() => {
|
||||||
|
this.ctx.map?.deactivateScenario();
|
||||||
|
});
|
||||||
|
this.ctx.map?.setSupplyChainPanel(supplyChainPanel);
|
||||||
|
}
|
||||||
|
|
||||||
this.createNewsPanel('africa', 'panels.africa');
|
this.createNewsPanel('africa', 'panels.africa');
|
||||||
this.createNewsPanel('latam', 'panels.latam');
|
this.createNewsPanel('latam', 'panels.latam');
|
||||||
|
|||||||
@@ -401,6 +401,7 @@ export class DeckGLMap {
|
|||||||
private tradeRouteSegments: TradeRouteSegment[] = resolveTradeRouteSegments();
|
private tradeRouteSegments: TradeRouteSegment[] = resolveTradeRouteSegments();
|
||||||
private storedChokepointData: GetChokepointStatusResponse | null = null;
|
private storedChokepointData: GetChokepointStatusResponse | null = null;
|
||||||
private scenarioState: ScenarioVisualState | null = null;
|
private scenarioState: ScenarioVisualState | null = null;
|
||||||
|
private affectedIso2Set: Set<string> = new Set();
|
||||||
private positiveEvents: PositiveGeoEvent[] = [];
|
private positiveEvents: PositiveGeoEvent[] = [];
|
||||||
private kindnessPoints: KindnessPoint[] = [];
|
private kindnessPoints: KindnessPoint[] = [];
|
||||||
private imageryScenes: ImageryScene[] = [];
|
private imageryScenes: ImageryScene[] = [];
|
||||||
@@ -1744,6 +1745,9 @@ export class DeckGLMap {
|
|||||||
const sanctionsLayer = this.createSanctionsChoroplethLayer();
|
const sanctionsLayer = this.createSanctionsChoroplethLayer();
|
||||||
if (sanctionsLayer) layers.push(sanctionsLayer);
|
if (sanctionsLayer) layers.push(sanctionsLayer);
|
||||||
}
|
}
|
||||||
|
// Scenario heat layer (affected countries tint)
|
||||||
|
const scenarioHeat = this.scenarioState ? this.createScenarioHeatLayer() : null;
|
||||||
|
if (scenarioHeat) layers.push(scenarioHeat);
|
||||||
// Phase 8: Species recovery zones
|
// Phase 8: Species recovery zones
|
||||||
if (mapLayers.speciesRecovery && this.speciesRecoveryZones.length > 0) {
|
if (mapLayers.speciesRecovery && this.speciesRecoveryZones.length > 0) {
|
||||||
layers.push(this.createSpeciesRecoveryLayer());
|
layers.push(this.createSpeciesRecoveryLayer());
|
||||||
@@ -3526,6 +3530,23 @@ export class DeckGLMap {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private createScenarioHeatLayer(): GeoJsonLayer | null {
|
||||||
|
if (!this.affectedIso2Set.size || !this.countriesGeoJsonData) return null;
|
||||||
|
return new GeoJsonLayer({
|
||||||
|
id: 'scenario-heat-layer',
|
||||||
|
data: this.countriesGeoJsonData,
|
||||||
|
stroked: false,
|
||||||
|
filled: true,
|
||||||
|
extruded: false,
|
||||||
|
pickable: false,
|
||||||
|
getFillColor: (feature: { properties?: Record<string, unknown> }) => {
|
||||||
|
const code = feature.properties?.['ISO3166-1-Alpha-2'] as string | undefined;
|
||||||
|
return (code && this.affectedIso2Set.has(code) ? [220, 60, 40, 80] : [0, 0, 0, 0]) as [number, number, number, number];
|
||||||
|
},
|
||||||
|
updateTriggers: { getFillColor: [this.scenarioState?.scenarioId ?? null] },
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
private createSpeciesRecoveryLayer(): ScatterplotLayer {
|
private createSpeciesRecoveryLayer(): ScatterplotLayer {
|
||||||
return new ScatterplotLayer({
|
return new ScatterplotLayer({
|
||||||
id: 'species-recovery-layer',
|
id: 'species-recovery-layer',
|
||||||
@@ -5434,6 +5455,7 @@ export class DeckGLMap {
|
|||||||
*/
|
*/
|
||||||
public setScenarioState(state: ScenarioVisualState | null): void {
|
public setScenarioState(state: ScenarioVisualState | null): void {
|
||||||
this.scenarioState = state;
|
this.scenarioState = state;
|
||||||
|
this.affectedIso2Set = new Set(state?.affectedIso2s ?? []);
|
||||||
this.render();
|
this.render();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -51,6 +51,7 @@ import { pinWebcam, isPinned } from '@/services/webcams/pinned-store';
|
|||||||
import type { WebcamEntry, WebcamCluster } from '@/generated/client/worldmonitor/webcam/v1/service_client';
|
import type { WebcamEntry, WebcamCluster } from '@/generated/client/worldmonitor/webcam/v1/service_client';
|
||||||
import type { TrafficAnomaly as ProtoTrafficAnomaly, DdosLocationHit } from '@/generated/client/worldmonitor/infrastructure/v1/service_client';
|
import type { TrafficAnomaly as ProtoTrafficAnomaly, DdosLocationHit } from '@/generated/client/worldmonitor/infrastructure/v1/service_client';
|
||||||
import type { RadiationObservation } from '@/services/radiation';
|
import type { RadiationObservation } from '@/services/radiation';
|
||||||
|
import type { ScenarioVisualState } from '@/config/scenario-templates';
|
||||||
|
|
||||||
const SAT_COUNTRY_COLORS: Record<string, string> = { CN: '#ff2020', RU: '#ff8800', US: '#4488ff', EU: '#44cc44', KR: '#aa66ff', IN: '#ff66aa', TR: '#ff4466', OTHER: '#ccccff' };
|
const SAT_COUNTRY_COLORS: Record<string, string> = { CN: '#ff2020', RU: '#ff8800', US: '#4488ff', EU: '#44cc44', KR: '#aa66ff', IN: '#ff66aa', TR: '#ff4466', OTHER: '#ccccff' };
|
||||||
const SAT_TYPE_EMOJI: Record<string, string> = { sar: '\u{1F4E1}', optical: '\u{1F4F7}', military: '\u{1F396}', sigint: '\u{1F4FB}' };
|
const SAT_TYPE_EMOJI: Record<string, string> = { sar: '\u{1F4E1}', optical: '\u{1F4F7}', military: '\u{1F396}', sigint: '\u{1F4FB}' };
|
||||||
@@ -396,7 +397,7 @@ interface GlobePath {
|
|||||||
interface GlobePolygon {
|
interface GlobePolygon {
|
||||||
coords: number[][][];
|
coords: number[][][];
|
||||||
name: string;
|
name: string;
|
||||||
_kind: 'cii' | 'conflict' | 'imageryFootprint' | 'forecastCone';
|
_kind: 'cii' | 'conflict' | 'imageryFootprint' | 'forecastCone' | 'scenario';
|
||||||
level?: string;
|
level?: string;
|
||||||
score?: number;
|
score?: number;
|
||||||
|
|
||||||
@@ -526,6 +527,7 @@ export class GlobeMap {
|
|||||||
private cableDegradedIds = new Set<string>();
|
private cableDegradedIds = new Set<string>();
|
||||||
private ciiScoresMap: Map<string, { score: number; level: string }> = new Map();
|
private ciiScoresMap: Map<string, { score: number; level: string }> = new Map();
|
||||||
private countriesGeoData: FeatureCollection<Geometry> | null = null;
|
private countriesGeoData: FeatureCollection<Geometry> | null = null;
|
||||||
|
private scenarioPolygons: GlobePolygon[] = [];
|
||||||
|
|
||||||
// Current layers state
|
// Current layers state
|
||||||
private layers: MapLayers;
|
private layers: MapLayers;
|
||||||
@@ -828,6 +830,7 @@ export class GlobeMap {
|
|||||||
if (d._kind === 'conflict') return GlobeMap.CONFLICT_CAP[d.intensity!] ?? GlobeMap.CONFLICT_CAP.low;
|
if (d._kind === 'conflict') return GlobeMap.CONFLICT_CAP[d.intensity!] ?? GlobeMap.CONFLICT_CAP.low;
|
||||||
if (d._kind === 'imageryFootprint') return 'rgba(0,0,0,0)';
|
if (d._kind === 'imageryFootprint') return 'rgba(0,0,0,0)';
|
||||||
if (d._kind === 'forecastCone') return 'rgba(255,140,60,0.2)';
|
if (d._kind === 'forecastCone') return 'rgba(255,140,60,0.2)';
|
||||||
|
if (d._kind === 'scenario') return 'rgba(220,60,40,0.3)';
|
||||||
return 'rgba(255,60,60,0.15)';
|
return 'rgba(255,60,60,0.15)';
|
||||||
})
|
})
|
||||||
.polygonSideColor((d: GlobePolygon) => {
|
.polygonSideColor((d: GlobePolygon) => {
|
||||||
@@ -835,6 +838,7 @@ export class GlobeMap {
|
|||||||
if (d._kind === 'conflict') return GlobeMap.CONFLICT_SIDE[d.intensity!] ?? GlobeMap.CONFLICT_SIDE.low;
|
if (d._kind === 'conflict') return GlobeMap.CONFLICT_SIDE[d.intensity!] ?? GlobeMap.CONFLICT_SIDE.low;
|
||||||
if (d._kind === 'imageryFootprint') return 'rgba(0,0,0,0)';
|
if (d._kind === 'imageryFootprint') return 'rgba(0,0,0,0)';
|
||||||
if (d._kind === 'forecastCone') return 'rgba(255,140,60,0.1)';
|
if (d._kind === 'forecastCone') return 'rgba(255,140,60,0.1)';
|
||||||
|
if (d._kind === 'scenario') return 'rgba(0,0,0,0)';
|
||||||
return 'rgba(255,60,60,0.08)';
|
return 'rgba(255,60,60,0.08)';
|
||||||
})
|
})
|
||||||
.polygonStrokeColor((d: GlobePolygon) => {
|
.polygonStrokeColor((d: GlobePolygon) => {
|
||||||
@@ -842,6 +846,7 @@ export class GlobeMap {
|
|||||||
if (d._kind === 'conflict') return GlobeMap.CONFLICT_STROKE[d.intensity!] ?? GlobeMap.CONFLICT_STROKE.low;
|
if (d._kind === 'conflict') return GlobeMap.CONFLICT_STROKE[d.intensity!] ?? GlobeMap.CONFLICT_STROKE.low;
|
||||||
if (d._kind === 'imageryFootprint') return '#00b4ff';
|
if (d._kind === 'imageryFootprint') return '#00b4ff';
|
||||||
if (d._kind === 'forecastCone') return 'rgba(255,140,60,0.5)';
|
if (d._kind === 'forecastCone') return 'rgba(255,140,60,0.5)';
|
||||||
|
if (d._kind === 'scenario') return 'transparent';
|
||||||
return '#ff4444';
|
return '#ff4444';
|
||||||
})
|
})
|
||||||
.polygonAltitude((d: GlobePolygon) => {
|
.polygonAltitude((d: GlobePolygon) => {
|
||||||
@@ -2088,11 +2093,33 @@ export class GlobeMap {
|
|||||||
polys.push(...this.stormConePolygons);
|
polys.push(...this.stormConePolygons);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (this.scenarioPolygons.length) {
|
||||||
|
polys.push(...this.scenarioPolygons);
|
||||||
|
}
|
||||||
|
|
||||||
(this.globe as any).polygonsData(polys);
|
(this.globe as any).polygonsData(polys);
|
||||||
}
|
}
|
||||||
|
|
||||||
// ─── Public data setters ──────────────────────────────────────────────────
|
// ─── Public data setters ──────────────────────────────────────────────────
|
||||||
|
|
||||||
|
public setScenarioState(state: ScenarioVisualState | null): void {
|
||||||
|
this.scenarioPolygons = [];
|
||||||
|
if (state?.affectedIso2s?.length && this.countriesGeoData) {
|
||||||
|
const affected = new Set(state.affectedIso2s);
|
||||||
|
for (const feat of this.countriesGeoData.features) {
|
||||||
|
const code = feat.properties?.['ISO3166-1-Alpha-2'] as string | undefined;
|
||||||
|
if (!code || !affected.has(code)) continue;
|
||||||
|
const geom = feat.geometry;
|
||||||
|
if (!geom) continue;
|
||||||
|
const rings = geom.type === 'Polygon' ? [geom.coordinates] : geom.type === 'MultiPolygon' ? geom.coordinates : [];
|
||||||
|
for (const ring of rings) {
|
||||||
|
this.scenarioPolygons.push({ coords: ring as number[][][], name: code, _kind: 'scenario' });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
this.flushPolygons();
|
||||||
|
}
|
||||||
|
|
||||||
public setCIIScores(scores: Array<{ code: string; score: number; level: string }>): void {
|
public setCIIScores(scores: Array<{ code: string; score: number; level: string }>): void {
|
||||||
this.ciiScoresMap = new Map(scores.map(s => [s.code, { score: s.score, level: s.level }]));
|
this.ciiScoresMap = new Map(scores.map(s => [s.code, { score: s.score, level: s.level }]));
|
||||||
this.flushPolygons();
|
this.flushPolygons();
|
||||||
|
|||||||
@@ -61,6 +61,7 @@ import { getAlertsNearLocation } from '@/services/geo-convergence';
|
|||||||
import { getCountryAtCoordinates, getCountryBbox } from '@/services/country-geometry';
|
import { getCountryAtCoordinates, getCountryBbox } from '@/services/country-geometry';
|
||||||
import type { CountryClickPayload } from './DeckGLMap';
|
import type { CountryClickPayload } from './DeckGLMap';
|
||||||
import { t } from '@/services/i18n';
|
import { t } from '@/services/i18n';
|
||||||
|
import type { ScenarioVisualState } from '@/config/scenario-templates';
|
||||||
|
|
||||||
export type TimeRange = '1h' | '6h' | '24h' | '48h' | '7d' | 'all';
|
export type TimeRange = '1h' | '6h' | '24h' | '48h' | '7d' | 'all';
|
||||||
export type MapView = 'global' | 'america' | 'mena' | 'eu' | 'asia' | 'latam' | 'africa' | 'oceania';
|
export type MapView = 'global' | 'america' | 'mena' | 'eu' | 'asia' | 'latam' | 'africa' | 'oceania';
|
||||||
@@ -3407,6 +3408,10 @@ export class MapComponent {
|
|||||||
this.popup.setChokepointData(data);
|
this.popup.setChokepointData(data);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public setScenarioState(_state: ScenarioVisualState | null): void {
|
||||||
|
// SVG renderer: scenario fill deferred (no iso2 data binding on country elements)
|
||||||
|
}
|
||||||
|
|
||||||
public setLayerLoading(layer: keyof MapLayers, loading: boolean): void {
|
public setLayerLoading(layer: keyof MapLayers, loading: boolean): void {
|
||||||
const btn = this.container.querySelector(`.layer-toggle[data-layer="${layer}"]`);
|
const btn = this.container.querySelector(`.layer-toggle[data-layer="${layer}"]`);
|
||||||
if (btn) {
|
if (btn) {
|
||||||
|
|||||||
@@ -49,6 +49,9 @@ import type { TrafficAnomaly as ProtoTrafficAnomaly, DdosLocationHit } from '@/g
|
|||||||
import type { DiseaseOutbreakItem } from '@/services/disease-outbreaks';
|
import type { DiseaseOutbreakItem } from '@/services/disease-outbreaks';
|
||||||
import type { GetChokepointStatusResponse } from '@/services/supply-chain';
|
import type { GetChokepointStatusResponse } from '@/services/supply-chain';
|
||||||
import type { ScenarioVisualState, ScenarioResult } from '@/config/scenario-templates';
|
import type { ScenarioVisualState, ScenarioResult } from '@/config/scenario-templates';
|
||||||
|
import { getAuthState } from '@/services/auth-state';
|
||||||
|
import { hasPremiumAccess } from '@/services/panel-gating';
|
||||||
|
import { trackGateHit } from '@/services/analytics';
|
||||||
|
|
||||||
export type { ScenarioVisualState, ScenarioResult };
|
export type { ScenarioVisualState, ScenarioResult };
|
||||||
|
|
||||||
@@ -90,6 +93,7 @@ export class MapContainer {
|
|||||||
private deckGLMap: DeckGLMap | null = null;
|
private deckGLMap: DeckGLMap | null = null;
|
||||||
private svgMap: MapComponent | null = null;
|
private svgMap: MapComponent | null = null;
|
||||||
private globeMap: GlobeMap | null = null;
|
private globeMap: GlobeMap | null = null;
|
||||||
|
private supplyChainPanel: import('@/components/SupplyChainPanel').SupplyChainPanel | null = null;
|
||||||
private initialState: MapContainerState;
|
private initialState: MapContainerState;
|
||||||
private useDeckGL: boolean;
|
private useDeckGL: boolean;
|
||||||
private useGlobe: boolean;
|
private useGlobe: boolean;
|
||||||
@@ -970,22 +974,31 @@ export class MapContainer {
|
|||||||
|
|
||||||
// ─── Scenario Engine ─────────────────────────────────────────────────────────
|
// ─── Scenario Engine ─────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
public setSupplyChainPanel(panel: import('@/components/SupplyChainPanel').SupplyChainPanel): void {
|
||||||
|
this.supplyChainPanel = panel;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Activate a scenario across all active renderers.
|
* Activate a scenario across all active renderers.
|
||||||
* PRO-gated — free users trigger `trackGateHit('scenario')` only.
|
* PRO-gated — free users trigger `trackGateHit('scenario-engine')` only.
|
||||||
*
|
*
|
||||||
* @param scenarioId Template ID from scenario-templates.ts
|
* @param scenarioId Template ID from scenario-templates.ts
|
||||||
* @param result Computed result from the scenario worker
|
* @param result Computed result from the scenario worker
|
||||||
*/
|
*/
|
||||||
public activateScenario(scenarioId: string, result: ScenarioResult): void {
|
public activateScenario(scenarioId: string, result: ScenarioResult): void {
|
||||||
|
if (!hasPremiumAccess(getAuthState())) {
|
||||||
|
trackGateHit('scenario-engine');
|
||||||
|
return;
|
||||||
|
}
|
||||||
const state: ScenarioVisualState = {
|
const state: ScenarioVisualState = {
|
||||||
scenarioId,
|
scenarioId,
|
||||||
disruptedChokepointIds: result.affectedChokepointIds,
|
disruptedChokepointIds: result.affectedChokepointIds,
|
||||||
affectedIso2s: result.topImpactCountries.map((c: { iso2: string }) => c.iso2),
|
affectedIso2s: result.topImpactCountries.map((c: { iso2: string }) => c.iso2),
|
||||||
};
|
};
|
||||||
// DeckGL is the primary renderer for scenario visuals.
|
|
||||||
// Globe and SVG support is deferred to Sprint D.
|
|
||||||
this.deckGLMap?.setScenarioState(state);
|
this.deckGLMap?.setScenarioState(state);
|
||||||
|
this.svgMap?.setScenarioState(state);
|
||||||
|
this.globeMap?.setScenarioState(state);
|
||||||
|
this.supplyChainPanel?.showScenarioSummary(scenarioId, result);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -993,6 +1006,9 @@ export class MapContainer {
|
|||||||
*/
|
*/
|
||||||
public deactivateScenario(): void {
|
public deactivateScenario(): void {
|
||||||
this.deckGLMap?.setScenarioState(null);
|
this.deckGLMap?.setScenarioState(null);
|
||||||
|
this.svgMap?.setScenarioState(null);
|
||||||
|
this.globeMap?.setScenarioState(null);
|
||||||
|
this.supplyChainPanel?.hideScenarioSummary();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Utility methods
|
// Utility methods
|
||||||
|
|||||||
@@ -6,6 +6,8 @@ import type {
|
|||||||
GetShippingStressResponse,
|
GetShippingStressResponse,
|
||||||
} from '@/services/supply-chain';
|
} from '@/services/supply-chain';
|
||||||
import { fetchBypassOptions } from '@/services/supply-chain';
|
import { fetchBypassOptions } from '@/services/supply-chain';
|
||||||
|
import type { ScenarioResult } from '@/config/scenario-templates';
|
||||||
|
import { SCENARIO_TEMPLATES } from '@/config/scenario-templates';
|
||||||
import { TransitChart } from '@/utils/transit-chart';
|
import { TransitChart } from '@/utils/transit-chart';
|
||||||
import { t } from '@/services/i18n';
|
import { t } from '@/services/i18n';
|
||||||
import { escapeHtml } from '@/utils/sanitize';
|
import { escapeHtml } from '@/utils/sanitize';
|
||||||
@@ -14,6 +16,7 @@ import { isDesktopRuntime } from '@/services/runtime';
|
|||||||
import { getAuthState, subscribeAuthState } from '@/services/auth-state';
|
import { getAuthState, subscribeAuthState } from '@/services/auth-state';
|
||||||
import { hasPremiumAccess } from '@/services/panel-gating';
|
import { hasPremiumAccess } from '@/services/panel-gating';
|
||||||
import { trackGateHit } from '@/services/analytics';
|
import { trackGateHit } from '@/services/analytics';
|
||||||
|
import { premiumFetch } from '@/services/premium-fetch';
|
||||||
|
|
||||||
type TabId = 'chokepoints' | 'shipping' | 'indicators' | 'minerals' | 'stress';
|
type TabId = 'chokepoints' | 'shipping' | 'indicators' | 'minerals' | 'stress';
|
||||||
|
|
||||||
@@ -31,6 +34,10 @@ export class SupplyChainPanel extends Panel {
|
|||||||
private chartMountTimer: ReturnType<typeof setTimeout> | null = null;
|
private chartMountTimer: ReturnType<typeof setTimeout> | null = null;
|
||||||
private bypassUnsubscribe: (() => void) | null = null;
|
private bypassUnsubscribe: (() => void) | null = null;
|
||||||
private bypassGateTracked = false;
|
private bypassGateTracked = false;
|
||||||
|
private onDismissScenario: (() => void) | null = null;
|
||||||
|
private onScenarioActivate: ((scenarioId: string, result: ScenarioResult) => void) | null = null;
|
||||||
|
private activeScenarioState: { scenarioId: string; result: ScenarioResult } | null = null;
|
||||||
|
private scenarioPollController: AbortController | null = null;
|
||||||
|
|
||||||
constructor() {
|
constructor() {
|
||||||
super({ id: 'supply-chain', title: t('panels.supplyChain'), defaultRowSpan: 2, infoTooltip: t('components.supplyChain.infoTooltip') });
|
super({ id: 'supply-chain', title: t('panels.supplyChain'), defaultRowSpan: 2, infoTooltip: t('components.supplyChain.infoTooltip') });
|
||||||
@@ -181,6 +188,16 @@ export class SupplyChainPanel extends Panel {
|
|||||||
this.chartMountTimer = null;
|
this.chartMountTimer = null;
|
||||||
}, 220);
|
}, 220);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Re-insert scenario banner after setContent replaces inner content.
|
||||||
|
if (this.activeScenarioState) {
|
||||||
|
this.showScenarioSummary(this.activeScenarioState.scenarioId, this.activeScenarioState.result);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Attach scenario trigger buttons for expanded chokepoint cards.
|
||||||
|
if (this.activeTab === 'chokepoints' && this.expandedChokepoint) {
|
||||||
|
this.attachScenarioTriggers();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private renderBypassSection(container: HTMLElement, chokepointId: string): void {
|
private renderBypassSection(container: HTMLElement, chokepointId: string): void {
|
||||||
@@ -305,6 +322,20 @@ export class SupplyChainPanel extends Panel {
|
|||||||
? `<div class="sc-bypass-section" data-bypass-cp="${escapeHtml(cp.id)}"><div class="sc-bypass-loading">Loading bypass options\u2026</div></div>`
|
? `<div class="sc-bypass-section" data-bypass-cp="${escapeHtml(cp.id)}"><div class="sc-bypass-loading">Loading bypass options\u2026</div></div>`
|
||||||
: '';
|
: '';
|
||||||
|
|
||||||
|
const scenarioSection = expanded ? (() => {
|
||||||
|
const template = SCENARIO_TEMPLATES.find(tmpl =>
|
||||||
|
tmpl.affectedChokepointIds.includes(cp.id) && tmpl.type !== 'tariff_shock'
|
||||||
|
);
|
||||||
|
if (!template) return '';
|
||||||
|
const isPro = hasPremiumAccess(getAuthState());
|
||||||
|
const btnClass = isPro ? 'sc-scenario-btn' : 'sc-scenario-btn sc-scenario-btn--gated';
|
||||||
|
return `<div class="sc-scenario-trigger" data-scenario-id="${escapeHtml(template.id)}" data-chokepoint-id="${escapeHtml(cp.id)}">
|
||||||
|
<button class="${btnClass}" ${!isPro ? 'data-gated="1"' : ''} aria-label="Simulate ${escapeHtml(template.name)}">
|
||||||
|
Simulate Closure
|
||||||
|
</button>
|
||||||
|
</div>`;
|
||||||
|
})() : '';
|
||||||
|
|
||||||
return `<div class="trade-restriction-card${expanded ? ' expanded' : ''}" data-cp-id="${escapeHtml(cp.name)}" style="cursor:pointer">
|
return `<div class="trade-restriction-card${expanded ? ' expanded' : ''}" data-cp-id="${escapeHtml(cp.name)}" style="cursor:pointer">
|
||||||
<div class="trade-restriction-header">
|
<div class="trade-restriction-header">
|
||||||
<span class="trade-country">${escapeHtml(cp.name)}</span>
|
<span class="trade-country">${escapeHtml(cp.name)}</span>
|
||||||
@@ -345,6 +376,7 @@ export class SupplyChainPanel extends Panel {
|
|||||||
${actionRow}
|
${actionRow}
|
||||||
${chartPlaceholder}
|
${chartPlaceholder}
|
||||||
${bypassSection}
|
${bypassSection}
|
||||||
|
${scenarioSection}
|
||||||
</div>
|
</div>
|
||||||
</div>`;
|
</div>`;
|
||||||
}).join('')}
|
}).join('')}
|
||||||
@@ -591,4 +623,91 @@ export class SupplyChainPanel extends Panel {
|
|||||||
</table>
|
</table>
|
||||||
</div>`;
|
</div>`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ─── Scenario banner ─────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
public showScenarioSummary(scenarioId: string, result: ScenarioResult): void {
|
||||||
|
this.activeScenarioState = { scenarioId, result };
|
||||||
|
this.content.querySelector('.sc-scenario-banner')?.remove();
|
||||||
|
const top5 = result.topImpactCountries.slice(0, 5);
|
||||||
|
const countriesHtml = top5.map(c =>
|
||||||
|
`<span class="sc-scenario-country">${escapeHtml(c.iso2)} <em>${(c.impactPct * 100).toFixed(0)}%</em></span>`
|
||||||
|
).join(' \u00B7 ');
|
||||||
|
const banner = document.createElement('div');
|
||||||
|
banner.className = 'sc-scenario-banner';
|
||||||
|
const scenarioName = SCENARIO_TEMPLATES.find(tmpl => tmpl.id === scenarioId)?.name ?? scenarioId.replace(/-/g, ' ');
|
||||||
|
banner.innerHTML = `<span class="sc-scenario-icon">\u26A0</span><span class="sc-scenario-name">${escapeHtml(scenarioName)}</span><span class="sc-scenario-countries">${countriesHtml}</span><button class="sc-scenario-dismiss" aria-label="Dismiss scenario">\u00D7</button>`;
|
||||||
|
banner.querySelector('.sc-scenario-dismiss')!.addEventListener('click', () => this.onDismissScenario?.());
|
||||||
|
this.content.prepend(banner);
|
||||||
|
}
|
||||||
|
|
||||||
|
public hideScenarioSummary(): void {
|
||||||
|
this.activeScenarioState = null;
|
||||||
|
this.content.querySelector('.sc-scenario-banner')?.remove();
|
||||||
|
this.content.querySelectorAll<HTMLButtonElement>('.sc-scenario-btn').forEach(btn => {
|
||||||
|
btn.disabled = false;
|
||||||
|
btn.textContent = 'Simulate Closure';
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
public setOnDismissScenario(cb: () => void): void {
|
||||||
|
this.onDismissScenario = cb;
|
||||||
|
}
|
||||||
|
|
||||||
|
public setOnScenarioActivate(cb: (scenarioId: string, result: ScenarioResult) => void): void {
|
||||||
|
this.onScenarioActivate = cb;
|
||||||
|
}
|
||||||
|
|
||||||
|
private attachScenarioTriggers(): void {
|
||||||
|
this.content.querySelectorAll<HTMLElement>('.sc-scenario-trigger').forEach(el => {
|
||||||
|
el.querySelector('.sc-scenario-btn')?.addEventListener('click', async (e) => {
|
||||||
|
e.stopPropagation();
|
||||||
|
const btn = el.querySelector<HTMLButtonElement>('.sc-scenario-btn')!;
|
||||||
|
if (btn.dataset.gated === '1') {
|
||||||
|
trackGateHit('scenario-engine');
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
this.scenarioPollController?.abort();
|
||||||
|
this.scenarioPollController = new AbortController();
|
||||||
|
const { signal } = this.scenarioPollController;
|
||||||
|
|
||||||
|
const scenarioId = el.dataset.scenarioId!;
|
||||||
|
btn.disabled = true;
|
||||||
|
btn.textContent = 'Computing\u2026';
|
||||||
|
try {
|
||||||
|
const runResp = await premiumFetch('/api/scenario/v1/run', {
|
||||||
|
method: 'POST',
|
||||||
|
headers: { 'Content-Type': 'application/json' },
|
||||||
|
body: JSON.stringify({ scenarioId }),
|
||||||
|
signal,
|
||||||
|
});
|
||||||
|
if (!runResp.ok) throw new Error('Run failed');
|
||||||
|
const { jobId } = await runResp.json() as { jobId: string };
|
||||||
|
let result: ScenarioResult | null = null;
|
||||||
|
for (let i = 0; i < 30; i++) {
|
||||||
|
if (signal.aborted || !this.content.isConnected) return;
|
||||||
|
if (i > 0) await new Promise(r => setTimeout(r, 2000));
|
||||||
|
const statusResp = await premiumFetch(`/api/scenario/v1/status?jobId=${encodeURIComponent(jobId)}`, { signal });
|
||||||
|
if (!statusResp.ok) throw new Error(`Status poll failed: ${statusResp.status}`);
|
||||||
|
const status = await statusResp.json() as { status: string; result?: ScenarioResult };
|
||||||
|
if (status.status === 'done') {
|
||||||
|
const r = status.result;
|
||||||
|
if (!r || !Array.isArray(r.topImpactCountries)) throw new Error('done without valid result');
|
||||||
|
result = r;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
if (status.status === 'failed') throw new Error('Scenario failed');
|
||||||
|
}
|
||||||
|
if (!result) throw new Error('Timeout');
|
||||||
|
if (signal.aborted || !this.content.isConnected) return;
|
||||||
|
this.onScenarioActivate?.(scenarioId, result);
|
||||||
|
btn.textContent = 'Active';
|
||||||
|
} catch (err) {
|
||||||
|
if (err instanceof Error && err.name === 'AbortError') return;
|
||||||
|
btn.textContent = 'Error \u2014 retry';
|
||||||
|
btn.disabled = false;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -10,3 +10,5 @@ export type {
|
|||||||
ScenarioVisualState,
|
ScenarioVisualState,
|
||||||
ScenarioResult,
|
ScenarioResult,
|
||||||
} from '../../server/worldmonitor/supply-chain/v1/scenario-templates';
|
} from '../../server/worldmonitor/supply-chain/v1/scenario-templates';
|
||||||
|
|
||||||
|
export { SCENARIO_TEMPLATES } from '../../server/worldmonitor/supply-chain/v1/scenario-templates';
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import {
|
|||||||
type GetCountryChokepointIndexResponse,
|
type GetCountryChokepointIndexResponse,
|
||||||
type GetBypassOptionsResponse,
|
type GetBypassOptionsResponse,
|
||||||
type GetCountryCostShockResponse,
|
type GetCountryCostShockResponse,
|
||||||
|
type GetSectorDependencyResponse,
|
||||||
type ShippingIndex,
|
type ShippingIndex,
|
||||||
type ChokepointInfo,
|
type ChokepointInfo,
|
||||||
type CriticalMineral,
|
type CriticalMineral,
|
||||||
@@ -28,6 +29,7 @@ export type {
|
|||||||
GetCountryChokepointIndexResponse,
|
GetCountryChokepointIndexResponse,
|
||||||
GetBypassOptionsResponse,
|
GetBypassOptionsResponse,
|
||||||
GetCountryCostShockResponse,
|
GetCountryCostShockResponse,
|
||||||
|
GetSectorDependencyResponse,
|
||||||
ShippingIndex,
|
ShippingIndex,
|
||||||
ChokepointInfo,
|
ChokepointInfo,
|
||||||
CriticalMineral,
|
CriticalMineral,
|
||||||
@@ -149,3 +151,21 @@ export async function fetchCountryCostShock(
|
|||||||
return empty;
|
return empty;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const emptySectorDependency: GetSectorDependencyResponse = {
|
||||||
|
iso2: '', hs2: '27', hs2Label: '', flags: [],
|
||||||
|
primaryExporterIso2: '', primaryExporterShare: 0,
|
||||||
|
primaryChokepointId: '', primaryChokepointExposure: 0,
|
||||||
|
hasViableBypass: false, fetchedAt: '',
|
||||||
|
};
|
||||||
|
|
||||||
|
export async function fetchSectorDependency(
|
||||||
|
iso2: string,
|
||||||
|
hs2 = '27',
|
||||||
|
): Promise<GetSectorDependencyResponse> {
|
||||||
|
try {
|
||||||
|
return await client.getSectorDependency({ iso2, hs2 });
|
||||||
|
} catch {
|
||||||
|
return { ...emptySectorDependency, iso2, hs2 };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -20,4 +20,6 @@ export const PREMIUM_RPC_PATHS = new Set<string>([
|
|||||||
'/api/sanctions/v1/list-sanctions-pressure',
|
'/api/sanctions/v1/list-sanctions-pressure',
|
||||||
'/api/trade/v1/list-comtrade-flows',
|
'/api/trade/v1/list-comtrade-flows',
|
||||||
'/api/trade/v1/get-tariff-trends',
|
'/api/trade/v1/get-tariff-trends',
|
||||||
|
'/api/scenario/v1/run',
|
||||||
|
'/api/scenario/v1/status',
|
||||||
]);
|
]);
|
||||||
|
|||||||
@@ -340,3 +340,103 @@ describe('Edge Function module isolation', () => {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('Scenario run endpoint (api/scenario/v1/run.ts)', () => {
|
||||||
|
const runPath = join(root, 'api', 'scenario', 'v1', 'run.ts');
|
||||||
|
|
||||||
|
it('exports edge config with runtime: edge', () => {
|
||||||
|
const src = readFileSync(runPath, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
src.includes("runtime: 'edge'") || src.includes('runtime: "edge"'),
|
||||||
|
'run.ts: must export config with runtime: edge',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('has a default export handler function', () => {
|
||||||
|
const src = readFileSync(runPath, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
src.includes('export default') && src.includes('function handler'),
|
||||||
|
'run.ts: must have a default export handler function',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns 405 for non-POST requests', () => {
|
||||||
|
const src = readFileSync(runPath, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
src.includes('405'),
|
||||||
|
'run.ts: must return 405 for non-POST requests',
|
||||||
|
);
|
||||||
|
assert.ok(
|
||||||
|
src.includes("!== 'POST'") || src.includes('!== "POST"'),
|
||||||
|
'run.ts: must check for POST method and reject other methods with 405',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('validates scenarioId is required', () => {
|
||||||
|
const src = readFileSync(runPath, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
src.includes('scenarioId'),
|
||||||
|
'run.ts: must validate scenarioId field',
|
||||||
|
);
|
||||||
|
assert.ok(
|
||||||
|
src.includes('400'),
|
||||||
|
'run.ts: must return 400 for invalid/missing scenarioId',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('uses per-user rate limiting', () => {
|
||||||
|
const src = readFileSync(runPath, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
src.includes('rate') && src.includes('429'),
|
||||||
|
'run.ts: must implement rate limiting with 429 response',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('uses AbortSignal.timeout on Redis fetch', () => {
|
||||||
|
const src = readFileSync(runPath, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
src.includes('AbortSignal.timeout'),
|
||||||
|
'run.ts: all Redis fetches must have AbortSignal.timeout to prevent hanging edge isolates',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('Scenario status endpoint (api/scenario/v1/status.ts)', () => {
|
||||||
|
const statusPath = join(root, 'api', 'scenario', 'v1', 'status.ts');
|
||||||
|
|
||||||
|
it('exports edge config with runtime: edge', () => {
|
||||||
|
const src = readFileSync(statusPath, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
src.includes("runtime: 'edge'") || src.includes('runtime: "edge"'),
|
||||||
|
'status.ts: must export config with runtime: edge',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns 400 for missing or invalid jobId', () => {
|
||||||
|
const src = readFileSync(statusPath, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
src.includes('400'),
|
||||||
|
'status.ts: must return 400 for missing or invalid jobId',
|
||||||
|
);
|
||||||
|
assert.ok(
|
||||||
|
src.includes('jobId'),
|
||||||
|
'status.ts: must validate jobId query parameter',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('validates jobId format to guard against path traversal', () => {
|
||||||
|
const src = readFileSync(statusPath, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
src.includes('JOB_ID_RE') || src.includes('/^scenario:'),
|
||||||
|
'status.ts: must validate jobId against a regex to prevent path traversal attacks (e.g. ../../etc/passwd)',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('uses AbortSignal.timeout on Redis fetch', () => {
|
||||||
|
const src = readFileSync(statusPath, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
src.includes('AbortSignal.timeout'),
|
||||||
|
'status.ts: Redis fetch must have AbortSignal.timeout to prevent hanging edge isolates',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
48
todos/152-complete-p1-t-shadows-i18n-import.md
Normal file
48
todos/152-complete-p1-t-shadows-i18n-import.md
Normal file
@@ -0,0 +1,48 @@
|
|||||||
|
---
|
||||||
|
status: complete
|
||||||
|
priority: p1
|
||||||
|
issue_id: "152"
|
||||||
|
tags: [code-review, quality, supply-chain]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# `t` Parameter Shadows i18n `t` Import in `renderChokepoints`
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
In `SupplyChainPanel.ts:324`, the arrow function parameter `t` inside `SCENARIO_TEMPLATES.find(t => ...)` shadows the module-level `import { t } from '@/services/i18n'` at line 12. Inside the callback, `t` refers to the template object, not the i18n function. Any translated string added inside this callback (or nearby refactor) will silently use the template object as a function, throwing at runtime.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/SupplyChainPanel.ts:324`
|
||||||
|
- **Code:** `const template = SCENARIO_TEMPLATES.find(t => t.affectedChokepointIds.includes(cp.id) && t.type !== 'tariff_shock');`
|
||||||
|
- Not a crash today — `t(...)` is not called inside the callback — but it is a maintenance trap
|
||||||
|
- Biome may not catch this as an error (depends on shadow rules configured)
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Rename the arrow function parameter (Recommended)
|
||||||
|
```ts
|
||||||
|
const template = SCENARIO_TEMPLATES.find(tmpl =>
|
||||||
|
tmpl.affectedChokepointIds.includes(cp.id) && tmpl.type !== 'tariff_shock'
|
||||||
|
);
|
||||||
|
```
|
||||||
|
**Pros:** One-line fix, obvious, eliminates shadow entirely
|
||||||
|
**Cons:** None
|
||||||
|
**Effort:** Small | **Risk:** None
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A immediately — 1-line fix._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/components/SupplyChainPanel.ts`
|
||||||
|
- **Line:** 324
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] Arrow function parameter renamed from `t` to `tmpl` (or similar)
|
||||||
|
- [ ] `npm run lint` passes with no shadow warnings
|
||||||
|
- [ ] `npm run typecheck` passes
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by kieran-typescript-reviewer during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
56
todos/153-complete-p1-polling-no-statusresp-ok-check.md
Normal file
56
todos/153-complete-p1-polling-no-statusresp-ok-check.md
Normal file
@@ -0,0 +1,56 @@
|
|||||||
|
---
|
||||||
|
status: complete
|
||||||
|
priority: p1
|
||||||
|
issue_id: "153"
|
||||||
|
tags: [code-review, quality, supply-chain, reliability]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# Missing `statusResp.ok` Guard in Scenario Polling Loop
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
In `SupplyChainPanel.ts:677`, the polling loop calls `statusResp.json()` without first checking `statusResp.ok`. A 429, 500, or network error causes `.json()` to throw or produce garbage, silently burning through all 30 iterations. The button ends up stuck on "Computing…" for 60 seconds before showing "Error — retry" with no useful signal.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/SupplyChainPanel.ts`, line 677
|
||||||
|
- **Code:**
|
||||||
|
```ts
|
||||||
|
const statusResp = await fetch(`/api/scenario/v1/status?jobId=${encodeURIComponent(jobId)}`);
|
||||||
|
const status = await statusResp.json() as { status: string; result?: ScenarioResult };
|
||||||
|
```
|
||||||
|
- No check on `statusResp.ok` before consuming body
|
||||||
|
- 30 × 2s = 60s of silent failure with no feedback to the user
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Guard before `.json()` (Recommended)
|
||||||
|
```ts
|
||||||
|
const statusResp = await fetch(`/api/scenario/v1/status?jobId=${encodeURIComponent(jobId)}`);
|
||||||
|
if (!statusResp.ok) throw new Error(`Status poll failed: ${statusResp.status}`);
|
||||||
|
const status = await statusResp.json() as { status: string; result?: ScenarioResult };
|
||||||
|
```
|
||||||
|
**Pros:** Fails fast on server error, exits loop immediately
|
||||||
|
**Cons:** None
|
||||||
|
**Effort:** Small | **Risk:** None
|
||||||
|
|
||||||
|
### Option B: Retry on non-fatal errors, throw on fatal
|
||||||
|
Skip 429/503, throw on 4xx. More complex, adds ~10 lines, probably not worth it for this use case.
|
||||||
|
**Effort:** Medium | **Risk:** Low
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A — guard before `.json()`. One line._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/components/SupplyChainPanel.ts`
|
||||||
|
- **Line:** 677
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] `statusResp.ok` checked before calling `.json()`
|
||||||
|
- [ ] Loop exits immediately (throws) on non-OK response
|
||||||
|
- [ ] Button shows "Error — retry" within one polling cycle on server error
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by kieran-typescript-reviewer during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
@@ -0,0 +1,64 @@
|
|||||||
|
---
|
||||||
|
status: complete
|
||||||
|
priority: p1
|
||||||
|
issue_id: "154"
|
||||||
|
tags: [code-review, quality, supply-chain, type-safety]
|
||||||
|
dependencies: [153]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Non-Null Assertion on Optional `result` Without Guard in Polling Loop
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
In `SupplyChainPanel.ts:679`, `result = status.result!` uses a non-null assertion on a field typed as optional. If the server sends `{ status: 'done' }` without `result` (version mismatch or worker bug), `undefined` is silently passed to `onScenarioActivate`, which crashes in `activateScenario` when it calls `result.topImpactCountries.map(...)`. Additionally, `impactPct * 100` in `showScenarioSummary` could produce `NaN` if the upstream type changes (prior art: `feedback_pizzint_spike_magnitude_type.md`).
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/SupplyChainPanel.ts`, line 679
|
||||||
|
- **Code:** `if (status.status === 'done') { result = status.result!; break; }`
|
||||||
|
- `status.result` is typed as optional (`result?: ScenarioResult`)
|
||||||
|
- If `status.result` is absent, `undefined` propagates to `activateScenario` → crash in `topImpactCountries.map(...)`
|
||||||
|
- `impactPct` is numeric but upstream could change to string (see MEMORY: `feedback_pizzint_spike_magnitude_type.md`)
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Explicit guard + type narrowing (Recommended)
|
||||||
|
```ts
|
||||||
|
if (status.status === 'done') {
|
||||||
|
const r = status.result;
|
||||||
|
if (!r || !Array.isArray(r.topImpactCountries)) throw new Error('done without valid result');
|
||||||
|
result = r;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
**Pros:** Explicit, catches server bugs, type-safe without `!`
|
||||||
|
**Cons:** Slight verbosity
|
||||||
|
**Effort:** Small | **Risk:** None
|
||||||
|
|
||||||
|
### Option B: Minimal guard only
|
||||||
|
```ts
|
||||||
|
if (status.status === 'done') {
|
||||||
|
if (!status.result) throw new Error('done without result');
|
||||||
|
result = status.result;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
Protects against `undefined` but not malformed `topImpactCountries`.
|
||||||
|
**Effort:** Small | **Risk:** Low
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A — adds one guard line, eliminates the `!` assertion._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/components/SupplyChainPanel.ts`
|
||||||
|
- **Lines:** 679
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] Non-null assertion `!` removed from `status.result`
|
||||||
|
- [ ] `topImpactCountries` presence verified before use
|
||||||
|
- [ ] `npm run typecheck` passes without `!` assertion
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by kieran-typescript-reviewer during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
|
- MEMORY: `feedback_pizzint_spike_magnitude_type.md`
|
||||||
@@ -0,0 +1,61 @@
|
|||||||
|
---
|
||||||
|
status: complete
|
||||||
|
priority: p1
|
||||||
|
issue_id: "155"
|
||||||
|
tags: [code-review, architecture, supply-chain, correctness]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# Scenario Callbacks Never Wired in `panel-layout.ts` — Feature Dead on Arrival
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
`SupplyChainPanel.setOnScenarioActivate()` and `setOnDismissScenario()` are declared and implemented but never called anywhere. When the user clicks "Simulate Closure" and the poll completes, `this.onScenarioActivate?.(...)` fires — but the callback is null, so no map overlays are applied. The dismiss button also does nothing. The entire scenario visual engine is non-functional in production.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/app/panel-layout.ts` — missing wiring
|
||||||
|
- `setOnScenarioActivate` and `setOnDismissScenario` public methods exist on `SupplyChainPanel` but are never called
|
||||||
|
- `MapContainer.setSupplyChainPanel()` also exists but is never called
|
||||||
|
- The feature compiles and type-checks but is a silent no-op at runtime
|
||||||
|
- Confirmed by architecture reviewer: "wiring is declared but not connected"
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Wire callbacks in `panel-layout.ts` (Recommended)
|
||||||
|
In `src/app/panel-layout.ts`, after `SupplyChainPanel` is created:
|
||||||
|
```ts
|
||||||
|
supplyChainPanel.setOnScenarioActivate((id, result) => {
|
||||||
|
this.ctx.map?.activateScenario(id, result);
|
||||||
|
});
|
||||||
|
supplyChainPanel.setOnDismissScenario(() => {
|
||||||
|
this.ctx.map?.deactivateScenario();
|
||||||
|
});
|
||||||
|
```
|
||||||
|
This follows the existing pattern for all other panel callbacks (e.g. `MonitorPanel.onChanged`).
|
||||||
|
**Pros:** Correct architecture, no circular reference, follows established pattern
|
||||||
|
**Cons:** Requires checking how `this.ctx.map` is typed/available in panel-layout.ts
|
||||||
|
**Effort:** Small | **Risk:** Low
|
||||||
|
|
||||||
|
### Option B: Keep `setSupplyChainPanel` on MapContainer, call it from panel-layout.ts
|
||||||
|
Also call `mapContainer.setSupplyChainPanel(supplyChainPanel)` in panel-layout.ts. Keeps the current back-reference architecture but at least makes it functional.
|
||||||
|
**Pros:** Minimal change to the current design
|
||||||
|
**Cons:** Preserves the bidirectional coupling (see todo #161)
|
||||||
|
**Effort:** Small | **Risk:** Low
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A — wire callbacks in `panel-layout.ts`. This is the minimum fix to make the feature functional and also the architecturally correct approach (see #161 for the full coupling cleanup)._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/app/panel-layout.ts` (wiring), `src/components/SupplyChainPanel.ts` (callbacks declared)
|
||||||
|
- **Lines in panel-layout.ts:** find where `SupplyChainPanel` is constructed (grep: `new SupplyChainPanel`)
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] `setOnScenarioActivate` wired in `panel-layout.ts` → calls `mapContainer.activateScenario`
|
||||||
|
- [ ] `setOnDismissScenario` wired in `panel-layout.ts` → calls `mapContainer.deactivateScenario`
|
||||||
|
- [ ] Clicking "Simulate Closure" (PRO, done scenario) applies arc recolor + heat layer on DeckGL
|
||||||
|
- [ ] Dismiss × on banner restores normal visual state
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by architecture-strategist during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
@@ -0,0 +1,56 @@
|
|||||||
|
---
|
||||||
|
status: complete
|
||||||
|
priority: p2
|
||||||
|
issue_id: "156"
|
||||||
|
tags: [code-review, quality, supply-chain, visual]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# GlobeMap `polygonStrokeColor` Missing `scenario` Branch — Falls Through to Red
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
In `GlobeMap.ts`, `polygonStrokeColor` has no `'scenario'` case. When scenario polygons are rendered, the callback falls through to the catch-all `return '#ff4444'` (conflict red), giving affected countries a visible red border that clashes with the intended orange/amber heat tint and visually conflates scenario state with conflict data.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/GlobeMap.ts`
|
||||||
|
- `polygonCapColor` and `polygonSideColor` both have `'scenario'` branches added in Sprint E
|
||||||
|
- `polygonStrokeColor` was missed — falls through to `'#ff4444'` (conflict) or `'transparent'`
|
||||||
|
- At runtime: scenario-affected countries appear with a red border matching conflict polygons
|
||||||
|
- Identified during architecture review of PR #2910
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Add `scenario` branch returning transparent (Recommended)
|
||||||
|
```ts
|
||||||
|
// In polygonStrokeColor callback:
|
||||||
|
if (d._kind === 'scenario') return 'transparent';
|
||||||
|
```
|
||||||
|
Scenario heat overlay has no border — consistent with how the DeckGL `GeoJsonLayer` has `stroked: false`.
|
||||||
|
**Pros:** Zero visual noise, matches DeckGL behavior, 1-line fix
|
||||||
|
**Cons:** None
|
||||||
|
**Effort:** Small | **Risk:** None
|
||||||
|
|
||||||
|
### Option B: Use a subtle amber stroke
|
||||||
|
```ts
|
||||||
|
if (d._kind === 'scenario') return 'rgba(220,120,40,0.4)';
|
||||||
|
```
|
||||||
|
Adds a faint amber outline to distinguish from other polygon types.
|
||||||
|
**Effort:** Small | **Risk:** Low (visual choice)
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A — transparent stroke for scenario polygons, matching DeckGL's `stroked: false`._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/components/GlobeMap.ts`
|
||||||
|
- **Grep:** `polygonStrokeColor` — find the callback and add the missing branch before the `conflict` case
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] `polygonStrokeColor` returns `'transparent'` (or no-stroke equivalent) when `d._kind === 'scenario'`
|
||||||
|
- [ ] No red border visible on Globe scenario overlay
|
||||||
|
- [ ] `npm run typecheck` passes
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified during /ce-review of PR #2910 (GlobeMap polygon rendering)
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
@@ -0,0 +1,60 @@
|
|||||||
|
---
|
||||||
|
status: complete
|
||||||
|
priority: p2
|
||||||
|
issue_id: "157"
|
||||||
|
tags: [code-review, architecture, supply-chain, import-boundaries]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# `SCENARIO_TEMPLATES` Imported Directly from `../../server/` — Module Boundary Violation
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
`SupplyChainPanel.ts` imports `SCENARIO_TEMPLATES` via a relative path crossing into `server/`:
|
||||||
|
```ts
|
||||||
|
import { SCENARIO_TEMPLATES } from '../../server/worldmonitor/supply-chain/v1/scenario-templates';
|
||||||
|
```
|
||||||
|
Client components must not import from `server/` directly — the `src/` → `server/` boundary keeps server-only deps (gRPC stubs, proto codegen) out of the browser bundle. This import will silently work today but will break if any server-only module is added upstream of `scenario-templates.ts`.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/SupplyChainPanel.ts`
|
||||||
|
- Direct relative import crosses `src/` → `server/` module boundary
|
||||||
|
- `src/config/scenario-templates.ts` already re-exports the `ScenarioResult` and `ScenarioVisualState` types from this file
|
||||||
|
- The `SCENARIO_TEMPLATES` const is not yet re-exported from `src/config/scenario-templates.ts`
|
||||||
|
- Pattern in codebase: all other panel files import from `@/config/`, `@/services/`, or `@/components/`
|
||||||
|
- Identified by architecture-strategist during PR #2910 review
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Re-export `SCENARIO_TEMPLATES` via `src/config/scenario-templates.ts` (Recommended)
|
||||||
|
In `src/config/scenario-templates.ts`, add:
|
||||||
|
```ts
|
||||||
|
export { SCENARIO_TEMPLATES } from '../../server/worldmonitor/supply-chain/v1/scenario-templates';
|
||||||
|
```
|
||||||
|
Then update `SupplyChainPanel.ts`:
|
||||||
|
```ts
|
||||||
|
import { SCENARIO_TEMPLATES } from '@/config/scenario-templates';
|
||||||
|
```
|
||||||
|
**Pros:** One canonical boundary crossing point, consistent with existing type re-exports, no runtime impact
|
||||||
|
**Cons:** Needs one extra line in config file
|
||||||
|
**Effort:** Small | **Risk:** None
|
||||||
|
|
||||||
|
### Option B: Add a `src/` copy or adapter
|
||||||
|
Duplicate the const in `src/config/`. More files, divergence risk.
|
||||||
|
**Effort:** Medium | **Risk:** Medium (divergence)
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A — add one re-export to `src/config/scenario-templates.ts` and update the import in `SupplyChainPanel.ts`._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/config/scenario-templates.ts` (add re-export), `src/components/SupplyChainPanel.ts` (update import)
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] `src/components/SupplyChainPanel.ts` no longer imports from `../../server/`
|
||||||
|
- [ ] `SCENARIO_TEMPLATES` accessible via `@/config/scenario-templates`
|
||||||
|
- [ ] `npm run typecheck` passes
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by architecture-strategist during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
@@ -0,0 +1,73 @@
|
|||||||
|
---
|
||||||
|
status: complete
|
||||||
|
priority: p2
|
||||||
|
issue_id: "158"
|
||||||
|
tags: [code-review, performance, supply-chain, deckgl]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# `createScenarioHeatLayer` Allocates New `Set` on Every `buildLayers()` Call
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
`DeckGLMap.createScenarioHeatLayer()` constructs `new Set(this.scenarioState.affectedIso2s)` inside the method body. `buildLayers()` is called on every frame/render cycle by DeckGL when layers need to be rebuilt. The `Set` allocation is O(n) on the number of affected ISO-2 codes and runs inside the hot DeckGL render path.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/DeckGLMap.ts`
|
||||||
|
- **Code:**
|
||||||
|
```ts
|
||||||
|
private createScenarioHeatLayer(): GeoJsonLayer | null {
|
||||||
|
if (!this.scenarioState?.affectedIso2s?.length || !this.countriesGeoJsonData) return null;
|
||||||
|
const affected = new Set(this.scenarioState.affectedIso2s); // ← allocated every call
|
||||||
|
return new GeoJsonLayer({ ..., getFillColor: (feature) => { const code = ...; return affected.has(code) ? ... } });
|
||||||
|
}
|
||||||
|
```
|
||||||
|
- `buildLayers()` is called whenever deck viewport or layers change — potentially dozens of times per second during pan/zoom
|
||||||
|
- The `Set` contents only change when `setScenarioState()` is called (rare)
|
||||||
|
- Identified by performance-oracle during PR #2910 review
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Cache the Set in `setScenarioState()` (Recommended)
|
||||||
|
```ts
|
||||||
|
private affectedIso2Set: Set<string> = new Set();
|
||||||
|
|
||||||
|
public setScenarioState(state: ScenarioVisualState | null): void {
|
||||||
|
this.scenarioState = state;
|
||||||
|
this.affectedIso2Set = new Set(state?.affectedIso2s ?? []);
|
||||||
|
this.rebuildLayers();
|
||||||
|
}
|
||||||
|
|
||||||
|
private createScenarioHeatLayer(): GeoJsonLayer | null {
|
||||||
|
if (!this.affectedIso2Set.size || !this.countriesGeoJsonData) return null;
|
||||||
|
return new GeoJsonLayer({ ..., getFillColor: (feature) => {
|
||||||
|
const code = feature.properties?.['ISO3166-1-Alpha-2'] as string | undefined;
|
||||||
|
return (code && this.affectedIso2Set.has(code) ? [220, 60, 40, 80] : [0, 0, 0, 0]) as [number,number,number,number];
|
||||||
|
}});
|
||||||
|
}
|
||||||
|
```
|
||||||
|
**Pros:** Set allocated once per state change (not per render), correct `updateTriggers` still invalidates DeckGL cache
|
||||||
|
**Cons:** Small memory overhead for the cached Set field
|
||||||
|
**Effort:** Small | **Risk:** Low
|
||||||
|
|
||||||
|
### Option B: Keep as-is with a comment
|
||||||
|
Acceptable if `buildLayers()` is only called on state change. But DeckGL calls it more often.
|
||||||
|
**Effort:** None | **Risk:** High (performance regression on active globe interactions)
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A — cache the Set in `setScenarioState()`._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/components/DeckGLMap.ts`
|
||||||
|
- Add private `affectedIso2Set: Set<string> = new Set()` field
|
||||||
|
- Move Set construction to `setScenarioState()`
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] `createScenarioHeatLayer` does not allocate a new `Set` on each call
|
||||||
|
- [ ] `setScenarioState()` rebuilds the cached Set
|
||||||
|
- [ ] `npm run typecheck` passes
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by performance-oracle during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
@@ -0,0 +1,73 @@
|
|||||||
|
---
|
||||||
|
status: pending
|
||||||
|
priority: p2
|
||||||
|
issue_id: "159"
|
||||||
|
tags: [code-review, performance, supply-chain, globe]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# `flushPolygons` Iterates All 250 GeoJSON Features on Every Flush — Pre-compute Scenario Polygons
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
`GlobeMap.flushPolygons()` iterates over `countriesGeoJsonData.features` to build scenario `GlobePolygon` objects on every call. `flushPolygons()` is called whenever ANY polygon layer changes (CII update, conflict update, imagery toggle, storm cone render). With ~250 GeoJSON features, this is ~250 object allocations + ISO-2 Set lookups on every flush, even when the scenario state hasn't changed.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/GlobeMap.ts`
|
||||||
|
- `flushPolygons()` constructs scenario polygons inline from `this.scenarioState.affectedIso2s` + `countriesGeoJsonData` on every call
|
||||||
|
- Every other polygon layer (CII, conflict, imagery) pre-builds its polygon array before `flushPolygons()` and simply concatenates
|
||||||
|
- `setScenarioState()` calls `flushPolygons()` but does not cache the pre-built polygon array
|
||||||
|
- Identified by performance-oracle during PR #2910 review
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Pre-compute scenario polygons in `setScenarioState()` (Recommended)
|
||||||
|
```ts
|
||||||
|
private scenarioPolygons: GlobePolygon[] = [];
|
||||||
|
|
||||||
|
public setScenarioState(state: ScenarioVisualState | null): void {
|
||||||
|
this.scenarioState = state;
|
||||||
|
if (!state?.affectedIso2s?.length || !this.countriesGeoJsonData) {
|
||||||
|
this.scenarioPolygons = [];
|
||||||
|
} else {
|
||||||
|
const affected = new Set(state.affectedIso2s);
|
||||||
|
this.scenarioPolygons = this.countriesGeoJsonData.features
|
||||||
|
.filter(f => affected.has(f.properties?.['ISO3166-1-Alpha-2'] as string))
|
||||||
|
.map(f => ({ ...buildGlobePolygon(f), _kind: 'scenario' as const }));
|
||||||
|
}
|
||||||
|
this.flushPolygons();
|
||||||
|
}
|
||||||
|
```
|
||||||
|
Then in `flushPolygons()`:
|
||||||
|
```ts
|
||||||
|
const polys = [...this.ciiPolygons, ...this.conflictPolygons, ...this.scenarioPolygons, ...];
|
||||||
|
(this.globe as any).polygonsData(polys);
|
||||||
|
```
|
||||||
|
**Pros:** O(1) in `flushPolygons()` for scenario layer, consistent with existing pattern for other polygon types
|
||||||
|
**Cons:** Slightly larger memory footprint (cached array)
|
||||||
|
**Effort:** Small | **Risk:** Low
|
||||||
|
|
||||||
|
### Option B: Keep inline, add early-exit guard
|
||||||
|
```ts
|
||||||
|
if (!this.scenarioState?.affectedIso2s?.length) { /* skip scenario loop */ }
|
||||||
|
```
|
||||||
|
Reduces cost to near-zero when no scenario active, but still O(n) when active.
|
||||||
|
**Effort:** Trivial | **Risk:** None (acceptable trade-off)
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A if the inline loop is confirmed to run frequently (profile first). Option B is an acceptable minimal fix if profiling shows low impact._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/components/GlobeMap.ts`
|
||||||
|
- Add `private scenarioPolygons: GlobePolygon[] = []`
|
||||||
|
- Move construction into `setScenarioState()`
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] `flushPolygons()` does not iterate GeoJSON features when scenario state is unchanged
|
||||||
|
- [ ] `setScenarioState()` pre-builds `scenarioPolygons`
|
||||||
|
- [ ] `npm run typecheck` passes
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by performance-oracle during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
64
todos/160-complete-p2-scenario-fetch-missing-auth-header.md
Normal file
64
todos/160-complete-p2-scenario-fetch-missing-auth-header.md
Normal file
@@ -0,0 +1,64 @@
|
|||||||
|
---
|
||||||
|
status: complete
|
||||||
|
priority: p2
|
||||||
|
issue_id: "160"
|
||||||
|
tags: [code-review, security, supply-chain, auth]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# Scenario Fetch Calls in `attachScenarioTriggers` Missing Auth Header
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
The `fetch` calls in `SupplyChainPanel.attachScenarioTriggers()` — both the initial `POST /api/scenario/v1/run` and the polling `GET /api/scenario/v1/status` — send no authentication header. The API endpoints use `validateApiKey()` which, for browser origins (trusted), silently returns `required: false` and skips key validation. This means PRO-gated endpoints are reachable unauthenticated from any browser tab that happens to have the right origin header.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/SupplyChainPanel.ts` — `attachScenarioTriggers()` polling loop
|
||||||
|
- No `X-WorldMonitor-Key` or `Authorization` header on either fetch call
|
||||||
|
- `validateApiKey(req)` with no `{ forceKey: true }` returns `{ required: false }` for browser origins (see MEMORY: `feedback_validateapikey_forcekey_vendor.md`)
|
||||||
|
- The PRO gate is only checked client-side in `MapContainer.activateScenario()` — bypassing it server-side is trivial via curl with `Origin: https://worldmonitor.app`
|
||||||
|
- Identified by security-sentinel during PR #2910 review
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Add auth header using `getApiKey()` (Recommended)
|
||||||
|
```ts
|
||||||
|
import { getApiKey } from '@/services/auth-state';
|
||||||
|
|
||||||
|
// In attachScenarioTriggers():
|
||||||
|
const headers: Record<string, string> = { 'Content-Type': 'application/json' };
|
||||||
|
const apiKey = getApiKey();
|
||||||
|
if (apiKey) headers['X-WorldMonitor-Key'] = apiKey;
|
||||||
|
|
||||||
|
const runResp = await fetch('/api/scenario/v1/run', { method: 'POST', headers, body: JSON.stringify(payload) });
|
||||||
|
// ...
|
||||||
|
const statusResp = await fetch(`/api/scenario/v1/status?jobId=...`, { headers });
|
||||||
|
```
|
||||||
|
**Pros:** Consistent with other authenticated panel fetches, enables server-side PRO gating
|
||||||
|
**Cons:** None
|
||||||
|
**Effort:** Small | **Risk:** None
|
||||||
|
|
||||||
|
### Option B: Add `forceKey: true` on the API endpoints + client auth header
|
||||||
|
Also update the API endpoints to use `validateApiKey(req, { forceKey: true })` to reject unauthenticated browser requests.
|
||||||
|
**Pros:** Defense in depth — server-side gate enforced regardless of client behavior
|
||||||
|
**Cons:** Requires API changes too
|
||||||
|
**Effort:** Small | **Risk:** Low
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A immediately (client auth header). Option B should follow as a separate API hardening change._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/components/SupplyChainPanel.ts`
|
||||||
|
- `api/scenario/v1/run.ts` and `api/scenario/v1/status.ts` — consider `forceKey: true` follow-up
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] Both fetch calls in `attachScenarioTriggers` include `X-WorldMonitor-Key` header when available
|
||||||
|
- [ ] No regression for users without an API key (header omitted gracefully)
|
||||||
|
- [ ] `npm run typecheck` passes
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by security-sentinel during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
|
- MEMORY: `feedback_validateapikey_forcekey_vendor.md`
|
||||||
|
- MEMORY: `feedback_is_caller_premium_trusted_origin.md`
|
||||||
@@ -0,0 +1,59 @@
|
|||||||
|
---
|
||||||
|
status: pending
|
||||||
|
priority: p2
|
||||||
|
issue_id: "161"
|
||||||
|
tags: [code-review, architecture, supply-chain, coupling]
|
||||||
|
dependencies: [155]
|
||||||
|
---
|
||||||
|
|
||||||
|
# MapContainer ↔ SupplyChainPanel Bidirectional Coupling via `setSupplyChainPanel`
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
`MapContainer` has a `setSupplyChainPanel(panel)` setter that stores a direct reference to `SupplyChainPanel`. This creates a bidirectional dependency: `SupplyChainPanel` calls into `MapContainer` via callbacks (correct direction), and `MapContainer` calls back into `SupplyChainPanel` via `showScenarioSummary` / `hideScenarioSummary` (reverse direction). This circular architecture makes both components harder to test and violates the unidirectional data flow established by every other panel in the system.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/MapContainer.ts` — `setSupplyChainPanel()` and `deactivateScenario()` calling `this.supplyChainPanel?.hideScenarioSummary()`
|
||||||
|
- **File:** `src/components/SupplyChainPanel.ts` — `setOnDismissScenario` / `setOnScenarioActivate` callbacks point to `MapContainer`
|
||||||
|
- All other panels (MonitorPanel, CountryDeepDivePanel, etc.) use one-way callbacks: panel → map, never map → panel
|
||||||
|
- The bidirectional link was noted in the Sprint E plan as deferred to this issue
|
||||||
|
- Identified by architecture-strategist during PR #2910 review (see todo #155 for the more urgent wiring issue)
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Remove `showScenarioSummary`/`hideScenarioSummary` from `MapContainer` dispatch (Recommended)
|
||||||
|
Instead of `MapContainer` calling `panel.showScenarioSummary()` and `panel.hideScenarioSummary()`:
|
||||||
|
- Have `SupplyChainPanel` observe scenario state changes through its existing `onScenarioActivate` and `onDismissScenario` callbacks
|
||||||
|
- The panel already receives `(id, result)` in `onScenarioActivate` — it can call `showScenarioSummary` on itself
|
||||||
|
- `MapContainer.activateScenario` fires `onScenarioActivate` → panel handles its own UI update
|
||||||
|
- `MapContainer.deactivateScenario` fires `onDismissScenario` → panel handles its own dismiss
|
||||||
|
- Remove `setSupplyChainPanel` from `MapContainer` entirely
|
||||||
|
|
||||||
|
**Pros:** Breaks the circular reference, consistent with all other panels
|
||||||
|
**Cons:** Requires refactoring how the panel receives activation events (it already does via callbacks)
|
||||||
|
**Effort:** Medium | **Risk:** Low
|
||||||
|
|
||||||
|
### Option B: Keep `setSupplyChainPanel` but document the coupling explicitly
|
||||||
|
Add a JSDoc comment acknowledging the bidirectional dependency and track via this todo.
|
||||||
|
**Pros:** No refactor needed now
|
||||||
|
**Cons:** Coupling persists indefinitely, harder to unit-test
|
||||||
|
**Effort:** Trivial | **Risk:** None (deferred tech debt)
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A in a follow-up PR after P1 issues (#155) are fixed. The coupling is architectural debt but not a blocking bug._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/components/MapContainer.ts`, `src/components/SupplyChainPanel.ts`
|
||||||
|
- Dependency: Fix todo #155 first (wiring), then restructure the callback flow here
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] `MapContainer` has no direct reference to `SupplyChainPanel`
|
||||||
|
- [ ] `setSupplyChainPanel()` removed from `MapContainer`
|
||||||
|
- [ ] Panel banner updates triggered via existing `onScenarioActivate` / `onDismissScenario` callbacks
|
||||||
|
- [ ] `npm run typecheck` passes
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by architecture-strategist during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
|
- Related: todo #155 (callbacks not wired — fix first)
|
||||||
@@ -0,0 +1,71 @@
|
|||||||
|
---
|
||||||
|
status: complete
|
||||||
|
priority: p2
|
||||||
|
issue_id: "162"
|
||||||
|
tags: [code-review, quality, supply-chain, reliability]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# `attachScenarioTriggers` Has No In-Flight Guard — Concurrent Polling Possible
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
`SupplyChainPanel.attachScenarioTriggers()` launches a polling loop but has no guard against being called while a previous poll is still running. If the user clicks "Simulate Closure" a second time before the first job completes (e.g., after navigating away and back to the chokepoint), two polling loops run concurrently. Both can call `onScenarioActivate`, resulting in a race between two job results applying visual state in an undefined order.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/SupplyChainPanel.ts`
|
||||||
|
- `attachScenarioTriggers()` is called from the "Simulate Closure" button click handler
|
||||||
|
- No `isPolling` flag or AbortController checked before starting a new poll
|
||||||
|
- Existing guard: `if (!button.isConnected) break` only exits on DOM removal, not on second click
|
||||||
|
- Two concurrent polls can overlap and call `onScenarioActivate` with different results
|
||||||
|
- Identified by kieran-typescript-reviewer during PR #2910 review
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: AbortController + in-flight flag (Recommended)
|
||||||
|
```ts
|
||||||
|
private scenarioPollController: AbortController | null = null;
|
||||||
|
|
||||||
|
private async attachScenarioTriggers(button: HTMLButtonElement, cp: Chokepoint): Promise<void> {
|
||||||
|
// Cancel any in-flight poll
|
||||||
|
this.scenarioPollController?.abort();
|
||||||
|
this.scenarioPollController = new AbortController();
|
||||||
|
const { signal } = this.scenarioPollController;
|
||||||
|
|
||||||
|
// ... in the polling loop:
|
||||||
|
if (signal.aborted || !button.isConnected) break;
|
||||||
|
const statusResp = await fetch(`...`, { signal });
|
||||||
|
}
|
||||||
|
```
|
||||||
|
**Pros:** Clean cancellation, prevents concurrent polls, mirrors standard fetch abort patterns
|
||||||
|
**Cons:** Need to handle `AbortError` gracefully (not show error banner)
|
||||||
|
**Effort:** Small | **Risk:** Low
|
||||||
|
|
||||||
|
### Option B: Simple boolean in-flight flag
|
||||||
|
```ts
|
||||||
|
private isScenarioPolling = false;
|
||||||
|
|
||||||
|
if (this.isScenarioPolling) return;
|
||||||
|
this.isScenarioPolling = true;
|
||||||
|
try { /* poll */ } finally { this.isScenarioPolling = false; }
|
||||||
|
```
|
||||||
|
Blocks second trigger rather than cancelling first. Simpler but less responsive (user can't restart a stalled poll).
|
||||||
|
**Effort:** Small | **Risk:** None
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A — AbortController for clean cancellation, matching the codebase's existing fetch patterns._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/components/SupplyChainPanel.ts`
|
||||||
|
- Add `private scenarioPollController: AbortController | null = null`
|
||||||
|
- Handle `AbortError` in the catch block (do not show error banner on abort)
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] Second click aborts previous polling loop
|
||||||
|
- [ ] `AbortError` not surfaced to user as "Error — retry"
|
||||||
|
- [ ] `npm run typecheck` passes
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by kieran-typescript-reviewer during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
45
todos/163-complete-p3-scenario-banner-shows-raw-id.md
Normal file
45
todos/163-complete-p3-scenario-banner-shows-raw-id.md
Normal file
@@ -0,0 +1,45 @@
|
|||||||
|
---
|
||||||
|
status: complete
|
||||||
|
priority: p3
|
||||||
|
issue_id: "163"
|
||||||
|
tags: [code-review, quality, supply-chain, ux]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# Scenario Banner Shows Raw Template ID Instead of Human-Readable Name
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
The scenario activation banner in `SupplyChainPanel.showScenarioSummary()` displays the raw `scenarioId` string (e.g., `"suez-full-closure-2026"`) rather than the template's `.name` property (e.g., `"Suez Canal Full Closure"`). Users see a machine identifier instead of a localized, readable label.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/components/SupplyChainPanel.ts`
|
||||||
|
- Banner HTML likely contains `${scenarioId}` where it should use `SCENARIO_TEMPLATES.find(tmpl => tmpl.id === scenarioId)?.name ?? scenarioId`
|
||||||
|
- `SCENARIO_TEMPLATES` is already imported in the same file
|
||||||
|
- Minor UX issue — doesn't block functionality
|
||||||
|
- Identified during code review of PR #2910
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Look up template name with fallback
|
||||||
|
```ts
|
||||||
|
const templateName = SCENARIO_TEMPLATES.find(tmpl => tmpl.id === scenarioId)?.name ?? scenarioId;
|
||||||
|
// Use templateName in banner heading
|
||||||
|
```
|
||||||
|
**Pros:** Shows human-readable label, graceful fallback to ID if template not found
|
||||||
|
**Effort:** Small | **Risk:** None
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A — 1-line lookup._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/components/SupplyChainPanel.ts` — `showScenarioSummary()`
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] Banner heading shows template `.name`, not raw `scenarioId`
|
||||||
|
- [ ] Falls back to raw ID if template not found
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
@@ -0,0 +1,59 @@
|
|||||||
|
---
|
||||||
|
status: pending
|
||||||
|
priority: p3
|
||||||
|
issue_id: "164"
|
||||||
|
tags: [code-review, quality, supply-chain, reliability]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# `fetchSectorDependency` Has No Circuit Breaker — Retries Indefinitely on Persistent Failure
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
`src/services/supply-chain/index.ts:fetchSectorDependency()` catches all errors and returns `emptySectorDependency`. While this prevents crashes, it means every call during a persistent outage (e.g., server restart, network partition) makes a live gRPC attempt before falling back. If the supply-chain panel calls this per-chokepoint on every render, a 30-chokepoint list during an outage = 30 sequential timeouts per render cycle.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `src/services/supply-chain/index.ts`
|
||||||
|
- **Code:**
|
||||||
|
```ts
|
||||||
|
export async function fetchSectorDependency(iso2, hs2 = '27') {
|
||||||
|
try {
|
||||||
|
return await client.getSectorDependency({ iso2, hs2 });
|
||||||
|
} catch {
|
||||||
|
return { ...emptySectorDependency, iso2, hs2 };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
- No timeout, no cached failure state, no back-off
|
||||||
|
- Minor issue today (called rarely), but will matter at scale
|
||||||
|
- Identified by kieran-typescript-reviewer during PR #2910 review
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Add a short timeout to the gRPC call
|
||||||
|
```ts
|
||||||
|
const ac = new AbortController();
|
||||||
|
setTimeout(() => ac.abort(), 3000);
|
||||||
|
return await client.getSectorDependency({ iso2, hs2 }, { signal: ac.signal });
|
||||||
|
```
|
||||||
|
Fails fast, reduces hung call duration.
|
||||||
|
**Effort:** Small | **Risk:** Low
|
||||||
|
|
||||||
|
### Option B: Deduplicate in-flight requests with a request Map
|
||||||
|
Cache the promise keyed by `${iso2}:${hs2}` — deduplicates concurrent calls for the same country.
|
||||||
|
**Effort:** Small | **Risk:** Low
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Combine A + B: timeout + in-flight dedup. Low-effort, prevents worst-case pile-up._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `src/services/supply-chain/index.ts`
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] `fetchSectorDependency` times out within ~3s rather than hanging indefinitely
|
||||||
|
- [ ] Concurrent calls for the same `(iso2, hs2)` share one in-flight promise
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by kieran-typescript-reviewer during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
@@ -0,0 +1,49 @@
|
|||||||
|
---
|
||||||
|
status: pending
|
||||||
|
priority: p3
|
||||||
|
issue_id: "165"
|
||||||
|
tags: [code-review, security, supply-chain, rate-limiting]
|
||||||
|
dependencies: []
|
||||||
|
---
|
||||||
|
|
||||||
|
# Scenario Rate Limiter Keys Off IP — Shared Egress Customers Share Rate Bucket
|
||||||
|
|
||||||
|
## Problem Statement
|
||||||
|
`api/scenario/v1/run.ts` rate-limits requests by client IP. In enterprise or office environments where many users share a single egress IP (NAT, VPN), all users share one rate bucket. A single heavy user can exhaust the quota for all colleagues on the same IP. The correct key for a PRO-gated endpoint is the authenticated API key identity.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
- **File:** `api/scenario/v1/run.ts`
|
||||||
|
- Rate limit key: likely `getClientIp(req)` (standard pattern in codebase)
|
||||||
|
- PRO endpoints in the codebase that handle multiple users per IP should key by API key identity
|
||||||
|
- See MEMORY: `feedback_is_caller_premium_trusted_origin.md` — the API key is extractable from `X-WorldMonitor-Key`
|
||||||
|
- Minor issue: scenario endpoint is PRO-only, low traffic volume — not urgent
|
||||||
|
- Identified by security-sentinel during PR #2910 review
|
||||||
|
|
||||||
|
## Proposed Solutions
|
||||||
|
|
||||||
|
### Option A: Key rate limit by API key when present, fall back to IP
|
||||||
|
```ts
|
||||||
|
const apiKey = req.headers.get('x-worldmonitor-key');
|
||||||
|
const rateLimitKey = apiKey ? `scenario:key:${apiKey}` : `scenario:ip:${getClientIp(req)}`;
|
||||||
|
```
|
||||||
|
**Pros:** Per-identity limiting for authenticated users, IP fallback for unauth
|
||||||
|
**Cons:** Unauthenticated requests still IP-keyed (acceptable)
|
||||||
|
**Effort:** Small | **Risk:** None
|
||||||
|
|
||||||
|
## Recommended Action
|
||||||
|
_Apply Option A in a follow-up. Not blocking — scenario endpoint is PRO-only and low-traffic._
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
- **Affected files:** `api/scenario/v1/run.ts`
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [ ] Rate limit key uses `X-WorldMonitor-Key` when present
|
||||||
|
- [ ] Falls back to IP for requests without a key
|
||||||
|
- [ ] Existing rate limit tests pass
|
||||||
|
|
||||||
|
## Work Log
|
||||||
|
- 2026-04-10: Identified by security-sentinel during PR #2910 review
|
||||||
|
|
||||||
|
## Resources
|
||||||
|
- PR: #2910
|
||||||
|
- MEMORY: `feedback_is_caller_premium_trusted_origin.md`
|
||||||
Reference in New Issue
Block a user