chienchuanw/asset-manager

refactor: FIRE calculator follow-up polish (PR #129 review nits)

Open

#130 geöffnet am 19. Mai 2026

Auf GitHub ansehen
 (0 Kommentare) (0 Reaktionen) (0 zugewiesene Personen)Go (0 Forks)github user discovery
good first issue

Repository-Metriken

Stars
 (2 Stars)
PR-Merge-Metriken
 (PR-Metriken ausstehend)

Beschreibung

Summary

Three non-blocking nits surfaced during the review of PR #129 (FIRE calculator, #128, merged to dev). None affect correctness; tracked here as follow-up cleanup.

Items

1. Form placeholders show raw numbers

frontend/src/app/fire/page.tsx — the assumptions form inputs use the echoed derived value directly as the placeholder (placeholder={data ? String(echoedValue(data, key)) : tCommon("loading")}). Net worth / expenses / savings should be formatted as currency (see formatCurrency in src/lib/format.ts or the Money component) and expectedReturn / withdrawalRate should display as percentages (e.g. 5% instead of 0.05).

2. Chart number formatting hardcodes the en locale

frontend/src/app/fire/page.tsx — the YAxis tickFormatter and Tooltip formatter call new Intl.NumberFormat("en", ...). This should use the active next-intl locale (zh-TW / en) so the projection chart respects the user's language. Pull the locale from next-intl rather than hardcoding "en".

3. Backend does not validate a negative expectedReturn

backend/internal/service/fire_service.goGetProjection only rejects withdrawalRate <= 0 (ErrInvalidWithdrawalRate). A negative expectedReturn is accepted and silently produces a shrinking projection. Decide and implement intended behavior: either reject negative expectedReturn with a 400 (mirroring the withdrawal-rate guard, plumb through fire_handler.go) or explicitly document that negative real returns are allowed. Add a corresponding fire_service_test.go case.

Acceptance criteria

  • Currency inputs (netWorth, annualExpenses, annualSavings) show formatted-currency placeholders; rate inputs (expectedReturn, withdrawalRate) show percentage placeholders
  • Chart axis/tooltip number formatting uses the active i18n locale instead of hardcoded "en"
  • Negative expectedReturn handling is explicit (rejected with 400 or documented as allowed), with a fire_service_test.go test covering the chosen behavior
  • pnpm tsc --noEmit (FIRE files) and make test-unit remain green

Out of scope

Any new FIRE functionality (variants, sliders, persistence) — this issue is cleanup only.

Contributor Guide