From 925897a779ca1821bc710c63ba6653c8828ddd60 Mon Sep 17 00:00:00 2001 From: Ian Watt Date: Tue, 11 Nov 2025 21:52:03 +0000 Subject: [PATCH 1/2] fix: don't just say USD when no conversion has taken place --- packages/econify/src/batch/batch.ts | 39 +++- packages/econify/src/normalization/explain.ts | 174 ++++++++++++------ 2 files changed, 152 insertions(+), 61 deletions(-) diff --git a/packages/econify/src/batch/batch.ts b/packages/econify/src/batch/batch.ts index 18367b0..b065050 100644 --- a/packages/econify/src/batch/batch.ts +++ b/packages/econify/src/batch/batch.ts @@ -640,9 +640,24 @@ function processItem( const scale = targetMagnitude ?? "ones"; normalizedUnit = scale === "ones" ? "ones" : titleCase(scale); } else { + // Choose label currency for units: + // - If FX would occur (known currency, target set, fx available and different), use target currency + // - Else, use known effective currency (if any) + const effectiveCurrencyKnown = (effectiveCurrency && + effectiveCurrency !== "UNKNOWN") + ? effectiveCurrency + : undefined; + const willConvert = !shouldSkipCurrency && + !!effectiveCurrencyKnown && + !!options.toCurrency && + !!options.fx && + effectiveCurrencyKnown !== options.toCurrency; + const labelCurrency = willConvert + ? options.toCurrency + : effectiveCurrencyKnown; normalizedUnit = buildNormalizedUnit( item.unit, - options.toCurrency, + labelCurrency, targetMagnitude, options.toTimeScale, indicatorType, @@ -716,7 +731,27 @@ function buildNormalizedUnit( ): string { const parsed = parseUnit(original); - const cur = (currency || parsed.currency)?.toUpperCase(); + // Special-case placeholder currencies (e.g., "National currency"): preserve original label, + // add magnitude if any, and append time dimension when appropriate. + if (parsed.currency === "UNKNOWN") { + const mag = magnitude ?? parsed.scale ?? getScale(original); + const ts = timeScale ?? parsed.timeScale; + const parts: string[] = [original]; + if (mag && mag !== "ones") parts.push(String(mag)); + let out = parts.join(" "); + const shouldIncludeTime = ts && + allowsTimeConversion(indicatorType, temporalAggregation); + if (shouldIncludeTime) { + out = `${out}${out ? " " : ""}per ${ts}`; + } + return out || original; + } + + // Sanitize provided currency: ignore placeholder values + const provided = currency && currency.toUpperCase() !== "UNKNOWN" + ? currency + : undefined; + const cur = (provided || parsed.currency)?.toUpperCase(); // Fallback to detect scale from unit text when parser misses singular forms (e.g., "Thousand") const mag = magnitude ?? parsed.scale ?? getScale(original); const ts = timeScale ?? parsed.timeScale; diff --git a/packages/econify/src/normalization/explain.ts b/packages/econify/src/normalization/explain.ts index b2e341c..4fcda4a 100644 --- a/packages/econify/src/normalization/explain.ts +++ b/packages/econify/src/normalization/explain.ts @@ -43,6 +43,8 @@ export function buildExplainMetadata( // Use explicit fields if provided, otherwise fall back to parsed values const effectiveCurrency = options.explicitCurrency || parsed.currency; + const hasKnownCurrency = !!effectiveCurrency && + effectiveCurrency !== "UNKNOWN"; const effectiveScale = options.explicitScale || parsed.scale; // Time scale priority: @@ -61,7 +63,7 @@ export function buildExplainMetadata( // FX information if ( - effectiveCurrency && options.toCurrency && options.fx && + hasKnownCurrency && options.toCurrency && options.fx && effectiveCurrency !== options.toCurrency ) { const rate = options.fx.rates[effectiveCurrency]; @@ -79,6 +81,9 @@ export function buildExplainMetadata( } } + // Determine if FX conversion actually occurred (based on presence of explain.fx) + const didFX = !!explain.fx; + // Magnitude information - only provide when scaling actually occurs const originalScale = effectiveScale || getScale(originalUnit); const targetScale = options.toMagnitude || originalScale; @@ -257,74 +262,123 @@ export function buildExplainMetadata( } } else { // Monetary/currency-based units (default behavior) - originalUnitString = buildOriginalUnitString( - effectiveCurrency, - originalScale, - ); - // For monetary units, only preserve original time scale if it was explicitly in the unit string - // (not just in metadata periodicity). This prevents adding "per month" to stock indicators. - const hasTimeInUnit = !!parsed.timeScale; - - // Use target time scale unless conversion was explicitly blocked - // Check if time conversion was blocked (not just "no conversion needed") - const timeWasBlocked = explain.periodicity?.adjusted === false && - explain.periodicity?.description?.includes("blocked"); - const effectiveTargetTime: TimeScale | undefined = timeWasBlocked - ? (hasTimeInUnit && originalTimeScale ? originalTimeScale : undefined) - : (options.toTimeScale || - (hasTimeInUnit && originalTimeScale ? originalTimeScale : undefined)); - - normalizedUnitString = buildNormalizedUnitString( - options.toCurrency || effectiveCurrency, - targetScale, - effectiveTargetTime, - ); - - // Build full unit strings with time periods - originalFullUnit = buildFullUnitString( - effectiveCurrency, - originalScale, - originalTimeScale || undefined, - ); - normalizedFullUnit = buildFullUnitString( - options.toCurrency || effectiveCurrency, - targetScale, - effectiveTargetTime, - ) || normalizedUnitString; - - // Per-capita: avoid adding scale label like 'millions' to currency units - if (isPerCapita) { - normalizedUnitString = buildNormalizedUnitString( - options.toCurrency || effectiveCurrency, - "ones", - options.toTimeScale, + // Special handling for placeholder currency (e.g., "National currency") with no FX: + // Present units using the original label without injecting a target currency. + if (!hasKnownCurrency && !didFX) { + const base = originalUnit; + // For placeholder currencies, reflect target time scale if provided + if (options.toTimeScale) { + originalUnitString = base; + normalizedUnitString = `${base} per ${options.toTimeScale}`; + originalFullUnit = originalUnitString; + normalizedFullUnit = normalizedUnitString; + } else { + originalUnitString = base; + normalizedUnitString = base; + originalFullUnit = base; + normalizedFullUnit = base; + } + } else { + // Known currency or FX occurred: build currency-based labels normally + originalUnitString = buildOriginalUnitString( + effectiveCurrency, + originalScale, ); - normalizedFullUnit = buildFullUnitString( - options.toCurrency || effectiveCurrency, - "ones", - options.toTimeScale, - ) || normalizedUnitString; - } + // For monetary units, only preserve original time scale if it was explicitly in the unit string + // (not just in metadata periodicity). This prevents adding "per month" to stock indicators. + const hasTimeInUnit = !!parsed.timeScale; + + // Use target time scale unless conversion was explicitly blocked + // Check if time conversion was blocked (not just "no conversion needed") + const timeWasBlocked = explain.periodicity?.adjusted === false && + explain.periodicity?.description?.includes("blocked"); + const effectiveTargetTime: TimeScale | undefined = timeWasBlocked + ? (hasTimeInUnit && originalTimeScale ? originalTimeScale : undefined) + : (options.toTimeScale || + (hasTimeInUnit && originalTimeScale ? originalTimeScale : undefined)); + + // Choose label currency: + // - If FX occurred, use target currency + // - Else, use the known effective currency + const labelCurrency = didFX + ? options.toCurrency + : (effectiveCurrency || undefined); - // Indicators with skipTimeInUnit: omit per-time in unit strings - // This includes: stock, balance, capacity, price, percentage, ratio, rate, index, etc. - if (skipTimeInUnitString) { normalizedUnitString = buildNormalizedUnitString( - options.toCurrency || effectiveCurrency, + labelCurrency, targetScale, - undefined, + effectiveTargetTime, ); + + // Build full unit strings with time periods originalFullUnit = buildFullUnitString( effectiveCurrency, originalScale, - undefined, + originalTimeScale || undefined, ); normalizedFullUnit = buildFullUnitString( - options.toCurrency || effectiveCurrency, + labelCurrency, targetScale, - undefined, + effectiveTargetTime, ) || normalizedUnitString; } + + // Per-capita: avoid adding scale label like 'millions' to currency units + if (isPerCapita) { + const perCapitaCurrency = didFX + ? options.toCurrency + : (effectiveCurrency || undefined); + if (perCapitaCurrency) { + normalizedUnitString = buildNormalizedUnitString( + perCapitaCurrency, + "ones", + options.toTimeScale, + ); + normalizedFullUnit = buildFullUnitString( + perCapitaCurrency, + "ones", + options.toTimeScale, + ) || normalizedUnitString; + } else { + // If currency is unknown, keep the base label without currency + const base = originalUnit; + normalizedUnitString = options.toTimeScale + ? `${base} per ${options.toTimeScale}` + : base; + normalizedFullUnit = normalizedUnitString; + } + } + + // Indicators with skipTimeInUnit: omit per-time in unit strings + // This includes: stock, balance, capacity, price, percentage, ratio, rate, index, etc. + if (skipTimeInUnitString) { + if (hasKnownCurrency || didFX) { + const labelCurrency = didFX + ? options.toCurrency + : (effectiveCurrency || undefined); + normalizedUnitString = buildNormalizedUnitString( + labelCurrency, + targetScale, + undefined, + ); + originalFullUnit = buildFullUnitString( + effectiveCurrency, + originalScale, + undefined, + ); + normalizedFullUnit = buildFullUnitString( + labelCurrency, + targetScale, + undefined, + ) || normalizedUnitString; + } else { + // Placeholder currency: keep base without per-time + const base = originalUnit; + normalizedUnitString = base; + originalFullUnit = base; + normalizedFullUnit = base; + } + } } explain.units = { @@ -619,11 +673,13 @@ export function buildExplainMetadata( // 🆕 Separate component fields for easy frontend access if ( !isNonCurrencyCategory && !isNonCurrencyDomain && - (effectiveCurrency || options.toCurrency) + (hasKnownCurrency || didFX) ) { explain.currency = { - original: effectiveCurrency, - normalized: options.toCurrency || effectiveCurrency || "USD", + original: hasKnownCurrency ? effectiveCurrency : undefined, + normalized: didFX + ? options.toCurrency + : (hasKnownCurrency ? effectiveCurrency : undefined), }; } From a2556638fac26bc743b531cdf5d34185c0b6cf7b Mon Sep 17 00:00:00 2001 From: Ian Watt Date: Tue, 11 Nov 2025 22:06:55 +0000 Subject: [PATCH 2/2] Apply suggested changes Apply suggested changes --- packages/econify/src/normalization/explain.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/econify/src/normalization/explain.ts b/packages/econify/src/normalization/explain.ts index 4fcda4a..fa829e2 100644 --- a/packages/econify/src/normalization/explain.ts +++ b/packages/econify/src/normalization/explain.ts @@ -675,12 +675,16 @@ export function buildExplainMetadata( !isNonCurrencyCategory && !isNonCurrencyDomain && (hasKnownCurrency || didFX) ) { - explain.currency = { - original: hasKnownCurrency ? effectiveCurrency : undefined, - normalized: didFX - ? options.toCurrency - : (hasKnownCurrency ? effectiveCurrency : undefined), - }; + const normalizedCurrency = didFX + ? (options.toCurrency || "USD") + : (hasKnownCurrency ? effectiveCurrency : undefined); + + if (normalizedCurrency) { + explain.currency = { + original: hasKnownCurrency ? effectiveCurrency : undefined, + normalized: normalizedCurrency, + }; + } } if (originalScale || targetScale) {