Add Crossover Report #5554
Add Crossover Report #5554
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
sjones512
force-pushed
the
feat/add-crossover-report
branch
from
0d22973 to
9fb7785
Compare
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller
Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded
Removed
Bigger No assets were bigger Smaller No assets were smaller Unchanged No assets were unchanged |
WalkthroughAdds a Crossover reporting feature: new routes (/crossover, /crossover/:id), a Crossover report UI and card (Crossover, CrossoverCard), a CrossoverGraph visualization, an AccountSelector component, and a crossover-spreadsheet module implementing data aggregation, Hampel/trend projections, and crossover detection. Dashboard types and import/validation are updated to accept a new Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
packages/loot-core/src/types/models/dashboard.ts (1)
66-77: Consider renamingincomeAccountIdstoinvestmentAccountIdsfor clarityThese accounts are used to derive investment returns and projected investment income, not income categories. The current name may be confused with income categories or transaction income. If renaming across the codebase is heavy, add a short doc comment clarifying that these are investment accounts.
- incomeAccountIds?: string[]; + // Accounts whose balances are treated as investments providing income via SWR + incomeAccountIds?: string[];packages/loot-core/src/server/dashboard/app.ts (1)
90-96: Optional: add shape validation forcrossover-cardmetaCurrently only custom reports have meta validated. If you want to harden imports, consider validating the
metashape forcrossover-card(e.g., arrays of ids, numeric ranges for SWR/returns). This would prevent malformed dashboards from being imported.packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (1)
141-199: ResponsiveContainer is redundant with explicit width/heightYou already measure width/height via Container’s AutoSizer and pass them to LineChart. Wrapping LineChart in ResponsiveContainer is unnecessary and adds complexity.
- {(width, height) => ( - <ResponsiveContainer> - <div style={{ ...(!compact && { marginTop: '15px' }) }}> - <LineChart - width={width} - height={height} - data={graphData.data} - margin={{ - top: 0, - right: 0, - left: compact ? 0 : 20, - bottom: compact ? 0 : 10, - }} - > + {(width, height) => ( + <div style={{ ...(!compact && { marginTop: '15px' }) }}> + <LineChart + width={width} + height={height} + data={graphData.data} + margin={{ + top: 0, + right: 0, + left: compact ? 0 : 20, + bottom: compact ? 0 : 10, + }} + > {!compact && <CartesianGrid strokeDasharray="3 3" />} <XAxis dataKey="x" hide={compact} tick={{ fill: theme.pageText }} tickLine={{ stroke: theme.pageText }} /> <YAxis hide={compact} tickFormatter={tickFormatter} tick={{ fill: theme.pageText }} tickLine={{ stroke: theme.pageText }} /> {showTooltip && ( <Tooltip content={<CustomTooltip />} isAnimationActive={false} /> )} {graphData.crossoverXLabel && ( <ReferenceLine x={graphData.crossoverXLabel} stroke={theme.noticeText} strokeDasharray="4 4" /> )} <Line type="monotone" dataKey="investmentIncome" dot={false} stroke={theme.reportsBlue} strokeWidth={2} animationDuration={0} /> <Line type="monotone" dataKey="expenses" dot={false} stroke={theme.reportsRed} strokeWidth={2} animationDuration={0} /> - </LineChart> - </div> - </ResponsiveContainer> + </LineChart> + </div> )}packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (3)
196-227: Inefficient per-month.findon balancesFor each account and month you do
acct.balances.find(...), which is O(n^2) across months. Probably fine for small ranges, but trivial to optimize by mappingbalancesby date once.- for (const acct of historicalAccounts) { + for (const acct of historicalAccounts) { + const balanceByMonth = new Map(acct.balances.map(b => [b.date, b.amount])); let runningBalance = acct.starting; for (let i = 0; i < months.length; i++) { const month = months[i]; - const found = acct.balances.find(b => b.date === month); - - if (found) { - runningBalance += found.amount; + const delta = balanceByMonth.get(month); + if (delta != null) { + runningBalance += delta; } historicalBalances[i] += runningBalance; } }
257-281: Annual return derivation should use compounding for accuracyYou compute a monthly CAGR and present “annual” as
monthly * 12. More accurate:(1 + monthly) ^ 12 - 1.- historicalReturn: - defaultMonthlyReturn != null ? defaultMonthlyReturn * 12 : null, + historicalReturn: + defaultMonthlyReturn != null + ? Math.pow(1 + defaultMonthlyReturn, 12) - 1 + : null,(Note: see full return block change below.)
320-352: Projection guardrails and zero-return behaviorIf no monthly return can be inferred or provided,
projectedBalanceremains flat, and the loop will never reach crossover unless expenses decline. This is acceptable, but consider adding a soft stop condition (e.g., “no projected crossover”) to avoid implying progress. You already cap at 600 months.packages/desktop-client/src/components/reports/AccountSelector.tsx (4)
172-174: Redundant array inclusion check.The
allOnBudgetSelectedvariable already confirms all items are selected, so checking if they're "included" again is redundant.This check can be simplified since we already know from
onBudgetSelected(line 52-54) whether all on-budget accounts are selected:- const allOnBudgetSelected = onBudgetAccountIds.every(id => - selectedAccountIds.includes(id), - ); - - if (allOnBudgetSelected) { + if (onBudgetSelected) {
261-263: Redundant array inclusion check for off-budget accounts.Similar to the on-budget section, this check is redundant.
Simplify by using the already computed
offBudgetSelected:- const allOffBudgetSelected = offBudgetAccountIds.every(id => - selectedAccountIds.includes(id), - ); - - if (allOffBudgetSelected) { + if (offBudgetSelected) {
349-351: Redundant array inclusion check for closed accounts.Similar to the other sections, this check is redundant.
Simplify by using the already computed
closedSelected:- const allClosedSelected = closedAccountIds.every(id => - selectedAccountIds.includes(id), - ); - - if (allClosedSelected) { + if (closedSelected) {
186-190: Consider using Set for better performance.When selecting all accounts in a group, the current implementation uses array operations which have O(n) complexity for includes() checks.
For better performance with larger account lists, consider using a Set:
} else { // Select all on-budget accounts - const newSelection = [...selectedAccountIds]; - onBudgetAccountIds.forEach(id => { - if (!newSelection.includes(id)) { - newSelection.push(id); - } - }); - setSelectedAccountIds(newSelection); + const selectionSet = new Set(selectedAccountIds); + onBudgetAccountIds.forEach(id => selectionSet.add(id)); + setSelectedAccountIds(Array.from(selectionSet)); }The same optimization applies to lines 274-280 and 362-368.
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (1)
195-197: Consider using a formatter for consistent decimal display.The years display uses
toFixed(1)inline, but currency formatting usesintegerToCurrency. Consider extracting a formatter for consistency.- {yearsToRetire != null - ? `${yearsToRetire.toFixed(1)} years` - : 'N/A'} + {yearsToRetire != null + ? t('{{years}} years', { years: yearsToRetire.toFixed(1) }) + : t('N/A')}packages/desktop-client/src/components/reports/reports/Crossover.tsx (3)
133-134: Inconsistent default behavior for expense categories.When no saved expense categories exist, the code defaults to all non-income categories. However, for income accounts (line 144), it defaults to all accounts when none are saved. This asymmetry might be confusing - consider whether both should follow the same pattern (either default to all or default to none).
418-421: Input validation could be more robust.The withdrawal rate input allows values from 0-100, but mathematically, a 100% withdrawal rate doesn't make practical sense. Consider adding more reasonable bounds.
setSwr( - Math.max(0, Math.min(100, Number(e.target.value))) / 100, + Math.max(0, Math.min(20, Number(e.target.value))) / 100, )Also update the max attribute:
- max={100} + max={20}
531-532: Currency formatting with potential precision loss.The code multiplies by 100 then rounds before formatting, which could introduce rounding errors for certain values.
Consider passing the raw value and let
integerToCurrencyhandle the conversion:- ? integerToCurrency(Math.round(targetMonthlyIncome * 100)) + ? integerToCurrency(targetMonthlyIncome)Note: This assumes
targetMonthlyIncomeis already in the integer format expected byintegerToCurrency. If not, verify the data format from the spreadsheet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
upcoming-release-notes/5554.mdis excluded by!**/*.md
📒 Files selected for processing (9)
packages/desktop-client/src/components/reports/AccountSelector.tsx(1 hunks)packages/desktop-client/src/components/reports/Overview.tsx(3 hunks)packages/desktop-client/src/components/reports/ReportRouter.tsx(2 hunks)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/Crossover.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts(1 hunks)packages/loot-core/src/server/dashboard/app.ts(1 hunks)packages/loot-core/src/types/models/dashboard.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError).
Favor named exports for components and utilities.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use objects or maps instead.
Avoid usinganyorunknownunless absolutely necessary. Look for type definitions in the codebase instead.
Avoid type assertions withasor!; prefer usingsatisfies.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsxpackages/desktop-client/src/components/reports/AccountSelector.tsxpackages/loot-core/src/types/models/dashboard.tspackages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.tspackages/desktop-client/src/components/reports/ReportRouter.tsxpackages/loot-core/src/server/dashboard/app.tspackages/desktop-client/src/components/reports/reports/CrossoverCard.tsxpackages/desktop-client/src/components/reports/Overview.tsxpackages/desktop-client/src/components/reports/reports/Crossover.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
Use declarative JSX, keeping JSX minimal and readable.
Files:
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsxpackages/desktop-client/src/components/reports/AccountSelector.tsxpackages/desktop-client/src/components/reports/ReportRouter.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsxpackages/desktop-client/src/components/reports/Overview.tsxpackages/desktop-client/src/components/reports/reports/Crossover.tsx
🧠 Learnings (2)
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/desktop-client/src/components/reports/AccountSelector.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-10-04T05:13:58.322Z
Learnt from: tlesicka
PR: actualbudget/actual#3554
File: packages/desktop-client/src/components/sidebar/Accounts.tsx:60-64
Timestamp: 2024-10-04T05:13:58.322Z
Learning: The `onReorder` function in `Accounts.tsx` was moved from `Sidebar.tsx`, and the `targetId` parameter remains typed as `unknown` intentionally.
Applied to files:
packages/desktop-client/src/components/reports/AccountSelector.tsx
🧬 Code Graph Analysis (7)
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (6)
packages/component-library/src/styles.ts (1)
CSSProperties(7-7)packages/desktop-client/src/hooks/usePrivacyMode.ts (1)
usePrivacyMode(3-6)packages/loot-core/src/shared/util.ts (2)
amountToCurrencyNoDecimal(437-442)amountToCurrency(433-435)packages/component-library/src/theme.ts (1)
theme(1-204)packages/component-library/src/View.tsx (1)
View(14-33)packages/desktop-client/src/components/reports/Container.tsx (1)
Container(14-31)
packages/desktop-client/src/components/reports/AccountSelector.tsx (7)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/component-library/src/View.tsx (1)
View(14-33)packages/component-library/src/Button.tsx (1)
Button(139-185)packages/component-library/src/icons/v2/index.ts (4)
SvgViewShow(52-52)SvgViewHide(51-51)SvgCheckAll(13-13)SvgUncheckAll(48-48)packages/component-library/src/Text.tsx (1)
Text(19-28)packages/desktop-client/src/components/reports/GraphButton.tsx (1)
GraphButton(17-47)packages/desktop-client/src/components/forms.tsx (1)
Checkbox(70-136)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (4)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/desktop-client/src/hooks/useSpreadsheet.tsx (1)
useSpreadsheet(19-25)packages/desktop-client/src/components/reports/spreadsheets/recalculate.ts (1)
recalculate(27-116)packages/loot-core/src/shared/util.ts (1)
integerToAmount(489-495)
packages/desktop-client/src/components/reports/ReportRouter.tsx (1)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (1)
Crossover(69-81)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (10)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/loot-core/src/types/models/dashboard.ts (1)
CrossoverWidget(66-77)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (1)
createCrossoverSpreadsheet(54-176)packages/desktop-client/src/components/reports/ReportCard.tsx (1)
ReportCard(34-119)packages/component-library/src/View.tsx (1)
View(14-33)packages/desktop-client/src/components/reports/ReportCardName.tsx (1)
ReportCardName(17-55)packages/component-library/src/Block.tsx (1)
Block(12-17)packages/component-library/src/styles.ts (1)
styles(16-157)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (1)
CrossoverGraph(43-202)packages/desktop-client/src/components/reports/LoadingIndicator.tsx (1)
LoadingIndicator(13-32)
packages/desktop-client/src/components/reports/Overview.tsx (2)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (1)
CrossoverCard(56-225)packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx (1)
accounts(1345-1345)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (10)
packages/desktop-client/src/hooks/useWidget.ts (1)
useWidget(8-21)packages/loot-core/src/types/models/dashboard.ts (2)
CrossoverWidget(66-77)TimeFrame(4-14)packages/loot-core/src/types/models/category.ts (1)
CategoryEntity(3-13)packages/desktop-client/src/hooks/useSpreadsheet.tsx (1)
useSpreadsheet(19-25)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (1)
createCrossoverSpreadsheet(54-176)packages/desktop-client/src/components/reports/Header.tsx (1)
Header(55-290)packages/desktop-client/src/components/reports/CategorySelector.tsx (1)
CategorySelector(31-271)packages/desktop-client/src/components/reports/AccountSelector.tsx (1)
AccountSelector(26-421)packages/loot-core/src/shared/util.ts (1)
integerToCurrency(422-431)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (1)
CrossoverGraph(43-202)
🔇 Additional comments (10)
packages/loot-core/src/types/models/dashboard.ts (2)
66-77: CrossoverWidget type looks consistent with existing widget patterns
- Meta is nullable, matching other widgets.
- Field choices (safeWithdrawalRate, estimatedReturn, projectionType) are sensible and align with the rest of the PR.
No blockers.
83-90: SpecializedWidget union updated correctlyIncluding CrossoverWidget in the union is necessary to plumb validation/export/import and widget management. Looks good.
packages/loot-core/src/server/dashboard/app.ts (1)
74-85: Whitelist addition for 'crossover-card' is correctThe new widget type is allowed during import/validation. This aligns with the new SpecializedWidget union.
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (1)
135-139: Compact mode may break AutoSizer heightOverriding Container height to 'auto' in compact mode can result in a zero or undefined height, causing the chart to render with zero height. Verify that parent layouts guarantee a fixed height when compact is true; otherwise, prefer a min-height or rely on the Container’s default fixed height.
You can inspect other graphs’ compact behavior or test visually. If it breaks, consider this adjustment:
- <Container - style={{ - ...style, - ...(compact && { height: 'auto' }), - }} - > + <Container + style={{ + ...style, + ...(compact && { minHeight: 160 }), // ensures AutoSizer has height + }} + >packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (1)
116-124: Confirmed — calculate() queries return a numericdata(no change needed)Short explanation: server-side runCompiledAqlQuery returns a scalar for calculation queries (returns the single column value or 0), and compileAndRunAqlQuery wraps that as { data, dependencies }. The client aqlQuery simply forwards the server response, so using .then(({ data }) => data as number) is correct.
Files inspected:
- packages/loot-core/src/server/aql/exec.ts (calculation handling & compileAndRunAqlQuery)
- packages/loot-core/src/server/aql/index.ts
- packages/desktop-client/src/queries/aqlQuery.ts
- packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (the startingBalance usage)
- packages/desktop-client/src/queries/pagedQuery.ts (similar calculate usage)
- tests: packages/desktop-client/src/queries/aqlQuery.test.ts and packages/loot-core/src/server/aql/exec.test.ts
Conclusion: the original concern is addressed by the server implementation — no fix required.
packages/desktop-client/src/components/reports/ReportRouter.tsx (1)
7-21: Routes for Crossover report are correctly registeredImports and routes for both list and detail paths match the pattern used by other reports. Looks good.
packages/desktop-client/src/components/reports/Overview.tsx (3)
28-28: LGTM! Clean integration of the CrossoverCard component.The import follows the existing pattern of importing card components from the reports subdirectory.
420-423: LGTM! Properly integrated into the widget menu.The new crossover-card menu item follows the established pattern and is appropriately positioned in the menu list.
560-568: LGTM! Rendering implementation is consistent.The CrossoverCard rendering block correctly passes all required props and follows the same pattern as other card widgets in the dashboard.
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (1)
107-108: Potential issue when accounts prop is empty.If the
accountsprop is an empty array,incomeAccountIdswill default to an empty array, which might not be the intended behavior.Should there be a fallback or validation to handle the case when no accounts are available?
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/reports/reports/Crossover.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (2)
92-105: Exclude tombstoned and split-parent transactions to avoid double-counting in aggregationsThe expense and balance queries don’t exclude tombstoned or split-parent transactions. This can double-count split parents (parent row + children) and include deleted rows. Previously flagged in an earlier review; your change to return zeros when no categories are selected fixes the “all transactions” issue, but parent/tombstone filtering is still outstanding.
Consider adding filters similar to the below (adapt fields to your schema — parent_id/is_parent, tombstone, etc.):
const query = q('transactions') .filter({ $and: [ { $or: expenseCategoryIds.map(id => ({ category: id })) }, { date: { $gte: monthUtils.firstDayOfMonth(start) } }, { date: { $lte: monthUtils.lastDayOfMonth(end) } }, + { tombstone: 0 }, + // Exclude split parents; adjust to your schema flags/shape + { $or: [{ is_parent: 0 }, { parent_id: { $ne: null } }] }, ], })And on balance queries:
q('transactions') .filter({ account: accountId }) .filter({ - date: { $lte: monthUtils.lastDayOfMonth(start) }, + date: { $lte: monthUtils.lastDayOfMonth(start) }, + tombstone: 0, + // Exclude parent split rows if present in schema + is_parent: 0, })q('transactions') .filter({ account: accountId, - date: { $gte: monthUtils.firstDayOfMonth(start) }, + date: { $gte: monthUtils.firstDayOfMonth(start) }, + tombstone: 0, + is_parent: 0, }) .filter({ $and: [{ date: { $lte: monthUtils.lastDayOfMonth(end) } }], })To quickly verify available flags in your schema:
#!/bin/bash # Discover split/tombstone fields for transactions rg -n -C2 -P --type=ts "(is_parent|parent_id|subtransactions|tombstone)\b" | sed -e 's/^/HIT: /' rg -n -C2 -P --type=ts "type\s+Transaction|export\s+type\s+Transaction|TransactionsTable"Also applies to: 114-121, 127-139
371-383: Normalize summary units and annualize return using compoundingThese summary fields are mixed raw cents vs. display units, and the annual return uses a simple multiply by 12. Normalize and compound for correctness.
return { graphData: { data, start: params.start, end: params.end, crossoverXLabel: crossoverIndex != null ? (data[crossoverIndex]?.x ?? null) : null, }, // Provide some summary numbers - lastKnownBalance: historicalBalances[historicalBalances.length - 1] || 0, - lastKnownMonthlyIncome: Math.round( - (historicalBalances[historicalBalances.length - 1] || 0) * monthlySWR, - ), - lastKnownMonthlyExpenses: lastExpense, - // Return the calculated default return for display purposes - historicalReturn: - defaultMonthlyReturn != null ? defaultMonthlyReturn * 12 : null, + lastKnownBalance: integerToAmount( + historicalBalances[historicalBalances.length - 1] || 0, + ), + lastKnownMonthlyIncome: integerToAmount( + Math.round( + (historicalBalances[historicalBalances.length - 1] || 0) * monthlySWR, + ), + ), + lastKnownMonthlyExpenses: integerToAmount(lastExpense), + // Return the calculated default annualized return (compound) + historicalReturn: + defaultMonthlyReturn != null + ? Math.pow(1 + defaultMonthlyReturn, 12) - 1 + : null,Note: Import
integerToAmountfrom your existing amount utilities (adjust the import path to match your codebase).
🧹 Nitpick comments (6)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (3)
206-224: Avoid O(n^2) scanning per account by indexing monthly changesYou linearly scan
acct.balanceswith.findfor every month, resulting in O(n_months × n_records) per account. Pre-index by date to reduce to O(n_months).- // Process each month in order - for (let i = 0; i < months.length; i++) { - const month = months[i]; - const found = acct.balances.find(b => b.date === month); - - if (found) { - // Add the monthly change to get the balance at the END of this month - runningBalance += found.amount; - } - - // Add this account's balance to the total for this month - historicalBalances[i] += runningBalance; - } + // Index monthly changes for O(1) lookup + const balanceByMonth = new Map(acct.balances.map(b => [b.date, b.amount] as const)); + // Process each month in order + for (let i = 0; i < months.length; i++) { + const month = months[i]; + const monthChange = balanceByMonth.get(month); + if (monthChange != null) { + // Add the monthly change to get the balance at the END of this month + runningBalance += monthChange; + } + // Add this account's balance to the total for this month + historicalBalances[i] += runningBalance; + }
237-251: Detect crossover in historical data to avoid unnecessary projectionCurrently, the crossover is only detected during projection. If the crossover already happened in the historical window, the reference line is never drawn and extra projection work is done.
Apply these changes to declare
crossoverIndexearlier, check historical points first, and only project when needed:let lastBalance = 0; let lastExpense = 0; + let crossoverIndex: number | null = null; months.forEach((month, idx) => { const balance = historicalBalances[idx]; // Use historical balances for data generation const monthlyIncome = balance * monthlySWR; const spend = expenseMap.get(month) || 0; data.push({ x: d.format(d.parseISO(month + '-01'), 'MMM yyyy'), investmentIncome: Math.round(monthlyIncome), expenses: spend, }); lastBalance = balance; lastExpense = spend; }); + + // Early crossover detection within historical data + for (let i = 0; i < data.length; i++) { + if (data[i].investmentIncome >= data[i].expenses) { + crossoverIndex = i; + break; + } + } - let crossoverIndex: number | null = null; - if (months.length > 0) { + if (months.length > 0 && crossoverIndex == null) { // If no explicit return provided, use the calculated default if (monthlyReturn == null) { monthlyReturn = defaultMonthlyReturn; } // Project up to 600 months max to avoid infinite loops (50 years) const maxProjectionMonths = 600; let projectedBalance = lastBalance; let monthCursor = d.parseISO(months[months.length - 1] + '-01');Also applies to: 281-349
318-349: Projection doesn’t model withdrawals reducing principal (intentional?)During projection,
projectedIncomeis computed from the growing balance, but that withdrawal isn’t deducted fromprojectedBalance. If you intend SWR to be a withdrawal, you might want to reduce the principal accordingly; otherwise this models “income potential” rather than “net-of-withdrawal balance.”If you want to model principal drawdown:
- if (monthlyReturn != null) { - projectedBalance = projectedBalance * (1 + monthlyReturn); - } - const projectedIncome = projectedBalance * monthlySWR; + if (monthlyReturn != null) { + projectedBalance = projectedBalance * (1 + monthlyReturn); + } + const projectedIncome = projectedBalance * monthlySWR; + // If SWR is a withdrawal, subtract it from the balance: + // projectedBalance = Math.max(0, projectedBalance - projectedIncome);Clarifying which model you want in the UI copy would also help user expectations.
packages/desktop-client/src/components/reports/reports/Crossover.tsx (3)
311-319: Consider surfacing a success notification after renaming the widget
onSaveWidgetshows a success toast;onSaveWidgetNamedoesn’t. For consistency and feedback parity, dispatch a success notification on rename as well.await send('dashboard-update-widget', { id: widget.id, meta: { ...(widget.meta ?? {}), name, }, }); + dispatch( + addNotification({ + notification: { + type: 'message', + message: t('Dashboard widget name updated.'), + }, + }), + );
48-67: Prefer interface over type alias for public data shapesProject guidelines prefer interfaces over type aliases. Swapping
type CrossoverDatafor aninterfacewill align with the convention.-type CrossoverData = { +interface CrossoverData { graphData: { data: Array<{ x: string; investmentIncome: number; expenses: number; isProjection?: boolean; }>; start: string; end: string; crossoverXLabel: string | null; }; lastKnownBalance: number; lastKnownMonthlyIncome: number; lastKnownMonthlyExpenses: number; historicalReturn: number | null; yearsToRetire: number | null; targetMonthlyIncome: number | null; -}; +}
470-503: Placeholder reflects simple annualization; ensure it matches spreadsheet’s compounded valueThe placeholder uses
historicalReturndirectly. If you adopt compounded annualization in the spreadsheet ((1 + monthly)^12 - 1), this UI will stay correct. If you keep simple multiply-by-12, the displayed placeholder will mislead users about the expected annual rate.No code change needed here if you implement the compounding fix in the spreadsheet. Otherwise, clarify the label (e.g., “simple annualized”) or update the computation to compounded annualization for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/Crossover.tsx(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError).
Favor named exports for components and utilities.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use objects or maps instead.
Avoid usinganyorunknownunless absolutely necessary. Look for type definitions in the codebase instead.
Avoid type assertions withasor!; prefer usingsatisfies.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.tspackages/desktop-client/src/components/reports/reports/Crossover.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
Use declarative JSX, keeping JSX minimal and readable.
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
🧠 Learnings (2)
📚 Learning: 2024-10-22T11:55:03.192Z
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx:181-196
Timestamp: 2024-10-22T11:55:03.192Z
Learning: After refactoring `nameError` in `DuplicateFileModal.tsx`, it's not necessary to use `setNameError` in the `onPress` handlers when validation fails.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-10-16T03:51:04.683Z
Learnt from: tlesicka
PR: actualbudget/actual#3593
File: packages/desktop-client/src/components/sidebar/BudgetName.tsx:114-136
Timestamp: 2024-10-16T03:51:04.683Z
Learning: In 'packages/desktop-client/src/components/sidebar/BudgetName.tsx', empty budget names are handled elsewhere, so additional error handling within the 'EditableBudgetName' component is unnecessary.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
🧬 Code Graph Analysis (2)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (3)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/desktop-client/src/hooks/useSpreadsheet.tsx (1)
useSpreadsheet(19-25)packages/desktop-client/src/components/reports/spreadsheets/recalculate.ts (1)
recalculate(27-116)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (6)
packages/loot-core/src/types/models/dashboard.ts (2)
CrossoverWidget(66-77)TimeFrame(4-14)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (1)
createCrossoverSpreadsheet(53-174)packages/desktop-client/src/hooks/useNavigate.ts (1)
useNavigate(12-48)packages/desktop-client/src/components/reports/CategorySelector.tsx (1)
CategorySelector(31-271)packages/desktop-client/src/components/reports/AccountSelector.tsx (1)
AccountSelector(26-421)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (1)
CrossoverGraph(39-195)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Analyze
- GitHub Check: test
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Functional Desktop App
🔇 Additional comments (1)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (1)
210-221: Good: user-friendly error handling instead of throwing in UI handlersReplacing the thrown error with a visible notification is the right UX choice here for a failed save due to missing widget.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (1)
92-105: Query pattern matches ActualQL norms; keep split/tombstone filters outFilters by category and bounded month range are correct. Per our learned context, there’s no need to add tombstone or split-parent filters; ActualQL’s default split behavior is already inline.
🧹 Nitpick comments (6)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (6)
26-41: Expose Hampel threshold as a parameter or constantHardcoding 3 is reasonable, but making it configurable (or at least a module-level const) will help experimentation and future tuning without code edits.
Apply this minimal change:
-const threshold = 3; // Standard threshold for outlier detection +const HAMPEL_OUTLIER_THRESHOLD = 3; // Standard threshold for outlier detection ... -const filteredExpenses = expenses.filter(expense => { +const filteredExpenses = expenses.filter(expense => { - const lowerBound = median - 1.4826 * mad * threshold; - const upperBound = median + 1.4826 * mad * threshold; + const lowerBound = median - 1.4826 * mad * HAMPEL_OUTLIER_THRESHOLD; + const upperBound = median + 1.4826 * mad * HAMPEL_OUTLIER_THRESHOLD; return expense >= lowerBound && expense <= upperBound; });
43-51: Type shape is clear; consider narrowing strings to YYYY-MMSince you pass around YYYY-MM (month keys) extensively, you might consider a branded type or a stricter alias to catch accidental full-date strings at compile time. Optional, but improves correctness at boundaries.
126-141: One query per account is fine; optional: batch by account+monthCurrent approach is simple and readable. If performance becomes an issue with many accounts, consider a single query grouped by month and account, then pivot in memory. Not required now.
206-224: Avoid O(M×N) finds when aggregating balances per monthUsing
findinside the month loop turns this into O(months × balances). Convert account monthly changes to a Map for O(1) lookup.Apply:
- for (const acct of historicalAccounts) { + for (const acct of historicalAccounts) { // Calculate running balance for each month // Start with the account's starting balance (balance at the end of the first month) let runningBalance = acct.starting; - - // Process each month in order - for (let i = 0; i < months.length; i++) { - const month = months[i]; - const found = acct.balances.find(b => b.date === month); - - if (found) { - // Add the monthly change to get the balance at the END of this month - runningBalance += found.amount; - } - - // Add this account's balance to the total for this month - historicalBalances[i] += runningBalance; - } + const monthlyChangeByDate = new Map(acct.balances.map(b => [b.date, b.amount])); + // Process each month in order + for (let i = 0; i < months.length; i++) { + const month = months[i]; + const delta = monthlyChangeByDate.get(month); + if (delta != null) { + // Add the monthly change to get the balance at the END of this month + runningBalance += delta; + } + // Add this account's balance to the total for this month + historicalBalances[i] += runningBalance; + } }
359-367: Years to retire uses calendar months; consider month-midpoint to reduce off-by-one feelOptional: using the first day of the current month for both sides or rounding up/down could make the displayed years feel more intuitive. Not blocking.
379-383: Consistent units: returning raw cents is good; add a brief doc commentPer our shared learning, keeping spreadsheet outputs in integer cents is preferred. Consider a brief comment near the return to document that all monetary values are in integer cents for downstream formatters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError).
Favor named exports for components and utilities.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use objects or maps instead.
Avoid usinganyorunknownunless absolutely necessary. Look for type definitions in the codebase instead.
Avoid type assertions withasor!; prefer usingsatisfies.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
🧠 Learnings (4)
📚 Learning: 2025-08-16T07:09:15.665Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:375-387
Timestamp: 2025-08-16T07:09:15.665Z
Learning: When handling financial data consistency issues in the desktop client, prefer keeping the data layer (spreadsheets) in raw integer cents and use the useFormat hook for all presentation formatting, rather than converting to display amounts in the data layer.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-06-16T17:45:40.807Z
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:64-72
Timestamp: 2025-06-16T17:45:40.807Z
Learning: The user misu-dev prefers strict type checking for financial format types in useFormat.ts as a long-term goal, but acknowledges that creating follow-up issues for cleanup should wait until after the current PR is merged, not during the development phase.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:14:02.055Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:0-0
Timestamp: 2025-08-16T07:14:02.055Z
Learning: In Actual Budget's Actual QL system, parent transactions for splits are automatically handled - queries return individual split transactions, not parents, so no manual filtering for is_parent is needed.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:14:02.055Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:0-0
Timestamp: 2025-08-16T07:14:02.055Z
Learning: Actual Budget reports do not use tombstone filtering for transactions in their spreadsheet queries.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
🧬 Code Graph Analysis (1)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (2)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/desktop-client/src/hooks/useSpreadsheet.tsx (1)
useSpreadsheet(19-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Functional Desktop App
🔇 Additional comments (6)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (6)
12-19: Median implementation looks solidCopy-on-sort avoids mutating inputs; even/odd handling is correct.
53-82: Graceful empty-state handling looks goodEarly return with zeroed metrics when dates or account selection are missing keeps consumers simple.
86-91: Confirm UX: no categories ⇒ zero expensesCurrent behavior returns 0 expenses for all months if no categories are selected. If the intended UX is “only selected categories count,” this is correct. If UX should default to “all expenses” when none selected, switch to querying negatives instead.
If defaulting to “all expenses” is desired, this patch would implement it:
- if (!expenseCategoryIds.length) { - return monthUtils - .rangeInclusive(start, end) - .map(date => ({ date, amount: 0 })); - } + const categoriesFilter = + expenseCategoryIds.length > 0 + ? { $or: expenseCategoryIds.map(id => ({ category: id })) } + : { amount: { $lt: 0 } }; // all outflows as expenses
196-202: Expense sign-flip is correct for visualizationSummed category outflows are negative; flipping to positive “spend” values is consistent.
300-320: Projection modes: clean separationLinear trend vs Hampel-filtered flat projection is straightforward and easy to follow. Nice.
371-377: Graph metadata looks consistentCrossover label resolution and start/end propagation are correct.
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/desktop-client/src/components/sidebar/Account.tsx (1)
272-276: Nit: consider consistent copy with header menu (“Close account”/“Reopen account”)In AccountHeader’s AccountMenu (lines 803–806 in that file), the labels are “Close account”/“Reopen account.” For consistency and clarity, mirror that here unless UX intentionally prefers the shorter strings in the sidebar.
Here’s a minimal diff to align the labels:
- { name: 'rename', text: t('Rename') }, - account.closed - ? { name: 'reopen', text: t('Reopen') } - : { name: 'close', text: t('Close') }, + { name: 'rename', text: t('Rename') }, + account.closed + ? { name: 'reopen', text: t('Reopen account') } + : { name: 'close', text: t('Close account') },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
upcoming-release-notes/5549.mdis excluded by!**/*.md
📒 Files selected for processing (4)
packages/desktop-client/src/components/accounts/Header.tsx(1 hunks)packages/desktop-client/src/components/reports/SaveReportName.tsx(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts(1 hunks)packages/desktop-client/src/components/sidebar/Account.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError).
Favor named exports for components and utilities.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use objects or maps instead.
Avoid usinganyorunknownunless absolutely necessary. Look for type definitions in the codebase instead.
Avoid type assertions withasor!; prefer usingsatisfies.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
packages/desktop-client/src/components/reports/SaveReportName.tsxpackages/desktop-client/src/components/sidebar/Account.tsxpackages/desktop-client/src/components/accounts/Header.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
Use declarative JSX, keeping JSX minimal and readable.
Files:
packages/desktop-client/src/components/reports/SaveReportName.tsxpackages/desktop-client/src/components/sidebar/Account.tsxpackages/desktop-client/src/components/accounts/Header.tsx
🧠 Learnings (5)
📚 Learning: 2024-10-12T23:57:22.683Z
Learnt from: jfdoming
PR: actualbudget/actual#3648
File: packages/desktop-client/src/components/HelpMenu.tsx:25-47
Timestamp: 2024-10-12T23:57:22.683Z
Learning: In `packages/desktop-client/src/components/HelpMenu.tsx`, when a `<Button>` component includes text content as a child, an explicit `aria-label` may not be required for accessibility, as the text content provides the accessible name.
Applied to files:
packages/desktop-client/src/components/reports/SaveReportName.tsx
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/desktop-client/src/components/sidebar/Account.tsxpackages/desktop-client/src/components/accounts/Header.tsx
📚 Learning: 2024-10-04T05:13:58.322Z
Learnt from: tlesicka
PR: actualbudget/actual#3554
File: packages/desktop-client/src/components/sidebar/Accounts.tsx:60-64
Timestamp: 2024-10-04T05:13:58.322Z
Learning: The `onReorder` function in `Accounts.tsx` was moved from `Sidebar.tsx`, and the `targetId` parameter remains typed as `unknown` intentionally.
Applied to files:
packages/desktop-client/src/components/sidebar/Account.tsx
📚 Learning: 2024-10-10T02:29:05.655Z
Learnt from: tlesicka
PR: actualbudget/actual#3593
File: packages/desktop-client/src/components/sidebar/Sidebar.tsx:112-116
Timestamp: 2024-10-10T02:29:05.655Z
Learning: In `packages/desktop-client/src/components/sidebar/BudgetName.tsx`, the `BudgetName` component consists of three parts: `BudgetName`, `EditBudgetName`, and the Menu. Keeping `EditBudgetName` as a separate component helps maintain cleaner code by separating concerns.
Applied to files:
packages/desktop-client/src/components/sidebar/Account.tsx
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
PR: actualbudget/actual#3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Applied to files:
packages/desktop-client/src/components/accounts/Header.tsx
🧬 Code Graph Analysis (1)
packages/desktop-client/src/components/accounts/Header.tsx (6)
packages/loot-core/src/shared/util.ts (1)
tsToRelativeTime(549-567)packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx (1)
dateFormat(1346-1346)packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts (1)
formatDate(107-118)packages/desktop-client/src/components/util/GenericInput.jsx (1)
dateFormat(47-47)packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx (1)
dateFormat(157-157)packages/desktop-client/src/components/filters/FiltersMenu.jsx (2)
dateFormat(277-277)dateFormat(456-456)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: Functional Desktop App
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Analyze
🔇 Additional comments (2)
packages/desktop-client/src/components/reports/SaveReportName.tsx (1)
82-82: LGTM: localized button labelSwitching to t('Add')/t('Update') is consistent with surrounding i18n usage. The button has visible text so no extra aria-label is needed (matches our past learning).
packages/desktop-client/src/components/accounts/Header.tsx (1)
395-411: No change needed: i18next tolerates whitespace in interpolation placeholdersi18next’s default interpolation engine trims any spaces inside
{{ … }}, so both{{relativeTimeAgo}}and{{ relativeTimeAgo }}resolve correctly. You can keep the existing spacing without impacting variable substitution.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (5)
84-108: Confirm UX: When no categories are selected, do we really want expenses to be zero?Current behavior returns zero for all months when
expenseCategoryIdsis empty. Most reports interpret “no filter” as “all expenses,” not zero. If the desired default is “all expenses,” filter on negative amounts instead of returning zeros.Proposed change (keeps behavior of only including expenses when no categories are selected, and preserves existing category-filtered query):
- if (!expenseCategoryIds.length) { - return monthUtils - .rangeInclusive(start, end) - .map(date => ({ date, amount: 0 })); - } - - const query = q('transactions') - .filter({ - $and: [ - { $or: expenseCategoryIds.map(id => ({ category: id })) }, - { date: { $gte: monthUtils.firstDayOfMonth(start) } }, - { date: { $lte: monthUtils.lastDayOfMonth(end) } }, - ], - }) - .groupBy({ $month: '$date' }) - .select([ - { date: { $month: '$date' } }, - { amount: { $sum: '$amount' } }, - ]); + if (!expenseCategoryIds.length) { + const query = q('transactions') + .filter({ + $and: [ + { amount: { $lt: 0 } }, // Only expenses + { date: { $gte: monthUtils.firstDayOfMonth(start) } }, + { date: { $lte: monthUtils.lastDayOfMonth(end) } }, + ], + }) + .groupBy({ $month: '$date' }) + .select([ + { date: { $month: '$date' } }, + { amount: { $sum: '$amount' } }, + ]); + const { data } = await aqlQuery(query); + return data as MonthlyAgg[]; + } + + const query = q('transactions') + .filter({ + $and: [ + { $or: expenseCategoryIds.map(id => ({ category: id })) }, + { date: { $gte: monthUtils.firstDayOfMonth(start) } }, + { date: { $lte: monthUtils.lastDayOfMonth(end) } }, + ], + }) + .groupBy({ $month: '$date' }) + .select([ + { date: { $month: '$date' } }, + { amount: { $sum: '$amount' } }, + ]);If the zero-by-default experience is intentional for the Crossover report, feel free to ignore this. Just wanted to confirm intent.
210-221: Avoid O(n^2) lookups when aggregating balances by month
findper month causes quadratic behavior w.r.t. number of months. Pre-index monthly changes by date for linear-time accumulation.- // Process each month in order - for (let i = 0; i < months.length; i++) { - const month = months[i]; - const found = acct.balances.find(b => b.date === month); - - if (found) { - // Add the monthly change to get the balance at the END of this month - runningBalance += found.amount; - } - - // Add this account's balance to the total for this month - historicalBalances[i] += runningBalance; - } + // Pre-index monthly changes by date to avoid O(n^2) lookups + const byMonth = new Map(acct.balances.map(b => [b.date, b.amount] as const)); + // Process each month in order + for (let i = 0; i < months.length; i++) { + const month = months[i]; + const delta = byMonth.get(month) ?? 0; + // Add the monthly change to get the balance at the END of this month + runningBalance += delta; + // Add this account's balance to the total for this month + historicalBalances[i] += runningBalance; + }
349-351: Use consistent rounding when checking the projected crossoverYou round both series for display/storage in
data, but the comparison uses unrounded values. Aligning the comparison with rounded cents avoids edge-case off-by-one months when values are near the threshold.- if (crossoverIndex == null && projectedIncome >= projectedExpenses) { + if ( + crossoverIndex == null && + Math.round(projectedIncome) >= Math.round(projectedExpenses) + ) {
355-368: Avoid parsing the formatted x-label to compute years-to-retireParsing
MMM yyyyis locale-sensitive and brittle. Prefer computing from a month key or index:
- Option A: Store a raw month key (e.g.,
monthKey: 'YYYY-MM') alongsidexand parse withparseISO(monthKey + '-01').- Option B: Compute months-from-now using the known index offset (historical months length + projection steps) without string parsing.
Happy to draft a small refactor that adds a
monthKeyfield and swaps the parse logic.
12-19: Optional guard in median helper for future-proofingCurrently callers ensure non-empty arrays, but adding a zero-length guard makes
calculateMediansafer if reused elsewhere.function calculateMedian(values: number[]): number { + if (values.length === 0) return 0; const sorted = [...values].sort((a, b) => a - b); const mid = Math.floor(sorted.length / 2); return sorted.length % 2 === 0 ? (sorted[mid - 1] + sorted[mid]) / 2 : sorted[mid]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError).
Favor named exports for components and utilities.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use objects or maps instead.
Avoid usinganyorunknownunless absolutely necessary. Look for type definitions in the codebase instead.
Avoid type assertions withasor!; prefer usingsatisfies.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
🧠 Learnings (4)
📚 Learning: 2025-08-16T07:09:15.665Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:375-387
Timestamp: 2025-08-16T07:09:15.665Z
Learning: When handling financial data consistency issues in the desktop client, prefer keeping the data layer (spreadsheets) in raw integer cents and use the useFormat hook for all presentation formatting, rather than converting to display amounts in the data layer.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:14:02.055Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:0-0
Timestamp: 2025-08-16T07:14:02.055Z
Learning: In Actual Budget's Actual QL system, parent transactions for splits are automatically handled - queries return individual split transactions, not parents, so no manual filtering for is_parent is needed.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:14:02.055Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:0-0
Timestamp: 2025-08-16T07:14:02.055Z
Learning: Actual Budget reports do not use tombstone filtering for transactions in their spreadsheet queries.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-06-21T04:15:23.727Z
Learnt from: matt-fidd
PR: actualbudget/actual#5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
🧬 Code Graph Analysis (1)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (2)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/desktop-client/src/hooks/useSpreadsheet.tsx (1)
useSpreadsheet(19-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Functional Desktop App
- GitHub Check: Analyze
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (windows-latest)
🔇 Additional comments (5)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (5)
110-121: Good guard on undefined starting balanceDefaulting
startingBalanceto 0 prevents NaN propagation when accounts have no prior transactions. Looks solid.
195-201: OK to flip sign for expenses; matches “negative = outflow” conventionStoring expenses as positive values for visualization by negating summed negatives is consistent with other reports.
251-254: Good: capture only the first historical crossoverThis avoids overwriting with later months and matches “first time income ≥ expenses.”
259-262: Good: effective monthly rate from annual returnUsing
(1 + r)^(1/12) - 1correctly accounts for compounding.
378-388: Data layer returns raw cents; annualization via compounding is correct
- Returning
lastKnown*andtargetMonthlyIncomein raw cents is the right separation-of-concerns for spreadsheets in this app (format in the UI).- Annualizing with
Math.pow(1 + defaultMonthlyReturn, 12) - 1is accurate.
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
Show resolved
Hide resolved
sjones512
force-pushed
the
feat/add-crossover-report
branch
from
db67f93 to
6e2deac
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
packages/loot-core/src/types/models/dashboard.ts (1)
66-77: CrossoverWidget meta shape looks solid; consider simplifying nullable/optional fields and documenting units
- estimatedReturn?: number | null adds a third state (undefined vs null vs number). Unless null has a specific semantic (e.g., “auto”), prefer keeping it optional-only to reduce ambiguity, and document the unit and range for both rates.
- Consider marking the ID arrays readonly to prevent accidental mutation at call sites.
Possible tightening (if semantics allow):
export type CrossoverWidget = AbstractWidget< 'crossover-card', { name?: string; - expenseCategoryIds?: string[]; - incomeAccountIds?: string[]; + expenseCategoryIds?: readonly string[]; + incomeAccountIds?: readonly string[]; timeFrame?: TimeFrame; safeWithdrawalRate?: number; // 0.04 default - estimatedReturn?: number | null; // annual + // annualized return in decimal form, e.g., 0.05 for 5% + estimatedReturn?: number; // leave undefined to auto-compute projectionType?: 'trend' | 'hampel'; // expense projection method } | null >;packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (6)
69-72: Initialize start/end to a safe default to avoid first-render fetch with empty paramsstart/end begin as empty strings, which may cause an initial report run with invalid dates and a visible flicker. Initialize both to the last completed month up front; the effect will refine start when the earliest transaction is fetched.
- const [start, setStart] = useState<string>(''); - const [end, setEnd] = useState<string>(''); + const [start, setStart] = useState<string>(() => + monthUtils.subMonths(monthUtils.currentMonth(), 1), + ); + const [end, setEnd] = useState<string>(() => + monthUtils.subMonths(monthUtils.currentMonth(), 1), + );
73-95: Avoid setting state after unmount in async effectsend('get-earliest-transaction') is async; if the component unmounts before it resolves, setStart/setEnd will warn. Guard with an isMounted flag and cleanup.
- useEffect(() => { - async function calculateDateRange() { + useEffect(() => { + let isMounted = true; + async function calculateDateRange() { const trans = await send('get-earliest-transaction'); const currentMonth = monthUtils.currentMonth(); const earliestMonth = trans - ? monthUtils.monthFromDate(d.parseISO(fromDateRepr(trans.date))) + ? monthUtils.monthFromDate(d.parseISO(fromDateRepr(trans.date))) : currentMonth; // Use saved timeFrame from meta or default range let startMonth = earliestMonth; let endMonth = monthUtils.subMonths(currentMonth, 1); // Exclude current month by default if (meta?.timeFrame?.start && meta?.timeFrame?.end) { startMonth = meta.timeFrame.start; endMonth = meta.timeFrame.end; } - setStart(startMonth); - setEnd(endMonth); + if (isMounted) { + setStart(startMonth); + setEnd(endMonth); + } } calculateDateRange(); - }, [meta?.timeFrame]); + return () => { + isMounted = false; + }; + }, [meta?.timeFrame]);
8-8: Prefer named import from date-fns for better tree-shakingImport only parseISO to reduce bundle bloat and make intent explicit.
-import * as d from 'date-fns'; +import { parseISO } from 'date-fns';And update usage:
- ? monthUtils.monthFromDate(d.parseISO(fromDateRepr(trans.date))) + ? monthUtils.monthFromDate(parseISO(fromDateRepr(trans.date)))Also applies to: 78-78
115-135: Optional: Guard spreadsheet param creation when range is incompleteIf you keep empty-string initialization, short-circuit param creation to avoid starting a report request with invalid dates. This becomes unnecessary if you adopt the default start/end initialization suggested earlier.
- const params = useMemo( - () => - createCrossoverSpreadsheet({ + const params = useMemo(() => { + if (!start || !end) { + // Defer param creation until date range is ready + return null; + } + return createCrossoverSpreadsheet({ start, end, expenseCategoryIds, incomeAccountIds, safeWithdrawalRate: swr, estimatedReturn, projectionType, - }), - [ + }); + }, [ start, end, expenseCategoryIds, incomeAccountIds, swr, estimatedReturn, projectionType, - ], - ); + ]);Note: This assumes useReport can handle a null-ish params; if not, prefer the initialization fix instead.
151-162: Don’t throw on unknown menu item; warn insteadThrowing here will crash the UI if new items are added upstream. A warning is safer.
onMenuSelect={item => { switch (item) { case 'rename': setNameMenuOpen(true); break; case 'remove': onRemove(); break; default: - throw new Error(`Unrecognized selection: ${item}`); + console.warn('Unrecognized CrossoverCard menu selection:', item); + break; } }}
185-199: Localize the “years” suffix and N/AThe numeric value is localized via toFixed but the “years” string and “N/A” aren’t. Use i18n so locales can provide correct wording/plurals.
- {yearsToRetire != null - ? `${yearsToRetire.toFixed(1)} years` - : 'N/A'} + {yearsToRetire != null + ? t('{{years}} years', { years: yearsToRetire.toFixed(1) }) + : t('N/A')}If your i18n setup supports pluralization, prefer a pluralized key instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
upcoming-release-notes/5554.mdis excluded by!**/*.md
📒 Files selected for processing (9)
packages/desktop-client/src/components/reports/AccountSelector.tsx(1 hunks)packages/desktop-client/src/components/reports/Overview.tsx(3 hunks)packages/desktop-client/src/components/reports/ReportRouter.tsx(2 hunks)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/Crossover.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts(1 hunks)packages/loot-core/src/server/dashboard/app.ts(1 hunks)packages/loot-core/src/types/models/dashboard.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/desktop-client/src/components/reports/Overview.tsx
- packages/loot-core/src/server/dashboard/app.ts
- packages/desktop-client/src/components/reports/ReportRouter.tsx
- packages/desktop-client/src/components/reports/AccountSelector.tsx
- packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx
- packages/desktop-client/src/components/reports/reports/Crossover.tsx
- packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError).
Favor named exports for components and utilities.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use objects or maps instead.
Avoid usinganyorunknownunless absolutely necessary. Look for type definitions in the codebase instead.
Avoid type assertions withasor!; prefer usingsatisfies.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
packages/loot-core/src/types/models/dashboard.tspackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
Use declarative JSX, keeping JSX minimal and readable.
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧠 Learnings (1)
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Functional Desktop App
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Analyze
- GitHub Check: test
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
🔇 Additional comments (3)
packages/loot-core/src/types/models/dashboard.ts (2)
87-87: Including CrossoverWidget in SpecializedWidget is correct and sufficient for export/import unionsThis ensures the new widget participates in Widget, ExportImportDashboardWidget, and related flows without further type tweaks here.
66-77: CrossoverWidget export and server whitelist confirmed
packages/loot-core/src/types/models/index.tsalready includes the type-only re-export
export type * from './dashboard', makingCrossoverWidgetavailable vialoot-core/types/models.packages/loot-core/src/server/dashboard/app.tshas'crossover-card'in the validation whitelist.No further changes needed.
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (1)
26-46: Ignore the import suggestion—no sharedCrossoverDataexists
I searched the repository and found no exportedCrossoverDatatype or crossover-report helper to reuse. Defining it locally here is appropriate until a common definition is introduced.Likely an incorrect or invalid review comment.
sjones512
force-pushed
the
feat/add-crossover-report
branch
from
6e2deac to
fb574fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (3)
22-37: Prefer interfaces over type aliases for propsAlign with the codebase guideline by converting the props alias to an interface.
-type CrossoverGraphProps = { +interface CrossoverGraphProps { style?: CSSProperties; graphData: { data: Array<{ x: string; investmentIncome: number; expenses: number; isProjection?: boolean; }>; start: string; end: string; crossoverXLabel?: string | null; }; compact?: boolean; showTooltip?: boolean; -}; +}
70-124: Returnnullexplicitly from CustomTooltip when inactiveMake the component’s return explicit to improve readability and avoid relying on implicit
undefined.const CustomTooltip = ({ active, payload }: CustomTooltipProps) => { if (active && payload && payload.length) { return ( <div className={css({ zIndex: 1000, pointerEvents: 'none', borderRadius: 2, boxShadow: '0 1px 6px rgba(0, 0, 0, .20)', backgroundColor: theme.menuBackground, color: theme.menuItemText, padding: 10, })} > <div> <div style={{ marginBottom: 10 }}> <strong>{payload[0].payload.x}</strong> {payload[0].payload.isProjection ? ( <span style={{ marginLeft: 8, opacity: 0.7 }}> {t('(projected)')} </span> ) : null} </div> <div style={{ lineHeight: 1.5 }}> <View className={css({ display: 'flex', justifyContent: 'space-between', })} > <div> <Trans>Monthly investment income:</Trans> </div> <div> {format(payload[0].payload.investmentIncome, 'financial')} </div> </View> <View className={css({ display: 'flex', justifyContent: 'space-between', })} > <div> <Trans>Monthly expenses:</Trans> </div> <div>{format(payload[0].payload.expenses, 'financial')}</div> </View> </div> </div> </div> ); } + return null; };
134-191: Drop redundant ResponsiveContainer wrapperYou already size the chart via Container’s width/height. The extra ResponsiveContainer (wrapping a div) is redundant and adds nesting without benefit.
- <ResponsiveContainer> - <div style={{ ...(!compact && { marginTop: '15px' }) }}> - <LineChart - width={width} - height={height} - data={graphData.data} - margin={{ - top: 0, - right: 0, - left: compact ? 0 : 20, - bottom: compact ? 0 : 10, - }} - > + <div style={{ ...(!compact && { marginTop: '15px' }) }}> + <LineChart + width={width} + height={height} + data={graphData.data} + margin={{ + top: 0, + right: 0, + left: compact ? 0 : 20, + bottom: compact ? 0 : 10, + }} + > {!compact && <CartesianGrid strokeDasharray="3 3" />} <XAxis dataKey="x" hide={compact} tick={{ fill: theme.pageText }} tickLine={{ stroke: theme.pageText }} /> <YAxis hide={compact} tickFormatter={tickFormatter} tick={{ fill: theme.pageText }} tickLine={{ stroke: theme.pageText }} /> {showTooltip && ( <Tooltip content={<CustomTooltip />} isAnimationActive={false} /> )} {graphData.crossoverXLabel && ( <ReferenceLine x={graphData.crossoverXLabel} stroke={theme.noticeText} strokeDasharray="4 4" /> )} <Line type="monotone" dataKey="investmentIncome" dot={false} stroke={theme.reportsBlue} strokeWidth={2} animationDuration={0} /> <Line type="monotone" dataKey="expenses" dot={false} stroke={theme.reportsRed} strokeWidth={2} animationDuration={0} /> </LineChart> </div> - </ResponsiveContainer>packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (4)
26-45: Prefer interfaces for structured data typesSwitch to interfaces for CrossoverData to follow the TS guideline used in this repo.
-// Type for the return value of the recalculate function -type CrossoverData = { +// Type for the return value of the recalculate function +interface CrossoverData { graphData: { data: Array<{ x: string; investmentIncome: number; expenses: number; isProjection?: boolean; }>; start: string; end: string; crossoverXLabel: string | null; }; lastKnownBalance: number; lastKnownMonthlyIncome: number; lastKnownMonthlyExpenses: number; historicalReturn: number | null; yearsToRetire: number | null; targetMonthlyIncome: number | null; -}; +}
47-54: Prefer interfaces for propsUse an interface for props to keep consistency with the TS coding guidelines.
-type CrossoverCardProps = { +interface CrossoverCardProps { widgetId: string; isEditing?: boolean; accounts: AccountEntity[]; meta?: CrossoverWidget['meta']; onMetaChange: (newMeta: CrossoverWidget['meta']) => void; onRemove: () => void; -}; +}
81-92: Guard against end < start (edge case when only current month exists)If the earliest month equals the current month,
endMonthbecomes previous month and can precedestartMonth. EnsureendMonth >= startMonthto avoid an empty or invalid range.if (meta?.timeFrame?.start && meta?.timeFrame?.end) { startMonth = meta.timeFrame.start; endMonth = meta.timeFrame.end; } + // Ensure end is not earlier than start + if ( + d.isAfter(d.parseISO(startMonth + '-01'), d.parseISO(endMonth + '-01')) + ) { + endMonth = startMonth; + }
194-199: Localize/remove hardcoded “years” and “N/A”Avoid hardcoded English text. Either localize the unit or just show the number and rely on the label below. Also localize “N/A”.
- {yearsToRetire != null - ? `${yearsToRetire.toFixed(1)} years` - : 'N/A'} + {yearsToRetire != null + ? yearsToRetire.toFixed(1) + : t('N/A')}packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (3)
10-10: Prefer interface for simple shapesMinor TS nit: use an interface for
MonthlyAgg.-type MonthlyAgg = { date: string; amount: number }; +interface MonthlyAgg { + date: string; + amount: number; +}
366-371: Anchor months-diff to month starts to avoid off-by-one driftParsing with
currentDateas the reference can inherit today’s day-of-month and skewdifferenceInMonths. Use start-of-month for both values.- const currentDate = new Date(); - const crossoverDate = d.parse(crossoverData.x, 'MMM yyyy', currentDate); - const monthsDiff = d.differenceInMonths(crossoverDate, currentDate); + const ref = d.startOfMonth(new Date()); + const crossoverDate = d.parse(crossoverData.x, 'MMM yyyy', ref); + const monthsDiff = d.differenceInMonths(crossoverDate, ref);
178-398: Consider extracting helpers to reduce function size and aid testing
recalculatemixes data shaping, CAGR math, projection algorithms, and crossover detection in a single large function. Extracting helpers (e.g., buildExpenseMap, computeHistoricalBalances, computeDefaultMonthlyReturn, projectExpensesTrend, projectExpensesHampel, detectCrossover) will improve readability and testability.If helpful, I can propose a refactor patch that introduces these helpers while preserving behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
upcoming-release-notes/5554.mdis excluded by!**/*.md
📒 Files selected for processing (9)
packages/desktop-client/src/components/reports/AccountSelector.tsx(1 hunks)packages/desktop-client/src/components/reports/Overview.tsx(3 hunks)packages/desktop-client/src/components/reports/ReportRouter.tsx(2 hunks)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/Crossover.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts(1 hunks)packages/loot-core/src/server/dashboard/app.ts(1 hunks)packages/loot-core/src/types/models/dashboard.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/loot-core/src/server/dashboard/app.ts
- packages/desktop-client/src/components/reports/ReportRouter.tsx
- packages/loot-core/src/types/models/dashboard.ts
- packages/desktop-client/src/components/reports/Overview.tsx
- packages/desktop-client/src/components/reports/AccountSelector.tsx
- packages/desktop-client/src/components/reports/reports/Crossover.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Write concise, technical TypeScript code
Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError)
Structure files with sections in order: exported page/component, GraphQL queries, helpers, static content, types
Favor named exports for components and utilities
Prefer interfaces over types in TypeScript
Avoid enums; use objects or maps instead
Avoid using any or unknown unless absolutely necessary; prefer existing types
Avoid type assertions with as or non-null !; prefer satisfies
Use the function keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Files:
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsxpackages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.tspackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
Use declarative JSX, keeping JSX minimal and readable
Files:
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧠 Learnings (8)
📚 Learning: 2025-08-16T07:05:59.493Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx:0-0
Timestamp: 2025-08-16T07:05:59.493Z
Learning: In Actual Budget reports, privacy mode behavior is designed to hide axis tick values (showing "...") but intentionally continue showing actual currency amounts in tooltips. This is consistent across all reports and is not considered a privacy leak.
Applied to files:
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx
📚 Learning: 2025-08-16T16:42:08.258Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:7-8
Timestamp: 2025-08-16T16:42:08.258Z
Learning: In the Actual Budget codebase, it's the established pattern and acceptable to import useSpreadsheet as a type-only import and then use ReturnType<typeof useSpreadsheet> in type annotations. This pattern is used consistently across all spreadsheet files including custom-spreadsheet.ts, grouped-spreadsheet.ts, spending-spreadsheet.ts, net-worth-spreadsheet.ts, cash-flow-spreadsheet.tsx, calendar-spreadsheet.ts, summary-spreadsheet.ts, and crossover-spreadsheet.ts.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:09:15.665Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:375-387
Timestamp: 2025-08-16T07:09:15.665Z
Learning: When handling financial data consistency issues in the desktop client, prefer keeping the data layer (spreadsheets) in raw integer cents and use the useFormat hook for all presentation formatting, rather than converting to display amounts in the data layer.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:14:02.055Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:0-0
Timestamp: 2025-08-16T07:14:02.055Z
Learning: In Actual Budget's Actual QL system, parent transactions for splits are automatically handled - queries return individual split transactions, not parents, so no manual filtering for is_parent is needed.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:14:02.055Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:0-0
Timestamp: 2025-08-16T07:14:02.055Z
Learning: Actual Budget reports do not use tombstone filtering for transactions in their spreadsheet queries.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-06-21T04:15:23.727Z
Learnt from: matt-fidd
PR: actualbudget/actual#5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T16:42:08.258Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:7-8
Timestamp: 2025-08-16T16:42:08.258Z
Learning: In the Actual Budget codebase, it's acceptable and consistent to import useSpreadsheet as a type-only import and then use ReturnType<typeof useSpreadsheet> in type annotations. This pattern is used consistently across spreadsheet files like custom-spreadsheet.ts and crossover-spreadsheet.ts.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧬 Code Graph Analysis (2)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (3)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/desktop-client/src/hooks/useSpreadsheet.tsx (1)
useSpreadsheet(19-25)packages/desktop-client/src/components/reports/spreadsheets/recalculate.ts (1)
recalculate(27-116)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (10)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/loot-core/src/types/models/dashboard.ts (1)
CrossoverWidget(66-77)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (1)
createCrossoverSpreadsheet(56-176)packages/desktop-client/src/components/reports/ReportCard.tsx (1)
ReportCard(34-119)packages/component-library/src/View.tsx (1)
View(14-33)packages/desktop-client/src/components/reports/ReportCardName.tsx (1)
ReportCardName(17-55)packages/component-library/src/styles.ts (1)
styles(16-157)packages/desktop-client/src/components/PrivacyFilter.tsx (1)
PrivacyFilter(47-68)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (1)
CrossoverGraph(39-195)packages/desktop-client/src/components/reports/LoadingIndicator.tsx (1)
LoadingIndicator(13-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Analyze
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Functional Desktop App
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
🔇 Additional comments (4)
packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (1)
49-54: LGTM: Y-axis tickFormatter respects privacy modeHiding tick values under privacy mode while keeping true values in tooltips matches established report behavior.
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (1)
212-218: LGTM: Tooltip gating and compact graph usageConditional tooltips based on editing and viewport width, combined with compact graph rendering, match established report UX patterns.
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (2)
87-111: Confirm intended UX when no categories are selectedWhen
expenseCategoryIdsis empty, monthly expenses are set to zero for the entire range. This yields immediate crossover for many users by default. If the intent is “explicit opt-in categories only,” this is fine; otherwise consider defaulting to “all expenses.”Would you like me to draft a patch to default to all negative-amount transactions when no categories are selected, or keep zero as the intentional default?
116-125: LGTM: Solid guards for missing data and robust CAGR calcDefaulting
startingBalanceto 0 avoids NaNs, and the monthly CAGR computation has sensible guards and fallbacks. This will behave well with sparse histories.Also applies to: 266-285
sjones512
force-pushed
the
feat/add-crossover-report
branch
from
fb574fe to
4d7b524
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (11)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (3)
26-45: Avoid duplicating the CrossoverData type; export and reuse a single source of truthCrossoverData is re-declared here and again in Crossover.tsx. Prefer exporting it from the spreadsheet module and importing it in both places to prevent drift.
Apply within this file:
-// Type for the return value of the recalculate function -type CrossoverData = { - graphData: { - data: Array<{ - x: string; - investmentIncome: number; - expenses: number; - isProjection?: boolean; - }>; - start: string; - end: string; - crossoverXLabel: string | null; - }; - lastKnownBalance: number; - lastKnownMonthlyIncome: number; - lastKnownMonthlyExpenses: number; - historicalReturn: number | null; - yearsToRetire: number | null; - targetMonthlyIncome: number | null; -}; +import type { CrossoverData } from '@desktop-client/components/reports/spreadsheets/crossover-spreadsheet';Additionally, in the spreadsheet module, export the interface (see my comment on that file for the corresponding change).
194-199: Localize the “years” suffix and N/AThe value string embeds “years” directly and uses a raw 'N/A'. Wrap both in i18n to support translations and pluralization.
- <PrivacyFilter activationFilters={[!isCardHovered]}> - {yearsToRetire != null - ? `${yearsToRetire.toFixed(1)} years` - : 'N/A'} - </PrivacyFilter> + <PrivacyFilter activationFilters={[!isCardHovered]}> + {yearsToRetire != null ? ( + <Trans + i18nKey="yearsToRetire_value" + values={{ value: yearsToRetire.toFixed(1) }} + > + {{ value: yearsToRetire.toFixed(1) }} years + </Trans> + ) : ( + <Trans>N/A</Trans> + )} + </PrivacyFilter>
115-137: Avoid an initial empty-data render by deferring useReport until dates are readyWith start/end initially empty, the spreadsheet’s early-exit sets empty data which then gets replaced after dates load. Minor UX flicker. Consider deferring params creation until start and end are initialized.
If useReport supports a null/undefined params to skip (check its signature), gate the memo like:
- const params = useMemo( - () => - createCrossoverSpreadsheet({ + const params = useMemo( + () => (start && end ? createCrossoverSpreadsheet({ start, end, expenseCategoryIds, incomeAccountIds, safeWithdrawalRate: swr, estimatedReturn, projectionType, - }), + }) : null), , [start, end, expenseCategoryIds, incomeAccountIds, swr, estimatedReturn, projectionType]);And call
useReport<CrossoverData>('crossover', params || undefined);Also applies to: 137-138
packages/desktop-client/src/components/reports/reports/Crossover.tsx (4)
48-67: Single-source CrossoverData type; reuse from spreadsheet moduleCrossoverData is defined here and also in CrossoverCard.tsx. Export it once (in the spreadsheet module) and import it in both files to prevent type drift.
-// Type for the return value of the recalculate function -type CrossoverData = { - graphData: { - data: Array<{ - x: string; - investmentIncome: number; - expenses: number; - isProjection?: boolean; - }>; - start: string; - end: string; - crossoverXLabel: string | null; - }; - lastKnownBalance: number; - lastKnownMonthlyIncome: number; - lastKnownMonthlyExpenses: number; - historicalReturn: number | null; - yearsToRetire: number | null; - targetMonthlyIncome: number | null; -}; +import type { CrossoverData } from '@desktop-client/components/reports/spreadsheets/crossover-spreadsheet';
398-400: Copy tweak: “Expense categories”The heading reads “Expenses categories”. Consider “Expense categories”.
- <Trans>Expenses categories</Trans> + <Trans>Expense categories</Trans>
533-538: Localize N/A strings for consistencyBoth top stats use a literal 'N/A'. Wrap with Trans to make them translatable.
- {yearsToRetire != null ? yearsToRetire.toFixed(1) : 'N/A'} + {yearsToRetire != null ? yearsToRetire.toFixed(1) : <Trans>N/A</Trans>}- ? format(targetMonthlyIncome, 'financial') - : 'N/A'} + ? format(targetMonthlyIncome, 'financial') + : <Trans>N/A</Trans>}Also applies to: 545-552
425-441: Prefer Input’s onChangeValue for simpler numeric handlingUsing onChangeValue avoids accessing e.target and keeps event handling consistent with the design of Input.
- <Input + <Input type="number" min={0} max={100} step={0.1} value={(swr * 100).toString()} - onChange={e => - setSwr( - Math.max(0, Math.min(100, Number(e.target.value))) / 100, - ) - } + onChangeValue={val => + setSwr(Math.max(0, Math.min(100, Number(val))) / 100) + } style={{ width: 120, marginBottom: 12 }} />Apply similarly for the Estimated return input below.
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (4)
10-10: Use an interface for MonthlyAgg for readability and alignment with codebase styleMinor style improvement; interfaces are preferred.
-type MonthlyAgg = { date: string; amount: number }; +interface MonthlyAgg { + date: string; + amount: number; +}
46-55: Consider exporting CrossoverData alongside CrossoverParamsDownstream components currently re-declare the result shape. Exporting a single interface reduces duplication and drift.
Add below CrossoverParams:
+export interface CrossoverData { + graphData: { + data: Array<{ + x: string; + investmentIncome: number; + expenses: number; + isProjection?: boolean; + }>; + start: string; + end: string; + crossoverXLabel: string | null; + }; + lastKnownBalance: number; + lastKnownMonthlyIncome: number; + lastKnownMonthlyExpenses: number; + historicalReturn: number | null; + yearsToRetire: number | null; + targetMonthlyIncome: number | null; +}Then update the setData signature and recalculate’s return type:
- setData: (data: ReturnType<typeof recalculate>) => void, + setData: (data: CrossoverData) => void,And annotate recalculate’s return type:
-function recalculate( ... ) { +function recalculate( ... ): CrossoverData {
196-224: Historical balance aggregation is correct and efficient enough; consider future batchingThe per-account double-query (starting balance + grouped deltas) is clear and workable. If performance becomes an issue with many accounts, consider a single grouped query across all accounts and account+month grouping to cut round-trips.
374-397: Return shape consistency and unitsReturning integer cents for amounts aligns with the app’s formatting strategy; historicalReturn is annualized correctly. Consider rounding expenses in the historical segment to match the income rounding or leave both unrounded and delegate all rounding to the formatter for visual consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
upcoming-release-notes/5554.mdis excluded by!**/*.md
📒 Files selected for processing (9)
packages/desktop-client/src/components/reports/AccountSelector.tsx(1 hunks)packages/desktop-client/src/components/reports/Overview.tsx(3 hunks)packages/desktop-client/src/components/reports/ReportRouter.tsx(2 hunks)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/Crossover.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts(1 hunks)packages/loot-core/src/server/dashboard/app.ts(1 hunks)packages/loot-core/src/types/models/dashboard.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/desktop-client/src/components/reports/ReportRouter.tsx
- packages/loot-core/src/server/dashboard/app.ts
- packages/desktop-client/src/components/reports/AccountSelector.tsx
- packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx
- packages/desktop-client/src/components/reports/Overview.tsx
- packages/loot-core/src/types/models/dashboard.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Write concise, technical TypeScript code
Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError)
Structure files with sections in order: exported page/component, GraphQL queries, helpers, static content, types
Favor named exports for components and utilities
Prefer interfaces over types in TypeScript
Avoid enums; use objects or maps instead
Avoid using any or unknown unless absolutely necessary; prefer existing types
Avoid type assertions with as or non-null !; prefer satisfies
Use the function keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.tspackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
Use declarative JSX, keeping JSX minimal and readable
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧠 Learnings (8)
📚 Learning: 2024-10-22T11:55:03.192Z
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx:181-196
Timestamp: 2024-10-22T11:55:03.192Z
Learning: After refactoring `nameError` in `DuplicateFileModal.tsx`, it's not necessary to use `setNameError` in the `onPress` handlers when validation fails.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-08-16T16:42:08.258Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:7-8
Timestamp: 2025-08-16T16:42:08.258Z
Learning: In the Actual Budget codebase, it's the established pattern and acceptable to import useSpreadsheet as a type-only import and then use ReturnType<typeof useSpreadsheet> in type annotations. This pattern is used consistently across all spreadsheet files including custom-spreadsheet.ts, grouped-spreadsheet.ts, spending-spreadsheet.ts, net-worth-spreadsheet.ts, cash-flow-spreadsheet.tsx, calendar-spreadsheet.ts, summary-spreadsheet.ts, and crossover-spreadsheet.ts.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:09:15.665Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:375-387
Timestamp: 2025-08-16T07:09:15.665Z
Learning: When handling financial data consistency issues in the desktop client, prefer keeping the data layer (spreadsheets) in raw integer cents and use the useFormat hook for all presentation formatting, rather than converting to display amounts in the data layer.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:14:02.055Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:0-0
Timestamp: 2025-08-16T07:14:02.055Z
Learning: In Actual Budget's Actual QL system, parent transactions for splits are automatically handled - queries return individual split transactions, not parents, so no manual filtering for is_parent is needed.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:14:02.055Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:0-0
Timestamp: 2025-08-16T07:14:02.055Z
Learning: Actual Budget reports do not use tombstone filtering for transactions in their spreadsheet queries.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-06-21T04:15:23.727Z
Learnt from: matt-fidd
PR: actualbudget/actual#5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T16:42:08.258Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:7-8
Timestamp: 2025-08-16T16:42:08.258Z
Learning: In the Actual Budget codebase, it's acceptable and consistent to import useSpreadsheet as a type-only import and then use ReturnType<typeof useSpreadsheet> in type annotations. This pattern is used consistently across spreadsheet files like custom-spreadsheet.ts and crossover-spreadsheet.ts.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧬 Code Graph Analysis (3)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (21)
packages/desktop-client/src/hooks/useWidget.ts (1)
useWidget(8-21)packages/loot-core/src/types/models/dashboard.ts (2)
CrossoverWidget(66-77)TimeFrame(4-14)packages/desktop-client/src/redux/index.ts (1)
useDispatch(18-18)packages/desktop-client/src/hooks/useAccounts.ts (1)
useAccounts(8-20)packages/desktop-client/src/hooks/useCategories.ts (1)
useCategories(8-20)packages/loot-core/src/shared/test-helpers.ts (1)
start(140-142)packages/loot-core/src/types/models/category.ts (1)
CategoryEntity(3-13)packages/desktop-client/src/notifications/notificationsSlice.ts (1)
addNotification(58-68)packages/desktop-client/src/hooks/useSpreadsheet.tsx (1)
useSpreadsheet(19-25)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (1)
createCrossoverSpreadsheet(56-176)packages/desktop-client/src/hooks/useNavigate.ts (1)
useNavigate(12-48)packages/desktop-client/src/components/Page.tsx (3)
Page(115-155)MobilePageHeader(47-105)PageHeader(16-38)packages/desktop-client/src/components/mobile/MobileBackButton.tsx (1)
MobileBackButton(13-44)packages/desktop-client/src/components/EditablePageHeaderTitle.tsx (1)
EditablePageHeaderTitle(15-88)packages/desktop-client/src/components/reports/Header.tsx (1)
Header(55-290)packages/component-library/src/View.tsx (1)
View(14-33)packages/desktop-client/src/components/reports/CategorySelector.tsx (1)
CategorySelector(31-271)packages/desktop-client/src/components/reports/AccountSelector.tsx (1)
AccountSelector(26-421)packages/component-library/src/Input.tsx (1)
Input(52-91)packages/component-library/src/Select.tsx (1)
Select(44-138)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (1)
CrossoverGraph(39-195)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (2)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/desktop-client/src/hooks/useSpreadsheet.tsx (1)
useSpreadsheet(19-25)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (10)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/loot-core/src/types/models/dashboard.ts (1)
CrossoverWidget(66-77)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (1)
createCrossoverSpreadsheet(56-176)packages/desktop-client/src/components/reports/ReportCard.tsx (1)
ReportCard(34-119)packages/component-library/src/View.tsx (1)
View(14-33)packages/desktop-client/src/components/reports/ReportCardName.tsx (1)
ReportCardName(17-55)packages/component-library/src/styles.ts (1)
styles(16-157)packages/desktop-client/src/components/PrivacyFilter.tsx (1)
PrivacyFilter(47-68)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (1)
CrossoverGraph(39-195)packages/desktop-client/src/components/reports/LoadingIndicator.tsx (1)
LoadingIndicator(13-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Analyze
- GitHub Check: lint
- GitHub Check: build (macos-latest)
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: autofix
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Functional Desktop App
🔇 Additional comments (7)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (1)
199-208: Double-check sliding-window date adjustment to avoid off-by-oneonChangeDates subtracts one month when mode === 'sliding-window', while Header already computes ranges (including “Live” toggling). Ensure this extra shift doesn’t double-exclude the current month for “Live” ranges.
If this proves redundant, remove the month subtraction:
- if (mode === 'sliding-window') { - // This is because we don't include the current month in the sliding window - start = monthUtils.subMonths(start, 1); - end = monthUtils.subMonths(end, 1); - }I can generate a quick test harness to compare ranges produced by Header conversions vs this local adjustment if helpful.
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (6)
12-45: Hampel utilities look solidMedian, MAD, and the filtered median implementation are correct and defensively handle small arrays.
95-107: Expense query: OK to include positives (refunds) given category scopingGiven you scope by selected (non-income) categories, summing amounts and flipping sign later is acceptable and allows reimbursements to reduce spend, which matches budget semantics.
114-152: Good defensive default for startingBalance and exclusion of first month deltasDefaulting non-numeric calculate() results to 0 prevents NaN; filtering out the first month from grouped deltas is correct given you seed with the end-of-first-month balance.
226-255: First-occurrence crossover detection is correctCapturing the first index where income ≥ expenses avoids overwriting later crossovers. Good fix.
260-279: Effective monthly rate and annualization via compounding are correctUsing (1 + r)^(1/12) - 1 and later annualizing with (1 + r_m)^12 - 1 is the right approach.
323-357: Projection loop bounds and safeguards look goodProjection is capped at 600 months (50 years), prevents infinite loops, and uses chosen projection method appropriately with non-negative expense clamp.
Use effective monthly rate from annual return Guard against undefined starting balance
Use consistent rounding when checking the projected crossover Avoid O(n^2) lookups when aggregating balances by month
sjones512
force-pushed
the
feat/add-crossover-report
branch
from
4d7b524 to
53b120a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (4)
73-95: Avoid setState on unmounted component (add effect cleanup)
The asynccalculateDateRangecan resolve after unmount. Add a cancellation guard to prevent state updates on unmounted.useEffect(() => { - async function calculateDateRange() { + let cancelled = false; + async function calculateDateRange() { const trans = await send('get-earliest-transaction'); const currentMonth = monthUtils.currentMonth(); const earliestMonth = trans ? monthUtils.monthFromDate(d.parseISO(fromDateRepr(trans.date))) : currentMonth; // Use saved timeFrame from meta or default range let startMonth = earliestMonth; let endMonth = monthUtils.subMonths(currentMonth, 1); // Exclude current month by default - if (meta?.timeFrame?.start && meta?.timeFrame?.end) { - startMonth = meta.timeFrame.start; - endMonth = meta.timeFrame.end; - } + if (meta?.timeFrame?.start) startMonth = meta.timeFrame.start; + if (meta?.timeFrame?.end) endMonth = meta.timeFrame.end; + // ensure start <= end for a valid range + if (d.isAfter(d.parseISO(startMonth + '-01'), d.parseISO(endMonth + '-01'))) { + startMonth = endMonth; + } - setStart(startMonth); - setEnd(endMonth); + if (!cancelled) { + setStart(startMonth); + setEnd(endMonth); + } } calculateDateRange(); -}, [meta?.timeFrame]); + return () => { + cancelled = true; + }; +}, [meta?.timeFrame]);
106-109: Exclude closed accounts from default selections
Defaulting to all accounts can include closed accounts, skewing projections. Filter them out by default; explicit selection can still add them if needed.- const incomeAccountIds = useMemo( - () => meta?.incomeAccountIds ?? accounts.map(a => a.id), - [meta?.incomeAccountIds, accounts], - ); + const incomeAccountIds = useMemo( + () => + meta?.incomeAccountIds ?? + accounts.filter(a => a.closed === 0).map(a => a.id), + [meta?.incomeAccountIds, accounts], + );
81-88: Allow partial timeFrame overrides (not only when both start and end provided)
Currently overrides apply only when bothstartandendexist. Supporting partial overrides improves resilience of persisted meta.See diff in the cleanup comment above (applies partial overrides independently).
195-198: Localize “N/A” and year label; use locale-aware number formatting
'N/A'is user-facing and should be translatable.- Use locale-aware formatting for
yearsToRetireinstead of manualtoFixed.- {yearsToRetire != null - ? `${yearsToRetire.toFixed(1)} years` - : 'N/A'} + {yearsToRetire != null + ? `${yearsToRetire.toLocaleString(undefined, { + minimumFractionDigits: 1, + maximumFractionDigits: 1, + })} ${t('years')}` + : t('N/A')}packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (4)
89-111: Confirm default behavior when no categories selected (zeros vs “all expenses”)
WhenexpenseCategoryIdsis empty you return zero for every month. This makes the default report show 0 expenses and immediate crossover, which may be misleading as an initial experience. Many reports default to “all expenses” when no filter is specified.If the desired default is “all expenses,” consider:
- if (!expenseCategoryIds.length) { - return monthUtils - .rangeInclusive(start, end) - .map(date => ({ date, amount: 0 })); - } + if (!expenseCategoryIds.length) { + const query = q('transactions') + .filter({ + $and: [ + { date: { $gte: monthUtils.firstDayOfMonth(start) } }, + { date: { $lte: monthUtils.lastDayOfMonth(end) } }, + { amount: { $lt: 0 } }, // expenses only + ], + }) + .groupBy({ $month: '$date' }) + .select([ + { date: { $month: '$date' } }, + { amount: { $sum: '$amount' } }, + ]); + const { data } = await aqlQuery(query); + return (data ?? []) as MonthlyAgg[]; + }If zero-by-default is intentional, consider showing an explicit UI hint prompting the user to select categories.
109-111: Harden aql results with nullish fallback
Defensively defaultdatato[]to avoid undefined propagation if the query returns no rows.- const { data } = await aqlQuery(query); - return data as MonthlyAgg[]; + const { data } = await aqlQuery(query); + return (data ?? []) as MonthlyAgg[];
363-371: Parsing month labels for “years to retire” can be locale-sensitive
d.parse(crossoverData.x, 'MMM yyyy', currentDate)assumes English month abbreviations. Since labels were generated withd.format(..., 'MMM yyyy')without an explicit locale, parsing should match—but if a different locale is introduced later this can break. Consider carrying the underlyingYYYY-MMmonth key alongsidexto avoid parsing UI strings.const data: Array<{ - x: string; + x: string; // formatted + key?: string; // machine-readable 'YYYY-MM' investmentIncome: number; expenses: number; isProjection?: boolean; }> = [];Then set
key = monthfor historical andkey = d.formatISO(monthCursor, { representation: 'date' }).slice(0, 7)for projections, and use that for duration calculations.
46-54: Prefer interface over type for param shape
House style favors interfaces for object shapes. Non-blocking, but easy to align.-export type CrossoverParams = { +export interface CrossoverParams { start: string; end: string; expenseCategoryIds: string[]; // which categories count as expenses incomeAccountIds: AccountEntity['id'][]; // selected accounts for both historical returns and projections safeWithdrawalRate: number; // annual percent, e.g. 0.04 for 4% estimatedReturn?: number | null; // optional annual return to project future balances projectionType: 'trend' | 'hampel'; // expense projection method -}; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
upcoming-release-notes/5554.mdis excluded by!**/*.md
📒 Files selected for processing (9)
packages/desktop-client/src/components/reports/AccountSelector.tsx(1 hunks)packages/desktop-client/src/components/reports/Overview.tsx(3 hunks)packages/desktop-client/src/components/reports/ReportRouter.tsx(2 hunks)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/Crossover.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts(1 hunks)packages/loot-core/src/server/dashboard/app.ts(1 hunks)packages/loot-core/src/types/models/dashboard.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/loot-core/src/server/dashboard/app.ts
- packages/desktop-client/src/components/reports/ReportRouter.tsx
- packages/desktop-client/src/components/reports/Overview.tsx
- packages/desktop-client/src/components/reports/reports/Crossover.tsx
- packages/desktop-client/src/components/reports/AccountSelector.tsx
- packages/loot-core/src/types/models/dashboard.ts
- packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Write concise, technical TypeScript code
Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError)
Structure files with sections in order: exported page/component, GraphQL queries, helpers, static content, types
Favor named exports for components and utilities
Prefer interfaces over types in TypeScript
Avoid enums; use objects or maps instead
Avoid using any or unknown unless absolutely necessary; prefer existing types
Avoid type assertions with as or non-null !; prefer satisfies
Use the function keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsxpackages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
Use declarative JSX, keeping JSX minimal and readable
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧠 Learnings (7)
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2025-08-16T16:42:08.258Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:7-8
Timestamp: 2025-08-16T16:42:08.258Z
Learning: In the Actual Budget codebase, it's the established pattern and acceptable to import useSpreadsheet as a type-only import and then use ReturnType<typeof useSpreadsheet> in type annotations. This pattern is used consistently across all spreadsheet files including custom-spreadsheet.ts, grouped-spreadsheet.ts, spending-spreadsheet.ts, net-worth-spreadsheet.ts, cash-flow-spreadsheet.tsx, calendar-spreadsheet.ts, summary-spreadsheet.ts, and crossover-spreadsheet.ts.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:09:15.665Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:375-387
Timestamp: 2025-08-16T07:09:15.665Z
Learning: When handling financial data consistency issues in the desktop client, prefer keeping the data layer (spreadsheets) in raw integer cents and use the useFormat hook for all presentation formatting, rather than converting to display amounts in the data layer.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:14:02.055Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:0-0
Timestamp: 2025-08-16T07:14:02.055Z
Learning: In Actual Budget's Actual QL system, parent transactions for splits are automatically handled - queries return individual split transactions, not parents, so no manual filtering for is_parent is needed.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T07:14:02.055Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:0-0
Timestamp: 2025-08-16T07:14:02.055Z
Learning: Actual Budget reports do not use tombstone filtering for transactions in their spreadsheet queries.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-06-21T04:15:23.727Z
Learnt from: matt-fidd
PR: actualbudget/actual#5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
📚 Learning: 2025-08-16T16:42:08.258Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:7-8
Timestamp: 2025-08-16T16:42:08.258Z
Learning: In the Actual Budget codebase, it's acceptable and consistent to import useSpreadsheet as a type-only import and then use ReturnType<typeof useSpreadsheet> in type annotations. This pattern is used consistently across spreadsheet files like custom-spreadsheet.ts and crossover-spreadsheet.ts.
Applied to files:
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
🧬 Code Graph Analysis (2)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (6)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/loot-core/src/types/models/dashboard.ts (1)
CrossoverWidget(66-77)packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (1)
createCrossoverSpreadsheet(56-176)packages/desktop-client/src/components/reports/ReportCard.tsx (1)
ReportCard(34-119)packages/desktop-client/src/components/PrivacyFilter.tsx (1)
PrivacyFilter(47-68)packages/desktop-client/src/components/reports/graphs/CrossoverGraph.tsx (1)
CrossoverGraph(39-195)
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (2)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/desktop-client/src/hooks/useSpreadsheet.tsx (1)
useSpreadsheet(19-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Functional Desktop App
- GitHub Check: test
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze
- GitHub Check: compare
🔇 Additional comments (4)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (1)
142-151: LGTM: Card wiring and menu handling
Clean integration with ReportCard, disableClick during rename, and deep-link viatolook solid.packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts (3)
226-235: LGTM: Consistent money units (cents) and rounding for series
Amounts are kept in integer cents with rounding at the data layer, aligning with report formatting conventions viauseFormat.
260-279: LGTM: Effective monthly rate and CAGR computation
Using(1 + r)^(1/12) - 1and guarding against invalid CAGR inputs is correct and avoids compounding errors.
241-255: First-crossover detection handled correctly
Capturing only the first month where income >= expenses avoids later overwrites; good fix.
This pull request adds a Crossover Point report for projecting when income from investments may be sufficient to cover expenses.
Selected categories are used as expenses and then forecast into the future.
Selected account balances are treated as investments and similarly forecast into the future using a configurable annual rate of return.
A configurable "Safe Withdrawal Rate" is used to determine how much income the investment accounts provide monthly.
The report projects out until the Projected Income exceeds the Projected Expenses or 50 years, whichever comes first.