Conversation
Replace the naive same-unit balance sum with a currency-aware earnings summary so the dashboard shows a correct total and real today / this-month figures instead of hardcoded $0.00 placeholders. - internal/exchange: CoinGecko (crypto->USD) + Frankfurter (USD->fiat) with a 15-min stale-graceful cache; ToUSD / FromUSD / ToDisplay / Convertible. Non- money points (GRASS) are never summed — shown separately. - store.ListDailyBalances: latest successful balance per (platform, day). - app.go: computeEarningsSummary — Total / Today / Month (+ change%), a 30-day daily-accrual chart series, per-service breakdown with payout progress (from the catalog cashout threshold, only when currencies are comparable), and a separate points list. Wired into AppState + a GetEarningsSummary binding; a background goroutine refreshes rates every CacheTTL. - frontend: real Total / Today / This-Month cards, FX-converted balances with the native amount as sub-text, payout progress bars, a points section, and a "rates stale" badge. Tests: internal/exchange (httptest), store.ListDailyBalances, and computeEarningsSummary (app_test.go). go build/vet + go test -race ./... green; frontend tsc --noEmit clean.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Address the review findings on the FX-normalized earnings summary: - Per-platform accrual deltas (was whole-portfolio subtraction): a platform's first-ever observation, or a baseline outside the fetch window, now contributes 0 to Today/Month/Daily instead of dumping its whole lifetime balance into the hero tiles and chart. - Widen the daily-balance window to reach the prior-month baseline so MonthChange is correct (was wrong ~2/3 of each month with a fixed 40-day window). - Classify points by intent (wire exchange.PointsCurrencies / IsPoints): a convertible currency that is temporarily unpriced (rate outage) is excluded from the total and flags rates-stale, rather than being mislabeled a non-money reward point and dropped. - store.ListDailyBalances: deterministic intra-day tie-break on MAX(id) — the RFC3339Nano created_at is lexicographically unsafe for same-second rows. - formatBalance: sanitize the currency code (closes two pre-existing unescaped innerHTML sinks); show the native balance when a display rate is missing. - Clamp payout Percent to [0,100]; require MinAmount>0 for Eligible. Tests: +6 (first-observation, carry-forward gap, month-window value, points-during-outage, id tie-break, IsPoints). go test -race ./... green; tsc clean.
Owner
Author
Review triage (Slice 2 earnings)Two in-loop review passes (earnings-math + frontend/tests) — no XSS in the new code; the findings were numeric-aggregation correctness issues, all fixed in Fixed
+6 regression tests (one fails under the old code, proving the fix). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slice 2 — FX-normalized multi-currency earnings
Stacked on Slice 0 (#29). Fixes the headline earnings bug: the dashboard summed mixed currencies as one unit (USD + crypto tokens + points) and showed hardcoded
$0.00for Today / This-Month.What this adds
internal/exchange— CoinGecko (crypto→USD) + Frankfurter (USD→fiat), 15-min stale-graceful cache;ToUSD/FromUSD/ToDisplay/Convertible. Non-money points (e.g. GRASS) are never summed.store.ListDailyBalances— latest successful balance per (platform, day).app.gocomputeEarningsSummary— Total / Today / This-Month (+ change %), a 30-day daily-accrual chart series, a per-service breakdown with payout progress (from the catalog cashout threshold, only when currencies are comparable), and a separate points list. Wired intoAppState+ aGetEarningsSummarybinding; a background goroutine refreshes rates every 15 min.Verified
go build/go vet/go test -race ./...green (incl. newinternal/exchange,ListDailyBalances, andcomputeEarningsSummarytests).tsc --noEmitclean.wails build(darwin/arm64) succeeds end-to-end on the Mac mini — bindings regenerate, frontend bundles,.appbuilds.Base is
slice0/reconcile-wip; it auto-retargets tomainwhen #29 merges.