Multi-currency: FX conversion and reporting currency by ItsThompson · Pull Request #56 · ItsThompson/gofin

ItsThompson · 2026-08-15T22:30:58Z

Summary

Implements FX-backed foreign-currency expense creation, mixed-currency expense suggestions and log, and reporting-currency authority across dashboard/history/health-score. Period reporting currency is the single source of truth for money formatting and computation. FX conversion safe-fails on provider outages: no ledger row is written and the form stays ready for retry. Legacy rows are migrated by mc/03 startup backfill; this PR adds integrity telemetry for rows missing required snapshot fields.

What changed

Backend (Go)

Expense FX wiring (fx_client.go, expense.go, config/): Injected FX gRPC client via FX_SERVICE_ADDR. Create-expense validates currencies, calls ConvertAmount when transaction != reporting currency, and writes both money snapshots. FX failures map to CONVERSION_UNAVAILABLE (503) with no ledger write.
Expense suggestions (model/suggestions.go, service/suggestions.go, repository/immudb.go): Suggestion models include canonical transaction amount and currency; repository selects and maps transaction_amount/transaction_currency directly.
Health score (healthscore_service.go, healthscore.go, healthscore_trend.go): computeHealthScore uses period.ReportingCurrency instead of default settings. FormulaVersion bumped 2 -> 3. Stored scores get backfilled reporting currency. Trend points carry per-period reporting currency.
Dashboard and reporting (dashboard.go, model/requests.go): Spending trends and historical comparison carry per-point reporting currency. HistoricalComparison adds PreviousReportingCurrency and Comparable; cross-currency pairs suppress change percent and rolling average.
Reporting currency migration (000006_add_period_reporting_currency.*.sql): Three-step backfill (default settings currency, auth user currency, app fallback). Validates every row has a supported currency before adding NOT NULL + CHECK constraint.
Snapshot integrity telemetry (repository/immudb.go): Rows missing required snapshot fields return a typed SnapshotIntegrityError and emit expense_snapshot_integrity_error telemetry.

Frontend (TypeScript/React)

Expense autocomplete: Suggestion items display transaction currency code. Accepting a suggestion fills transaction amount and currency; foreign-currency suggestions trigger FX conversion on submit.
Expense log (expense-table-columns.tsx): Mixed-currency rows show transaction amount and secondary reporting amount labeled "Budget impact". Amount column sorts by reporting amount. Transaction and reporting currency filters are distinct.
History: Rows format with row.period.reportingCurrency instead of user.currency. Adjacent rows with different currencies show "Δ not comparable (different currency)".
Dashboard widgets: Historical comparison formats previous period in its own currency and guards change percent with comparable. Recent expenses use reportingAmount.
Core types: Added reportingCurrency to HealthScore, HealthScoreTrendPoint, and TrendPoint; added previousReportingCurrency and comparable to HistoricalComparison; made transactionAmount, reportingAmount, and reportingCurrency required on Expense.

Tests

All suites pass: expense, finance, fx, and frontend (finance 53 files/509 tests, core 88, api 187). gofmt and go vet clean.

Notes

Migration 000006 hardcodes USD as the app fallback currency; a configurable fallback can be a follow-up.
SpendingTrendChart uses a single period currency for Y-axis formatting; mixed-currency trend-axis normalization is a candidate follow-up.
Formula version bump 2 -> 3 forces lazy recompute of stored health scores (old scores used the default settings currency).

Wire Expense Service to FX Service for non-identity expense creation. When transactionCurrency != reportingCurrency, the service calls FX ConvertAmount before writing the ledger row, stores the provider snapshot (transaction amount/currency, converted reporting amount/currency, exchange rate, source, timestamp, expiry), and returns both money fields in the response. FX failures (CONVERSION_UNAVAILABLE, provider auth failure, provider response invalid, missing live rate) map to safe REST error CONVERSION_UNAVAILABLE (503) with no ledger write. - Add FX_SERVICE_ADDR config and GRPCFxClient in fx_client.go - Add FxClient interface with ConvertAmount, inject into ExpenseService - Foreign-currency path calls FX then builds provider snapshot - All FX gRPC errors map to conversionUnavailableError (no ledger write) - Unsupported transaction currency validated before FX call (400) - Frontend shows conversion-unavailable guidance toast and preserves form - docker-compose.yml adds FX_SERVICE_ADDR to expense-service - Tests: exact FX requests, exact repo writes, no writes on failure, REST/gRPC error mappings, frontend FX failure banner

… reporting currency to response models - computeHealthScore now uses period.ReportingCurrency instead of fetching default settings, so health-score insights remain stable after default currency changes - Added ReportingCurrency field to HealthScore and HealthScoreTrendPoint models - Bumped FormulaVersion from 2 to 3 (JSON shape change) and generated v3 golden snapshot - Backfill ReportingCurrency on stored scores persisted before the field existed - HistoricalComparison now carries PreviousReportingCurrency and Comparable flag to guard cross-currency amount comparisons - TrendPoint now carries ReportingCurrency per point for mixed-currency trends - ComputeSpendingTrends sets ReportingCurrency from each period

…d types - History rows now format with each period's reportingCurrency instead of user.currency; adjacent rows with different currencies hide amount deltas with a 'not comparable' label - HistoricalComparisonWidget formats previous period in its own currency and guards change-percent display with the comparable flag - RecentExpenses uses reportingAmount (budget impact) instead of transaction amount, with amount fallback for legacy rows - Added reportingCurrency to HealthScore, HealthScoreTrendPoint, TrendPoint, and HistoricalComparison TypeScript types - Added money snapshot fields (reportingAmount, reportingCurrency, etc.) to Expense type for budget-impact display - Updated test fixtures and added tests for JPY formatting, mixed-currency delta guards, and same-currency delta display

…ration

…resolution

…ency across read paths Move the service-layer legacy snapshot resolution out of the FX-wiring commit. Every read path (expense list, detail, correction history, pro-rata group, and export stream) resolves repository-synthesized legacy rows to the period reporting currency and emits normalization telemetry, without failing the read when period context is unavailable.

…mount sort and currency filters

- gofmt suggestions model/service files - rename misleading legacy-suggestion test - name and document the reporting-currency fallback - label dual amounts as budget impact for accessibility - scope mobile mixed-currency test to the mobile list - make canonical suggestion fields optional so fallbacks are meaningful - add foreign-suggestion-currency integration test

…er review-11

…l work

- Collapse mapFxError to a single safe CONVERSION_UNAVAILABLE mapping; the dead gRPC-code switch implied distinctions that do not exist. - Normalize the period reporting currency once in CreateExpense and use it for validation, the identity-vs-FX decision, snapshots, and the FX target. - Require a non-nil FxClient: NewExpenseService panics on nil instead of leaking a test-only runtime fallback; tests use a loud stubFxClient. - Return and close the FX gRPC connection in main.go, matching the finance client pattern. - Drop unused FxConvertResponse echo fields and use the shared exchange source constant in tests. - Assert RequestedAt in FX success tests via a fixed clock seam.

Spec 05 requires Expense to preserve FX error categories rather than collapse every gRPC status into 503. mapFxError now inspects status.Code: - Unavailable/FailedPrecondition -> 503 CONVERSION_UNAVAILABLE - InvalidArgument UNSUPPORTED_CURRENCY -> 400 UNSUPPORTED_CURRENCY - InvalidArgument INVALID_AMOUNT -> 400 VALIDATION_ERROR - Internal/unclassified -> 500 INTERNAL_SERVER_ERROR (reported) - non-gRPC transport failure -> 503 CONVERSION_UNAVAILABLE Also address review-7 nits: report an unsupported period reporting currency as a 500 internal invariant violation (not a retryable 503), rename the misnamed FX-client-unavailable test, and replace the always-true repo matcher with mock.AnythingOfType.

…g currency first - Log non-503 FX failures (Internal/unclassified) as Error with event foreign_currency_conversion_failed instead of the misleading foreign_currency_conversion_unavailable Info event. - Validate the period reporting currency before resolving the transaction currency so a corrupted reporting currency surfaces as a 500 internal invariant violation in every branch, including the no-currency-fields defaulting path, instead of a 400 transaction-currency error. - Add tests for both behaviors.

- Remove LegacySynthesized/PartialSnapshotFields and service-layer legacy normalization; rely on InitSchema backfill and version-0 error. - Read suggestion transaction columns directly without the legacy amount/currency fallback; keep version-1 integrity telemetry. - Update finance tests for the removed reporting-amount fallback and read the shared currency catalog from code.

- Make transactionAmount, reportingAmount, and reportingCurrency required and read them directly without legacy fallbacks. - Update finance and shell fixtures/mocks to supply the required snapshot fields. - Fix shell mock reporting-currency fields for comparison, health score, and trends.

ItsThompson · 2026-08-20T16:44:29Z

+  const toggleTransactionCurrency = useCallback((currency: string) => {
+    setCriteria((prev) => {
+      const next = new Set(prev.selectedTransactionCurrencies);
+      if (next.has(currency)) {
+        next.delete(currency);
+      } else {
+        next.add(currency);
+      }
+      return { ...prev, selectedTransactionCurrencies: next };
+    });
+  }, []);
+
+  const toggleReportingCurrency = useCallback((currency: string) => {
+    setCriteria((prev) => {
+      const next = new Set(prev.selectedReportingCurrencies);
+      if (next.has(currency)) {
+        next.delete(currency);
+      } else {
+        next.add(currency);
+      }
+      return { ...prev, selectedReportingCurrencies: next };
+    });
+  }, []);


we repeat the same pattern here, wondering if we can just abstract it out?

ItsThompson · 2026-08-20T16:46:11Z

 export interface ExpenseFilters {
  /** Current filter criteria. */
  criteria: FilterCriteria;
  /** Whether the filter panel is visible. */
  showFilters: boolean;
  /** Derived: true if any filter is active. */
  hasActiveFilters: boolean;
  /** Toggle a type in/out of the selected set. */
  toggleType: (type: string) => void;
  /** Toggle a tag in/out of the selected set. Syncs URL param. */
  toggleTag: (tagId: string) => void;
+  /** Toggle a transaction currency in/out of the selected set. */
+  toggleTransactionCurrency: (currency: string) => void;
+  /** Toggle a reporting currency in/out of the selected set. */
+  toggleReportingCurrency: (currency: string) => void;
  /** Toggle filter panel visibility. */
  toggleFilters: () => void;
  /** Reset all filters to empty. */
  clearFilters: () => void;
  /** Set the start date filter. */
  setDateFrom: (value: string) => void;
  /** Set the end date filter. */
  setDateTo: (value: string) => void;
 }


Comments should explain the WHY not the WHAT. Please remove.

ItsThompson · 2026-08-20T17:18:23Z

+ * loaded period: it only prevents an undefined-currency pass into the detail
+ * modal and row enrichment during the initial fetch window.
+ */
+export const FALLBACK_REPORTING_CURRENCY = "USD";


no this is not correct, there shouldnt be a constant for FALLBACK_REPORTING_CURRENCY.

If the period isn't loaded yet, there shouldnt be a fallback, we should wait for the period to load instead.

If it fails to load, then we should just return an error instead. Its like if expenses fail to load (i.e. empty expense log, error toast, etc.)

ItsThompson · 2026-08-20T17:20:27Z

+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+export function HistoryFeature(_props: FinancePageProps) {


// eslint-disable-next-line @typescript-eslint/no-unused-vars is an antipattern, unless there is an actual reason that I didnt see, lets remove the unused var instead please.

ItsThompson · 2026-08-20T17:20:55Z

comments in the entire file... WHY not WHAT.

ItsThompson · 2026-08-20T18:27:44Z

+type SnapshotIntegrityError struct {
+	ExpenseID string
+}
+
+func (e *SnapshotIntegrityError) Error() string {
+	return fmt.Sprintf("expense row %s: missing required snapshot fields", e.ExpenseID)
+}


The SnapshotIntegrityError path reports to Sentry via errkit.Report, but the propagated error only carries the ExpenseID, and it reaches Sentry embedded in the message text, not as structured data. The missing-field detail lives only in the log record (expense_snapshot_integrity_error), so a report on a corrupt row shows which row failed but not what is wrong with it.

Suggest carrying the detail on the error so it propagates with the wrap chain:

Add a MissingFields []string field to SnapshotIntegrityError and populate it in rowToExpense; each required field is already checked there, so the value is known at the point of failure.

Include the missing fields in Error() so the wrapped message that reaches Sentry names them.

At the report site, extract the error via errors.As and pass the fields into Meta.Data so the report carries them as structured fields in the "gofin" context block.

This keeps the log and the Sentry report aligned with no extra plumbing.

ItsThompson · 2026-08-20T18:30:54Z

+// severity matches the failure: retryable conversion outages are Info, while
+// unexpected FX server failures (500) are Error so they are visible as defects
+// rather than as routine provider outages.
+func logFxConversionFailure(logger *slog.Logger, err error, transactionCurrency, reportingCurrency string) {


we should be logging to sentry via errkit.Report rather than using logger.Error.

ItsThompson · 2026-08-20T18:56:02Z

+			Amount:              group.latest.TransactionAmount,   // Deprecated: mirrors transaction value.
+			Currency:            group.latest.TransactionCurrency, // Deprecated: mirrors transaction value.


Same as my comment in services/expense/internal/model/suggestions.go: Please remove Amount and Currency since they are now deprecated rather than keeping them.

ItsThompson · 2026-08-20T19:08:11Z

Comments in this entire file, why not what.

a lot of the comments can just be removed.

ItsThompson · 2026-08-20T19:09:29Z

 	if len(priorPeriods) > 0 {
 		prevSpent := spent[1]
 		result.PreviousSpent = prevSpent
-
-		if prevSpent > 0 {
+		result.PreviousReportingCurrency = priorPeriods[0].ReportingCurrency
+
+		// Cross-currency comparison guard: when the previous period's reporting
+		// currency differs from the current period's, the amount delta and
+		// change percent are not meaningful and the frontend must guard the display.
+		currentCurrency := periods[requestedIdx].ReportingCurrency
+		if currentCurrency != "" && priorPeriods[0].ReportingCurrency != "" && currentCurrency != priorPeriods[0].ReportingCurrency {
+			result.Comparable = false
+			result.ChangePercent = 0
+		} else if prevSpent > 0 {
 			result.ChangePercent = math.Round(float64(spent[0]-prevSpent)/float64(prevSpent)*10000) / 100
 		} else if spent[0] > 0 {
 			result.ChangePercent = 100.0
 		}
 	}

-	// Rolling average: need at least 3 prior periods
+	// Rolling average: need at least 3 prior periods in the same reporting currency.
+	// When the prior periods span mixed currencies the average is not meaningful.
 	if hasRollingAverage {
-		avg := (spent[1] + spent[2] + spent[3]) / 3
-		result.RollingAverage = &avg
+		currentCurrency := periods[requestedIdx].ReportingCurrency
+		allSameCurrency := currentCurrency != ""
+		for _, pp := range priorPeriods[:3] {
+			if pp.ReportingCurrency != currentCurrency {
+				allSameCurrency = false
+				break
+			}
+		}
+		if allSameCurrency {
+			avg := (spent[1] + spent[2] + spent[3]) / 3
+			result.RollingAverage = &avg
+		}
 	}


can we please flatten the nested if statements please.

ItsThompson changed the title ~~mc/04 fx conversion legacy reporting~~ Multi-currency: FX conversion, legacy snapshot migration, and reporting Aug 15, 2026

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch from 228597a to 91a8e19 Compare August 15, 2026 22:49

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch from 91a8e19 to 83ef524 Compare August 15, 2026 22:55

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch from 83ef524 to da072b0 Compare August 15, 2026 23:10

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch from 402e662 to 2d68e9b Compare August 15, 2026 23:53

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch from 2d68e9b to acd9ed1 Compare August 16, 2026 14:52

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch from acd9ed1 to f548caf Compare August 16, 2026 15:17

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch 2 times, most recently from 3e6fa9a to ea2bf5f Compare August 16, 2026 18:45

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch from ea2bf5f to 002148a Compare August 18, 2026 23:36

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch 2 times, most recently from 9bc77ae to 60d4235 Compare August 18, 2026 23:51

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch from 60d4235 to 89e07fb Compare August 19, 2026 06:30

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch 3 times, most recently from dd99103 to 7f1e87f Compare August 20, 2026 14:58

Base automatically changed from mc/03-identity-money-snapshots to main August 20, 2026 15:10

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch from 7f1e87f to 34e0698 Compare August 20, 2026 15:10

ItsThompson added 20 commits August 21, 2026 00:02

feat(finance): record auth and app fallback in reporting currency mig…

06184d7

…ration

feat(expense): flag legacy rows and partial snapshot fields for read …

18f8b58

…resolution

test(expense): cover legacy snapshot resolution across read paths

d16af74

feat(expense): preserve transaction currency in expense suggestions

6c9ea2f

feat(frontend): autofill transaction currency from expense suggestions

2210cb3

feat(frontend): show mixed-currency expense log rows with reporting-a…

d6018d9

…mount sort and currency filters

fix(expense): wire integrity telemetry and harden legacy resolution p…

02ee2c1

…er review-11

chore: consolidate fx Dockerfile and workspace checksums from paralle…

54d7de5

…l work

fix(expense): check requireAPIError return value in test

de39858

fix(expense): copy fx module in Dockerfile

43790ef

ItsThompson force-pushed the mc/04-fx-conversion-legacy-reporting branch from 34e0698 to 5eeafb3 Compare August 20, 2026 16:04

ItsThompson changed the title ~~Multi-currency: FX conversion, legacy snapshot migration, and reporting~~ Multi-currency: FX conversion and reporting currency Aug 20, 2026

docs: document FX service env vars in .env.example

2286d96

ItsThompson commented Aug 20, 2026

View reviewed changes

Navigation Menu

Multi-currency: FX conversion and reporting currency - #56

Multi-currency: FX conversion and reporting currency#56
ItsThompson wants to merge 21 commits into
mainfrom
mc/04-fx-conversion-legacy-reporting

ItsThompson commented Aug 15, 2026 •

edited

Loading

Uh oh!

ItsThompson Aug 20, 2026

Uh oh!

ItsThompson Aug 20, 2026

Uh oh!

ItsThompson Aug 20, 2026

Uh oh!

ItsThompson Aug 20, 2026

Uh oh!

ItsThompson Aug 20, 2026

Uh oh!

ItsThompson Aug 20, 2026

Uh oh!

ItsThompson Aug 20, 2026

Uh oh!

ItsThompson Aug 20, 2026

Uh oh!

ItsThompson Aug 20, 2026

Uh oh!

ItsThompson Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		// eslint-disable-next-line @typescript-eslint/no-unused-vars
		export function HistoryFeature(_props: FinancePageProps) {

		Amount: group.latest.TransactionAmount, // Deprecated: mirrors transaction value.
		Currency: group.latest.TransactionCurrency, // Deprecated: mirrors transaction value.

Conversation

ItsThompson commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Backend (Go)

Frontend (TypeScript/React)

Tests

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ItsThompson commented Aug 15, 2026 •

edited

Loading