Phase 0 PR2: Forecast region filter quick win#2942
Conversation
The getForecasts RPC already accepts a region parameter (free-text substring match on f.region in server/worldmonitor/forecast/v1/get-forecasts.ts) but the UI never passed it. Adds a second filter row alongside the existing domain filter, scoped to 8 regions matching the existing forecast region strings. The selected region string is passed end-to-end from UI to server with no client-side re-filtering. Earlier review caught that double-filtering with mismatched taxonomies hides valid forecasts. Spec: docs/internal/pro-regional-intelligence-upgrade.md (Feature 1, Phase 0)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds a region filter pill row to
Confidence Score: 3/5Not safe to merge as-is — two P1 defects cause the region filter to silently show wrong data. The 30-minute scheduled refresh is guaranteed to overwrite region-filtered state on every cycle, meaning the feature's core guarantee breaks predictably in production. The error-path inconsistency is a second independent P1. Both are on the primary user path of this feature. src/components/ForecastPanel.ts — updateForecasts and refetchForRegion both need fixes before merge. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ForecastPanel
participant fetchForecasts as fetchForecasts RPC
participant RefreshScheduler
User->>ForecastPanel: click region pill (e.g. MENA)
ForecastPanel->>ForecastPanel: selectedRegion = 'MENA', regionFetchSeq++
ForecastPanel->>ForecastPanel: render() — MENA pill highlighted (old data shown)
ForecastPanel->>fetchForecasts: fetchForecasts(undefined, 'MENA')
fetchForecasts-->>ForecastPanel: MENA forecasts
ForecastPanel->>ForecastPanel: updateForecasts(menaForecasts) → render()
Note over RefreshScheduler,ForecastPanel: 30 min later (REFRESH_INTERVALS.forecasts)
RefreshScheduler->>ForecastPanel: updateForecasts(allForecasts) — no region arg
ForecastPanel->>ForecastPanel: this.forecasts = allForecasts (selectedRegion still 'MENA')
Note over ForecastPanel: MENA pill active but all-region data displayed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| private async refetchForRegion(): Promise<void> { | ||
| const seq = ++this.regionFetchSeq; | ||
| // Repaint immediately so the clicked pill's active state updates. | ||
| this.render(); | ||
| try { | ||
| const forecasts = await fetchForecasts(undefined, this.selectedRegion || undefined); | ||
| if (seq !== this.regionFetchSeq) return; | ||
| this.updateForecasts(forecasts); | ||
| } catch { | ||
| // Silent failure: premium RPC, same pattern as the initial load in data-loader. | ||
| } | ||
| } |
There was a problem hiding this comment.
this.selectedRegion is updated by the caller before refetchForRegion() is invoked, and the immediately following this.render() already paints the new pill as active. If the fetchForecasts call then throws, the catch is silent: this.forecasts still holds data from the previous region (or all regions), but the UI continues to show the new region pill as selected.
Rolling back selectedRegion inside the catch restores a consistent state:
private async refetchForRegion(): Promise<void> {
const seq = ++this.regionFetchSeq;
const prevRegion = this.selectedRegion;
this.render();
try {
const forecasts = await fetchForecasts(undefined, this.selectedRegion || undefined);
if (seq !== this.regionFetchSeq) return;
this.updateForecasts(forecasts);
} catch {
if (seq === this.regionFetchSeq) {
this.selectedRegion = prevRegion;
this.render();
}
}
}| const regionBtn = target.closest('[data-fc-region]') as HTMLElement | null; | ||
| if (regionBtn) { | ||
| const nextRegion = regionBtn.dataset.fcRegion ?? ''; | ||
| if (nextRegion === this.selectedRegion) return; | ||
| this.selectedRegion = nextRegion; | ||
| void this.refetchForRegion(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
After clicking a region pill the initial render() in refetchForRegion immediately highlights the new pill, but the data below it still reflects the old region until the RPC completes. There is no visual cue (spinner, skeleton, opacity) to tell the user a request is in flight, making the panel feel inconsistent for the duration of the round-trip.
… review Records 19 P2 and 3 P3 todos from the kieran-typescript, security-sentinel, performance-oracle, architecture-strategist, code-simplicity, agent-native, and learnings researcher reviews of PR #2940 and PR #2942. P2 (19): isCloseToThreshold inverted for lt; sequential readLatestSnapshot 1600ms; sequential persist 1600ms; seeder bypasses runSeed; region taxonomy 3 sources of truth; Redis keys coupled to compute modules; stored XSS risk; AbortController missing; getForecasts no cachedFetchJson; REGIONS.find duplicated; undated inputs treated as fresh; inconsistent unknown-region error handling; writeExtraKeyWithMeta positional args; pipeline non-atomic persist; trigger watching runs on delta operators; regime.transition_driver always empty; orphan scripts/shared mirror; dangling docs/internal refs; refetchForRegion silent error swallowing. P3 (3): redundant JSON.stringify(caseFile) hot loops; helper/config cleanups (round dedup, generateSnapshotId entropy, matchesHorizon regex, coercive_pressure dead 0.30 weight); perf micro-cleanups (buildPreMeta x8, signal indexing, prototype pollution guards).
Addresses 2 P1 review findings on PR #2942. P1 #1: Region pills keyed to labels that do not match persisted Forecast.region. The seed-forecasts writes Forecast.region with country names (Israel, Iran, Taiwan, United States...) and theater/market values (Red Sea, Black Sea, Baltic Sea, South China Sea, Eastern Mediterranean, Strait of Hormuz, Western Pacific...). The server RPC only does a substring .includes() match, so pills like 'Middle East' and 'Europe' miss most of the data. The seed already computes macroRegion via MACRO_REGION_MAP but that field is not in the Forecast proto, so the client never sees it. Fix: mirror MACRO_REGION_MAP to new shared/forecast-macro-regions.js and filter client-side by macro region. Pills now use MENA, EAST_ASIA, EUROPE, AMERICAS, SOUTH_ASIA, AFRICA as the six macro regions that match the publisher's taxonomy. P1 #2: Region filter state lost on refresh. selectedRegion was only consumed by refetchForRegion(). When the data loader or scheduler called updateForecasts(fullForecasts), this.forecasts was overwritten and the region filter disappeared. render() did not know about the filter. Fix: apply region filter inside the render pipeline (matching the existing activeDomain pattern). this.forecasts stays as the full unfiltered set. Clicking a pill no longer issues an RPC; it just updates selectedRegion and re-renders. Removed the regionFetchSeq counter since no network call fires. Side effect: improved perf (no redundant RPC on pill clicks) and instant response. The getForecasts RPC is called once on initial load, same as before.
|
P1 review fixes landed in 01965eb — addresses both issues from the review. P1 #1: Region pills now match the seed's actual region vocabulary The seed already computes a P1 #2: Region filter no longer wiped on refresh Fix: the region filter now lives in Side effect: instant pill response (no roundtrip) and one fewer premium RPC per click. Files touched (3):
Checks:
|
…from filter Addresses 1 P1 + 1 P2 review finding on PR #2942. P1: narrow macro map silently dropped valid forecasts. The hardcoded FORECAST_MACRO_REGION_MAP only covered ~50 strings, but seed-forecasts.mjs emits conflict/political/cyber/infrastructure rows with region set to ANY source country name (Algeria, Niger, Kazakhstan, Uruguay, Vietnam...) and market rows with 'Global' or 'Northern Europe'. Any row whose region string was not in the 50-entry map returned null and vanished on every pill click. Fix: replace the narrow map with a 3-stage lookup: 1. Theater / geo-label map for multi-country strings (Red Sea, Baltic Sea, Northern Europe, Sahel, Horn of Africa, South China Sea, etc.) 2. Country name -> ISO2 via a 302-entry normalized dictionary mirroring shared/country-names.json. 3. ISO2 -> region via an inlined 218-entry ISO2_TO_REGION map (the World Bank taxonomy with strategic overrides: AF/PK/LK -> south-asia, TR -> mena, MX -> north-america, TW -> east-asia). Unknown strings return null. 'Global' resolves to a distinct 'global' id so callers can tell "truly global" from "unclassified"; both only appear under the All Regions pill since no specific pill matches them. Pills now use the 7 region IDs from the regional intelligence taxonomy (mena, east-asia, europe, north-america, south-asia, latam, sub-saharan-africa), replacing the old uppercase enum that was tied to the narrow MACRO_REGION_MAP. P2: status badge reflected filter result, not fetch success. Picking a region with zero matches flipped the badge to 'unavailable' even though the RPC succeeded and the feed was healthy. Badge is now tied to this.forecasts.length (fetch success) and the empty-state copy differentiates "No forecasts match the current filter" (filter miss) from "No forecasts available" (fetch miss) so users can tell the two apart without mistaking a filter miss for a broken feed.
|
Addressed 1 P1 + 1 P2 review finding in P1 — Narrow macro map drops valid forecastsThe old
Any row whose region string was not in the 50-entry map returned null and silently disappeared on every pill click. Replaced the narrow map with a 3-stage lookup in
Unknown strings return null. Pills now use the 7 canonical region IDs: P2 — Badge reflects filter result, not fetch successThe status badge was tied to the filtered Fixed: badge now reads Verification
Files changed:
|
1 participant
Summary
Phase 0 PR2 of the Regional Intelligence Model. Independent of PR #2940 (the foundation snapshot writer); both can merge in any order.
The existing
getForecasts(domain, region)server RPC accepts a region parameter but the UI never passed it. Adds a second filter row toForecastPanel.tsmatching the existing domain filter pattern. The selected region string is passed end-to-end with no client-side re-filtering.Spec
docs/internal/pro-regional-intelligence-upgrade.mdFeature 1, Phase 0 quick win.Testing
npm run typecheck- cleannpm run linton the touched file - cleanPost-Deploy Monitoring & Validation
UI-only change with no backend impact.
ForecastPanelTest plan