feat: Smart digest with weekly financial summary#279
feat: Smart digest with weekly financial summary#279sirrodgepodge wants to merge 1 commit intorohitdash08:mainfrom
Conversation
- Weekly summary generation: aggregates expenses by category, income, net flow, and week-over-week spending trends - AI-powered insights via Gemini (highlights + actionable tip) - Scheduled job via APScheduler (Monday 08:00 UTC) - Delivery via email/WhatsApp using existing notification channels - API endpoints: GET /digest/weekly, POST /digest/weekly/send - Frontend Digest page with summary cards, trends, category breakdown, upcoming bills, and AI insights sections - Navigation link added to navbar - Backend tests (8 tests) and frontend integration test - README updated with digest documentation Closes rohitdash08#121
There was a problem hiding this comment.
Pull request overview
Implements the “weekly financial digest” feature end-to-end (backend aggregation + delivery, API endpoints, scheduler job, frontend page + API client, and tests/docs) to satisfy issue #121.
Changes:
- Added backend digest generation + formatting service, with optional Gemini-powered insights.
- Added
/digest/weekly+/digest/weekly/sendendpoints and registered the digest blueprint. - Added APScheduler-based weekly delivery + frontend Digest page, with accompanying tests and README updates.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/backend/app/services/digest.py | Weekly digest aggregation, trends, upcoming bills, AI insights, and plain-text formatting. |
| packages/backend/app/routes/digest.py | Adds authenticated weekly digest fetch + send endpoints. |
| packages/backend/app/routes/init.py | Registers the new digest blueprint. |
| packages/backend/app/scheduler.py | Adds APScheduler job to generate/send weekly digests. |
| packages/backend/app/init.py | Starts scheduler during app creation (skipped in testing per comment). |
| packages/backend/tests/test_digest.py | Backend tests for digest computation, endpoints, formatting, and scheduler import. |
| app/src/api/digest.ts | Frontend API client + types for weekly digest. |
| app/src/pages/Digest.tsx | New Digest UI (summary cards, trends, breakdown, bills, AI panel, send button). |
| app/src/components/layout/Navbar.tsx | Adds “Digest” navigation entry. |
| app/src/App.tsx | Adds protected /digest route. |
| app/src/tests/Digest.integration.test.tsx | Frontend integration test for Digest rendering. |
| README.md | Documents digest endpoints and scheduled behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,65 @@ | |||
| """Digest API – weekly financial summary endpoints.""" | |||
|
|
|||
| from datetime import date, timedelta | |||
There was a problem hiding this comment.
timedelta is imported but never used, which will fail the repo's flake8 CI step (unused import). Remove it from the from datetime import ... import list.
| from datetime import date, timedelta | |
| from datetime import date |
| uid = int(get_jwt_identity()) | ||
| ref = request.args.get("ref_date") | ||
| ref_date = date.fromisoformat(ref) if ref else None | ||
|
|
||
| digest = generate_weekly_digest(uid, ref_date=ref_date) | ||
| return jsonify(digest) |
There was a problem hiding this comment.
ref_date = date.fromisoformat(ref) will raise ValueError on invalid input and currently returns a 500. Consider validating/parsing ref_date with a try/except and returning a 400 with a clear error message when the query param is not a valid ISO date.
| _scheduler = BackgroundScheduler(daemon=True) | ||
| _scheduler.add_job( | ||
| _send_weekly_digests, | ||
| trigger="cron", | ||
| day_of_week="mon", | ||
| hour=8, | ||
| minute=0, | ||
| args=[app], | ||
| id="weekly_digest", | ||
| replace_existing=True, | ||
| ) | ||
| _scheduler.start() |
There was a problem hiding this comment.
Starting an in-process BackgroundScheduler inside the web app will run once per Gunicorn worker and per replica (repo k8s manifests run 2 replicas × 2 workers). That will send duplicate weekly digests. Consider running the scheduler in a single dedicated process (separate Deployment/CronJob) or adding a robust single-leader guard (e.g., DB/Redis lock) so only one instance executes the job.
| <FinancialCard> | ||
| <FinancialCardHeader> | ||
| <FinancialCardTitle className="flex items-center gap-2"> | ||
| {trendIcon(summary.net_flow >= 0 ? 'down' : 'up')} Net Flow |
There was a problem hiding this comment.
The Net Flow card uses trendIcon(summary.net_flow >= 0 ? 'down' : 'up'), which shows a green down icon for a positive net flow and a red up icon for a negative net flow. This is inconsistent with the Dashboard’s net flow semantics and is likely inverted/misleading—consider using an icon/color mapping where positive net flow is shown as "up/green" and negative as "down/red".
| {trendIcon(summary.net_flow >= 0 ? 'down' : 'up')} Net Flow | |
| {summary.net_flow >= 0 ? ( | |
| <TrendingUp className="h-4 w-4 text-green-500" /> | |
| ) : ( | |
| <TrendingDown className="h-4 w-4 text-red-500" /> | |
| )}{' '} | |
| Net Flow |
| <ul className="space-y-2"> | ||
| {ai_insights.highlights.map((h, i) => ( | ||
| <li key={i} className="flex items-start gap-2 text-sm"> | ||
| <span className="text-primary mt-0.5">•</span> | ||
| <span>{h}</span> | ||
| </li> | ||
| ))} | ||
| </ul> |
There was a problem hiding this comment.
ai_insights.highlights.map(...) assumes highlights is always present when ai_insights exists. Since the backend accepts arbitrary model output (no schema validation), highlights could be missing or non-array and this will throw at runtime. Add a defensive guard (e.g. default to an empty list) before mapping.
| uid = int(get_jwt_identity()) | ||
| data = request.get_json(silent=True) or {} | ||
| ref = data.get("ref_date") | ||
| ref_date = date.fromisoformat(ref) if ref else None |
There was a problem hiding this comment.
Same issue in the send endpoint: date.fromisoformat(ref) can raise ValueError for bad client input and will surface as a 500. Handle parse failures and return 400 so the API behaves predictably.
| ref_date = date.fromisoformat(ref) if ref else None | |
| if ref: | |
| try: | |
| ref_date = date.fromisoformat(ref) | |
| except ValueError: | |
| return jsonify({"error": "Invalid ref_date format; expected ISO date YYYY-MM-DD."}), 400 | |
| else: | |
| ref_date = None |
| def _week_range(ref: date | None = None) -> tuple[date, date]: | ||
| """Return (Monday, Sunday) of the week containing *ref* (default: last full week).""" | ||
| today = ref or date.today() | ||
| # Last full week: Monday–Sunday before today's week | ||
| last_sunday = today - timedelta(days=today.isoweekday()) | ||
| last_monday = last_sunday - timedelta(days=6) | ||
| return last_monday, last_sunday |
There was a problem hiding this comment.
The _week_range docstring says it returns the week containing ref, but the implementation always returns the previous Monday–Sunday ("last full week" relative to ref). Update the docstring (or the logic) so callers aren’t misled.
| _ensure_schema_compatibility(app) | ||
|
|
||
| # Start background scheduler (weekly digest, etc.) – skip in testing | ||
| if not app.config.get("TESTING"): |
There was a problem hiding this comment.
create_app() starts the APScheduler before tests set app.config['TESTING']=True (see packages/backend/tests/conftest.py). This means tests will still start the background scheduler thread despite the comment "skip in testing". Consider moving scheduler startup out of create_app(), or keying off an env/config value available during app factory execution (e.g. app.testing, FLASK_ENV, or an explicit SCHEDULER_ENABLED setting).
| if not app.config.get("TESTING"): | |
| scheduler_enabled = os.getenv("SCHEDULER_ENABLED", "true").lower() == "true" | |
| flask_env = os.getenv("FLASK_ENV", "").lower() | |
| if scheduler_enabled and flask_env != "testing" and not app.config.get("TESTING"): |
| def init_scheduler(app): | ||
| """Start the background scheduler with the weekly digest job. | ||
|
|
||
| Runs every Monday at 08:00 UTC. | ||
| """ | ||
| global _scheduler | ||
| if _scheduler is not None: | ||
| return _scheduler | ||
|
|
||
| _scheduler = BackgroundScheduler(daemon=True) | ||
| _scheduler.add_job( | ||
| _send_weekly_digests, | ||
| trigger="cron", | ||
| day_of_week="mon", | ||
| hour=8, | ||
| minute=0, | ||
| args=[app], | ||
| id="weekly_digest", | ||
| replace_existing=True, | ||
| ) |
There was a problem hiding this comment.
The scheduler is documented as running "every Monday at 08:00 UTC", but BackgroundScheduler() defaults to the server’s local timezone unless explicitly configured. To make the behavior match the docstring, set the scheduler/trigger timezone to UTC (e.g. BackgroundScheduler(timezone="UTC") or add_job(..., timezone="UTC")).
|
Closing — no longer pursuing this bounty. |
Summary
Implements the weekly financial digest feature requested in #121.
What's included
Backend
packages/backend/app/services/digest.py– Core digest generation servicepackages/backend/app/routes/digest.py– API endpointsGET /digest/weekly?ref_date=YYYY-MM-DD– Retrieve latest weekly digestPOST /digest/weekly/send– Generate and deliver digest via email/WhatsApppackages/backend/app/scheduler.py– APScheduler background jobFrontend
app/src/pages/Digest.tsx– Full digest page with:app/src/api/digest.ts– API clientTests
packages/backend/tests/test_digest.py– 8 tests covering:app/src/__tests__/Digest.integration.test.tsx– Frontend component testDocumentation
How it works
Closes #121