From 6c8171cd6f23e201a2dc8844cad183541f7e8e22 Mon Sep 17 00:00:00 2001 From: jacobvjk Date: Tue, 6 Oct 2026 12:13:03 +0200 Subject: [PATCH] refactor(geography): drop the spelling folds #945 needed Timeseries now carry the metadata's own geography labels, and validateTimeseries enforces it (#974), so three workarounds for "South East Asia" vs "Southeast Asia" are no longer needed: - resolveGeography's labelVariant arm - regionMemberCodes' fallback to canonicalGeographyKey - hasGeography's label and spelling-fold attempts; it is now a plain lookup of the metadata token canonicalGeographyKey stays: comparisonScope uses it to spot two publishers using one region name with different members, which is a cross-publication question, not a data mismatch. The tests for the removed behaviour now assert the new contract: a different spelling is a different geography. Co-Authored-By: Claude Opus 5.5 --- src/utils/geographyFallback.test.ts | 96 ++++++------------------ src/utils/geographyFallback.ts | 30 +------- src/utils/geographyUtils.test.tsx | 13 ++-- src/utils/geographyUtils.ts | 18 ++--- src/utils/timeseriesAvailability.test.ts | 44 ++--------- src/utils/timeseriesAvailability.ts | 34 +++------ 6 files changed, 56 insertions(+), 179 deletions(-) diff --git a/src/utils/geographyFallback.test.ts b/src/utils/geographyFallback.test.ts index ed72297f..1e8d9866 100644 --- a/src/utils/geographyFallback.test.ts +++ b/src/utils/geographyFallback.test.ts @@ -134,72 +134,26 @@ describe("resolveGeography", () => { }); }); -describe("resolveGeography — spelling variants", () => { - it("matches a region spelled differently, and calls it a match not a fallback", () => { - // The live #945 case: IEA-APS declares "Southeast Asia" in its metadata - // while its own timeseries carries "South East Asia". Before this arm the - // request fell through to Global while the exact series sat in the file. - const result = resolveGeography( - ["Global", "South East Asia"], - "Southeast Asia", - SEA, - ); - - expect(result).toEqual({ - used: "South East Asia", - requested: "Southeast Asia", - fellBack: false, - reason: "labelVariant", - }); - }); - - it("finds the containing region when metadata and data spell it differently", () => { - // The other half of #945: a country request. The data carries "South East - // Asia", the metadata declares its members under "Southeast Asia"; the - // member lookup has to fold the spelling too, or VN lands on Global. - const metadata: Geography = { - global: true, - regions: { "Southeast Asia": ["VN", "TH"] }, - country: [], - }; +describe("resolveGeography — labels match exactly", () => { + it("does not fold spellings: timeseries carry the metadata's own label", () => { + // #945 used to need a spelling fold here, because IEA's timeseries said + // "South East Asia" where its metadata said "Southeast Asia". The labels + // now agree (validateTimeseries enforces it), so a different spelling is a + // different geography and an honest, reported fallback. expect( - resolveGeography(["Global", "South East Asia"], "VN", metadata), - ).toMatchObject({ - used: "South East Asia", - fellBack: true, - reason: "containingRegion", - }); - }); - - it("folds case and punctuation too", () => { - expect(resolveGeography(["Asia-Pacific"], "asia pacific", SEA).used).toBe( - "Asia-Pacific", - ); - }); - - it("prefers an exact match over a variant", () => { - const result = resolveGeography( - ["South East Asia", "Southeast Asia"], - "Southeast Asia", - SEA, - ); - expect(result.used).toBe("Southeast Asia"); - expect(result.reason).toBe("exact"); + resolveGeography(["Global", "South East Asia"], "Southeast Asia", SEA), + ).toMatchObject({ used: "Global", fellBack: true, reason: "global" }); }); - it("is a spelling fold, not a synonym table", () => { - // Different words stay different: no ISO-similarity threshold is involved. - // "ASEAN" still reaches the only option available, but as a reported - // fallback rather than a match — which is the distinction that matters. - expect(resolveGeography(["South East Asia"], "ASEAN", SEA)).toMatchObject({ - used: "South East Asia", - fellBack: true, - reason: "broadest", - }); - + it("matches the declared label exactly", () => { expect( - resolveGeography(["South Asia", "Global"], "South East Asia", SEA), - ).toMatchObject({ used: "Global", fellBack: true, reason: "global" }); + resolveGeography(["Global", "Southeast Asia"], "Southeast Asia", SEA), + ).toEqual({ + used: "Southeast Asia", + requested: "Southeast Asia", + fellBack: false, + reason: "exact", + }); }); }); @@ -217,16 +171,14 @@ describe("geographyFallbackNote", () => { }); it("says nothing when the request resolved", () => { - for (const reason of ["exact", "labelVariant"] as const) { - expect( - geographyFallbackNote({ - used: "South East Asia", - requested: "Southeast Asia", - fellBack: false, - reason, - }), - ).toBeNull(); - } + expect( + geographyFallbackNote({ + used: "South East Asia", + requested: "South East Asia", + fellBack: false, + reason: "exact", + }), + ).toBeNull(); }); it("says nothing when nothing was requested or nothing resolved", () => { diff --git a/src/utils/geographyFallback.ts b/src/utils/geographyFallback.ts index 3eac5e08..fa1f2a5d 100644 --- a/src/utils/geographyFallback.ts +++ b/src/utils/geographyFallback.ts @@ -1,6 +1,5 @@ import type { Geography } from "../types"; import { - canonicalGeographyKey, geographyKind, geographyLabel, normalizeGeography, @@ -26,13 +25,7 @@ export type GeographyResolution = { requested: string | null; /** True when `used` is broader than (or simply different from) `requested`. */ fellBack: boolean; - reason: - | "exact" - | "labelVariant" - | "containingRegion" - | "global" - | "broadest" - | "none"; + reason: "exact" | "containingRegion" | "global" | "broadest" | "none"; }; /** @@ -85,27 +78,6 @@ export function resolveGeography( return { used: exact, requested: wanted, fellBack: false, reason: "exact" }; } - /* - The same region spelled differently is still the same region, so this is a - match rather than a fallback. Without it, IEA-APS asking for its own - declared "Southeast Asia" misses its timeseries' "South East Asia", fails - the ISO step below (a region label is not an ISO code) and lands on Global — - reporting a fallback while the exact series sits in the file. See #945 for - the underlying data inconsistency, which this makes harmless. - */ - const wantedKey = canonicalGeographyKey(wanted); - const variant = ranked.find( - (geo) => canonicalGeographyKey(geo) === wantedKey, - ); - if (variant !== undefined) { - return { - used: variant, - requested: wanted, - fellBack: false, - reason: "labelVariant", - }; - } - // A requested country may be covered by a region the data does carry. const wantedISO = toISO2(wanted); if (wantedISO) { diff --git a/src/utils/geographyUtils.test.tsx b/src/utils/geographyUtils.test.tsx index 8a8ac0dc..c283529e 100644 --- a/src/utils/geographyUtils.test.tsx +++ b/src/utils/geographyUtils.test.tsx @@ -257,14 +257,11 @@ describe("regionMemberCodes", () => { ).toEqual(["DE"]); }); - it("matches the same region spelled differently (#945)", () => { - // A plot badge carries the timeseries spelling; the members live under the - // metadata's. Different words still do not match. - const geo = { - regions: { "Southeast Asia": ["VN"], "South Asia": ["IN"] }, - } as Geography; - expect(regionMemberCodes(geo, "South East Asia")).toEqual(["VN"]); - expect(regionMemberCodes(geo, "ASEAN")).toEqual([]); + it("matches the declared label only, not another spelling of it (#945)", () => { + // Timeseries now carry the metadata's own label, so no fold is needed. + const geo = { regions: { "Southeast Asia": ["VN"] } } as Geography; + expect(regionMemberCodes(geo, "Southeast Asia")).toEqual(["VN"]); + expect(regionMemberCodes(geo, "South East Asia")).toEqual([]); }); it("returns [] for a region the publication left unmapped", () => { diff --git a/src/utils/geographyUtils.ts b/src/utils/geographyUtils.ts index 2ae8f796..c8aa8258 100644 --- a/src/utils/geographyUtils.ts +++ b/src/utils/geographyUtils.ts @@ -81,13 +81,11 @@ export function regionMemberCodes( if (!wanted) return []; // Match on the normalized form so the lookup accepts the same token // `flattenGeography` emitted, even if the raw key carries stray whitespace. - // Failing that, the same words spelled differently: a timeseries label such - // as IEA's "South East Asia" names the metadata's "Southeast Asia" (#945). - const keys = Object.keys(geo.regions); - const wantedKey = canonicalGeographyKey(wanted); - const key = - keys.find((k) => normalizeGeography(k) === wanted) ?? - keys.find((k) => canonicalGeographyKey(k) === wantedKey); + // Exact otherwise: timeseries labels are the metadata's own (#945, enforced + // by validateTimeseries), so no spelling fold is needed. + const key = Object.keys(geo.regions).find( + (k) => normalizeGeography(k) === wanted, + ); if (key === undefined) return []; const members = geo.regions[key]; if (!Array.isArray(members)) return []; @@ -162,8 +160,10 @@ export function geographyKind(raw: string): GeographyKind { * A publication-independent identity for a geography token, folding away * spelling differences only: case, spacing and punctuation. * - * Publishers spell the same region differently — IEA declares "Southeast Asia" - * while ACE and IEA's own timeseries files carry "South East Asia" (#945). This + * Different publishers spell the same region differently — IEA writes + * "Southeast Asia", JRC "South East Asia" — which matters when one page sets + * pathways side by side (comparisonScope). Within one pathway no fold is + * needed: its timeseries use its metadata's own labels (#945). This * is deliberately a spelling fold and NOT a synonym table: "ASEAN" does not * merge with "South East Asia", and no ISO-similarity threshold is involved, * because a fuzzy match is unreviewable. Two tokens share a key only when they diff --git a/src/utils/timeseriesAvailability.test.ts b/src/utils/timeseriesAvailability.test.ts index 5d257b12..44820a51 100644 --- a/src/utils/timeseriesAvailability.test.ts +++ b/src/utils/timeseriesAvailability.test.ts @@ -87,48 +87,20 @@ describe("pathwayToolAvailability", () => { expect(av.hasGeography("North America")).toBe(false); }); - it("maps ISO2 country codes to display names", () => { - // "DE" maps to "Germany" via geographyLabel — confirmed by PathwayCard tests. - const av = pathwayToolAvailability([ - { - summary: { - sectors: [], - geographies: ["Germany"], - metrics: [], - }, - }, - ]); - expect(av.hasGeography("DE")).toBe(true); + it("matches the metadata token exactly, so a different spelling is absent", () => { + // Timeseries carry the metadata's own labels (#945, enforced by + // validateTimeseries), so no spelling fold or name lookup is involved. + expect(av.hasGeography("Southeast Asia")).toBe(false); + expect(av.hasGeography("south east asia")).toBe(false); }); - it("returns false for an ISO2 code whose display name is not present", () => { + it("matches an ISO2 country by its code, as summaries record it", () => { const av = pathwayToolAvailability([ - { summary: { sectors: [], geographies: ["Germany"], metrics: [] } }, + { summary: { sectors: [], geographies: ["DE"], metrics: [] } }, ]); + expect(av.hasGeography("DE")).toBe(true); expect(av.hasGeography("FR")).toBe(false); }); - - it("matches a region the publisher spells differently in its own data", () => { - // The live #945 case: IEA-APS declares "Southeast Asia" in metadata while - // its timeseries carries "South East Asia". resolveGeography plots it, so - // reporting it as absent from the tool would contradict the charts. - expect(av.hasGeography("Southeast Asia")).toBe(true); - }); - - it("folds case and punctuation, like resolveGeography", () => { - const av = pathwayToolAvailability([ - { - summary: { sectors: [], geographies: ["Asia-Pacific"], metrics: [] }, - }, - ]); - expect(av.hasGeography("asia pacific")).toBe(true); - }); - - it("stays a spelling fold, not a synonym table", () => { - // Different words remain different: no similarity threshold is involved. - expect(av.hasGeography("ASEAN")).toBe(false); - expect(av.hasGeography("South Asia")).toBe(false); - }); }); describe("merging multiple datasets", () => { diff --git a/src/utils/timeseriesAvailability.ts b/src/utils/timeseriesAvailability.ts index fc9c5a2d..475c0752 100644 --- a/src/utils/timeseriesAvailability.ts +++ b/src/utils/timeseriesAvailability.ts @@ -1,10 +1,6 @@ // src/utils/timeseriesAvailability.ts import { SECTORS_BY_KEY } from "./timeseriesTaxonomy"; -import { - canonicalGeographyKey, - geographyLabel, - normalizeGeography, -} from "./geographyUtils"; +import { normalizeGeography } from "./geographyUtils"; interface TimeseriesSummary { sectors?: string[]; @@ -70,37 +66,25 @@ export function pathwayToolAvailability( const sectorNames = new Set(); const metricNames = new Set(); - const geoStrings = new Set(); - const geoKeys = new Set(); + const geographies = new Set(); for (const ds of datasets) { for (const v of sectorDisplayNames(ds.summary)) sectorNames.add(v); for (const v of metricDisplayNames(ds.summary)) metricNames.add(v); - for (const v of parseSummary(ds.summary).geographies ?? []) { - geoStrings.add(v); - geoKeys.add(canonicalGeographyKey(v)); - } + for (const v of parseSummary(ds.summary).geographies ?? []) + geographies.add(normalizeGeography(v)); } return { hasSector: (name) => sectorNames.has(name), hasMetric: (name) => metricNames.has(name), /* - Three attempts, widening: the display label, the raw token, then a - spelling fold. - - The fold matters because a publisher's metadata and its own timeseries can - spell one region two ways — IEA-APS declares "Southeast Asia" while its - data carries "South East Asia" (#945). `resolveGeography` already matches - those through `canonicalGeographyKey`, so without this an exact-match test - would report the region as absent from the tool while the charts plot it - perfectly. The two surfaces have to tell the same story. + A plain lookup of the metadata token. Timeseries geographies are the + labels the pathway's metadata declares (#945, enforced by + validateTimeseries), so the tokens a caller passes match the summary's + exactly; only stray whitespace is normalized away. */ - hasGeography: (rawGeo) => { - const label = geographyLabel(normalizeGeography(rawGeo)); - if (geoStrings.has(label) || geoStrings.has(rawGeo)) return true; - return geoKeys.has(canonicalGeographyKey(rawGeo)); - }, + hasGeography: (rawGeo) => geographies.has(normalizeGeography(rawGeo)), }; }