feat: international currency support (issue #7)#10
Conversation
Captures the agreed approach (convert at boundary), config schema rename (totalPortfolioValueUSD → totalPortfolioValue + defaultCurrency), FX sourcing via yahoo-finance2 FX pairs, GBp/sub-unit handling, and file-by-file change list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
13-task plan derived from the approved design spec. Each task is ordered so commits leave the repo in a working state — schema rename lands before FX wiring, FX conversion lands before display updates, etc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the duplicated fmt$ helpers across email.ts, intradayEmail.ts, weeklyEmail.ts, telegram.ts. Supports USD/GBP/EUR/JPY/AUD/CAD/NZD/CHF/ HKD/SGD with currency-specific prefixes; unknown codes fall back to "<amount> <CODE>".
Single-call batch lookup for converting source currencies to a target default. Same yahoo-finance2 instance, no new dependency. Same-currency short-circuits to rate=1; individual failures log and omit from result.
…efaultCurrency Hard-cut schema migration. Loader throws a helpful error if the old field is still present. defaultCurrency is validated against an allowlist (USD/GBP/EUR/AUD/CAD/JPY/CHF/HKD/SGD/NZD); missing field defaults to USD with a one-line warning. This commit only renames — no FX conversion is wired yet, so behavior is unchanged for USD users (which is everyone today).
QuoteData gains currency (post-conversion) and originalCurrency (raw Yahoo). For tickers in SUB_UNIT_FIX (GBp/GBX/ILA/ZAc), divide all monetary fields by 100 and upgrade the currency to its real form (GBp → GBP, etc.). No FX conversion yet — currency = originalCurrency at this point. The next commit wires fetchFxRates to convert across currencies.
fetchAllPrices renamed to fetchPrices with defaultCurrency param. Batches a one-pass FX lookup across all unique source currencies, then applies conversion to every monetary field on QuoteData. Tickers whose FX rate can't be fetched are listed in result.skipped and the run continues without them.
…ndicators Rebases the 365-day chart series through the current FX rate so sma50/sma200/atr14/bollinger/recent-low values come out denominated in defaultCurrency. Tiny error if FX has drifted across the window, acceptable for entry-timing use.
fmt$ now delegates to formatMoney(_, defaultCurrency). Holdings header shows active currency. When any ticker has a non-default original currency, the email gains a footer caveat about limit prices. FX- skipped tickers shown in an amber footer line when present.
…alerts Threads originalCurrency through AIBuyRecommendation → IntradayAlert so the intraday email can render the cross-currency footer caveat. fmt$ now delegates to formatMoney; limit order hardcoded $ replaced. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e symbol
Steps 2, 3, and remaining $ fix from Task 8:
- buildIntradayEmailHtml: adds · ${defaultCurrency} to signal-count header
- sendRefreshEmail: adds · ${defaultCurrency} to date/time header
- sendRefreshEmail: replaces hardcoded $${quote.price.toFixed(2)} with fmt$()
- aiAnalysis: console limit log now shows originalCurrency instead of hardcoded $ - intradayEmail: comment clarifies hasCrossCurrency is alert-scoped (not portfolio)
Threads originalCurrency through AllocationItem so the weekly email can render its multi-currency footer when applicable. fmt$ now delegates to formatMoney; header shows defaultCurrency.
fmt$ now delegates to formatMoney(_, defaultCurrency). Each message type appends the active currency to its totals line and adds a single-line caveat when a non-default original currency is present.
Adds a CURRENCY: preamble to all three prompts (Stage 1 observation, Stage 2 decision, detailed thesis) so Gemini knows the active currency. For tickers whose Yahoo source currency differs from default, appends "(originally GBP)" etc. as audit context — keeps the model's reasoning from sounding confused about magnitudes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Renames the deprecated totalPortfolioValueUSD reference and documents the new defaultCurrency option + supported currency list.
All three pass: - smoke-fx: GBP→USD 1.35, EUR→USD 1.17, JPY→USD 0.0063, USD→USD 1 (sanity-bounded) - smoke-conversion: AAPL stays USD, TSCO.L GBp→USD (4.80 GBP × 1.35 = 6.50), SAP.DE EUR→USD (148.96 EUR × 1.17 = 174.63) - smoke-money-format: all 15 cases pass including fallback ZZZ suffix
…_FIX 63 tests across 6 describe blocks. Pure functions only — no network, no config.json dependency, CI-safe. applyFxRate and SUB_UNIT_FIX moved from fetchPrices.ts to util.ts so tests can import them without triggering config.ts (which throws on missing config.json). fetchPrices.ts re-imports from util.ts. Adds npm test script (node --import=tsx/esm --test) and CI step.
There was a problem hiding this comment.
Pull request overview
Adds international currency support across the quote-fetch → analysis → notification pipeline by normalizing sub-unit currencies, fetching FX rates, converting quote/technicals into a configured defaultCurrency, and updating outputs/prompts/config/docs accordingly.
Changes:
- Introduce FX-rate fetching (
src/fetchFx.ts) and integrate batch FX conversion into price fetching (src/fetchPrices.ts) with skip reporting. - Add currency-aware formatting/helpers (
formatMoney,applyFxRate,SUB_UNIT_FIX) and propagatedefaultCurrency+originalCurrencythrough analysis, AI prompts, and notifications. - Add unit tests + CI test step, plus smoke scripts for live FX/conversion sanity checks and docs/config migration updates.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/util.test.ts | Unit tests for formatMoney, applyFxRate, and SUB_UNIT_FIX. |
| src/util.ts | Adds sub-unit map, FX application helper, and shared money formatter. |
| src/fetchFx.ts | New Yahoo-based FX rate fetcher (GBPUSD=X style). |
| src/fetchPrices.ts | Captures currency, applies sub-unit fixes, batch-fetches FX, converts monetary fields, returns { quotes, skipped, fxRates }. |
| src/fetchTechnicals.ts | Applies FX conversion to OHLC before indicator computation; expands function signature to accept priceData/fxRates. |
| src/config.ts | Migrates totalPortfolioValueUSD → totalPortfolioValue; adds/validates defaultCurrency. |
| src/analyze.ts | Uses totalPortfolioValue; adds originalCurrency to AllocationItem. |
| src/index.ts | Wires new fetchPrices result shape, passes defaultCurrency/fxRates, and forwards fxSkipped into email. |
| src/email.ts | Uses formatMoney, adds currency header/caveats, and shows FX-skipped tickers. |
| src/intradayEmail.ts | Uses formatMoney, adds default currency label + cross-currency caveat. |
| src/weeklyEmail.ts | Uses formatMoney, adds default currency label + multi-currency caveat. |
| src/telegram.ts | Uses formatMoney, adds default currency labels + cross-currency caveats. |
| src/aiAnalysis.ts | Adds currency preamble + (originally XXX) annotations; records originalCurrency on recs. |
| src/detailedAnalysis.ts | Adds currency preamble and currency-aware formatting/annotations in prompt text. |
| src/intradayCompare.ts | Adds originalCurrency to IntradayAlert. |
| smoke/smoke-fx.ts | Live FX-rate smoke test. |
| smoke/smoke-conversion.ts | Live price fetch + FX/sub-unit conversion smoke test. |
| smoke/smoke-money-format.ts | CLI smoke for formatMoney expected outputs. |
| smoke/smoke-tooltip.ts | Smoke script for tooltip escaping behavior (synthetic HTML generation). |
| smoke/smoke-tooltip.html | Checked-in output artifact for tooltip smoke test. |
| package.json | Adds npm test and npm run smoke; expands prettier globs to include tests. |
| .github/workflows/ci.yml | Runs unit tests in CI. |
| docs/how-it-works.md | Documents new config field name + currency support behavior. |
| config.example.json | Updates config example for new portfolio value field + defaultCurrency. |
| README.md | Documents new npm test and npm run smoke commands. |
| CLAUDE.md | Updates dev command list and architecture notes for new modules. |
| .gitignore | Adjusts Claude settings ignore entry. |
Comments suppressed due to low confidence (1)
src/fetchTechnicals.ts:429
fetchTechnicalsnow optionally FX-converts OHLC before computing indicators, but the per-ticker log line still hardcodes$for moving averages. IfdefaultCurrencyis not USD, this becomes misleading. Suggest either logging without a currency symbol, passingdefaultCurrencyin and usingformatMoney, or including the currency code in the log output.
console.log(
` ✓ ${ticker}: 50MA=$${data.sma50}` +
(data.sma200 != null ? ` 200MA=$${data.sma200}` : "") +
` RSI=${data.rsi14}` +
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: string | null; | ||
| longName: string | null; | ||
| currency: string; // post-conversion currency (= defaultCurrency once Task 5 lands) | ||
| originalCurrency: string; // raw Yahoo currency (audit / logging) |
There was a problem hiding this comment.
QuoteData.originalCurrency is commented as “raw Yahoo currency”, but for sub-unit feeds you normalize it (GBp/GBX -> GBP) before storing. Either update this comment/field name to reflect that it’s normalized, or store both the raw feed currency and the normalized major currency for audit/logging.
| originalCurrency: string; // raw Yahoo currency (audit / logging) | |
| originalCurrency: string; // Yahoo/feed currency before post-fetch conversion; may be normalized from sub-units (e.g. GBp/GBX -> GBP) |
| ` — ${rec.reason}` + | ||
| (rec.suggestedLimitPrice ? ` [limit: $${rec.suggestedLimitPrice}]` : ""), | ||
| (rec.suggestedLimitPrice | ||
| ? ` [limit: ${rec.suggestedLimitPrice} ${rec.originalCurrency}]` |
There was a problem hiding this comment.
The summary log labels suggestedLimitPrice with rec.originalCurrency, but the prompts/state treat all monetary values (including limits) as being denominated in defaultCurrency. For cross-currency tickers this will print the wrong currency next to the limit. Suggest logging the limit in defaultCurrency (or explicitly log both: defaultCurrency amount + original currency note).
| ? ` [limit: ${rec.suggestedLimitPrice} ${rec.originalCurrency}]` | |
| ? ` [limit: ${rec.suggestedLimitPrice} ${defaultCurrency}]` |
| const rawCurrency = result.price?.currency ?? "USD"; | ||
| const subUnit = SUB_UNIT_FIX[rawCurrency]; | ||
| const originalCurrency = subUnit ? subUnit.realCurrency : rawCurrency; | ||
| const priceDivisor = subUnit ? subUnit.divisor : 1; |
There was a problem hiding this comment.
When result.price?.currency is missing you default to "USD" silently. The PR description says this case should emit a console.warn; consider warning here when the currency field is absent so users can detect ambiguous Yahoo data (especially if their defaultCurrency is not USD).
| USD: { prefix: "$", decimals: 0 }, | ||
| GBP: { prefix: "£", decimals: 0 }, | ||
| EUR: { prefix: "€", decimals: 0 }, | ||
| JPY: { prefix: "¥", decimals: 0 }, | ||
| AUD: { prefix: "A$", decimals: 0 }, | ||
| CAD: { prefix: "CA$", decimals: 0 }, | ||
| NZD: { prefix: "NZ$", decimals: 0 }, | ||
| CHF: { prefix: "CHF ", decimals: 0 }, | ||
| HKD: { prefix: "HK$", decimals: 0 }, | ||
| SGD: { prefix: "S$", decimals: 0 }, | ||
| }; | ||
|
|
||
| export function formatMoney(amount: number, currency: string): string { | ||
| const fmt = CURRENCY_FORMAT[currency]; | ||
| if (!fmt) { | ||
| const negative = amount < 0; | ||
| const rounded = Math.round(Math.abs(amount)).toLocaleString("en-US"); | ||
| return `${negative ? "-" : ""}${rounded} ${currency}`; | ||
| } | ||
| const negative = amount < 0; | ||
| const rounded = Math.round(Math.abs(amount)).toLocaleString("en-US", { | ||
| minimumFractionDigits: fmt.decimals, | ||
| maximumFractionDigits: fmt.decimals, | ||
| }); | ||
| return `${negative ? "-" : ""}${fmt.prefix}${rounded}`; |
There was a problem hiding this comment.
formatMoney currently rounds via Math.round(Math.abs(amount)) and formats with minimum/maximumFractionDigits: fmt.decimals, but fmt.decimals is 0 for all supported currencies. This means per-share prices and limit prices will be rounded to whole units anywhere this helper is used. Consider supporting currency-appropriate decimals (e.g. 2 for most, 0 for JPY) and/or adding an explicit decimals override for callers that need precision.
| USD: { prefix: "$", decimals: 0 }, | |
| GBP: { prefix: "£", decimals: 0 }, | |
| EUR: { prefix: "€", decimals: 0 }, | |
| JPY: { prefix: "¥", decimals: 0 }, | |
| AUD: { prefix: "A$", decimals: 0 }, | |
| CAD: { prefix: "CA$", decimals: 0 }, | |
| NZD: { prefix: "NZ$", decimals: 0 }, | |
| CHF: { prefix: "CHF ", decimals: 0 }, | |
| HKD: { prefix: "HK$", decimals: 0 }, | |
| SGD: { prefix: "S$", decimals: 0 }, | |
| }; | |
| export function formatMoney(amount: number, currency: string): string { | |
| const fmt = CURRENCY_FORMAT[currency]; | |
| if (!fmt) { | |
| const negative = amount < 0; | |
| const rounded = Math.round(Math.abs(amount)).toLocaleString("en-US"); | |
| return `${negative ? "-" : ""}${rounded} ${currency}`; | |
| } | |
| const negative = amount < 0; | |
| const rounded = Math.round(Math.abs(amount)).toLocaleString("en-US", { | |
| minimumFractionDigits: fmt.decimals, | |
| maximumFractionDigits: fmt.decimals, | |
| }); | |
| return `${negative ? "-" : ""}${fmt.prefix}${rounded}`; | |
| USD: { prefix: "$", decimals: 2 }, | |
| GBP: { prefix: "£", decimals: 2 }, | |
| EUR: { prefix: "€", decimals: 2 }, | |
| JPY: { prefix: "¥", decimals: 0 }, | |
| AUD: { prefix: "A$", decimals: 2 }, | |
| CAD: { prefix: "CA$", decimals: 2 }, | |
| NZD: { prefix: "NZ$", decimals: 2 }, | |
| CHF: { prefix: "CHF ", decimals: 2 }, | |
| HKD: { prefix: "HK$", decimals: 2 }, | |
| SGD: { prefix: "S$", decimals: 2 }, | |
| }; | |
| export function formatMoney(amount: number, currency: string, decimals?: number): string { | |
| const fmt = CURRENCY_FORMAT[currency]; | |
| const negative = amount < 0; | |
| const fractionDigits = decimals ?? fmt?.decimals ?? 0; | |
| const formatted = Math.abs(amount).toLocaleString("en-US", { | |
| minimumFractionDigits: fractionDigits, | |
| maximumFractionDigits: fractionDigits, | |
| }); | |
| if (!fmt) { | |
| return `${negative ? "-" : ""}${formatted} ${currency}`; | |
| } | |
| return `${negative ? "-" : ""}${fmt.prefix}${formatted}`; |
| <tr> | ||
| <td style="padding:5px 3px;border-bottom:1px solid ${S.border};font-weight:bold;" title="${escapeHtmlAttr(item.tickerFullName ?? item.ticker)}">${item.ticker}</td> | ||
| <td style="padding:5px 3px;border-bottom:1px solid ${S.border};text-align:right;">$${item.price.toLocaleString("en-US", { maximumFractionDigits: 2 })}</td> | ||
| <td style="padding:5px 3px;border-bottom:1px solid ${S.border};text-align:right;">${fmt$(item.price)}</td> | ||
| <td style="padding:5px 3px;border-bottom:1px solid ${S.border};text-align:right;">${item.currentPct.toFixed(1)}%</td> |
There was a problem hiding this comment.
Allocation table Price values are now formatted with fmt$/formatMoney, which rounds to whole units. Previously this column allowed up to 2 fractional digits; rounding prices can materially change what the user sees and can impact decisions. Suggest formatting per-share prices with 2 decimals (or a dedicated price formatter) while keeping whole-unit formatting for portfolio totals.
| ${a.suggestedBuyValue > 0 ? `<div style="font-size:13px;font-weight:bold;color:#fff;">Suggested: ${fmt$(a.suggestedBuyValue)}</div>` : ""} | ||
| ${a.currentAction === "STRONG BUY" && a.suggestedLimitPrice && a.suggestedLimitPrice > 0 ? `<div style="font-size:12px;color:${S.green};margin-top:4px;">Limit order: $${a.suggestedLimitPrice.toFixed(2)}${a.limitPriceReason ? ` — ${a.limitPriceReason}` : ""}</div>` : ""} | ||
| ${a.currentAction === "STRONG BUY" && a.suggestedLimitPrice && a.suggestedLimitPrice > 0 ? `<div style="font-size:12px;color:${S.green};margin-top:4px;">Limit order: ${fmt$(a.suggestedLimitPrice)}${a.limitPriceReason ? ` — ${a.limitPriceReason}` : ""}</div>` : ""} | ||
| ${a.bottomSignal && a.bottomSignal !== "" ? `<div style="font-size:11px;color:${S.yellow};margin-top:4px;">Bottom signal: ${a.bottomSignal}</div>` : ""} |
There was a problem hiding this comment.
Intraday limit prices and refresh prices are now rendered via fmt$/formatMoney, which currently rounds to whole units. Limit/quote prices generally need cents/pips precision (and the previous implementation used toFixed(2)). Consider using a 2-decimal formatter for prices/limits (currency-specific, e.g. JPY=0) to avoid losing precision in alerts.
| lines.push(""); | ||
| lines.push( | ||
| `💰 <b>${fmt$(report.totalCurrentValue)}</b>` + | ||
| `💰 <b>${fmt$(report.totalCurrentValue)}</b> ${defaultCurrency}` + |
There was a problem hiding this comment.
This line appends ${defaultCurrency} after formatMoney(...). For currencies where formatMoney already includes an explicit code prefix (e.g. CHF -> "CHF 1,234"), the output becomes redundant/odd (e.g. "CHF 1,234 CHF"). Recommend either (a) removing the appended currency code when using formatMoney, or (b) changing formatMoney to always include/always omit the ISO code so call sites don’t need to guess.
| `💰 <b>${fmt$(report.totalCurrentValue)}</b> ${defaultCurrency}` + | |
| `💰 <b>${fmt$(report.totalCurrentValue)}</b>` + |
|
Hi @furic |
Summary
src/fetchFx.ts: fetches FX rates from Yahoo Finance (GBPUSD=Xconvention), one batch per run, in-memory onlysrc/fetchPrices.ts: captures Yahoocurrencyfield, applies sub-unit fix for GBp/GBX/ILA/ZAc (÷100), converts all 9 monetary fields todefaultCurrencyvia FX rate; emitscurrency+originalCurrencyon everyQuoteData; returns{ quotes, skipped, fxRates }src/fetchTechnicals.ts: converts OHLCV todefaultCurrencybefore computing indicators (SMA, ATR, Bollinger, recent lows) so all derived values are already in the user's currencytotalPortfolioValueUSD→totalPortfolioValue(hard error if old name present); newdefaultCurrencyfield (defaults to"USD", validated against an allowlist of 10 currencies)src/util.ts: newformatMoney(amount, currency)helper replaces four duplicatedfmt$helpers in email/Telegram modulesemail.ts,intradayEmail.ts,weeklyEmail.ts,telegram.ts):fmt$delegates toformatMoney; header shows· ${defaultCurrency}; conditional footer caveat when cross-currency tickers are present; FX-skipped tickers shown in daily email footeraiAnalysis.ts,detailedAnalysis.ts):CURRENCY:preamble in all three prompts; cross-currency tickers annotated with(originally GBP)for model contextsrc/analyze.ts:AllocationItemgainsoriginalCurrency: string; singleMath.maxrenamehow-it-works.mdupdated with new field name and currency support notesMigration required
If you have a
config.jsonwithtotalPortfolioValueUSD, rename it tototalPortfolioValueand add"defaultCurrency": "USD". The app throws a clear error if the old name is detected.Error handling
currencyfield missingdefaultCurrency;console.warndefaultCurrencynot in allowlistTest plan
npm run typecheck— passes cleannpm run format:check— passes cleanTSCO.L(GBp) andSAP.DE(EUR) totargetPortfolio, runnpm run dev, verify header shows correct currency, prices are USD-magnitude, footer caveat appears, original currencies are loggedCloses #7
🤖 Generated with Claude Code