chienchuanw/asset-manager

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

Open

#130 aperta il 19 mag 2026

Vedi su GitHub
 (0 commenti) (0 reazioni) (0 assegnatari)Go (0 fork)github user discovery
good first issue

Metriche repository

Star
 (2 star)
Metriche merge PR
 (Metriche PR in attesa)

Descrizione

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.

Guida contributor