V2.0.4 - #27
Merged
Merged
Conversation
Introduce achievement search by title/description/category, memoize category derivation, and sort locked achievements by unlock progress so users can find relevant goals faster. Also limit initial unlocked/locked items with “show all” behavior (disabled while searching) to reduce UI clutter and improve scanability.feat(achievements): add search, sorting, and progressive reveal Introduce achievement search by title/description/category, memoize category derivation, and sort locked achievements by unlock progress so users can find relevant goals faster. Also limit initial unlocked/locked items with “show all” behavior (disabled while searching) to reduce UI clutter and improve scanability.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 24 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (32)
📒 Files selected for processing (56)
📝 WalkthroughWalkthroughThis PR introduces biometric cross-device enrollment functionality, new goal and debt-based income allocation modes for transactions, persistent achievement celebration tracking via user profile, release categorization, and enhanced wallet data operations. It also removes unused utility functions (validateEmail, debounce, backup/restore) and updates various components and hooks to support these features. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
components/portfolio/portfolio-list.tsx (1)
2929-2949:⚠️ Potential issue | 🟡 Minor"My Holdings" header counters disagree with the rendered list when "Sold Stocks" is toggled on.
The badge count (Line 2931), the up/down breakdown (Lines 2934, 2937), and the "Synced …" timestamp (Lines 2941, 2944) are all pinned to
activePortfolioItemsForCalculations(units > 0). However, the list rendered below (filteredPortfolio.map, Line 3013) honorsshowSoldStocksand switches tounits <= 0when the toggle is on. Net effect: with "Sold Stocks" enabled, the header still advertises the active-holdings count/up-down/sync, while the body shows a totally different set (often a zero/N mismatch in either direction). The "Synced" line can also disappear entirely even though the listed sold rows have validlastUpdatedvalues.Consider deriving the header counters from what's actually rendered (e.g.,
filteredPortfolio) or branching onshowSoldStocksso the badge, up/down chips, and Synced label match the list view.🔧 Suggested adjustment
<h3 className="font-bold text-lg flex items-center gap-2"> My Holdings - <Badge variant="secondary" className="rounded-full font-black">{activePortfolioItemsForCalculations.length}</Badge> + <Badge variant="secondary" className="rounded-full font-black">{filteredPortfolio.length}</Badge> <div className="flex gap-1 ml-1"> <Badge className="bg-success/10 text-success border-success/20 text-[9px] font-black py-0 px-1.5 h-4"> - {activePortfolioItemsForCalculations.filter(p => (p.currentPrice || p.buyPrice) > p.buyPrice).length}↑ + {filteredPortfolio.filter(p => (p.currentPrice || p.buyPrice) > p.buyPrice).length}↑ </Badge> <Badge className="bg-error/10 text-error border-error/20 text-[9px] font-black py-0 px-1.5 h-4"> - {activePortfolioItemsForCalculations.filter(p => (p.currentPrice || p.buyPrice) < p.buyPrice).length}↓ + {filteredPortfolio.filter(p => (p.currentPrice || p.buyPrice) < p.buyPrice).length}↓ </Badge> </div> </h3> - {activePortfolioItemsForCalculations.length > 0 && activePortfolioItemsForCalculations[0]?.lastUpdated && ( + {filteredPortfolio.some(item => item.lastUpdated) && ( <span className="text-[10px] font-black text-muted-foreground/60 uppercase tracking-widest flex items-center gap-1.5 ml-1"> <RefreshCcw className="w-2.5 h-2.5" /> - Synced {new Date(activePortfolioItemsForCalculations.reduce((latest, item) => { + Synced {new Date(filteredPortfolio.reduce((latest, item) => { const itemDate = new Date(item.lastUpdated || 0).getTime(); return itemDate > latest ? itemDate : latest; }, 0)).toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' })} </span> )}(As a side benefit, the
some(item => item.lastUpdated)guard also fixes the existing edge case where the first element lackslastUpdatedbut subsequent ones don't — today the Synced label hides even whenreducewould produce a valid timestamp.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/portfolio/portfolio-list.tsx` around lines 2929 - 2949, The header counts and "Synced" timestamp are being computed from activePortfolioItemsForCalculations while the rendered list uses filteredPortfolio (which flips when showSoldStocks is toggled), causing mismatched numbers and missing timestamps; fix by deriving the badge count, the up/down breakdown, and the synced timestamp from filteredPortfolio (or conditionally choose filteredPortfolio when showSoldStocks is true and activePortfolioItemsForCalculations otherwise) — update the components that render those values (references: activePortfolioItemsForCalculations, filteredPortfolio, showSoldStocks, and the Reduce-based timestamp logic) and change the synced guard to check filteredPortfolio.some(item => item.lastUpdated) and compute the latest timestamp safely via reduce over filteredPortfolio so the header always matches the displayed rows.components/portfolio/modals/add-transaction-modal.tsx (1)
64-78:⚠️ Potential issue | 🟡 MinorThrown error becomes an unhandled rejection — add a
catch.There's no
catchblock, so thethrow new Error(...)on line 69 (and any error fromres.json()) bubbles out of the async function and, becauseloadCoins()is invoked viavoid loadCoins(), becomes an unhandled promise rejection. The user still sees no feedback — the suggestions panel just shows "No matching coin found" silently. This matches the gap your ownCODEBASE_ANALYSIS.mdflags ("No error feedback to user when API fails").🛡️ Suggested fix
const loadCoins = async () => { setIsLoadingCoins(true) try { const res = await fetch("/api/crypto/coinlore/popular") if (!res.ok) { throw new Error(`Failed to load coins: ${res.status}`) } const data = await res.json() if (mounted && Array.isArray(data?.coins)) { setPopularCoins(data.coins) - } + } + } catch (err) { + if (mounted) { + console.error("Failed to load popular coins", err) + // Optionally surface via toast so users know the suggestions list is unavailable + } } finally { if (mounted) setIsLoadingCoins(false) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/portfolio/modals/add-transaction-modal.tsx` around lines 64 - 78, The async function loadCoins currently throws inside the try and has no catch, causing unhandled promise rejections; add a catch clause to handle errors from fetch and res.json() and ensure user feedback and cleanup: inside loadCoins add a catch(err) that (1) logs or sends the error to your error reporting, (2) sets an error state or calls a visible notification so the UI doesn’t silently show “No matching coin found”, and (3) still clears the loading flag via setIsLoadingCoins(false) (use mounted guard like existing code); update references to setPopularCoins only on success and ensure mounted is checked before any state updates in both success and error paths.components/dashboard/transaction-dialog.tsx (1)
1853-1857:⚠️ Potential issue | 🟠 MajorThe helper text is backwards for the new income Goal/Debt flows.
For income, choosing Goal means “move money from the goal into main balance”, and choosing Debt means “borrow into main balance”. The current copy still describes the expense actions (“add to goal” / “repay debt”), so the UI explains the opposite of what will happen.
Suggested fix
) : ( <p className="text-[10px] text-muted-foreground/80 pl-1 italic"> - {formData.category === "Goal" - ? "This money will be added directly to your savings goal." - : "This will record a repayment toward the selected debt."} + {formData.category === "Goal" + ? (type === "income" + ? "This will move money from the selected goal into your main balance." + : "This money will be added directly to your savings goal.") + : (type === "income" + ? "This will borrow money from the selected debt account into your main balance." + : "This will record a repayment toward the selected debt.")} </p> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/dashboard/transaction-dialog.tsx` around lines 1853 - 1857, The helper text under the <p> element using formData.category is reversed for the income Goal/Debt flows; update the copy so when formData.category === "Goal" it explains moving money from the goal into the main balance (e.g., "This will move money from your savings goal into your main balance.") and when the category is Debt it explains borrowing into the main balance from the selected debt (e.g., "This will record borrowing into your main balance from the selected debt."); modify the conditional branch in transaction-dialog.tsx around formData.category to swap the two messages accordingly.
🧹 Nitpick comments (11)
components/tools/shift-tracker.tsx (1)
58-65: Consider adding bounds checking for month indices.Both
fdandmnamefunctions access the month arrays usingparseInt(...) - 1without validating that the result is within bounds (0-11). If an invalid month value is provided, the functions will returnundefined, which could lead to display issues like "5 undefined 2024".While the input should always be valid ISO date strings in practice, defensive programming would improve robustness.
🛡️ Proposed defensive implementation
function fd(d: string) { const [y, mo, day] = d.split("-"); - return `${parseInt(day, 10)} ${MONTHS_SHORT[parseInt(mo, 10) - 1]} ${y}`; + const monthIndex = parseInt(mo, 10) - 1; + const monthName = MONTHS_SHORT[monthIndex] ?? "???"; + return `${parseInt(day, 10)} ${monthName} ${y}`; } function mname(m: string) { - return MONTHS_FULL[parseInt(m, 10) - 1]; + const monthIndex = parseInt(m, 10) - 1; + return MONTHS_FULL[monthIndex] ?? "Invalid Month"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/tools/shift-tracker.tsx` around lines 58 - 65, fd and mname currently index MONTHS_SHORT/MONTHS_FULL using parseInt(m,10)-1 without validating the index, which can yield undefined for invalid months; update both functions (fd and mname) to parse the month to an integer, check that monthIndex is between 0 and MONTHS_SHORT.length-1 (0–11), and if out of range return a safe fallback (e.g., a placeholder string like "??" or the original numeric month) instead of indexing the array; ensure fd uses the validated month name when building the formatted date and mname returns the safe fallback when the month is invalid.app/releases/page.tsx (2)
115-122: Multiple releases withstatus: "current"would all paint the primary accent stripe.The left accent bar at Lines 118-122 is keyed solely on
release.status === "current". If the JSON ever ends up with more than onecurrententry (easy to do during a release cut-over — see related comment inreleases.json), every such entry will get the primary stripe and "Current" badge. Consider either enforcing a single-current invariant when loading (e.g., only the first entry in the sorted list keepscurrent), or relying on array index 0 for the visual treatment instead of thestatusfield.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/releases/page.tsx` around lines 115 - 122, The visual "current" treatment should be based on the first item in the rendered list instead of the status field to avoid multiple items showing as current; update the conditional logic inside releases.map (the expression using release.status === "current" that controls the left stripe and any "Current" badge rendering) to use the mapped index (index === 0) or combine them (index === 0 || release.status === "current") so only the first entry receives the primary stripe and badge; adjust any related className and badge checks (inside the same releases.map render) to reference index rather than relying solely on release.status.
31-39: Category badge styling may render poorly in dark mode and silently break for unknown categories.Two concerns with
categoryColors:
- Dark-mode contrast. The fixed
border-*-200borders andtext-*-600text are tuned for light backgrounds; on dark mode they can look washed out / low-contrast againstbg-background. Consider usingdark:variants or opacity-based borders that adapt to theme.- Silent style breakage on unknown categories.
releasesData.releasesis cast withas ReleaseItem[](Line 42), so the TypeScript union does not validate the JSON at runtime. Any typo or new category indata/releases.jsonwill result incategoryColors[release.category]returningundefined, which gets interpolated into theclassNameliterally as the string"undefined", producing a styleless badge.♻️ Suggested adjustment
-const categoryColors: Record<ReleaseCategory, string> = { - Feature: "bg-blue-500/10 text-blue-600 border-blue-200", - Bugfix: "bg-red-500/10 text-red-600 border-red-200", - Improvement: "bg-green-500/10 text-green-600 border-green-200", - Major: "bg-purple-500/10 text-purple-600 border-purple-200", - UX: "bg-pink-500/10 text-pink-600 border-pink-200", - Security: "bg-amber-500/10 text-amber-600 border-amber-200", - Performance: "bg-cyan-500/10 text-cyan-600 border-cyan-200", -} +const categoryColors: Record<ReleaseCategory, string> = { + Feature: "bg-blue-500/10 text-blue-600 border-blue-500/30 dark:text-blue-400", + Bugfix: "bg-red-500/10 text-red-600 border-red-500/30 dark:text-red-400", + Improvement: "bg-green-500/10 text-green-600 border-green-500/30 dark:text-green-400", + Major: "bg-purple-500/10 text-purple-600 border-purple-500/30 dark:text-purple-400", + UX: "bg-pink-500/10 text-pink-600 border-pink-500/30 dark:text-pink-400", + Security: "bg-amber-500/10 text-amber-600 border-amber-500/30 dark:text-amber-400", + Performance: "bg-cyan-500/10 text-cyan-600 border-cyan-500/30 dark:text-cyan-400", +} +const defaultCategoryClass = "bg-muted text-muted-foreground border-border"And at the usage site (Line 132):
- <Badge variant="outline" className={`text-xs ${categoryColors[release.category]}`}> + <Badge variant="outline" className={`text-xs ${categoryColors[release.category] ?? defaultCategoryClass}`}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/releases/page.tsx` around lines 31 - 39, The categoryColors mapping (categoryColors: Record<ReleaseCategory, string>) uses light-mode specific tokens and can return undefined for unknown categories from releasesData.releases (cast as ReleaseItem[]), so update it to use theme-aware classes (add dark: variants or neutral/opacity-based classes like dark:border-*/dark:text-* for each key) and ensure the lookup at the badge render site uses a safe fallback (e.g., resolve const cls = categoryColors[release.category] ?? defaultBadgeClass) so an unknown or malformed release.category never injects "undefined" into className; also consider adding a small runtime validation or enum-check (ReleaseCategory) before lookup to make behavior explicit.components/portfolio/portfolio-list.tsx (2)
406-422:portfolioCryptoOptionsnot aligned with the newshowSoldStocksbehavior inportfolioStockOptions.
portfolioStockOptions(Lines 392-405) was updated to honorshowSoldStocksand only include unit-bearing items by default.portfolioCryptoOptionswas left unchanged and still surfaces every crypto holding for the active portfolio regardless of units, so a fully-sold crypto position keeps appearing in the Add-Transaction crypto picker even when the user has not opted in to viewing sold holdings. Consider mirroring the same gating for parity.♻️ Suggested change
const portfolioCryptoOptions = useMemo(() => { const byKey = new Map<string, { id?: string; symbol: string; name?: string }>() portfolio - .filter((item) => item.portfolioId === activePortfolioId) + .filter((item) => item.portfolioId === activePortfolioId && (showSoldStocks || item.units > 0)) .filter((item) => item.assetType === "crypto" || Boolean(item.cryptoId)) .forEach((item) => { const key = item.cryptoId || item.symbol if (!byKey.has(key)) { byKey.set(key, { id: item.cryptoId, symbol: item.symbol, name: item.assetName, }) } }) return Array.from(byKey.values()).sort((a, b) => a.symbol.localeCompare(b.symbol)) - }, [portfolio, activePortfolioId]) + }, [portfolio, activePortfolioId, showSoldStocks])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/portfolio/portfolio-list.tsx` around lines 406 - 422, portfolioCryptoOptions is currently returning all crypto keys regardless of units and doesn’t respect the same showSoldStocks gating used by portfolioStockOptions; update portfolioCryptoOptions to filter items by units (only include items with positive units) unless showSoldStocks is true, mirror the same predicate logic used in portfolioStockOptions when building the byKey map, and add showSoldStocks to the useMemo dependency array so the crypto picker reflects toggling sold holdings consistently.
744-769: Stale dep listed for the totals memo — switch toactivePortfolioItemsForCalculationsfor consistency.The body now reads exclusively from
activePortfolioItemsForCalculations, but the dep array on Line 769 still listsactivePortfolioItems. It happens to work becauseactivePortfolioItemsForCalculationsis derived fromactivePortfolioItems(so its identity changes whenever the source does), but it's inconsistent with the sibling memos (Line 830, Line 889) andreact-hooks/exhaustive-depswill flag it.- }, [activePortfolioItems, safeNumber]) + }, [activePortfolioItemsForCalculations, safeNumber])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/portfolio/portfolio-list.tsx` around lines 744 - 769, The useMemo that computes totalInvestment/currentValue/totalProfitLoss (the memo returning totalInvestment, currentValue, totalProfitLoss, totalProfitLossPercentage, todayChange, todayChangePercentage) currently lists activePortfolioItems in its dependency array but reads from activePortfolioItemsForCalculations; replace activePortfolioItems with activePortfolioItemsForCalculations in the dependency array (keep safeNumber) so the memo dependencies match the values used and satisfy react-hooks/exhaustive-deps.components/goals/goals-list.tsx (1)
666-755: Optional: reduce duplicatedEdit/HistoryJSX across the two branches.Both branches render essentially the same
EditandHistorybuttons; only the column layout (and slot order) differs based onchallengeSummary. Hoisting these into reusable elements (or driving the layout withcol-spanprops) would shrink the JSX and make future tweaks (e.g., a fourth action) less error-prone. Functionally the current code is correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/goals/goals-list.tsx` around lines 666 - 755, The JSX duplicates the same Edit and History buttons across the challengeSummary branches; refactor by extracting reusable button elements or small render helpers and then conditionally apply layout classes (e.g., "col-span-2" vs default) instead of duplicating markup. Locate the block that checks challengeSummary and the handlers setTransferDialog, setInvestmentDialog, handleEditGoal, setHistoryDialog and replace the duplicated Edit and History Button JSX with a shared constant or function (e.g., renderEditButton and renderHistoryButton) that accepts props for className/size/layout; then use those shared components in both places, passing "col-span-2" when challengeSummary is false to preserve the current layout.components/settings/security-settings.tsx (1)
636-647: Optional: only pass the partial update.
updateUserProfileacceptsPartial<UserProfile>(perhooks/use-wallet-data.ts), so spreading the fulluserProfilehere is unnecessary —updateUserProfile({ biometricEnabledOnAnyDevice: true })is equivalent and avoids re-sending unrelated fields (which can mask reducer-side merging bugs in future). Functionally fine as-is.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/settings/security-settings.tsx` around lines 636 - 647, The BiometricAuth onEnrollmentSuccess handler currently calls updateUserProfile({...userProfile, biometricEnabledOnAnyDevice: true}) which re-sends the entire userProfile; instead call updateUserProfile with the minimal partial update: updateUserProfile({ biometricEnabledOnAnyDevice: true }) (keep the existing guard userProfile && !userProfile.biometricEnabledOnAnyDevice and the same callback location in the BiometricAuth component).components/dashboard/my-wallet-page-client.tsx (1)
58-69: SpreadinguserProfilefrom the render closure can clobber concurrent updates.
onUpdateProfilecapturesuserProfilefrom this render and spreads it intoupdateUserProfile. If anything else updates the profile between renders (e.g., a debounced save, an effect, another prompt), this callback will overwrite those fields with the older snapshot when it fires.
updateUserProfilealready merges over the latest profile internally (seehooks/use-wallet-data.ts:1636-1651which spreadsuserProfileRef.current), so passing only the delta is both safer and simpler:♻️ Suggested change
onUpdateProfile={(updates) => { - if (userProfile) { - void updateUserProfile({ ...userProfile, ...updates }) - } + updateUserProfile(updates) }}The
userProfilenull-guard is also unnecessary here because the earlyreturnabove already handles that case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/dashboard/my-wallet-page-client.tsx` around lines 58 - 69, The onUpdateProfile callback in the BiometricCrossDevicePrompt should stop spreading the render-captured userProfile snapshot because that can clobber concurrent updates; instead pass only the updates (delta) to updateUserProfile so the hook’s internal merge (see updateUserProfile in hooks/use-wallet-data where it uses userProfileRef.current) can safely merge against the latest state, and remove the redundant null-guard since the parent already returns early when userProfile is absent; update the onUpdateProfile handler to call updateUserProfile(updates) (or void updateUserProfile(updates)) rather than void updateUserProfile({ ...userProfile, ...updates }).hooks/use-wallet-data.ts (3)
2933-2933: DeadsaveDataWithIntegrity("balance", ...)calls.
balanceis not in thesensitiveKeyslist (Line 1179-1190 / 1225-1236) and, more importantly,loadDataWithIntegrityChecknever reads a"balance"key — balance is always reconstructed fromtransactions. These two writes persist a value that is never consumed and they unnecessarily run through the integrity-record updater. Recommend dropping them; rely on transaction-derived balance like the other operations in this file (transferToGoal,makeDebtPayment,completeTransactionWithDebt).♻️ Proposed cleanup (apply at both sites)
setBalance(newBalance) - await saveDataWithIntegrity("balance", newBalance)Also applies to: 3014-3014
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-data.ts` at line 2933, Remove the dead saveDataWithIntegrity("balance", ...) calls: loadDataWithIntegrityCheck never reads "balance" and balance is reconstructed from transactions, so these writes are unused and just update the integrity record unnecessarily; delete the two saveDataWithIntegrity("balance", newBalance) invocations (both occurrences) and rely on the transaction-driven balance reconstruction pattern used in transferToGoal, makeDebtPayment, and completeTransactionWithDebt instead.
1146-1148: Consider preserving the raw value when JSON parse fails during migration.The current behavior keeps
parsed = raw(the original string) and proceeds tosaveToLocalStorage(storageKey, parsed, true). That's fine, but the warning is silent about whichstorageKeyfailed — including it would speed up triage of corrupted entries.♻️ Optional improvement
- } catch (error) { - console.warn("Failed to parse migration data:", error) + } catch (error) { + console.warn(`Failed to parse migration data for ${storageKey}:`, error) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-data.ts` around lines 1146 - 1148, The catch block that handles JSON.parse failures in hooks/use-wallet-data.ts should include the storage key and raw value in the warning so corrupted entries are easier to triage; update the catch for the parsing logic (where `parsed = raw` and `saveToLocalStorage(storageKey, parsed, true)` occurs) to log the `storageKey` and the original `raw` value along with the error (e.g., include `storageKey`, `raw`, and `error` in the message) and then continue preserving `parsed = raw` before calling `saveToLocalStorage`.
2979-3005: PreferrebuildDebtAccountsFromHistoryfor the debt account update.
addDebtToAccount(Line 1762-1769) appends the charge to history and then derives the account balances viarebuildDebtAccountsFromHistory(debtAccounts, updatedDebtTransactions). Here,addFromDebtinstead manually mapsbalance: d.balance + amountand also appends achargeentry, so the account balance is set ahead of time and then duplicated in history — currently consistent becauseoriginalBalance + Σcharges − Σpaymentsmatchesd.balance + amount, but it diverges from the project's "history is the source of truth" pattern and is fragile iforiginalBalance/history ever drift.Also note: persisted
debtAccountsget rebuilt from history on next load (Line 1324-1327), so the manualbalanceincrement is effectively redundant.♻️ Align with `addDebtToAccount`
- const updatedDebtAccounts = debtAccounts.map((d) => { - if (d.id === debtAccountId) { - return { ...d, balance: d.balance + amount, updatedAt: new Date().toISOString() } - } - return d - }) - - setDebtAccounts(updatedDebtAccounts) - await saveDataWithIntegrity("debtAccounts", updatedDebtAccounts) - - // Create debt credit transaction entry for history - const newDebtBalance = debtAccount.balance + amount const debtCharge: DebtCreditTransaction = { id: generateId('debt-tx'), accountId: debtAccountId, accountType: "debt", type: "charge", amount: amount, date: incomeTransaction.date, description: `Loan from ${debtAccount.name} to main balance`, - balanceAfter: newDebtBalance, + balanceAfter: debtAccount.balance + amount, sourceTransactionId: incomeTransaction.id, } const updatedDebtTransactions = [...debtCreditTransactions, debtCharge] setDebtCreditTransactions(updatedDebtTransactions) await saveDataWithIntegrity("debtCreditTransactions", updatedDebtTransactions) + + const updatedDebtAccounts = rebuildDebtAccountsFromHistory(debtAccounts, updatedDebtTransactions) + setDebtAccounts(updatedDebtAccounts) + await saveDataWithIntegrity("debtAccounts", updatedDebtAccounts)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-data.ts` around lines 2979 - 3005, The current addFromDebt block manually increments each debt account balance and then appends a charge entry, which duplicates logic and breaks the "history is source of truth" pattern; instead, append the new DebtCreditTransaction to debtCreditTransactions, then call rebuildDebtAccountsFromHistory(debtAccounts, updatedDebtTransactions) to derive updated debtAccounts, use that rebuilt account's balance for balanceAfter, call setDebtAccounts(rebuilt) and persist both rebuilt debtAccounts and updatedDebtTransactions via saveDataWithIntegrity; find this logic in the addFromDebt function where debtAccounts, debtCreditTransactions, newDebtBalance, debtCharge, setDebtAccounts and saveDataWithIntegrity are referenced and replace the manual mapping with the rebuild step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CODEBASE_ANALYSIS.md`:
- Around line 278-291: The two unlabeled fenced code blocks showing the
directory-tree and the Feature-Based Organization example are triggering
markdownlint MD040; update each opening fence from ``` to include a language
identifier (use ```text) for the directory-tree blocks so they become ```text
and silence the warning while preserving plain-text formatting and viewer
highlighting.
In `@components/dashboard/transaction-dialog.tsx`:
- Around line 594-599: The code is passing UI sentinel labels ("Goal"/"Debt")
from formData.category into addFromGoal/addFromDebt calls which overrides hook
defaults and pollutes category reporting; update the call sites (the addFromGoal
invocation shown and the similar add call around the other block) to map
sentinel values to real categories before calling: if formData.category ===
"Goal" use "Goal Transfer", if formData.category === "Debt" use "Debt Loan",
otherwise use formData.category (falling back to the existing default behavior),
and pass that mappedCategory into addFromGoal/addFromDebt instead of
formData.category.
In `@components/debt-credit/dialogs/debt-details-dialog.tsx`:
- Line 55: The Balance After display can crash because some income transactions
from addFromDebt lack tx.balanceAfter; in the DebtDetailsDialog rendering where
you call formatCurrency(tx.balanceAfter, ...) guard the output so you only call
formatCurrency when tx.balanceAfter is not null/undefined (e.g., tx.balanceAfter
!= null && formatCurrency(...)); alternatively, ensure addFromDebt sets a
numeric balanceAfter when creating those income transactions — update either the
rendering in debt-details-dialog.tsx (the Balance After render path) or the
addFromDebt creator to prevent passing undefined into formatCurrency.
In `@components/security/biometric-auth.tsx`:
- Around line 175-176: The enrollment success callback onEnrollmentSuccess is
only called in the immediate-wrap path; update the delayed "Finish Setup" flow
(the branch that executes when the user enrolls now but wraps PIN later) to also
invoke onEnrollmentSuccess after the delayed wrap completes successfully (e.g.,
in the completion handler for the Finish Setup / PIN wrap flow), ensuring you
call onEnrollmentSuccess?.() after persistence of biometricEnabledOnAnyDevice
and before any navigation/close so the parent and cross-device state stay in
sync.
In `@components/security/biometric-cross-device-prompt.tsx`:
- Around line 83-89: The prompt dismissal currently only toggles local UI state
via handleDismiss (setShowPrompt(false)) so users who tap “Not Now” are
re-prompted on next mount; persist the dismissal choice (e.g., write a flag to
localStorage or update the user preference via an API) when handleDismiss is
called and check that persisted flag (in the same place that reads
biometricEnabledOnAnyDevice) before showing the prompt; also ensure handleEnable
still calls onEnableBiometric and clears/sets the persisted flag appropriately
so the prompt won’t reappear after enabling.
In `@components/transactions/transaction-details-modal.tsx`:
- Around line 19-20: The modal currently imports the raw hook useWalletData from
the hooks module which creates a fresh instance instead of reading the shared
provider state; replace that import with the wallet-data context consumer (the
useWalletData exported by the wallet-data context/provider) so the component
reads from the shared provider snapshot, and update the import at the top of
transaction-details-modal.tsx and the other occurrence (around line 68) so all
usages reference the context-backed useWalletData.
- Around line 316-363: The debt_loan allocation is linked to debt account
balances and must be non-editable; update the transaction-details-modal UI to
treat transactions with transaction.allocationType === "debt_loan" as read-only
by disabling/hiding any edit controls and preventing save actions (i.e., check
transaction.allocationType before rendering editor fields or edit buttons), and
ensure the same logic is enforced by consulting canEditTransaction() where the
modal is opened so debt_loan rows are not considered editable; reference the
transaction object/allocationType in this component and align behavior with the
linkage logic in hooks/use-wallet-data (debt account handling) to avoid breaking
the debt-account balance coupling.
In `@data/releases.json`:
- Line 2: The currentBuild field in releases.json is set to "2026.04.25" but
should reflect the actual release indicated in this PR; update the
"currentBuild" value to the correct release date (e.g., "2026.04.26" if you
added a 2.0.4 entry) so currentBuild matches the intended release included in
this PR; modify the currentBuild string in releases.json accordingly.
- Around line 4-17: Add a new top-most release object for version "2.0.4" in
releases.json with status "current" and the correct date/title/highlights,
change the existing "version": "2.0.3" entry's "status" from "current" to
"stable", and update package.json's "version" field to "2.0.4" so repo metadata
and release notes align with the PR/branch naming; locate the JSON objects by
the "version" keys and the package.json "version" property to make these edits.
In `@hooks/use-achievements.tsx`:
- Around line 337-343: The effect that initializes celebratedAchievements
(inside the useEffect watching userProfile.celebratedAchievements) only sets
celebratedAchievements.current when the value is an array, leaving old IDs in
memory when the profile field is cleared or switched; change the effect so that
when userProfile.celebratedAchievements is not an array (null/undefined/other),
you explicitly reset celebratedAchievements.current = new Set() so the in-memory
set is cleared on profile reset/switch and new achievements will re-celebrate.
In `@hooks/use-wallet-data.ts`:
- Around line 2909-2933: The addFromGoal flow creates an income Transaction in
addFromGoal with actual: 0 which causes reconstructed wallet balance to ignore
the transferred amount (because code sums tx.actual ?? tx.amount and 0 is
treated as present); update the incomeTransaction creation in addFromGoal to set
actual: amount (like addFromDebt does), keep allocationType: "goal_transfer",
then persist via setTransactions/saveDataWithIntegrity and setBalance as before
so reloads correctly reflect the transfer.
- Around line 2962-2977: The income transaction creation in addFromDebt
currently ignores the function's description parameter and always uses the fixed
template; update the incomeTransaction construction (the object assigned to
incomeTransaction in addFromDebt) to set description to the caller-provided
description when present (e.g., description || `Debt transfer from
${debtAccount.name} to main balance`) so it mirrors addFromGoal's behavior and
preserves passed-in descriptions while falling back to the existing template.
- Around line 2926-2940: addFromGoal currently appends an income transaction and
updates transactions/balance but does not mutate the matching goal's
currentAmount, causing UI staleness; modify addFromGoal to find the goal by
goalId in the goals array, create an updatedGoals array that updates
goal.currentAmount (subtracting the transferred amount) following the same
pattern used in spendFromGoal/transferToGoal, call setGoals(updatedGoals) and
await saveDataWithIntegrity("goals", updatedGoals), and then keep returning
calculateGoalNetSavedAmount(goalId, updatedTransactions) as before.
---
Outside diff comments:
In `@components/dashboard/transaction-dialog.tsx`:
- Around line 1853-1857: The helper text under the <p> element using
formData.category is reversed for the income Goal/Debt flows; update the copy so
when formData.category === "Goal" it explains moving money from the goal into
the main balance (e.g., "This will move money from your savings goal into your
main balance.") and when the category is Debt it explains borrowing into the
main balance from the selected debt (e.g., "This will record borrowing into your
main balance from the selected debt."); modify the conditional branch in
transaction-dialog.tsx around formData.category to swap the two messages
accordingly.
In `@components/portfolio/modals/add-transaction-modal.tsx`:
- Around line 64-78: The async function loadCoins currently throws inside the
try and has no catch, causing unhandled promise rejections; add a catch clause
to handle errors from fetch and res.json() and ensure user feedback and cleanup:
inside loadCoins add a catch(err) that (1) logs or sends the error to your error
reporting, (2) sets an error state or calls a visible notification so the UI
doesn’t silently show “No matching coin found”, and (3) still clears the loading
flag via setIsLoadingCoins(false) (use mounted guard like existing code); update
references to setPopularCoins only on success and ensure mounted is checked
before any state updates in both success and error paths.
In `@components/portfolio/portfolio-list.tsx`:
- Around line 2929-2949: The header counts and "Synced" timestamp are being
computed from activePortfolioItemsForCalculations while the rendered list uses
filteredPortfolio (which flips when showSoldStocks is toggled), causing
mismatched numbers and missing timestamps; fix by deriving the badge count, the
up/down breakdown, and the synced timestamp from filteredPortfolio (or
conditionally choose filteredPortfolio when showSoldStocks is true and
activePortfolioItemsForCalculations otherwise) — update the components that
render those values (references: activePortfolioItemsForCalculations,
filteredPortfolio, showSoldStocks, and the Reduce-based timestamp logic) and
change the synced guard to check filteredPortfolio.some(item =>
item.lastUpdated) and compute the latest timestamp safely via reduce over
filteredPortfolio so the header always matches the displayed rows.
---
Nitpick comments:
In `@app/releases/page.tsx`:
- Around line 115-122: The visual "current" treatment should be based on the
first item in the rendered list instead of the status field to avoid multiple
items showing as current; update the conditional logic inside releases.map (the
expression using release.status === "current" that controls the left stripe and
any "Current" badge rendering) to use the mapped index (index === 0) or combine
them (index === 0 || release.status === "current") so only the first entry
receives the primary stripe and badge; adjust any related className and badge
checks (inside the same releases.map render) to reference index rather than
relying solely on release.status.
- Around line 31-39: The categoryColors mapping (categoryColors:
Record<ReleaseCategory, string>) uses light-mode specific tokens and can return
undefined for unknown categories from releasesData.releases (cast as
ReleaseItem[]), so update it to use theme-aware classes (add dark: variants or
neutral/opacity-based classes like dark:border-*/dark:text-* for each key) and
ensure the lookup at the badge render site uses a safe fallback (e.g., resolve
const cls = categoryColors[release.category] ?? defaultBadgeClass) so an unknown
or malformed release.category never injects "undefined" into className; also
consider adding a small runtime validation or enum-check (ReleaseCategory)
before lookup to make behavior explicit.
In `@components/dashboard/my-wallet-page-client.tsx`:
- Around line 58-69: The onUpdateProfile callback in the
BiometricCrossDevicePrompt should stop spreading the render-captured userProfile
snapshot because that can clobber concurrent updates; instead pass only the
updates (delta) to updateUserProfile so the hook’s internal merge (see
updateUserProfile in hooks/use-wallet-data where it uses userProfileRef.current)
can safely merge against the latest state, and remove the redundant null-guard
since the parent already returns early when userProfile is absent; update the
onUpdateProfile handler to call updateUserProfile(updates) (or void
updateUserProfile(updates)) rather than void updateUserProfile({ ...userProfile,
...updates }).
In `@components/goals/goals-list.tsx`:
- Around line 666-755: The JSX duplicates the same Edit and History buttons
across the challengeSummary branches; refactor by extracting reusable button
elements or small render helpers and then conditionally apply layout classes
(e.g., "col-span-2" vs default) instead of duplicating markup. Locate the block
that checks challengeSummary and the handlers setTransferDialog,
setInvestmentDialog, handleEditGoal, setHistoryDialog and replace the duplicated
Edit and History Button JSX with a shared constant or function (e.g.,
renderEditButton and renderHistoryButton) that accepts props for
className/size/layout; then use those shared components in both places, passing
"col-span-2" when challengeSummary is false to preserve the current layout.
In `@components/portfolio/portfolio-list.tsx`:
- Around line 406-422: portfolioCryptoOptions is currently returning all crypto
keys regardless of units and doesn’t respect the same showSoldStocks gating used
by portfolioStockOptions; update portfolioCryptoOptions to filter items by units
(only include items with positive units) unless showSoldStocks is true, mirror
the same predicate logic used in portfolioStockOptions when building the byKey
map, and add showSoldStocks to the useMemo dependency array so the crypto picker
reflects toggling sold holdings consistently.
- Around line 744-769: The useMemo that computes
totalInvestment/currentValue/totalProfitLoss (the memo returning
totalInvestment, currentValue, totalProfitLoss, totalProfitLossPercentage,
todayChange, todayChangePercentage) currently lists activePortfolioItems in its
dependency array but reads from activePortfolioItemsForCalculations; replace
activePortfolioItems with activePortfolioItemsForCalculations in the dependency
array (keep safeNumber) so the memo dependencies match the values used and
satisfy react-hooks/exhaustive-deps.
In `@components/settings/security-settings.tsx`:
- Around line 636-647: The BiometricAuth onEnrollmentSuccess handler currently
calls updateUserProfile({...userProfile, biometricEnabledOnAnyDevice: true})
which re-sends the entire userProfile; instead call updateUserProfile with the
minimal partial update: updateUserProfile({ biometricEnabledOnAnyDevice: true })
(keep the existing guard userProfile && !userProfile.biometricEnabledOnAnyDevice
and the same callback location in the BiometricAuth component).
In `@components/tools/shift-tracker.tsx`:
- Around line 58-65: fd and mname currently index MONTHS_SHORT/MONTHS_FULL using
parseInt(m,10)-1 without validating the index, which can yield undefined for
invalid months; update both functions (fd and mname) to parse the month to an
integer, check that monthIndex is between 0 and MONTHS_SHORT.length-1 (0–11),
and if out of range return a safe fallback (e.g., a placeholder string like "??"
or the original numeric month) instead of indexing the array; ensure fd uses the
validated month name when building the formatted date and mname returns the safe
fallback when the month is invalid.
In `@hooks/use-wallet-data.ts`:
- Line 2933: Remove the dead saveDataWithIntegrity("balance", ...) calls:
loadDataWithIntegrityCheck never reads "balance" and balance is reconstructed
from transactions, so these writes are unused and just update the integrity
record unnecessarily; delete the two saveDataWithIntegrity("balance",
newBalance) invocations (both occurrences) and rely on the transaction-driven
balance reconstruction pattern used in transferToGoal, makeDebtPayment, and
completeTransactionWithDebt instead.
- Around line 1146-1148: The catch block that handles JSON.parse failures in
hooks/use-wallet-data.ts should include the storage key and raw value in the
warning so corrupted entries are easier to triage; update the catch for the
parsing logic (where `parsed = raw` and `saveToLocalStorage(storageKey, parsed,
true)` occurs) to log the `storageKey` and the original `raw` value along with
the error (e.g., include `storageKey`, `raw`, and `error` in the message) and
then continue preserving `parsed = raw` before calling `saveToLocalStorage`.
- Around line 2979-3005: The current addFromDebt block manually increments each
debt account balance and then appends a charge entry, which duplicates logic and
breaks the "history is source of truth" pattern; instead, append the new
DebtCreditTransaction to debtCreditTransactions, then call
rebuildDebtAccountsFromHistory(debtAccounts, updatedDebtTransactions) to derive
updated debtAccounts, use that rebuilt account's balance for balanceAfter, call
setDebtAccounts(rebuilt) and persist both rebuilt debtAccounts and
updatedDebtTransactions via saveDataWithIntegrity; find this logic in the
addFromDebt function where debtAccounts, debtCreditTransactions, newDebtBalance,
debtCharge, setDebtAccounts and saveDataWithIntegrity are referenced and replace
the manual mapping with the rebuild step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bdc6b3b0-8353-43b7-9c39-b52ffb0dc8f2
📒 Files selected for processing (24)
CODEBASE_ANALYSIS.mdapp/releases/page.tsxcomponents/achievements/achievements-profile.tsxcomponents/dashboard/my-wallet-page-client.tsxcomponents/dashboard/transaction-dialog.tsxcomponents/debt-credit/dialogs/debt-details-dialog.tsxcomponents/goals/goals-list.tsxcomponents/portfolio/modals/add-transaction-modal.tsxcomponents/portfolio/portfolio-list.tsxcomponents/security/biometric-auth.tsxcomponents/security/biometric-cross-device-prompt.tsxcomponents/settings/security-settings.tsxcomponents/settings/user-settings.tsxcomponents/tools/shift-tracker.tsxcomponents/transactions/transaction-details-modal.tsxcontexts/wallet-data-context.tsxdata/releases.jsonhooks/use-achievements.tsxhooks/use-wallet-data.tslib/goal-calculations.tslib/utils.test.tslib/utils.tslib/wallet-utils.tstypes/wallet.ts
💤 Files with no reviewable changes (2)
- lib/utils.ts
- lib/wallet-utils.ts
Comment on lines
+278
to
+291
| ``` | ||
| contexts/ | ||
| wallet-data-context.tsx // Core finance data | ||
| portfolio-context.tsx // Stock/crypto data | ||
| security-context.tsx // Auth/PIN/encryption | ||
| notification-context.tsx // Push notifications | ||
| ``` | ||
|
|
||
| ### B. **Feature-Based Organization** | ||
|
|
||
| **Current:** Mixed organization | ||
| **Recommended:** | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Markdownlint (MD040) flags the unlabeled fences at lines 278 and 291. Use ```text for the directory-tree blocks to silence the warning and improve syntax-highlight behavior in viewers.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 278-278: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 291-291: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CODEBASE_ANALYSIS.md` around lines 278 - 291, The two unlabeled fenced code
blocks showing the directory-tree and the Feature-Based Organization example are
triggering markdownlint MD040; update each opening fence from ``` to include a
language identifier (use ```text) for the directory-tree blocks so they become
```text and silence the warning while preserving plain-text formatting and
viewer highlighting.
Comment on lines
+594
to
+599
| const result = await addFromGoal( | ||
| formData.allocationTarget, | ||
| numAmount, | ||
| formData.description.trim() || `Transfer from ${selectedGoal.title || selectedGoal.name || "Goal"}`, | ||
| formData.category || "Income" | ||
| ) |
Contributor
There was a problem hiding this comment.
Don’t save the special selector labels as real income categories.
In these new income paths, formData.category is often the sentinel "Goal" / "Debt" from the special picker, and passing it through here overrides the hook defaults ("Goal Transfer" / "Debt Loan"). That will skew category-based reporting with UI control values instead of actual transaction categories.
Suggested fix
const result = await addFromGoal(
formData.allocationTarget,
numAmount,
formData.description.trim() || `Transfer from ${selectedGoal.title || selectedGoal.name || "Goal"}`,
- formData.category || "Income"
+ formData.category === "Goal" ? undefined : formData.category.trim() || undefined
)
@@
const result = await addFromDebt(
formData.allocationTarget,
numAmount,
formData.description.trim() || `Loan from ${selectedDebt.name || "Debt Account"}`,
- formData.category || "Income"
+ formData.category === "Debt" ? undefined : formData.category.trim() || undefined
)Also applies to: 621-626
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/dashboard/transaction-dialog.tsx` around lines 594 - 599, The code
is passing UI sentinel labels ("Goal"/"Debt") from formData.category into
addFromGoal/addFromDebt calls which overrides hook defaults and pollutes
category reporting; update the call sites (the addFromGoal invocation shown and
the similar add call around the other block) to map sentinel values to real
categories before calling: if formData.category === "Goal" use "Goal Transfer",
if formData.category === "Debt" use "Debt Loan", otherwise use formData.category
(falling back to the existing default behavior), and pass that mappedCategory
into addFromGoal/addFromDebt instead of formData.category.
| if (!accountId) return [] | ||
| return transactions | ||
| .filter((t: any) => t.accountId === accountId) | ||
| .filter((t: any) => t.accountId === accountId || t.debtAccountId === accountId) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether debt-related income transactions populate balanceAfter and whether
# any other code paths set debtAccountId for non-payment/charge transactions.
rg -nP --type=ts -C2 'balanceAfter\s*[:=]'
rg -nP --type=ts -C3 'debtAccountId\s*:\s*'Repository: Shubhamnpk/mywallet
Length of output: 12483
🏁 Script executed:
# Get full context of addFromDebt function to see what balanceAfter is set to for the income transaction
sed -n '2946,3010p' hooks/use-wallet-data.tsRepository: Shubhamnpk/mywallet
Length of output: 2279
🏁 Script executed:
# Check the debt-details-dialog.tsx around line 233 to see how balanceAfter is rendered
sed -n '220,245p' components/debt-credit/dialogs/debt-details-dialog.tsxRepository: Shubhamnpk/mywallet
Length of output: 2390
🏁 Script executed:
# Find the formatCurrency function to understand how it handles undefined/null
rg -n "const formatCurrency|function formatCurrency" -A 10Repository: Shubhamnpk/mywallet
Length of output: 50377
Fix runtime error when rendering debt-loan income transactions.
The filter expansion to include debtAccountId === accountId will now pull in income transactions created by addFromDebt (with type: "income", allocationType: "debt_loan"). However, these income transactions do not have a balanceAfter property set, but line 233 unconditionally renders:
Balance After: {formatCurrency(tx.balanceAfter, ...)}
Since formatCurrency calls amount.toLocaleString() without null/undefined guards, this will crash with "Cannot read property 'toLocaleString' of undefined" when attempting to render these rows.
Guard the Balance After display to handle cases where balanceAfter == null:
{tx.balanceAfter != null && formatCurrency(tx.balanceAfter, ...)}
Alternatively, set balanceAfter in addFromDebt when creating the income transaction.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/debt-credit/dialogs/debt-details-dialog.tsx` at line 55, The
Balance After display can crash because some income transactions from
addFromDebt lack tx.balanceAfter; in the DebtDetailsDialog rendering where you
call formatCurrency(tx.balanceAfter, ...) guard the output so you only call
formatCurrency when tx.balanceAfter is not null/undefined (e.g., tx.balanceAfter
!= null && formatCurrency(...)); alternatively, ensure addFromDebt sets a
numeric balanceAfter when creating those income transactions — update either the
rendering in debt-details-dialog.tsx (the Balance After render path) or the
addFromDebt creator to prevent passing undefined into formatCurrency.
Comment on lines
+2909
to
+2933
| const incomeTransaction: Transaction = { | ||
| id: generateId('tx'), | ||
| type: "income", | ||
| amount: amount, | ||
| description: `${goal.title || goal.name || "Goal"}: ${description}`, | ||
| category: category || "Goal Transfer", | ||
| date: new Date().toISOString(), | ||
| allocationType: "goal_transfer", | ||
| allocationTarget: goalId, | ||
| timeEquivalent: userProfile ? calculateTimeEquivalent(amount, userProfile) : undefined, | ||
| total: amount, | ||
| actual: 0, // actual: 0 means this counts as spending from the goal | ||
| debtUsed: 0, | ||
| debtAccountId: null, | ||
| status: "normal", | ||
| } | ||
|
|
||
| const updatedTransactions = [...transactions, incomeTransaction] | ||
| setTransactions(updatedTransactions) | ||
| await saveDataWithIntegrity("transactions", updatedTransactions) | ||
|
|
||
| // Update balance - add income amount to main balance | ||
| const newBalance = balance + amount | ||
| setBalance(newBalance) | ||
| await saveDataWithIntegrity("balance", newBalance) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how goal_transfer is treated in goal calculations and whether the cash-balance reducer special-cases it.
fd -t f 'goal-calculations.ts' --exec cat
echo '--- callers / treatment of goal_transfer ---'
rg -nP -C2 '\bgoal_transfer\b'
echo '--- balance reconstruction patterns ---'
rg -nP -C2 'actual\s*\?\?\s*(tx|t)\.amount'Repository: Shubhamnpk/mywallet
Length of output: 16107
Critical: addFromGoal loses the transferred amount from the wallet balance on reload.
The new income transaction is created with actual: 0 (Line 2920), but the wallet balance is reconstructed elsewhere via the nullish-coalescing pattern:
if (tx.type === "income") return sum + (tx.actual ?? tx.amount)(Lines 101, 1367, 1621, 1908, 2647, 2794 in use-wallet-data.ts). The operator ?? only checks for null/undefined—it does not treat 0 as missing. With actual: 0, the coalescing returns 0, so this transaction contributes nothing to the reconstructed balance. The setBalance(balance + amount) on Line 2932 is therefore in-memory only. After a page refresh or refreshData call, the wallet balance silently snaps back and the user appears to lose the goal-to-wallet transfer.
The goal-side accounting (in goal-calculations.ts) treats goal_transfer with actual: 0 as a spending outflow, correctly reducing the goal's net saved amount. The issue is the wallet side only.
Compare addFromDebt (Line 2973), which uses actual: amount and reconstructs correctly.
Suggested fix: set actual: amount
- total: amount,
- actual: 0, // actual: 0 means this counts as spending from the goal
+ total: amount,
+ actual: amount, // cash actually arrives in the walletThis keeps the goal-side logic intact (goal_transfer already filters as outflow) and ensures the transaction reconstructs correctly on reload.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-wallet-data.ts` around lines 2909 - 2933, The addFromGoal flow
creates an income Transaction in addFromGoal with actual: 0 which causes
reconstructed wallet balance to ignore the transferred amount (because code sums
tx.actual ?? tx.amount and 0 is treated as present); update the
incomeTransaction creation in addFromGoal to set actual: amount (like
addFromDebt does), keep allocationType: "goal_transfer", then persist via
setTransactions/saveDataWithIntegrity and setBalance as before so reloads
correctly reflect the transfer.
Comment on lines
+2926
to
+2940
| const updatedTransactions = [...transactions, incomeTransaction] | ||
| setTransactions(updatedTransactions) | ||
| await saveDataWithIntegrity("transactions", updatedTransactions) | ||
|
|
||
| // Update balance - add income amount to main balance | ||
| const newBalance = balance + amount | ||
| setBalance(newBalance) | ||
| await saveDataWithIntegrity("balance", newBalance) | ||
|
|
||
| return { | ||
| success: true, | ||
| transaction: incomeTransaction, | ||
| remainingGoalAmount: calculateGoalNetSavedAmount(goalId, updatedTransactions), | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find UI usages of goal.currentAmount to confirm staleness impact.
rg -nP -C2 '\.currentAmount\b' --type=ts --type=tsx -g '!**/use-wallet-data.ts'Repository: Shubhamnpk/mywallet
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Find UI usages of goal.currentAmount
rg -nP -C2 '\.currentAmount\b' --type=ts -g '!**/use-wallet-data.ts'Repository: Shubhamnpk/mywallet
Length of output: 10076
🏁 Script executed:
#!/bin/bash
# View the addFromGoal function at lines 2926-2940
sed -n '2900,2945p' hooks/use-wallet-data.tsRepository: Shubhamnpk/mywallet
Length of output: 1761
🏁 Script executed:
#!/bin/bash
# View the spendFromGoal function around line 2856-2861
sed -n '2840,2880p' hooks/use-wallet-data.tsRepository: Shubhamnpk/mywallet
Length of output: 1322
🏁 Script executed:
#!/bin/bash
# Find the updateGoalContribution function around line 1628
sed -n '1600,1650p' hooks/use-wallet-data.tsRepository: Shubhamnpk/mywallet
Length of output: 1781
🏁 Script executed:
#!/bin/bash
# Find and examine updateGoalContribution function
rg -nA15 'const updateGoalContribution|function updateGoalContribution' hooks/use-wallet-data.tsRepository: Shubhamnpk/mywallet
Length of output: 790
🏁 Script executed:
#!/bin/bash
# Find and examine updateGoalContributionHelper function
rg -nA20 'const updateGoalContributionHelper|function updateGoalContributionHelper' hooks/use-wallet-data.tsRepository: Shubhamnpk/mywallet
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Search for updateGoalContributionHelper across entire codebase
rg -n 'updateGoalContributionHelper' --type=tsRepository: Shubhamnpk/mywallet
Length of output: 706
Update goal.currentAmount when transferring money from a goal.
addFromGoal does not update goal.currentAmount while the sibling functions spendFromGoal and transferToGoal do. This creates UI staleness—components that render goal.currentAmount directly (e.g., financial-health-score.tsx, transaction-dialog.tsx, goal-progress-visualization.tsx, insights-panel.tsx) will display stale balances until the next data reload.
The function relies on calculateGoalNetSavedAmount(...) only for the returned value, but never mutates the goals array. Add the same pattern used in spendFromGoal:
const updatedTransactions = [...transactions, incomeTransaction]
+ const updatedGoals = goals.map((g) =>
+ g.id === goalId
+ ? { ...g, currentAmount: Math.max(0, g.currentAmount - amount), updatedAt: new Date().toISOString() }
+ : g,
+ )
+ setGoals(updatedGoals)
+ await saveDataWithIntegrity("goals", updatedGoals)
setTransactions(updatedTransactions)
await saveDataWithIntegrity("transactions", updatedTransactions)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-wallet-data.ts` around lines 2926 - 2940, addFromGoal currently
appends an income transaction and updates transactions/balance but does not
mutate the matching goal's currentAmount, causing UI staleness; modify
addFromGoal to find the goal by goalId in the goals array, create an
updatedGoals array that updates goal.currentAmount (subtracting the transferred
amount) following the same pattern used in spendFromGoal/transferToGoal, call
setGoals(updatedGoals) and await saveDataWithIntegrity("goals", updatedGoals),
and then keep returning calculateGoalNetSavedAmount(goalId, updatedTransactions)
as before.
Comment on lines
+2962
to
+2977
| const incomeTransaction: Transaction = { | ||
| id: generateId('tx'), | ||
| type: "income", | ||
| amount: amount, | ||
| description: `Debt transfer from ${debtAccount.name} to main balance`, | ||
| category: category || "Debt Loan", | ||
| date: new Date().toISOString(), | ||
| allocationType: "debt_loan", | ||
| allocationTarget: debtAccountId, | ||
| timeEquivalent: userProfile ? calculateTimeEquivalent(amount, userProfile) : undefined, | ||
| total: amount, | ||
| actual: amount, | ||
| debtUsed: 0, | ||
| debtAccountId: debtAccountId, | ||
| status: "normal", | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm callers do pass a meaningful description into addFromDebt.
rg -nP -C3 '\baddFromDebt\s*\('Repository: Shubhamnpk/mywallet
Length of output: 632
🏁 Script executed:
# Find addFromDebt function definition and signature
rg -nP 'const\s+addFromDebt|function\s+addFromDebt' hooks/use-wallet-data.ts -A 10Repository: Shubhamnpk/mywallet
Length of output: 471
🏁 Script executed:
# Also search for the function declaration in different patterns
ast-grep --pattern 'const addFromDebt = ($_) => { $$$ }'Repository: Shubhamnpk/mywallet
Length of output: 45
🏁 Script executed:
# Check addFromGoal for comparison to validate the claim
rg -nP 'const\s+addFromGoal|function\s+addFromGoal' hooks/use-wallet-data.ts -A 15Repository: Shubhamnpk/mywallet
Length of output: 533
🏁 Script executed:
# Check addFromGoal transaction creation around line 2913
sed -n '2900,2930p' hooks/use-wallet-data.tsRepository: Shubhamnpk/mywallet
Length of output: 1331
🏁 Script executed:
# Check addFromDebt transaction creation at lines 2962-2977
sed -n '2962,2980p' hooks/use-wallet-data.tsRepository: Shubhamnpk/mywallet
Length of output: 727
Hard-coded description ignores the caller-provided description parameter.
The addFromDebt function accepts a description parameter but discards it entirely when creating the transaction, using only a fixed template: Debt transfer from ${debtAccount.name} to main balance. In contrast, addFromGoal (line 2913) embeds the provided description into the transaction record. Callers do pass meaningful descriptions (e.g., from transaction-dialog.tsx:624), which are silently lost.
🛠 Proposed fix
- description: `Debt transfer from ${debtAccount.name} to main balance`,
+ description: description?.trim()
+ ? `${debtAccount.name}: ${description}`
+ : `Debt transfer from ${debtAccount.name} to main balance`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-wallet-data.ts` around lines 2962 - 2977, The income transaction
creation in addFromDebt currently ignores the function's description parameter
and always uses the fixed template; update the incomeTransaction construction
(the object assigned to incomeTransaction in addFromDebt) to set description to
the caller-provided description when present (e.g., description || `Debt
transfer from ${debtAccount.name} to main balance`) so it mirrors addFromGoal's
behavior and preserves passed-in descriptions while falling back to the existing
template.
Enhance the mobile Tools experience by auto-scrolling to tool content when the tab opens, making the second row of cards easier to reach without manual scrolling. Also redesigned mobile tool cards into a compact 4-column layout, simplified card content for quicker scanning, reordered tool tabs (Shift Tracker before Insights), and removed the extra helper banner to reduce UI clutter. Included biometric auth module updates and lockfile sync to support the related security/dependency changes.feat(dashboard): improve mobile tools tab flow and layout Enhance the mobile Tools experience by auto-scrolling to tool content when the tab opens, making the second row of cards easier to reach without manual scrolling. Also redesigned mobile tool cards into a compact 4-column layout, simplified card content for quicker scanning, reordered tool tabs (Shift Tracker before Insights), and removed the extra helper banner to reduce UI clutter. Included biometric auth module updates and lockfile sync to support the related security/dependency changes.
Store a local dismissal flag for the cross-device biometric prompt so it doesn’t keep reappearing after users close it, and treat dialog close events as a dismiss action. Also prevent editing of `debt_loan`-linked transactions (including a runtime guard in save) to avoid breaking linked balance integrity, and update the helper copy accordingly. Update release metadata by adding v2.0.4 as current and marking v2.0.3 as stable.fix: persist biometric dismiss state and lock linked edits Store a local dismissal flag for the cross-device biometric prompt so it doesn’t keep reappearing after users close it, and treat dialog close events as a dismiss action. Also prevent editing of `debt_loan`-linked transactions (including a runtime guard in save) to avoid breaking linked balance integrity, and update the helper copy accordingly. Update release metadata by adding v2.0.4 as current and marking v2.0.3 as stable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor