chienchuanw/asset-manager

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

Open

#130 创建于 2026年5月19日

在 GitHub 查看
 (0 评论) (0 反应) (0 负责人)Go (0 fork)github user discovery
good first issue

仓库指标

Star
 (2 star)
PR 合并指标
 (PR 指标待抓取)

描述

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.

贡献者指南