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 No assets were 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 feature-flagged "Crossover" report and widget across client and server. New frontend components: AccountSelector, CrossoverGraph, Crossover report page, CrossoverCard, and a crossover-spreadsheet module that queries monthly expenses and historical balances, applies median/MAD/Hampel filtering, projects income/expenses, and detects a crossover point. Overview and ReportRouter are updated to surface the Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Potential hotspots to review:
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
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
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
Show resolved
Hide resolved
packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts
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
|
Ok, after some more digging. Its just that if you start from zero, it makes the historical return look really big, which is technically true, but the first few years have a really really high "return" because contributions are such a huge portion of the gains. Aside from splitting out contributions and growth, maybe the historical return could look at just the last few years, or the current window, whichever is smaller. |
|
The dashboard card is reporting different values than the opened report |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (3)
73-94: Optimize date range calculation and add cleanup.The effect always fetches the earliest transaction even when
meta.timeFrameis already set. Additionally, the async operation lacks cleanup to prevent state updates on an unmounted component.Apply this diff to optimize and add cleanup:
useEffect(() => { + let isMounted = true; async function calculateDateRange() { + // Use saved timeFrame from meta if available + if (meta?.timeFrame?.start && meta?.timeFrame?.end) { + setStart(meta.timeFrame.start); + setEnd(meta.timeFrame.end); + return; + } + const trans = await send('get-earliest-transaction'); + if (!isMounted) return; + 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; - } - - setStart(startMonth); - setEnd(endMonth); + setStart(earliestMonth); + setEnd(monthUtils.subMonths(currentMonth, 1)); // Exclude current month by default } calculateDateRange(); + return () => { + isMounted = false; + }; }, [meta?.timeFrame]);
194-198: Consider using format utility for consistency.The
toFixed(1)method is used directly for number formatting. For consistency with the rest of the codebase, consider using theformatutility (available viauseFormathook) which already handles privacy mode and locale preferences.Example refactor:
+ const format = useFormat(); + // ... in the JSX <PrivacyFilter activationFilters={[!isCardHovered]}> {yearsToRetire != null - ? `${yearsToRetire.toFixed(1)} years` + ? t('{{years}} years', { years: format(yearsToRetire, 'decimal1') }) : 'N/A'} </PrivacyFilter>Note: Verify that a
'decimal1'format type exists in the format utility, or use an appropriate alternative.
151-162: Consider logging unrecognized menu selections instead of throwing.Throwing an error for an unrecognized menu selection could crash the component. While this helps catch developer errors during development, consider logging the error instead for a more graceful failure in production.
Apply this diff:
+import { logger } from 'loot-core/platform/client/logger'; + // ... in the callback onMenuSelect={item => { switch (item) { case 'rename': setNameMenuOpen(true); break; case 'remove': onRemove(); break; default: - throw new Error(`Unrecognized selection: ${item}`); + logger.error(`Unrecognized menu selection: ${item}`); } }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Prefertypeoverinterfacein TypeScript
Avoidenum; use objects or maps instead
Avoidanyandunknownunless absolutely necessary
Avoid type assertions (as, non-null!); prefersatisfiesfor narrowing
Use inline type imports:import { type X } from '...'
Favor functional/declarative patterns; avoid classes
Use thefunctionkeyword for pure functions
Use named exports for components and utilities (avoid default exports except specific cases)
Maintain import order groups (React, Node built-ins, externals, Actual packages, parent, sibling, index) with newlines between groups
Do not directly reference platform-specific imports by extension (.api,.web,.electron)
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not useconsole.*; use the logger instead
Importuuidwith destructuring:import { v4 as uuidv4 } from 'uuid'
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/{desktop-client,component-library}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/{desktop-client,component-library}/src/**/*.{ts,tsx}: All user-facing strings must be translated
Prefer<Trans>component overt()when possible
Do not useReact.*namespace imports; use named imports
Use theme tokens; do not import colors directly
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/{desktop-client,component-library}/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
packages/{desktop-client,component-library}/src/**/*.tsx: Do not useReact.FC/React.FunctionComponent; type props directly
Avoid unstable nested components in JSX
Prefer declarative, minimal JSX; avoid unnecessary curly braces and prefer concise conditional expressions (condition && <Component />)
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/desktop-client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use
<Link>instead of<a>tags
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/desktop-client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/desktop-client/src/**/*.{ts,tsx}: Use custom navigation hook fromsrc/hooksinstead of react-router directly
UseuseDispatch,useSelector,useStorefromsrc/redux(not react-redux)
Use absolute imports in desktop-client
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧬 Code graph analysis (1)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (8)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/loot-core/src/types/models/dashboard.ts (1)
CrossoverWidget(66-78)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/desktop-client/src/components/PrivacyFilter.tsx (1)
PrivacyFilter(47-68)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). (6)
- GitHub Check: build (ubuntu-22.04)
- 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: Generate VRT Updates
🔇 Additional comments (3)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (3)
27-54: LGTM! Type definitions are well-structured.The
CrossoverDataandCrossoverCardPropstypes correctly model the component's data contracts and align with the spreadsheet module and widget metadata fromdashboard.ts.
12-15: Use inline type imports per coding guidelines.The
AccountEntityandCrossoverWidgetimports should use the inlinetypekeyword for consistency with the project's TypeScript conventions.As per coding guidelines
Apply this diff:
-import { - type AccountEntity, - type CrossoverWidget, -} from 'loot-core/types/models'; +import { type AccountEntity, type CrossoverWidget } from 'loot-core/types/models';Likely an incorrect or invalid review comment.
106-109: Default account behavior is intentional and consistent with Crossover.tsx—no changes needed.Both
CrossoverCard.tsxandCrossover.tsxuse the same default: whenmeta.incomeAccountIdsis not set, they default to all accounts. This appears intentional—the Crossover report is designed to be user-configurable. Once a user makes selections in the Crossover report, those selections are saved tometaand then used byCrossoverCard. The initial default to all accounts occurs only when no prior selections exist, which is expected first-use behavior.Other reports filter accounts in query logic rather than at the input level, so the pattern differs but is not inconsistent.
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
Outdated
Show resolved
Hide resolved
I'm not too familiar with CoastFI, but love the idea of supporting additional forms of retirement planning. I imagine that would be a separate PR though?
Good catch. It looks like I broke it when I changed how the percent inputs are handled. Should be fixed now Regarding the inflated historic returns when account balances are small/zero, and contributions are a much more significant that investment returns, I'm not sure that there is a change we need to make right now unless we can identify a good solution. I'll admit it is a bit quirky when there is limited data, but the user can control for this by changing the start date of the report and overriding the Estimated Return. |
Use format utility for years to retire Replaced hardcoded color with theme token
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (2)
1-1: Remove unused default React import.JSX doesn’t require React default import; keep named hooks only. As per coding guidelines.
-import React, { useEffect, useMemo, useState, useCallback } from 'react'; +import { useEffect, useMemo, useState, useCallback } from 'react';
221-242: Add try/catch and user notifications on save failures.Network errors currently fall through silently. Wrap send(...) calls and notify on failure.
- await send('dashboard-update-widget', { + try { + await send('dashboard-update-widget', { id: widget.id, meta: { ...(widget.meta ?? {}), expenseCategoryIds: selectedExpenseCategories.map(c => c.id), incomeAccountIds: selectedIncomeAccountIds, safeWithdrawalRate: Math.max(0, Math.min(100, swr)) / 100, estimatedReturn: estimatedReturn == null ? null : Math.max(0, Math.min(100, estimatedReturn)) / 100, projectionType, showHiddenCategories, timeFrame: { start, end, mode }, }, - }); - dispatch( - addNotification({ - notification: { - type: 'message', - message: t('Dashboard widget successfully saved.'), - }, - }), - ); + }); + dispatch( + addNotification({ + notification: { + type: 'message', + message: t('Dashboard widget successfully saved.'), + }, + }), + ); + } catch (e) { + dispatch( + addNotification({ + notification: { + type: 'error', + message: t('Save failed. Please try again.'), + }, + }), + ); + } @@ - const name = newName || t('Crossover Point'); - await send('dashboard-update-widget', { - id: widget.id, - meta: { - ...(widget.meta ?? {}), - name, - }, - }); + const name = newName || t('Crossover Point'); + try { + await send('dashboard-update-widget', { + id: widget.id, + meta: { + ...(widget.meta ?? {}), + name, + }, + }); + } catch (e) { + dispatch( + addNotification({ + notification: { + type: 'error', + message: t('Rename failed. Please try again.'), + }, + }), + ); + }Also applies to: 313-321
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/desktop-client/src/components/reports/reports/Crossover.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Prefertypeoverinterfacein TypeScript
Avoidenum; use objects or maps instead
Avoidanyandunknownunless absolutely necessary
Avoid type assertions (as, non-null!); prefersatisfiesfor narrowing
Use inline type imports:import { type X } from '...'
Favor functional/declarative patterns; avoid classes
Use thefunctionkeyword for pure functions
Use named exports for components and utilities (avoid default exports except specific cases)
Maintain import order groups (React, Node built-ins, externals, Actual packages, parent, sibling, index) with newlines between groups
Do not directly reference platform-specific imports by extension (.api,.web,.electron)
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not useconsole.*; use the logger instead
Importuuidwith destructuring:import { v4 as uuidv4 } from 'uuid'
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
packages/{desktop-client,component-library}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/{desktop-client,component-library}/src/**/*.{ts,tsx}: All user-facing strings must be translated
Prefer<Trans>component overt()when possible
Do not useReact.*namespace imports; use named imports
Use theme tokens; do not import colors directly
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
packages/{desktop-client,component-library}/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
packages/{desktop-client,component-library}/src/**/*.tsx: Do not useReact.FC/React.FunctionComponent; type props directly
Avoid unstable nested components in JSX
Prefer declarative, minimal JSX; avoid unnecessary curly braces and prefer concise conditional expressions (condition && <Component />)
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
packages/desktop-client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use
<Link>instead of<a>tags
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
packages/desktop-client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/desktop-client/src/**/*.{ts,tsx}: Use custom navigation hook fromsrc/hooksinstead of react-router directly
UseuseDispatch,useSelector,useStorefromsrc/redux(not react-redux)
Use absolute imports in desktop-client
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
🧬 Code graph analysis (1)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (17)
packages/desktop-client/src/hooks/useWidget.ts (1)
useWidget(8-21)packages/loot-core/src/types/models/dashboard.ts (2)
CrossoverWidget(66-78)TimeFrame(4-14)packages/desktop-client/src/components/reports/LoadingIndicator.tsx (1)
LoadingIndicator(13-32)packages/desktop-client/src/hooks/useAccounts.ts (1)
useAccounts(8-20)packages/desktop-client/src/hooks/useCategories.ts (1)
useCategories(8-22)packages/desktop-client/src/hooks/useFormat.ts (1)
useFormat(109-282)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(56-176)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(56-300)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/PrivacyFilter.tsx (1)
PrivacyFilter(47-68)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). (11)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Functional Desktop App
- GitHub Check: validate-cli
- GitHub Check: test
- GitHub Check: web
- GitHub Check: Analyze
- GitHub Check: Generate VRT Updates
- GitHub Check: compare
🔇 Additional comments (1)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (1)
45-46: Fix type-only import with ReturnType.Using ReturnType against a type-only import is invalid. Prefer typeof import(...) to avoid a value import.
-import { type useSpreadsheet } from '@desktop-client/hooks/useSpreadsheet'; +// remove the type-only import; use inline typeof import(...) where needed @@ - async ( - spreadsheet: ReturnType<typeof useSpreadsheet>, + async ( + spreadsheet: ReturnType< + typeof import('@desktop-client/hooks/useSpreadsheet').useSpreadsheet + >, setData: (data: CrossoverData) => void, ) => {Also applies to: 253-257
⛔ Skipped due to learnings
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.306Z 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.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.306Z 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.
packages/desktop-client/src/components/reports/reports/Crossover.tsx
Outdated
Show resolved
Hide resolved
| type="number" | ||
| min={0} | ||
| max={100} | ||
| step={0.1} | ||
| value={swr == null ? '' : swr} | ||
| onChange={e => setSwr(e.target.valueAsNumber)} | ||
| style={{ width: 120, marginBottom: 12 }} | ||
| /> |
There was a problem hiding this comment.
Handle empty input to avoid NaN in SWR.
Clearing the SWR field sets valueAsNumber to NaN, which propagates to the spreadsheet and breaks math.
- <Input
+ <Input
type="number"
min={0}
max={100}
step={0.1}
value={swr == null ? '' : swr}
- onChange={e => setSwr(e.target.valueAsNumber)}
+ onChange={e => {
+ if (e.target.value === '') {
+ setSwr(0);
+ } else {
+ setSwr(e.target.valueAsNumber);
+ }
+ }}
style={{ width: 120, marginBottom: 12 }}
/>🤖 Prompt for AI Agents
In packages/desktop-client/src/components/reports/reports/Crossover.tsx around
lines 488 to 495, clearing the SWR input produces NaN because valueAsNumber
returns NaN for empty strings; change the onChange to detect empty input and set
the SWR state to a safe empty value (null or '') instead of NaN (e.g., if
e.target.value === '' then setSwr(null) else setSwr(e.target.valueAsNumber) ),
ensure the SWR state type accepts that empty value, and keep value bound to swr
== null ? '' : swr so the input stays empty without propagating NaN to the
spreadsheet.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (1)
131-131: Simplify redundant ternary.The expression
estimatedReturn == null ? null : estimatedReturnis equivalent to justestimatedReturn.- estimatedReturn: estimatedReturn == null ? null : estimatedReturn, + estimatedReturn,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/desktop-client/src/components/reports/reports/Crossover.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Prefertypeoverinterfacein TypeScript
Avoidenum; use objects or maps instead
Avoidanyandunknownunless absolutely necessary
Avoid type assertions (as, non-null!); prefersatisfiesfor narrowing
Use inline type imports:import { type X } from '...'
Favor functional/declarative patterns; avoid classes
Use thefunctionkeyword for pure functions
Use named exports for components and utilities (avoid default exports except specific cases)
Maintain import order groups (React, Node built-ins, externals, Actual packages, parent, sibling, index) with newlines between groups
Do not directly reference platform-specific imports by extension (.api,.web,.electron)
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not useconsole.*; use the logger instead
Importuuidwith destructuring:import { v4 as uuidv4 } from 'uuid'
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/{desktop-client,component-library}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/{desktop-client,component-library}/src/**/*.{ts,tsx}: All user-facing strings must be translated
Prefer<Trans>component overt()when possible
Do not useReact.*namespace imports; use named imports
Use theme tokens; do not import colors directly
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/{desktop-client,component-library}/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
packages/{desktop-client,component-library}/src/**/*.tsx: Do not useReact.FC/React.FunctionComponent; type props directly
Avoid unstable nested components in JSX
Prefer declarative, minimal JSX; avoid unnecessary curly braces and prefer concise conditional expressions (condition && <Component />)
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/desktop-client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use
<Link>instead of<a>tags
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/desktop-client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/desktop-client/src/**/*.{ts,tsx}: Use custom navigation hook fromsrc/hooksinstead of react-router directly
UseuseDispatch,useSelector,useStorefromsrc/redux(not react-redux)
Use absolute imports in desktop-client
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧠 Learnings (26)
📚 Learning: 2024-11-01T20:29:18.673Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3744
File: packages/desktop-client/src/components/reports/reports/CustomReport.tsx:157-157
Timestamp: 2024-11-01T20:29:18.673Z
Learning: In the `CustomReport` component (`packages/desktop-client/src/components/reports/reports/CustomReport.tsx`), the session storage references are necessary and should not be removed.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 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/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-10-04T18:16:45.140Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3566
File: packages/desktop-client/src/components/reports/Overview.tsx:100-101
Timestamp: 2024-10-04T18:16:45.140Z
Learning: In `packages/desktop-client/src/components/reports/Overview.tsx`, when filtering `baseLayout`, if `item.type === 'custom-report'`, `item.meta.id` will always be defined.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-11-12T19:52:52.889Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In `packages/desktop-client/src/components/reports/reports/Summary.tsx`, API calls like `get-earliest-transaction` are used without explicit error handling to maintain consistency with other components.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-10-22T11:55:03.192Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 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
Repo: actualbudget/actual PR: 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
📚 Learning: 2024-10-24T05:09:44.115Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/server/backups.web.ts:203-207
Timestamp: 2024-10-24T05:09:44.115Z
Learning: In `packages/loot-core/src/server/backups.web.ts`, developers prefer to keep the error handling for cloud storage uploads inline rather than extracting it into a separate function. Avoid suggesting this refactoring in future reviews.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-08-17T13:48:35.570Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 5533
File: packages/desktop-client/src/components/modals/BudgetAutomationsModal.tsx:116-131
Timestamp: 2025-08-17T13:48:35.570Z
Learning: In the Actual Budget app, the BudgetAutomationsModal's send() calls for 'budget/set-category-automations' have adequate error handling through the global error handling system that shows toast notifications, as confirmed by direct testing where manually injected errors resulted in appropriate toast notifications to the user.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-06-03T23:19:44.814Z
Learnt from: elijaholmos
Repo: actualbudget/actual PR: 5076
File: packages/desktop-client/src/components/CommandBar.tsx:70-70
Timestamp: 2025-06-03T23:19:44.814Z
Learning: The useReports hook in packages/desktop-client/src/hooks/useReports.ts always returns an array for the data property due to initialData: [] and default value fallback (data: customReports = []), so it never returns undefined and doesn't need additional undefined checks when used.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget mobile app, the callback functions `onEditCategory` (implemented as `onOpenCategoryMenuModal`) and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget app, the callback functions `onEditCategory` and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-06-16T17:45:40.807Z
Learnt from: misu-dev
Repo: actualbudget/actual PR: 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/reports/Crossover.tsx
📚 Learning: 2025-01-21T11:39:38.461Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4154
File: packages/desktop-client/src/components/spreadsheet/CellValue.tsx:47-47
Timestamp: 2025-01-21T11:39:38.461Z
Learning: In the CellValue component of packages/desktop-client/src/components/spreadsheet/CellValue.tsx, the children prop needs an additional length check (children && children.length > 0) because the Trans component can pass an empty array at runtime despite the type definition expecting a function.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-11-14T01:17:25.783Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3792
File: packages/desktop-client/src/components/reports/spreadsheets/summary-spreadsheet.ts:284-287
Timestamp: 2024-11-14T01:17:25.783Z
Learning: In the `calculatePercentage` function in `packages/desktop-client/src/components/reports/spreadsheets/summary-spreadsheet.ts`, division by zero is acceptable and should result in `Infinity` as intended.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-10-24T18:57:16.406Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3732
File: packages/desktop-client/src/components/util/AmountInput.tsx:102-108
Timestamp: 2024-10-24T18:57:16.406Z
Learning: In `packages/desktop-client/src/components/util/AmountInput.tsx`, zero amounts are handled in the `onSwitch` function, so zero amounts are intentionally ignored in the `fireUpdate` function.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-09-28T08:34:21.188Z
Learnt from: misu-dev
Repo: actualbudget/actual PR: 5639
File: packages/desktop-client/src/components/util/GenericInput.jsx:68-73
Timestamp: 2025-09-28T08:34:21.188Z
Learning: In packages/desktop-client/src/components/util/GenericInput.jsx, for amount filtering with inflow/outflow options, the sign prop should always be '+' (for both inflow and outflow) because the UI layer keeps all amounts positive, while the server-side transaction rules engine handles the actual sign conversion and negation logic when querying the database.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-10-21T06:59:54.870Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-21T06:59:54.870Z
Learning: Applies to packages/{desktop-client,component-library}/src/**/*.{ts,tsx} : Prefer `<Trans>` component over `t()` when possible
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-10-21T06:59:54.870Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-21T06:59:54.870Z
Learning: Applies to packages/{desktop-client,component-library}/src/**/*.{ts,tsx} : All user-facing strings must be translated
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-10-08T15:46:15.739Z
Learnt from: qedi-r
Repo: actualbudget/actual PR: 3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is best avoided, and we should almost always prefer using the useTranslation hook if possible.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-11-26T13:07:02.794Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3891
File: packages/loot-core/src/shared/rules.ts:209-212
Timestamp: 2024-11-26T13:07:02.794Z
Learning: The file `packages/loot-core/src/shared/rules.ts` is not yet translated, so internationalization using the `t()` function is not required here.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-10-24T21:29:26.090Z
Learnt from: csenel
Repo: actualbudget/actual PR: 5991
File: packages/desktop-client/src/components/mobile/budget/BudgetCell.tsx:82-89
Timestamp: 2025-10-24T21:29:26.090Z
Learning: Notification messages in BudgetCell.tsx will be translated in a separate PR to handle all translations together, as there are multiple messages to go through.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-10-05T10:58:55.008Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts:16-24
Timestamp: 2024-10-05T10:58:55.008Z
Learning: In `packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts`, the `parseDate` function's `str` parameter should maintain its current type `string | number | null | Array<unknown> | object`, as narrowing it to `string | null` is not suitable.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-01-18T20:08:55.203Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4181
File: packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx:252-254
Timestamp: 2025-01-18T20:08:55.203Z
Learning: Notification messages in BudgetTable.jsx will be translated in a separate PR to handle all translations together, as there are multiple messages to go through.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
Repo: actualbudget/actual PR: 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/reports/reports/Crossover.tsx
📚 Learning: 2025-10-21T06:59:54.870Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-21T06:59:54.870Z
Learning: Applies to packages/{desktop-client,component-library}/src/**/*.{ts,tsx} : Use theme tokens; do not import colors directly
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-11-12T18:16:38.182Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3828
File: packages/desktop-client/src/style/themes/development.ts:217-217
Timestamp: 2024-11-12T18:16:38.182Z
Learning: In `packages/desktop-client/src/style/themes/development.ts`, the constants `calendarCellBackground` and `calendarBackground` are used for different calendar components, so they may share the same color value.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧬 Code graph analysis (2)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (24)
packages/desktop-client/src/hooks/useWidget.ts (1)
useWidget(8-21)packages/loot-core/src/types/models/dashboard.ts (2)
CrossoverWidget(66-78)TimeFrame(4-14)packages/desktop-client/src/components/reports/LoadingIndicator.tsx (1)
LoadingIndicator(13-32)packages/desktop-client/src/hooks/useAccounts.ts (1)
useAccounts(8-20)packages/desktop-client/src/hooks/useCategories.ts (1)
useCategories(8-22)packages/desktop-client/src/hooks/useFormat.ts (1)
useFormat(109-282)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(56-300)packages/component-library/src/Button.tsx (1)
Button(139-185)packages/component-library/src/View.tsx (1)
View(14-33)packages/component-library/src/Text.tsx (1)
Text(19-28)packages/component-library/src/Tooltip.tsx (1)
Tooltip(20-78)packages/component-library/src/styles.ts (1)
styles(16-163)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/theme.ts (1)
theme(1-204)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/component-library/src/Paragraph.tsx (1)
Paragraph(12-32)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (9)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/loot-core/src/types/models/dashboard.ts (1)
CrossoverWidget(66-78)packages/desktop-client/src/hooks/useFormat.ts (1)
useFormat(109-282)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/reports/ReportCardName.tsx (1)
ReportCardName(17-55)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). (11)
- GitHub Check: web
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (macos-latest)
- GitHub Check: Functional Desktop App
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Generate VRT Updates
- GitHub Check: Analyze
- GitHub Check: test
- GitHub Check: validate-cli
- GitHub Check: compare
packages/desktop-client/src/components/reports/reports/Crossover.tsx
Outdated
Show resolved
Hide resolved
… simplify estimatedReturn assignment
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/desktop-client/src/components/reports/reports/Crossover.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/src/components/reports/reports/Crossover.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Prefertypeoverinterfacein TypeScript
Avoidenum; use objects or maps instead
Avoidanyandunknownunless absolutely necessary
Avoid type assertions (as, non-null!); prefersatisfiesfor narrowing
Use inline type imports:import { type X } from '...'
Favor functional/declarative patterns; avoid classes
Use thefunctionkeyword for pure functions
Use named exports for components and utilities (avoid default exports except specific cases)
Maintain import order groups (React, Node built-ins, externals, Actual packages, parent, sibling, index) with newlines between groups
Do not directly reference platform-specific imports by extension (.api,.web,.electron)
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not useconsole.*; use the logger instead
Importuuidwith destructuring:import { v4 as uuidv4 } from 'uuid'
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/{desktop-client,component-library}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/{desktop-client,component-library}/src/**/*.{ts,tsx}: All user-facing strings must be translated
Prefer<Trans>component overt()when possible
Do not useReact.*namespace imports; use named imports
Use theme tokens; do not import colors directly
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/{desktop-client,component-library}/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
packages/{desktop-client,component-library}/src/**/*.tsx: Do not useReact.FC/React.FunctionComponent; type props directly
Avoid unstable nested components in JSX
Prefer declarative, minimal JSX; avoid unnecessary curly braces and prefer concise conditional expressions (condition && <Component />)
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/desktop-client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use
<Link>instead of<a>tags
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/desktop-client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/desktop-client/src/**/*.{ts,tsx}: Use custom navigation hook fromsrc/hooksinstead of react-router directly
UseuseDispatch,useSelector,useStorefromsrc/redux(not react-redux)
Use absolute imports in desktop-client
Files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧠 Learnings (7)
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 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: 2024-11-01T20:29:18.673Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3744
File: packages/desktop-client/src/components/reports/reports/CustomReport.tsx:157-157
Timestamp: 2024-11-01T20:29:18.673Z
Learning: In the `CustomReport` component (`packages/desktop-client/src/components/reports/reports/CustomReport.tsx`), the session storage references are necessary and should not be removed.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-10-04T18:16:45.140Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3566
File: packages/desktop-client/src/components/reports/Overview.tsx:100-101
Timestamp: 2024-10-04T18:16:45.140Z
Learning: In `packages/desktop-client/src/components/reports/Overview.tsx`, when filtering `baseLayout`, if `item.type === 'custom-report'`, `item.meta.id` will always be defined.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-11-12T19:52:52.889Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In `packages/desktop-client/src/components/reports/reports/Summary.tsx`, API calls like `get-earliest-transaction` are used without explicit error handling to maintain consistency with other components.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-10-04T05:13:58.322Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 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/reports/CrossoverCard.tsx
📚 Learning: 2025-10-21T06:59:54.870Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-21T06:59:54.870Z
Learning: Applies to packages/{desktop-client,component-library}/src/**/*.{ts,tsx} : Use theme tokens; do not import colors directly
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-11-12T18:16:38.182Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3828
File: packages/desktop-client/src/style/themes/development.ts:217-217
Timestamp: 2024-11-12T18:16:38.182Z
Learning: In `packages/desktop-client/src/style/themes/development.ts`, the constants `calendarCellBackground` and `calendarBackground` are used for different calendar components, so they may share the same color value.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧬 Code graph analysis (1)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (13)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/loot-core/src/types/models/dashboard.ts (1)
CrossoverWidget(66-78)packages/desktop-client/src/hooks/useFormat.ts (1)
useFormat(109-282)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/Block.tsx (1)
Block(12-17)packages/component-library/src/styles.ts (1)
styles(16-163)packages/desktop-client/src/components/PrivacyFilter.tsx (1)
PrivacyFilter(47-68)packages/component-library/src/theme.ts (1)
theme(1-204)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). (11)
- GitHub Check: Functional Desktop App
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: web
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: Analyze
- GitHub Check: validate-cli
- GitHub Check: test
- GitHub Check: Generate VRT Updates
- GitHub Check: compare
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
Show resolved
Hide resolved
…ensure proper handling of the latest month
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/desktop-client/src/components/reports/reports/Crossover.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Prefertypeoverinterfacein TypeScript
Avoidenum; use objects or maps instead
Avoidanyandunknownunless absolutely necessary
Avoid type assertions (as, non-null!); prefersatisfiesfor narrowing
Use inline type imports:import { type X } from '...'
Favor functional/declarative patterns; avoid classes
Use thefunctionkeyword for pure functions
Use named exports for components and utilities (avoid default exports except specific cases)
Maintain import order groups (React, Node built-ins, externals, Actual packages, parent, sibling, index) with newlines between groups
Do not directly reference platform-specific imports by extension (.api,.web,.electron)
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not useconsole.*; use the logger instead
Importuuidwith destructuring:import { v4 as uuidv4 } from 'uuid'
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/{desktop-client,component-library}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/{desktop-client,component-library}/src/**/*.{ts,tsx}: All user-facing strings must be translated
Prefer<Trans>component overt()when possible
Do not useReact.*namespace imports; use named imports
Use theme tokens; do not import colors directly
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/{desktop-client,component-library}/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
packages/{desktop-client,component-library}/src/**/*.tsx: Do not useReact.FC/React.FunctionComponent; type props directly
Avoid unstable nested components in JSX
Prefer declarative, minimal JSX; avoid unnecessary curly braces and prefer concise conditional expressions (condition && <Component />)
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/desktop-client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use
<Link>instead of<a>tags
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
packages/desktop-client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/desktop-client/src/**/*.{ts,tsx}: Use custom navigation hook fromsrc/hooksinstead of react-router directly
UseuseDispatch,useSelector,useStorefromsrc/redux(not react-redux)
Use absolute imports in desktop-client
Files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧠 Learnings (30)
📚 Learning: 2024-11-01T20:29:18.673Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3744
File: packages/desktop-client/src/components/reports/reports/CustomReport.tsx:157-157
Timestamp: 2024-11-01T20:29:18.673Z
Learning: In the `CustomReport` component (`packages/desktop-client/src/components/reports/reports/CustomReport.tsx`), the session storage references are necessary and should not be removed.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 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/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-10-04T18:16:45.140Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3566
File: packages/desktop-client/src/components/reports/Overview.tsx:100-101
Timestamp: 2024-10-04T18:16:45.140Z
Learning: In `packages/desktop-client/src/components/reports/Overview.tsx`, when filtering `baseLayout`, if `item.type === 'custom-report'`, `item.meta.id` will always be defined.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-11-12T19:52:52.889Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In `packages/desktop-client/src/components/reports/reports/Summary.tsx`, API calls like `get-earliest-transaction` are used without explicit error handling to maintain consistency with other components.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsxpackages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-10-22T11:55:03.192Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 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
Repo: actualbudget/actual PR: 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
📚 Learning: 2024-10-24T05:09:44.115Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/server/backups.web.ts:203-207
Timestamp: 2024-10-24T05:09:44.115Z
Learning: In `packages/loot-core/src/server/backups.web.ts`, developers prefer to keep the error handling for cloud storage uploads inline rather than extracting it into a separate function. Avoid suggesting this refactoring in future reviews.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-08-17T13:48:35.570Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 5533
File: packages/desktop-client/src/components/modals/BudgetAutomationsModal.tsx:116-131
Timestamp: 2025-08-17T13:48:35.570Z
Learning: In the Actual Budget app, the BudgetAutomationsModal's send() calls for 'budget/set-category-automations' have adequate error handling through the global error handling system that shows toast notifications, as confirmed by direct testing where manually injected errors resulted in appropriate toast notifications to the user.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget mobile app, the callback functions `onEditCategory` (implemented as `onOpenCategoryMenuModal`) and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget app, the callback functions `onEditCategory` and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-06-16T17:45:40.807Z
Learnt from: misu-dev
Repo: actualbudget/actual PR: 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/reports/Crossover.tsx
📚 Learning: 2025-01-21T11:39:38.461Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4154
File: packages/desktop-client/src/components/spreadsheet/CellValue.tsx:47-47
Timestamp: 2025-01-21T11:39:38.461Z
Learning: In the CellValue component of packages/desktop-client/src/components/spreadsheet/CellValue.tsx, the children prop needs an additional length check (children && children.length > 0) because the Trans component can pass an empty array at runtime despite the type definition expecting a function.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-11-14T01:17:25.783Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3792
File: packages/desktop-client/src/components/reports/spreadsheets/summary-spreadsheet.ts:284-287
Timestamp: 2024-11-14T01:17:25.783Z
Learning: In the `calculatePercentage` function in `packages/desktop-client/src/components/reports/spreadsheets/summary-spreadsheet.ts`, division by zero is acceptable and should result in `Infinity` as intended.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-10-24T18:57:16.406Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3732
File: packages/desktop-client/src/components/util/AmountInput.tsx:102-108
Timestamp: 2024-10-24T18:57:16.406Z
Learning: In `packages/desktop-client/src/components/util/AmountInput.tsx`, zero amounts are handled in the `onSwitch` function, so zero amounts are intentionally ignored in the `fireUpdate` function.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-09-28T08:34:21.188Z
Learnt from: misu-dev
Repo: actualbudget/actual PR: 5639
File: packages/desktop-client/src/components/util/GenericInput.jsx:68-73
Timestamp: 2025-09-28T08:34:21.188Z
Learning: In packages/desktop-client/src/components/util/GenericInput.jsx, for amount filtering with inflow/outflow options, the sign prop should always be '+' (for both inflow and outflow) because the UI layer keeps all amounts positive, while the server-side transaction rules engine handles the actual sign conversion and negation logic when querying the database.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-10-21T06:59:54.870Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-21T06:59:54.870Z
Learning: Applies to packages/{desktop-client,component-library}/src/**/*.{ts,tsx} : Prefer `<Trans>` component over `t()` when possible
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-10-21T06:59:54.870Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-21T06:59:54.870Z
Learning: Applies to packages/{desktop-client,component-library}/src/**/*.{ts,tsx} : All user-facing strings must be translated
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-10-08T15:46:15.739Z
Learnt from: qedi-r
Repo: actualbudget/actual PR: 3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is best avoided, and we should almost always prefer using the useTranslation hook if possible.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-10-24T21:29:26.090Z
Learnt from: csenel
Repo: actualbudget/actual PR: 5991
File: packages/desktop-client/src/components/mobile/budget/BudgetCell.tsx:82-89
Timestamp: 2025-10-24T21:29:26.090Z
Learning: Notification messages in BudgetCell.tsx will be translated in a separate PR to handle all translations together, as there are multiple messages to go through.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-11-26T13:07:02.794Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3891
File: packages/loot-core/src/shared/rules.ts:209-212
Timestamp: 2024-11-26T13:07:02.794Z
Learning: The file `packages/loot-core/src/shared/rules.ts` is not yet translated, so internationalization using the `t()` function is not required here.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-01-18T20:08:55.203Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4181
File: packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx:252-254
Timestamp: 2025-01-18T20:08:55.203Z
Learning: Notification messages in BudgetTable.jsx will be translated in a separate PR to handle all translations together, as there are multiple messages to go through.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
Repo: actualbudget/actual PR: 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/reports/reports/Crossover.tsx
📚 Learning: 2024-10-05T10:58:55.008Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts:16-24
Timestamp: 2024-10-05T10:58:55.008Z
Learning: In `packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts`, the `parseDate` function's `str` parameter should maintain its current type `string | number | null | Array<unknown> | object`, as narrowing it to `string | null` is not suitable.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-01-17T12:11:23.669Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3840
File: packages/loot-core/src/server/budget/template-notes.ts:12-12
Timestamp: 2025-01-17T12:11:23.669Z
Learning: In server-side code or non-React contexts where React hooks cannot be used, prefer using `import { t } from 'i18next'` and directly using `t(string)` for translations, instead of using react-i18next hooks or other approaches.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2024-10-12T23:57:22.683Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 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/reports/Crossover.tsx
📚 Learning: 2024-10-21T02:24:38.823Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3699
File: packages/loot-core/src/client/actions/app.ts:56-74
Timestamp: 2024-10-21T02:24:38.823Z
Learning: The team has decided to handle the `/accounts` route in the default case within the `getPageDocs` function in `packages/loot-core/src/client/actions/app.ts`, as discussed in a previous PR.
Applied to files:
packages/desktop-client/src/components/reports/reports/Crossover.tsx
📚 Learning: 2025-10-21T06:59:54.870Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-21T06:59:54.870Z
Learning: Applies to packages/{desktop-client,component-library}/src/**/*.{ts,tsx} : Use theme tokens; do not import colors directly
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-11-12T18:16:38.182Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3828
File: packages/desktop-client/src/style/themes/development.ts:217-217
Timestamp: 2024-11-12T18:16:38.182Z
Learning: In `packages/desktop-client/src/style/themes/development.ts`, the constants `calendarCellBackground` and `calendarBackground` are used for different calendar components, so they may share the same color value.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2025-01-16T14:29:13.188Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:200-200
Timestamp: 2025-01-16T14:29:13.188Z
Learning: In the scheduled transactions implementation within `packages/loot-core/src/client/data-hooks/transactions.ts`, the `upcoming` flag is set based on `schedules.length > 0` to act as an override, where the first occurrence gets `false` and subsequent occurrences get `true`. This is intentional and should not be changed to date-based comparison.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
📚 Learning: 2024-10-23T16:53:06.163Z
Learnt from: youngcw
Repo: actualbudget/actual PR: 3714
File: packages/loot-core/src/server/budget/goaltemplates.ts:684-688
Timestamp: 2024-10-23T16:53:06.163Z
Learning: In `packages/loot-core/src/server/budget/goaltemplates.ts`, when calculating the number of days in a month for daily limits, using `monthUtils.differenceInCalendarDays(monthUtils.addMonths(month, 1), month)` is correct and adding 1 is not required.
Applied to files:
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx
🧬 Code graph analysis (2)
packages/desktop-client/src/components/reports/reports/Crossover.tsx (13)
packages/desktop-client/src/hooks/useWidget.ts (1)
useWidget(8-21)packages/loot-core/src/types/models/dashboard.ts (2)
CrossoverWidget(66-78)TimeFrame(4-14)packages/desktop-client/src/components/reports/LoadingIndicator.tsx (1)
LoadingIndicator(13-32)packages/desktop-client/src/hooks/useAccounts.ts (1)
useAccounts(8-20)packages/desktop-client/src/hooks/useCategories.ts (1)
useCategories(8-22)packages/desktop-client/src/hooks/useFormat.ts (1)
useFormat(109-282)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/components/reports/Header.tsx (1)
Header(56-300)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/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/reports/CrossoverCard.tsx (12)
packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/loot-core/src/types/models/dashboard.ts (1)
CrossoverWidget(66-78)packages/desktop-client/src/hooks/useFormat.ts (1)
useFormat(109-282)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-163)packages/desktop-client/src/components/PrivacyFilter.tsx (1)
PrivacyFilter(47-68)packages/component-library/src/theme.ts (1)
theme(1-204)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). (11)
- GitHub Check: Generate VRT Updates
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: Analyze
- GitHub Check: web
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: test
- GitHub Check: validate-cli
- GitHub Check: Functional Desktop App
- GitHub Check: compare
🔇 Additional comments (9)
packages/desktop-client/src/components/reports/reports/CrossoverCard.tsx (1)
1-239: LGTM! Implementation follows established patterns.The component properly integrates with the dashboard widget system, handles date range initialization correctly (preventing reversed ranges when only current-month data exists), uses theme tokens for styling, and provides proper translations for all user-facing strings. The memoization strategy and spreadsheet parameter construction align well with other report components in the codebase.
packages/desktop-client/src/components/reports/reports/Crossover.tsx (8)
1-86: LGTM! Clean component structure and proper imports.The outer
Crossovercomponent follows React 19 best practices with direct prop typing, uses the established widget loading pattern, and maintains proper import organization with inline type imports.
90-161: LGTM! State initialization properly handles both new and existing widgets.The initialization effect correctly runs for both new widgets (meta absent) and existing widgets (meta present), defaulting to sensible selections (all non-income categories, all accounts) and preventing re-initialization via the
selectionsInitializedflag. This addresses the past review concern about empty selections on new widgets.
163-212: LGTM! Date range management is consistent and handles all cases.The date initialization properly computes defaults from transaction history, respects saved timeFrame from widget meta, and correctly adjusts sliding-window dates to exclude the current month.
214-248: LGTM! Widget persistence includes proper error handling.The save handler correctly guards against missing widget with a user-friendly notification (addressing past review feedback), saves all configuration including fractions for rates, and confirms success to the user.
251-301: LGTM! Data pipeline correctly filters and transforms values.The memoized
expenseCategoryIdsproperly respects theshowHiddenCategoriestoggle (addressing past review feedback), and the params callback passes fraction values to the spreadsheet as expected bycreateCrossoverSpreadsheet.
306-327: LGTM! Name save handler uses consistent error handling.Properly guards against missing widget with notifications, matching the pattern established in
onSaveWidget.
603-617: LGTM! Input conversion maintains fraction/percent consistency.Both SWR and estimated return inputs correctly:
- Display as percents (
value * 100)- Store as fractions (
input / 100)- Handle NaN gracefully (converting to 0 or null)
- Maintain consistency with save logic and spreadsheet parameters
This addresses past review concerns about unit mismatches and NaN handling.
Also applies to: 707-740
340-846: LGTM! Complete report implementation with proper composition.The render section properly:
- Uses responsive layout with conditional sidebar
- Includes comprehensive tooltips with helpful information
- Displays stats with privacy filtering
- Integrates the CrossoverGraph component
- Provides clear documentation in the description
- Uses translations throughout (Trans and t())
- Applies theme tokens consistently
All past review concerns (N/A translation, error handling, unit consistency) have been addressed.
…" in user instructions
Its another way to look at retirement savings if a person doesn't actually feel the need to stop working entirely. It points to more of a "I work on my own terms now" state. But yes, a separate PR would be best. |
youngcw
left a comment
youngcw
left a comment
There was a problem hiding this comment.
Double check the code rabbit recommendations, but other than that I think we are ready.
|
I forgot before. We need to make a feedback issue ticket and link that feedback issue to the link when enabling the feature |
|
@sjones512 I don't know if you're planning to continue developing this feature or not, but just in case: there have been several suggestions mentioned on the feedback issue (#6134) already. Thanks for the contribution, in any case. |
6 participants
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.