-
Notifications
You must be signed in to change notification settings - Fork 0
Complete Phase 2 refactor: Budget/Goals UI, AI categorization, mobile support #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: kayodebristol <3579196+kayodebristol@users.noreply.github.com>
Co-authored-by: kayodebristol <3579196+kayodebristol@users.noreply.github.com>
Co-authored-by: kayodebristol <3579196+kayodebristol@users.noreply.github.com>
Co-authored-by: kayodebristol <3579196+kayodebristol@users.noreply.github.com>
Co-authored-by: kayodebristol <3579196+kayodebristol@users.noreply.github.com>
Co-authored-by: kayodebristol <3579196+kayodebristol@users.noreply.github.com>
Co-authored-by: kayodebristol <3579196+kayodebristol@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR completes Phase 2 of the Financial Advisor refactor, implementing budget management, goals tracking, advanced reporting with Chart.js visualizations, an AI categorization framework, and mobile platform support for iOS and Android. The implementation adds significant functionality with a clean architecture, though there are some areas requiring attention around data validation, async operation handling, and documentation dates.
Key Changes
- Budget and goals management with full CRUD operations and visual progress tracking
- Interactive financial charts using Chart.js with proper lifecycle management
- AI categorization framework with vector embedding support and fallback logic
- Mobile platform configuration for iOS (13.0+) and Android (SDK 24+)
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/routes/reports/+page.svelte |
Added Chart.js integration with pie, bar, and doughnut charts for financial visualization |
src/routes/goals/+page.svelte |
New goals tracking UI with modal-based progress updates and completion indicators |
src/routes/budgets/+page.svelte |
New budget management UI with CRUD operations and period support |
src/routes/+page.svelte |
Added navigation links for budgets and goals pages |
src/lib/stores/financial.ts |
Enhanced stores with budget and goal CRUD operations |
src/lib/praxis/logic.ts |
Added async AI categorization support alongside existing rule-based logic |
src/lib/pluresdb/store.ts |
Added budget/goal persistence methods with localStorage implementation |
src/lib/ai/categorizer.ts |
New AI categorization service with vector embedding framework |
src-tauri/tauri.conf.json |
Added iOS and Android mobile platform configurations |
package.json |
Added Chart.js dependency and mobile build scripts |
docs/PHASE2_COMPLETION.md |
Comprehensive completion report documenting all Phase 2 achievements |
docs/MOBILE.md |
Mobile platform setup and deployment guide |
README.md |
Updated to mention mobile platform support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
| // Reactive statement to initialize charts when data is loaded | ||
| $: if ($transactions.length >= 0 && $accounts.length >= 0 && !chartsInitialized) { |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reactive statement condition checks if arrays have length >= 0, which is always true for arrays. This means the charts will be initialized immediately when the component mounts, even if the data hasn't loaded yet. Consider checking if the data has actually loaded (e.g., checking for length > 0 or using a separate loading flag) to avoid initializing charts with empty data before the async load operations complete.
| <div class="budget-progress"> | ||
| <div class="progress-bar"> | ||
| <div class="progress-fill" style="width: 0%"></div> | ||
| </div> | ||
| <p class="progress-text">$0.00 spent of ${budget.amount.toFixed(2)}</p> |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The budget progress is hardcoded to 0% with static values. This means budgets will always show as not used, regardless of actual spending. The progress should be calculated based on actual transactions matching the budget's category and time period to provide meaningful budget tracking.
| if (!newGoal.name || !newGoal.targetAmount) { | ||
| errors = ['Please fill in all required fields']; | ||
| return; | ||
| } | ||
| if (newGoal.targetAmount <= 0) { | ||
| errors = ['Target amount must be positive']; | ||
| return; | ||
| } | ||
| const goal: Goal = { | ||
| id: `goal-${Date.now()}`, | ||
| name: newGoal.name, | ||
| targetAmount: newGoal.targetAmount, |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation only checks if the amount is less than or equal to 0, but it doesn't validate that the amount is a valid number. User input could potentially be NaN or other invalid numeric values. Consider adding additional validation to ensure the amount is a valid positive number (e.g., !isNaN(newGoal.targetAmount) && isFinite(newGoal.targetAmount)).
| if (!newGoal.name || !newGoal.targetAmount) { | |
| errors = ['Please fill in all required fields']; | |
| return; | |
| } | |
| if (newGoal.targetAmount <= 0) { | |
| errors = ['Target amount must be positive']; | |
| return; | |
| } | |
| const goal: Goal = { | |
| id: `goal-${Date.now()}`, | |
| name: newGoal.name, | |
| targetAmount: newGoal.targetAmount, | |
| const targetAmount = Number(newGoal.targetAmount); | |
| if (!newGoal.name || Number.isNaN(targetAmount)) { | |
| errors = ['Please fill in all required fields']; | |
| return; | |
| } | |
| if (!Number.isFinite(targetAmount) || targetAmount <= 0) { | |
| errors = ['Target amount must be a valid positive number']; | |
| return; | |
| } | |
| const goal: Goal = { | |
| id: `goal-${Date.now()}`, | |
| name: newGoal.name, | |
| targetAmount: targetAmount, |
| const goal = $goals.find(g => g.id === id); | ||
| if (goal) { | ||
| const newAmount = Math.max(0, goal.currentAmount || 0) + amount; | ||
| goals.update({ | ||
| ...goal, | ||
| currentAmount: newAmount, | ||
| isCompleted: newAmount >= goal.targetAmount | ||
| }); | ||
| } | ||
| showProgressForm = false; | ||
| progressAmount = 0; | ||
| selectedGoalId = ''; |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The progress amount input allows any numeric value without validation. Users could enter negative numbers (which are handled by Math.max) or invalid values like NaN. The validation should occur before calling updateGoalProgress to provide better user feedback. Consider adding validation in the form submission to ensure progressAmount is a valid positive number.
| const goal = $goals.find(g => g.id === id); | |
| if (goal) { | |
| const newAmount = Math.max(0, goal.currentAmount || 0) + amount; | |
| goals.update({ | |
| ...goal, | |
| currentAmount: newAmount, | |
| isCompleted: newAmount >= goal.targetAmount | |
| }); | |
| } | |
| showProgressForm = false; | |
| progressAmount = 0; | |
| selectedGoalId = ''; | |
| // Clear previous errors before validating | |
| errors = []; | |
| // Coerce and validate the progress amount | |
| const parsedAmount = Number(amount); | |
| if (!Number.isFinite(parsedAmount) || parsedAmount <= 0) { | |
| errors = ['Progress amount must be a positive number']; | |
| return; | |
| } | |
| const goal = $goals.find((g) => g.id === id); | |
| if (goal) { | |
| const newAmount = Math.max(0, goal.currentAmount || 0) + parsedAmount; | |
| goals.update({ | |
| ...goal, | |
| currentAmount: newAmount, | |
| isCompleted: newAmount >= goal.targetAmount | |
| }); | |
| // Only reset and close the form after a successful update | |
| showProgressForm = false; | |
| progressAmount = 0; | |
| selectedGoalId = ''; | |
| } |
| // Budget operations | ||
| async saveBudget(budget: any) { | ||
| await this.initialize(); | ||
| const budgets = this.getBudgets(); | ||
| const index = budgets.findIndex((b: any) => b.id === budget.id); | ||
|
|
||
| if (index >= 0) { | ||
| budgets[index] = budget; | ||
| } else { | ||
| budgets.push(budget); | ||
| } | ||
|
|
||
| this.storage?.setItem('fa_budgets', JSON.stringify(budgets)); | ||
| return budget; | ||
| } | ||
|
|
||
| getBudgets(): any[] { | ||
| if (!this.storage) return []; | ||
| const data = this.storage.getItem('fa_budgets'); | ||
| return data ? JSON.parse(data) : []; | ||
| } | ||
|
|
||
| async deleteBudget(id: string) { | ||
| const budgets = this.getBudgets(); | ||
| const filtered = budgets.filter((b: any) => b.id !== id); | ||
| this.storage?.setItem('fa_budgets', JSON.stringify(filtered)); | ||
| return true; | ||
| } | ||
|
|
||
| // Goal operations | ||
| async saveGoal(goal: any) { | ||
| await this.initialize(); | ||
| const goals = this.getGoals(); | ||
| const index = goals.findIndex((g: any) => g.id === goal.id); | ||
|
|
||
| if (index >= 0) { | ||
| goals[index] = goal; | ||
| } else { | ||
| goals.push(goal); | ||
| } | ||
|
|
||
| this.storage?.setItem('fa_goals', JSON.stringify(goals)); | ||
| return goal; | ||
| } | ||
|
|
||
| getGoals(): any[] { | ||
| if (!this.storage) return []; | ||
| const data = this.storage.getItem('fa_goals'); | ||
| return data ? JSON.parse(data) : []; | ||
| } | ||
|
|
||
| async deleteGoal(id: string) { | ||
| const goals = this.getGoals(); | ||
| const filtered = goals.filter((g: any) => g.id !== id); | ||
| this.storage?.setItem('fa_goals', JSON.stringify(filtered)); | ||
| return true; | ||
| } |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The localStorage operations don't have error handling. If localStorage is full, disabled, or inaccessible (e.g., in private browsing mode), these operations will throw errors that aren't caught. Consider wrapping localStorage operations in try-catch blocks to handle quota exceeded errors and other storage failures gracefully.
| add: async (budget: Budget) => { | ||
| await dataStore.saveBudget(budget); | ||
| update(budgets => [...budgets, budget]); | ||
| }, | ||
| update: async (budget: Budget) => { | ||
| await dataStore.saveBudget(budget); | ||
| update(budgets => budgets.map(b => b.id === budget.id ? budget : b)); | ||
| }, | ||
| remove: async (id: string) => { | ||
| await dataStore.deleteBudget(id); | ||
| update(budgets => budgets.filter(b => b.id !== id)); | ||
| } | ||
| }; |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The add, update, and remove methods are marked as async and use await, but the dataStore methods (saveBudget, deleteBudget) are also async. However, the store updates (update() calls) happen synchronously without waiting for the dataStore operations to complete. If a dataStore operation fails, the in-memory store state will be out of sync with the persisted state. Consider awaiting the dataStore operations before updating the store state, or implement error handling to revert the store state if persistence fails.
| add: async (goal: Goal) => { | ||
| await dataStore.saveGoal(goal); | ||
| update(goals => [...goals, goal]); | ||
| }, | ||
| update: async (goal: Goal) => { | ||
| await dataStore.saveGoal(goal); | ||
| update(goals => goals.map(g => g.id === goal.id ? goal : g)); | ||
| }, | ||
| remove: async (id: string) => { | ||
| await dataStore.deleteGoal(id); | ||
| update(goals => goals.filter(g => g.id !== id)); | ||
| } | ||
| }; | ||
| } |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The add, update, and remove methods are marked as async and use await, but the dataStore methods (saveGoal, deleteGoal) are also async. However, the store updates (update() calls) happen synchronously without waiting for the dataStore operations to complete. If a dataStore operation fails, the in-memory store state will be out of sync with the persisted state. Consider awaiting the dataStore operations before updating the store state, or implement error handling to revert the store state if persistence fails.
| * - Fallback to rule-based categorization | ||
| */ | ||
|
|
||
| import { dataStore } from '../pluresdb/store'; |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement uses a relative path with an alias that starts with '../', but it should use the '$lib' alias for consistency with the rest of the codebase. Consider changing this to 'import { dataStore } from "$lib/pluresdb/store";' to maintain consistent import patterns.
| import { dataStore } from '../pluresdb/store'; | |
| import { dataStore } from '$lib/pluresdb/store'; |
| if (newBudget.amount <= 0) { | ||
| errors = ['Budget amount must be positive']; | ||
| return; | ||
| } |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation only checks if the amount is less than or equal to 0, but it doesn't validate that the amount is a valid number. User input could potentially be NaN or other invalid numeric values. Consider adding additional validation to ensure the amount is a valid positive number (e.g., !isNaN(newBudget.amount) && isFinite(newBudget.amount)).
Implements remaining Phase 2 features: budget management, goal tracking, advanced reporting with charts, AI-powered categorization framework, and mobile platform configuration.
Budget Management
Goals Tracking
prompt())Advanced Reporting
onDestroycleanup, reactive initialization)AI Categorization Framework
Data Layer
FinancialDataStorewith budget/goal persistenceMobile Platform
tauri.conf.jsonnpm run tauri:ios:build,npm run tauri:android:builddocs/MOBILE.mdCode Quality
setTimeoutwith reactive statements for chart initializationOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.