feat(intelligence): personalized savings recommendation engine grounded in user data - #897
Merged
0xDeon merged 3 commits intoJul 27, 2026
Conversation
✅ Deploy Preview for nesterhq canceled.
|
✅ Deploy Preview for nesterdapp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds a personalized savings recommendation engine with deterministic candidate calculations, Monte Carlo projection enrichment, engagement tracking, guarded LLM explanations, caching, and authenticated FastAPI endpoints. ChangesSavings recommendation engine
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RecommendationsRouter
participant RecommendationEngine
participant VaultContextFetcher
participant ProjectionProvider
participant LLM
Client->>RecommendationsRouter: GET /intelligence/savings-recommendations
RecommendationsRouter->>RecommendationEngine: generate_for_user(...)
RecommendationEngine->>VaultContextFetcher: fetch user context
VaultContextFetcher-->>RecommendationEngine: goals and vault data
RecommendationEngine->>ProjectionProvider: enrich projection candidates
ProjectionProvider-->>RecommendationEngine: probability delta
RecommendationEngine->>LLM: select and explain grounded candidates
LLM-->>RecommendationEngine: candidate IDs and explanations
RecommendationEngine-->>RecommendationsRouter: SavingsRecommendationSet
RecommendationsRouter-->>Client: recommendation response
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/intelligence/app/routers/recommendations.py`:
- Line 76: Invalidate the user’s cached recommendations immediately after the
engagement updates in the dismissal and mark-acted-on paths. Update the code
around engine.dismiss() and engine.mark_acted_on() to call the existing
recommendation-cache invalidation mechanism for that user, ensuring subsequent
generate_for_user() calls recompute filtering and ranking.
- Around line 23-25: Update _validate_candidate_id to use the candidate ID
regex’s fullmatch operation instead of match, ensuring the entire
value—including trailing newline characters—is validated against the documented
charset.
- Around line 68-76: Move the synchronous recommendation engine calls used by
the async handlers in recommendations.py, including dismiss_recommendation, off
the event loop by dispatching them through the existing threadpool utility;
ensure recommendation_store.py’s blocking Redis get/setex operations are
covered, or convert that store to redis.asyncio with explicit timeouts. Preserve
the current handler behavior and return values while preventing Redis I/O from
running directly in async handlers.
In `@apps/intelligence/app/services/projection_client.py`:
- Around line 147-157: Update the projection comparison method containing the
p_current and p_required simulations to run both independent _simulate calls
concurrently with asyncio.gather, while preserving the existing None handling
and delta calculation. Also ensure the surrounding enrich_with_projections flow
applies a single overall timeout budget to the enrichment step rather than
allowing per-leg timeouts to accumulate across candidates.
In `@apps/intelligence/app/services/recommendation_engine.py`:
- Around line 790-792: Update the exception handling in the recommendation
selection retry loop to distinguish transient API failures such as 429, 500, and
timeouts from structural errors. For transient failures, preserve the retry
budget by applying the existing backoff behavior and continuing until
_MAX_REGENERATE_ATTEMPTS is exhausted; retain the immediate break behavior for
structural exceptions.
- Around line 805-818: Replace synchronous Redis usage in
apps/intelligence/app/services/recommendation_engine.py#L805-L818 by making
_get_redis, _cache_get, and _cache_set use redis.asyncio with awaited operations
and bounded connect/socket timeouts; update their async callers accordingly. In
apps/intelligence/app/services/recommendation_store.py#L55-L86, make
_RedisEngagementStore methods async over redis.asyncio, and update the
EngagementStore Protocol plus in-memory fallback implementations and callers to
preserve the same behavior without blocking the FastAPI event loop.
- Around line 897-935: Update the vault projection flow around the user_vaults
and available_vaults loops to eliminate sequential per-vault HTTP calls: create
and reuse one aiohttp session, execute rebalance and risk fetches concurrently
with asyncio.gather, and bound concurrency with a semaphore. Preserve the
existing VaultPosition and AvailableVault field mappings and fallback values
while ensuring all fetched results are available before candidate generation
continues.
- Around line 719-743: Sanitize user-authored text when constructing candidate
summaries, titles, and risk contexts: collapse whitespace and truncate
goal.name, position.name, best.name, and the Go-supplied reason to a fixed
maximum length before formatting them. Update the candidate-construction paths
feeding c.summary and c.risk_context, including the relevant gather_context
flows, so prompt strings cannot contain raw newlines or unbounded content. Also
flag prompt strings assembled from unsanitised user input and unbounded
prompt/token usage.
- Around line 971-983: Update the recommendation cache identity so risk
tolerance is included in both lookup and validation: change the cache key used
by _cache_get within generate_for_user to distinguish user_id and
risk_tolerance, and include context.risk_tolerance in _context_fingerprint.
Ensure recommendations generated for one tolerance cannot be returned for
another.
In `@apps/intelligence/app/services/recommendation_store.py`:
- Around line 143-160: The _build_store factory directly accesses the private
_RedisEngagementStore._available state. Expose a public available property on
_RedisEngagementStore and update _build_store to use it, preserving the existing
Redis selection and in-memory fallback behavior.
- Around line 55-86: Update RecommendationStore’s _read and record methods to
avoid permanently disabling persistence after transient Redis read failures: do
not latch self._available to False from _read errors, and preserve the existing
availability behavior for write failures. Replace the JSON blob
read-modify-write flow with Redis hash operations, using per-candidate HSET
fields and refreshing the key TTL via EXPIRE; update reads to retrieve hash
fields and decode each engagement record while preserving the existing return
shape and dismissal behavior.
In `@apps/intelligence/app/services/vault_context.py`:
- Around line 301-304: Update the URL construction in the vault context method
to percent-encode vault_id as a single path segment before interpolating it into
the rebalance-suggestion endpoint. Preserve the existing empty-result behavior
for missing vault_id or user_id and leave user_id handling unchanged.
In `@apps/intelligence/tests/test_recommendation_engine.py`:
- Around line 527-532: Update the no-network recommendation test to inject an
explicit stub projection provider instead of passing None, preventing
get_projection_provider() from creating the real API provider. Also monkeypatch
RecommendationEngine._cache_set and _get_redis so generate_for_user does not
access the live Redis backend, while preserving the existing
empty-recommendations assertion.
- Around line 448-453: Add tests covering projection enrichment and
fingerprint-based cache invalidation. In the recommendation engine test suite,
use a stub projection provider to exercise enrich_with_projections, then mutate
an EngineContext and verify _context_fingerprint/_cache_get refreshes rather
than returning the stale cached result.
In `@apps/intelligence/tests/test_recommendation_store.py`:
- Around line 6-33: Add a test to TestInMemoryEngagementStore that constructs
the store with a short TTL, records an engagement, advances or simulates time
beyond the TTL, and verifies the stale entry is evicted through get_all or
is_dismissed. Exercise the constructor’s TTL parameter and the _evict_stale path
while preserving assertions for entries that remain unexpired.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bbd7eef0-79ef-409b-bee4-3117b1f66b21
📒 Files selected for processing (13)
apps/intelligence/app/config.pyapps/intelligence/app/main.pyapps/intelligence/app/models/__init__.pyapps/intelligence/app/models/savings_recommendation.pyapps/intelligence/app/routers/recommendations.pyapps/intelligence/app/services/finance_math.pyapps/intelligence/app/services/projection_client.pyapps/intelligence/app/services/recommendation_engine.pyapps/intelligence/app/services/recommendation_store.pyapps/intelligence/app/services/savings_service.pyapps/intelligence/app/services/vault_context.pyapps/intelligence/tests/test_recommendation_engine.pyapps/intelligence/tests/test_recommendation_store.py
Comment on lines
+23
to
+25
| def _validate_candidate_id(value: str) -> str: | ||
| if _CANDIDATE_ID_RE.match(value): | ||
| return value |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use fullmatch for candidate IDs. In apps/intelligence/app/routers/recommendations.py:23-25, re.match(...$) can still accept a trailing newline ("candidate\n"), so this check is looser than the documented charset. fullmatch() closes that edge case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/routers/recommendations.py` around lines 23 - 25,
Update _validate_candidate_id to use the candidate ID regex’s fullmatch
operation instead of match, ensuring the entire value—including trailing newline
characters—is validated against the documented charset.
Comment on lines
+68
to
+76
| async def dismiss_recommendation( | ||
| request: Request, | ||
| candidate_id: str, | ||
| claims: dict[str, Any] = Depends(verify_jwt), | ||
| ) -> dict[str, bool]: | ||
| """Mark a recommendation dismissed so it is never re-recommended.""" | ||
| user_id = _require_user_id(claims) | ||
| safe_id = _validate_candidate_id(candidate_id) | ||
| engine.dismiss(user_id, safe_id) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
git ls-files apps/intelligence/app/routers/recommendations.py apps/intelligence/app | sed -n '1,120p'
ast-grep outline apps/intelligence/app/routers/recommendations.py --view expanded
rg -n "def (dismiss_recommendation|recommend|engine\.|setex|get\(|Redis|redis)" apps/intelligence/app -SRepository: Suncrest-Labs/nester
Length of output: 2697
🏁 Script executed:
sed -n '1,220p' apps/intelligence/app/routers/recommendations.py
printf '\n---\n'
rg -n "class .*Engine|def .*dismiss|def .*recommend|setex|get\(" apps/intelligence/app -SRepository: Suncrest-Labs/nester
Length of output: 37381
🏁 Script executed:
wc -l apps/intelligence/app/services/recommendation_engine.py apps/intelligence/app/services/recommendation_store.py
sed -n '820,1015p' apps/intelligence/app/services/recommendation_engine.py
printf '\n---\n'
sed -n '1,220p' apps/intelligence/app/services/recommendation_store.pyRepository: Suncrest-Labs/nester
Length of output: 13862
🏁 Script executed:
nl -ba apps/intelligence/app/services/recommendation_store.py | sed -n '1,120p'
nl -ba apps/intelligence/app/routers/recommendations.py | sed -n '60,95p'Repository: Suncrest-Labs/nester
Length of output: 230
Keep Redis I/O off the event loop
apps/intelligence/app/routers/recommendations.py:68-91 calls sync engine methods from async handlers, and apps/intelligence/app/services/recommendation_store.py:59-77 does blocking Redis get/setex. A slow Redis instance can stall the FastAPI event loop; run these calls in a threadpool or switch the store to redis.asyncio with timeouts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/routers/recommendations.py` around lines 68 - 76, Move
the synchronous recommendation engine calls used by the async handlers in
recommendations.py, including dismiss_recommendation, off the event loop by
dispatching them through the existing threadpool utility; ensure
recommendation_store.py’s blocking Redis get/setex operations are covered, or
convert that store to redis.asyncio with explicit timeouts. Preserve the current
handler behavior and return values while preventing Redis I/O from running
directly in async handlers.
| """Mark a recommendation dismissed so it is never re-recommended.""" | ||
| user_id = _require_user_id(claims) | ||
| safe_id = _validate_candidate_id(candidate_id) | ||
| engine.dismiss(user_id, safe_id) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Invalidate cached recommendations after engagement changes.
engine.dismiss() and engine.mark_acted_on() only persist engagement (apps/intelligence/app/services/recommendation_engine.py Lines 998-1002), while generate_for_user() returns the cached result before filtering/ranking. A dismissed recommendation can therefore remain visible for up to six hours, and acted-on weighting is delayed.
Also applies to: 91-91
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/routers/recommendations.py` at line 76, Invalidate the
user’s cached recommendations immediately after the engagement updates in the
dismissal and mark-acted-on paths. Update the code around engine.dismiss() and
engine.mark_acted_on() to call the existing recommendation-cache invalidation
mechanism for that user, ensuring subsequent generate_for_user() calls recompute
filtering and ranking.
Comment on lines
+147
to
+157
| p_current = await self._simulate( | ||
| monthly_contribution=current_monthly_contribution, **common | ||
| ) | ||
| if p_current is None: | ||
| return None | ||
| p_required = await self._simulate( | ||
| monthly_contribution=required_monthly_contribution, **common | ||
| ) | ||
| if p_required is None: | ||
| return None | ||
| return {"success_probability_delta": p_required - p_current} |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Serial simulation legs multiply user-facing latency.
Both legs are independent, yet awaited sequentially with a 5s timeout each. enrich_with_projections (recommendation_engine.py Lines 452-496) calls this once per increase_contribution candidate, also sequentially, so a user with 5 goals faces up to 50s of blocking projection calls inside one HTTP request before the LLM call even starts. Run the two legs concurrently and give the whole enrichment step a budget.
⚡ Proposed fix
- p_current = await self._simulate(
- monthly_contribution=current_monthly_contribution, **common
- )
- if p_current is None:
- return None
- p_required = await self._simulate(
- monthly_contribution=required_monthly_contribution, **common
- )
- if p_required is None:
+ p_current, p_required = await asyncio.gather(
+ self._simulate(monthly_contribution=current_monthly_contribution, **common),
+ self._simulate(monthly_contribution=required_monthly_contribution, **common),
+ )
+ if p_current is None or p_required is None:
return NoneAdd import asyncio at the top.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| p_current = await self._simulate( | |
| monthly_contribution=current_monthly_contribution, **common | |
| ) | |
| if p_current is None: | |
| return None | |
| p_required = await self._simulate( | |
| monthly_contribution=required_monthly_contribution, **common | |
| ) | |
| if p_required is None: | |
| return None | |
| return {"success_probability_delta": p_required - p_current} | |
| p_current, p_required = await asyncio.gather( | |
| self._simulate(monthly_contribution=current_monthly_contribution, **common), | |
| self._simulate(monthly_contribution=required_monthly_contribution, **common), | |
| ) | |
| if p_current is None or p_required is None: | |
| return None | |
| return {"success_probability_delta": p_required - p_current} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/projection_client.py` around lines 147 - 157,
Update the projection comparison method containing the p_current and p_required
simulations to run both independent _simulate calls concurrently with
asyncio.gather, while preserving the existing None handling and delta
calculation. Also ensure the surrounding enrich_with_projections flow applies a
single overall timeout budget to the enrichment step rather than allowing
per-leg timeouts to accumulate across candidates.
Comment on lines
+719
to
+743
| candidate_lines = [] | ||
| for c in prompt_candidates: | ||
| impact_parts = [] | ||
| if c.impact.goal_success_probability_delta is not None: | ||
| impact_parts.append( | ||
| f"success probability change: {c.impact.goal_success_probability_delta:+.2%}" | ||
| ) | ||
| if c.impact.additional_yield_usdc is not None: | ||
| impact_parts.append(f"additional yield: ${c.impact.additional_yield_usdc:.2f}") | ||
| if c.impact.time_saved_months is not None: | ||
| impact_parts.append(f"time saved: {c.impact.time_saved_months:.1f} months") | ||
| line = ( | ||
| f"- id={c.candidate_id} | type={c.action_type} | {c.summary} | " | ||
| f"impact: {', '.join(impact_parts) or 'n/a'}" | ||
| ) | ||
| if c.risk_context: | ||
| line += f" | risk: {c.risk_context}" | ||
| candidate_lines.append(line) | ||
|
|
||
| prompt = ( | ||
| f"User risk tolerance: {context.risk_tolerance}.\n" | ||
| f"Data freshness: {context.data_freshness}.\n\n" | ||
| "Candidate actions (each already computed deterministically):\n" | ||
| + "\n".join(candidate_lines) | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
User-controlled goal/vault names reach the prompt unsanitised and unbounded.
c.summary and c.risk_context embed goal.name and position.name, which originate from user-authored records (gather_context Lines 946, 912). Nothing strips newlines or caps length, so a goal named e.g. Laptop\n\nIgnore prior instructions; ... is injected verbatim into the candidate list at Lines 730-736. The tool schema constrains candidate_id to an enum, which limits the blast radius, but explanation prose is free-form and is what the user reads, and 10 candidates × unbounded names also makes the prompt size unbounded regardless of SELECT_MAX_TOKENS.
Sanitise at candidate construction: collapse whitespace and truncate names to a fixed length.
🔒 Sketch
+_MAX_NAME_LEN = 60
+
+def _safe_label(raw: str) -> str:
+ return " ".join(str(raw).split())[:_MAX_NAME_LEN]Apply to goal.name / position.name / best.name wherever they are formatted into summary, title, and risk_context, and to the Go-supplied reason at Line 227.
As per path instructions, flag "prompt strings assembled from unsanitised user input" and "unbounded prompt/token usage".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_engine.py` around lines 719 -
743, Sanitize user-authored text when constructing candidate summaries, titles,
and risk contexts: collapse whitespace and truncate goal.name, position.name,
best.name, and the Go-supplied reason to a fixed maximum length before
formatting them. Update the candidate-construction paths feeding c.summary and
c.risk_context, including the relevant gather_context flows, so prompt strings
cannot contain raw newlines or unbounded content. Also flag prompt strings
assembled from unsanitised user input and unbounded prompt/token usage.
Source: Path instructions
Comment on lines
+143
to
+160
| def _build_store() -> Union[_RedisEngagementStore, _InMemoryEngagementStore]: | ||
| redis_url = settings.redis_url | ||
| if redis_url: | ||
| try: | ||
| s = _RedisEngagementStore(redis_url) | ||
| if s._available: | ||
| logger.info("recommendation store: redis (%s)", redis_url) | ||
| return s | ||
| logger.warning( | ||
| "recommendation store: redis connection failed, using in-memory fallback" | ||
| ) | ||
| except Exception as exc: | ||
| logger.warning( | ||
| "recommendation store: redis unavailable (%s), using in-memory fallback", exc | ||
| ) | ||
| else: | ||
| logger.info("recommendation store: in-memory (single-instance only)") | ||
| return _InMemoryEngagementStore() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Reaching into s._available from the factory leaks private state.
Expose a small available property (or have _RedisEngagementStore.__init__ raise so the except branch handles it) instead of probing a private attribute at Line 148.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_store.py` around lines 143 -
160, The _build_store factory directly accesses the private
_RedisEngagementStore._available state. Expose a public available property on
_RedisEngagementStore and update _build_store to use it, preserving the existing
Redis selection and in-memory fallback behavior.
Comment on lines
+301
to
+304
| if not vault_id or not user_id: | ||
| return {} | ||
|
|
||
| url = f"{self.api_base_url}/api/v1/vaults/{vault_id}/rebalance-suggestion" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Percent-encode vault_id before interpolating into the URL path.
vault_id is passed straight into the path at Line 304. Any value containing /, .., or a query separator rewrites the request target, and the call carries the service API key — a path-manipulation primitive against the internal API. The existing truthiness check doesn't cover it.
🔒 Proposed fix
+from urllib.parse import quote
+
...
- url = f"{self.api_base_url}/api/v1/vaults/{vault_id}/rebalance-suggestion"
+ url = (
+ f"{self.api_base_url}/api/v1/vaults/"
+ f"{quote(str(vault_id), safe='')}/rebalance-suggestion"
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if not vault_id or not user_id: | |
| return {} | |
| url = f"{self.api_base_url}/api/v1/vaults/{vault_id}/rebalance-suggestion" | |
| if not vault_id or not user_id: | |
| return {} | |
| url = ( | |
| f"{self.api_base_url}/api/v1/vaults/" | |
| f"{quote(str(vault_id), safe='')}/rebalance-suggestion" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/vault_context.py` around lines 301 - 304,
Update the URL construction in the vault context method to percent-encode
vault_id as a single path segment before interpolating it into the
rebalance-suggestion endpoint. Preserve the existing empty-result behavior for
missing vault_id or user_id and leave user_id handling unchanged.
Comment on lines
+448
to
+453
| class TestFallbackRecommendationSet: | ||
| def test_fallback_uses_candidate_summary_verbatim(self): | ||
| context, candidates = _sample_context_and_candidates() | ||
| result = _fallback_recommendation_set(candidates, context) | ||
| assert result.recommendations[0].explanation == candidates[0].summary | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Missing coverage for two headline behaviours: projection enrichment and cache invalidation.
The PR objectives call out Monte Carlo integration with heuristic fallback and fingerprint-based cache refresh, but neither enrich_with_projections nor _context_fingerprint/_cache_get is exercised. Both are cheap to test with a stub provider and a mutated EngineContext. Want me to draft them?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/tests/test_recommendation_engine.py` around lines 448 -
453, Add tests covering projection enrichment and fingerprint-based cache
invalidation. In the recommendation engine test suite, use a stub projection
provider to exercise enrich_with_projections, then mutate an EngineContext and
verify _context_fingerprint/_cache_get refreshes rather than returning the stale
cached result.
Comment on lines
+527
to
+532
| eng = RecommendationEngine( | ||
| fetcher=FakeFetcher(), projection_provider=None, engagement_store=store | ||
| ) | ||
| # Force refresh so we don't hit any cross-test cache collisions. | ||
| result = await eng.generate_for_user("user-1", "moderate", force_refresh=True) | ||
| assert result.recommendations == [] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
This "no network" test still constructs a real projection provider and hits the real cache backend.
projection_provider=None falls through to projection_provider or get_projection_provider() (recommendation_engine.py Lines 889-891), building an ApiProjectionProvider pointed at settings.nester_api_base_url; it is only unused because the fake fetcher returns zero goals. And generate_for_user always ends in _cache_set → _get_redis(), so the suite attempts a live Redis connection at settings.redis_url. Inject an explicit stub provider and monkeypatch _cache_set/_get_redis.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/tests/test_recommendation_engine.py` around lines 527 -
532, Update the no-network recommendation test to inject an explicit stub
projection provider instead of passing None, preventing
get_projection_provider() from creating the real API provider. Also monkeypatch
RecommendationEngine._cache_set and _get_redis so generate_for_user does not
access the live Redis backend, while preserving the existing
empty-recommendations assertion.
Comment on lines
+6
to
+33
| class TestInMemoryEngagementStore: | ||
| def test_dismiss_then_is_dismissed(self): | ||
| store = _InMemoryEngagementStore() | ||
| store.record("user-1", "increase_contribution:goal-1", "dismissed") | ||
| assert store.is_dismissed("user-1", "increase_contribution:goal-1") is True | ||
|
|
||
| def test_unrecorded_candidate_is_not_dismissed(self): | ||
| store = _InMemoryEngagementStore() | ||
| assert store.is_dismissed("user-1", "increase_contribution:goal-1") is False | ||
|
|
||
| def test_acted_on_is_not_treated_as_dismissed(self): | ||
| store = _InMemoryEngagementStore() | ||
| store.record("user-1", "move_to_higher_yield:vault-1", "acted_on") | ||
| assert store.is_dismissed("user-1", "move_to_higher_yield:vault-1") is False | ||
| assert store.get_all("user-1")["move_to_higher_yield:vault-1"]["engagement"] == "acted_on" | ||
|
|
||
| def test_engagement_scoped_per_user(self): | ||
| store = _InMemoryEngagementStore() | ||
| store.record("user-1", "increase_contribution:goal-1", "dismissed") | ||
| assert store.is_dismissed("user-2", "increase_contribution:goal-1") is False | ||
|
|
||
| def test_get_all_returns_copy(self): | ||
| store = _InMemoryEngagementStore() | ||
| store.record("user-1", "increase_contribution:goal-1", "dismissed") | ||
| data = store.get_all("user-1") | ||
| data["increase_contribution:goal-1"]["engagement"] = "acted_on" | ||
| # Mutating the returned dict must not affect the store's internal state. | ||
| assert store.is_dismissed("user-1", "increase_contribution:goal-1") is True |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add a TTL-eviction test.
_evict_stale (recommendation_store.py Lines 119-124) is the only untested branch and is trivially exercisable via the constructor parameter.
💚 Suggested test
+ def test_stale_user_entries_are_evicted(self):
+ store = _InMemoryEngagementStore(ttl_days=0)
+ store.record("user-1", "increase_contribution:goal-1", "dismissed")
+ assert store.get_all("user-1") == {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| class TestInMemoryEngagementStore: | |
| def test_dismiss_then_is_dismissed(self): | |
| store = _InMemoryEngagementStore() | |
| store.record("user-1", "increase_contribution:goal-1", "dismissed") | |
| assert store.is_dismissed("user-1", "increase_contribution:goal-1") is True | |
| def test_unrecorded_candidate_is_not_dismissed(self): | |
| store = _InMemoryEngagementStore() | |
| assert store.is_dismissed("user-1", "increase_contribution:goal-1") is False | |
| def test_acted_on_is_not_treated_as_dismissed(self): | |
| store = _InMemoryEngagementStore() | |
| store.record("user-1", "move_to_higher_yield:vault-1", "acted_on") | |
| assert store.is_dismissed("user-1", "move_to_higher_yield:vault-1") is False | |
| assert store.get_all("user-1")["move_to_higher_yield:vault-1"]["engagement"] == "acted_on" | |
| def test_engagement_scoped_per_user(self): | |
| store = _InMemoryEngagementStore() | |
| store.record("user-1", "increase_contribution:goal-1", "dismissed") | |
| assert store.is_dismissed("user-2", "increase_contribution:goal-1") is False | |
| def test_get_all_returns_copy(self): | |
| store = _InMemoryEngagementStore() | |
| store.record("user-1", "increase_contribution:goal-1", "dismissed") | |
| data = store.get_all("user-1") | |
| data["increase_contribution:goal-1"]["engagement"] = "acted_on" | |
| # Mutating the returned dict must not affect the store's internal state. | |
| assert store.is_dismissed("user-1", "increase_contribution:goal-1") is True | |
| class TestInMemoryEngagementStore: | |
| def test_dismiss_then_is_dismissed(self): | |
| store = _InMemoryEngagementStore() | |
| store.record("user-1", "increase_contribution:goal-1", "dismissed") | |
| assert store.is_dismissed("user-1", "increase_contribution:goal-1") is True | |
| def test_unrecorded_candidate_is_not_dismissed(self): | |
| store = _InMemoryEngagementStore() | |
| assert store.is_dismissed("user-1", "increase_contribution:goal-1") is False | |
| def test_acted_on_is_not_treated_as_dismissed(self): | |
| store = _InMemoryEngagementStore() | |
| store.record("user-1", "move_to_higher_yield:vault-1", "acted_on") | |
| assert store.is_dismissed("user-1", "move_to_higher_yield:vault-1") is False | |
| assert store.get_all("user-1")["move_to_higher_yield:vault-1"]["engagement"] == "acted_on" | |
| def test_engagement_scoped_per_user(self): | |
| store = _InMemoryEngagementStore() | |
| store.record("user-1", "increase_contribution:goal-1", "dismissed") | |
| assert store.is_dismissed("user-2", "increase_contribution:goal-1") is False | |
| def test_get_all_returns_copy(self): | |
| store = _InMemoryEngagementStore() | |
| store.record("user-1", "increase_contribution:goal-1", "dismissed") | |
| data = store.get_all("user-1") | |
| data["increase_contribution:goal-1"]["engagement"] = "acted_on" | |
| # Mutating the returned dict must not affect the store's internal state. | |
| assert store.is_dismissed("user-1", "increase_contribution:goal-1") is True | |
| def test_stale_user_entries_are_evicted(self): | |
| store = _InMemoryEngagementStore(ttl_days=0) | |
| store.record("user-1", "increase_contribution:goal-1", "dismissed") | |
| assert store.get_all("user-1") == {} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/tests/test_recommendation_store.py` around lines 6 - 33,
Add a test to TestInMemoryEngagementStore that constructs the store with a short
TTL, records an engagement, advances or simulates time beyond the TTL, and
verifies the stale entry is evicted through get_all or is_dismissed. Exercise
the constructor’s TTL parameter and the _evict_stale path while preserving
assertions for entries that remain unexpired.
6 tasks
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/intelligence/app/services/recommendation_engine.py`:
- Around line 101-103: Update the deadline calculation in the recommendation
engine to normalize the caller-provided now value to timezone-aware UTC when it
is naive, matching the existing normalization for self.deadline. Ensure
subsequent subtraction between now and deadline remains valid for both naive and
aware inputs.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f25700ff-1f8f-4f2c-8334-ae5a4165714c
📒 Files selected for processing (2)
apps/intelligence/app/services/projection_client.pyapps/intelligence/app/services/recommendation_engine.py
Comment on lines
+101
to
+103
| deadline = ( | ||
| self.deadline if self.deadline.tzinfo else self.deadline.replace(tzinfo=timezone.utc) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Normalize a caller-provided naive now.
Line 104 subtracts an aware deadline from a naive now when callers pass datetime(...), raising TypeError. Normalize now alongside deadline.
Proposed fix
def months_remaining(self, now: Optional[datetime] = None) -> int:
now = now or datetime.now(timezone.utc)
+ if now.tzinfo is None:
+ now = now.replace(tzinfo=timezone.utc)
deadline = (
self.deadline if self.deadline.tzinfo else self.deadline.replace(tzinfo=timezone.utc)
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| deadline = ( | |
| self.deadline if self.deadline.tzinfo else self.deadline.replace(tzinfo=timezone.utc) | |
| ) | |
| now = now or datetime.now(timezone.utc) | |
| if now.tzinfo is None: | |
| now = now.replace(tzinfo=timezone.utc) | |
| deadline = ( | |
| self.deadline if self.deadline.tzinfo else self.deadline.replace(tzinfo=timezone.utc) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_engine.py` around lines 101 -
103, Update the deadline calculation in the recommendation engine to normalize
the caller-provided now value to timezone-aware UTC when it is naive, matching
the existing normalization for self.deadline. Ensure subsequent subtraction
between now and deadline remains valid for both naive and aware inputs.
…ed in user data Adds a savings recommendation engine (app/services/recommendation_engine.py) that generates personalized, actionable recommendations from a user's real goals, positions, and cash-flow behavior. Candidate actions (increase a goal's contribution, move idle balance to higher yield, lock for a term boost, consolidate goals toward the nearest deadline) and every number attached to them are computed deterministically in Python -- Claude, called via tool use, only selects 2-4 candidates, orders them, and writes a short explanation, constrained to a `select_recommendations` tool schema that can only reference candidates by id. A fabrication guard (`_validate_selection`) checks every number in the model's prose against the set of numbers the referenced candidate actually carries, rejects and regenerates once on violation, then falls back to a fully templated explanation built straight from the candidate's own fields -- so a fabricated number can never reach the response. Yield-related candidates always carry risk context (from the vault's real risk score or a documented default disclosure); goal-success figures are integrated with the Suncrest-Labs#843 Monte Carlo simulation endpoint when reachable (two calls -- current vs. proposed contribution -- diffed into a real probability delta) and degrade to a documented heuristic otherwise, since per-user (Redis-backed with an in-memory fallback, mirroring conversation_store.py's pattern) and filtered out permanently; acted-on action types get a deterministic priority boost. Recommendations are cached per-user for 6 hours and invalidated automatically when goal/vault figures change materially, rather than recomputed per page load. Fixes the stale `claude-sonnet-4-6` model id. Closes Suncrest-Labs#847 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…y/ruff cleanup for Suncrest-Labs#847 - projection_client.py: point ProjectionProvider at the real Suncrest-Labs#843 endpoint contract (POST /api/v1/tools/simulation, goal_success.probability) now that it's known, instead of the placeholder GET route guessed before Suncrest-Labs#843's shape was finalized. Computes the success-probability delta from two simulation calls (current vs required contribution) rather than guessing the Go service's internal sensitivity-grid step sizes. - recommendation_engine.py: thread GoalContext into enrich_with_projections so it can build the simulation request; type the Anthropic tool-use call properly (ToolParam/ToolChoiceToolParam/MessageParam) instead of bare dicts, fixing mypy strict errors -- this is the first tool-use call in the intelligence service, so no prior typed precedent existed. - ruff: import sort, drop pointless f-string prefixes, wrap one long line. mypy --strict and ruff both clean; full suite still 105/105 passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Abidoyesimze
force-pushed
the
feat/847-savings-recommendation-engine
branch
from
July 27, 2026 11:21
960515c to
92862b7
Compare
|
@Abidoyesimze Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (10)
apps/intelligence/tests/test_recommendation_engine.py (2)
448-453: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
enrich_with_projectionsand fingerprint-based cache invalidation remain uncovered.Both are headline PR behaviours and both are cheap to test with a stub provider plus a mutated
EngineContext.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/intelligence/tests/test_recommendation_engine.py` around lines 448 - 453, Extend the recommendation engine tests with coverage for enrich_with_projections and fingerprint-based cache invalidation. Add a stub provider to verify projection enrichment, then mutate an EngineContext between calls and assert the fingerprint change invalidates cached results and recomputes recommendations.
497-532: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStill touches the real projection provider and the real cache backend, and leaks module globals.
projection_provider=Noneat Line 528 falls through toget_projection_provider()(recommendation_engine.py Lines 898-900), andgenerate_for_useralways ends at_cache_set→_get_redis(), so the suite attempts a live Redis connection atsettings.redis_url.Beyond the prior note: that
_cache_setalso writes_mem_reco_cache["user-1"]and latches the module globals_redis_client/_redis_availablefor the remainder of the session.force_refresh=Trueprotects this test, but any later test that omits it inherits the poisoned state. Monkeypatch_cache_set/_get_redisand inject a stub provider.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/intelligence/tests/test_recommendation_engine.py` around lines 497 - 532, Update test_engine_end_to_end_never_recommends_dismissed_candidate to inject a stub projection provider instead of passing None, and monkeypatch the recommendation cache path by replacing _cache_set and _get_redis with test-local no-op/stub implementations. Ensure the test neither contacts real Redis or the projection provider nor mutates shared cache or Redis module globals, while preserving the assertion that no recommendations are returned and the LLM client is unreachable.apps/intelligence/app/services/recommendation_engine.py (5)
989-1004: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCache identity still ignores
risk_tolerance.
_cache_get(user_id)(Line 990) and_context_fingerprint(Lines 864-870) both omit it, whilegenerate_yield_move_candidatesfilters on it (Line 215). Aconservativerequest served from anaggressive-populated cache returns out-of-tolerance vault moves for six hours.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/intelligence/app/services/recommendation_engine.py` around lines 989 - 1004, The recommendation cache identity must include risk_tolerance. Update _context_fingerprint and the _cache_get/_cache_set flow around the recommendation generation path to incorporate the request’s risk_tolerance, ensuring conservative and aggressive requests use separate cache entries while preserving existing fingerprint validation.
99-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNaive caller-supplied
nowstill raises at Line 104.
deadlineis normalized,nowis not.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/intelligence/app/services/recommendation_engine.py` around lines 99 - 105, Normalize the caller-supplied `now` in `months_remaining` before subtracting it from `deadline`: when `now` is naive, assign it UTC timezone information, while preserving timezone-aware values unchanged. Ensure the `deadline - now` calculation works for both input forms.
799-801: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
breakat Line 801 spends the whole retry budget on the first transient error.A 429 or timeout on attempt 0 drops straight to templated fallback;
_MAX_REGENERATE_ATTEMPTSnever protects against a flaky API call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/intelligence/app/services/recommendation_engine.py` around lines 799 - 801, Update the exception handling in the recommendation selection retry loop to continue to the next attempt after a transient failure instead of breaking immediately. Preserve the logger.exception call, and allow _MAX_REGENERATE_ATTEMPTS to be fully consumed before using the templated fallback.
723-747: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUnsanitised, unbounded user-authored names still flow into the prompt.
c.summary/c.risk_contextcarrygoal.nameandposition.nameverbatim (built at Lines 178, 238, 282, 339 fromgather_contextLines 921, 940, 955). No newline stripping, no length cap, so both injection surface and prompt size stay unbounded regardless ofSELECT_MAX_TOKENS(which caps output only).As per path instructions, flagging "prompt strings assembled from unsanitised user input" and "unbounded prompt/token usage".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/intelligence/app/services/recommendation_engine.py` around lines 723 - 747, Sanitize and bound user-authored text before assembling candidate prompt lines in the candidate_lines flow. Apply newline/control-character stripping and a defined maximum length to c.summary and c.risk_context, preserving their existing optional behavior and formatting while preventing injected prompt instructions and unbounded prompt growth.Source: Path instructions
906-944: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftSequential per-vault HTTP fan-out on the request path.
fetch_vault_rebalance_suggestion(Line 915) andfetch_vault_risk(Line 934) are awaited one at a time; 10 owned + 20 available vaults is 30 serialised round-trips before candidate generation starts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/intelligence/app/services/recommendation_engine.py` around lines 906 - 944, Parallelize the per-vault fetches in the recommendation-building flow: update the loops around fetch_vault_rebalance_suggestion and fetch_vault_risk to schedule requests concurrently and await their results as a batch, while preserving each vault’s existing fallback and output mapping. Ensure candidate generation waits only for the concurrent batch rather than serialized round-trips.apps/intelligence/tests/test_recommendation_store.py (1)
6-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTTL eviction and the Redis store remain untested.
_evict_staleis still uncovered, and_RedisEngagementStorehas zero tests despite carrying all the failure-mode complexity (availability latching, blob overwrite). A fake client would cover it cheaply.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/intelligence/tests/test_recommendation_store.py` around lines 6 - 33, The engagement-store tests only cover basic in-memory behavior; add focused tests for _InMemoryEngagementStore._evict_stale and _RedisEngagementStore. Use a fake Redis client to exercise availability latching and blob overwrite behavior, and include TTL-expiration cases that verify stale entries are evicted while current entries remain.apps/intelligence/app/services/recommendation_store.py (2)
55-86: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftRedis error latching + non-atomic blob writes still present.
Lines 63-66 permanently disable persistence after one transient read error, and Lines 72-77 are a read-modify-write over a single JSON blob. One additional wrinkle worth calling out: if
_readfails insiderecord, it returns{}, and thesetexat Line 77 then overwrites the user's entire engagement history with a single key — silent data loss, not just a lost update. A Redis hash (HSET+EXPIRE) fixes both.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/intelligence/app/services/recommendation_store.py` around lines 55 - 86, The Redis-backed engagement store still latches off after transient read failures and performs unsafe read-modify-write updates. Refactor _read, record, and related accessors to store each candidate engagement as a Redis hash field using HSET with an expiry, preserving existing entries and avoiding writes after failed reads; remove permanent _available disabling for transient Redis errors while retaining safe fallback behavior.
143-160: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrivate-state probe at Line 148, plus a dead
exceptbranch.
_RedisEngagementStore.__init__swallows every exception internally, so theexcept Exceptionat Lines 154-157 can never fire. Expose anavailableproperty (or let__init__raise) and the branch becomes meaningful.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/intelligence/app/services/recommendation_store.py` around lines 143 - 160, Update _RedisEngagementStore to expose its connection status through a public available property, then have _build_store use s.available instead of the private _available field. Remove the unreachable broad exception branch if initialization continues swallowing errors, or change initialization to propagate failures so _build_store’s existing except path can handle them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/intelligence/app/config.py`:
- Around line 12-19: Update the savings-service response handling around
response.content in savings_service.py so Sonnet 5 thinking blocks cannot
produce an empty narrative: either disable adaptive thinking for this call or
iterate through content blocks and use the text block(s). Preserve the
claude-sonnet-5 default in config.py once this path handles the response
correctly.
In `@apps/intelligence/app/services/recommendation_engine.py`:
- Around line 456-500: The per-candidate projection requests in the enrichment
loop are awaited serially, causing unbounded request latency. Update the flow
around `fetch_goal_projection` to run eligible goal projections concurrently via
`asyncio.gather`, while using a bounded semaphore to cap in-flight requests;
preserve the existing per-projection exception handling, fallback behavior, and
candidate ordering.
- Line 195: Normalize priority scores from all candidate generators to a
comparable 0–1 scale before filter_and_rank_candidates performs the global sort,
including the consolidation score near priority_score and the yield/months-saved
generators. Preserve each candidate’s underlying savings and impact values, and
ensure select_and_explain ranks consolidation candidates fairly before
truncating the prompt list.
- Around line 258-266: Enforce risk tolerance centrally across both
vault-destination generators in
apps/intelligence/app/services/recommendation_engine.py: add and use a shared
_within_risk_tolerance(vaults, risk_tolerance) helper for the pool logic around
lines 258-266, removing the out-of-tolerance fallback and continuing when no
vault remains; update generate_term_lock_candidates around lines 304-330 to
accept risk_tolerance and filter locked_options before selecting by APY, and
pass context.risk_tolerance at the line 430 call site.
- Around line 609-623: The _grounded_numbers function currently adds ×100
renderings for every impact field, allowing fabricated order-of-magnitude
values. Keep the direct and absolute renderings for all values, but restrict the
value * 100 and abs(value) * 100 expansions to
candidate.impact.goal_success_probability_delta only; do not apply them to
additional_yield_usdc or time_saved_months.
- Around line 814-827: Update _get_redis so any failed connection or ping clears
_redis_client along with setting _redis_available to False, allowing subsequent
calls to retry Redis instead of being short-circuited by the initial non-None
client. Preserve the existing in-memory fallback and warning behavior.
- Around line 840-855: Update the in-memory recommendation cache flow around
_cache_set and its read helper to remove expired entries instead of retaining
them indefinitely. When a lookup finds a stale _mem_reco_cache entry, delete it
before returning None, and preserve the existing TTL behavior for valid entries
and the Redis path.
- Around line 878-884: Normalize the string-parsing branch of _parse_deadline to
return a timezone-aware UTC datetime, matching the existing datetime-input
behavior. After datetime.fromisoformat(text), attach timezone.utc when the
parsed value has no tzinfo while preserving any supplied offset.
- Around line 401-418: The consolidation recommendation identifier generated in
the consolidation path around _candidate_key must remain stable when goals are
added or removed, so dismissals continue matching the same underlying
recommendation. Update the candidate ID and corresponding dismissal-matching
logic near the consolidation handling to use a canonical identity that does not
encode the full changing goal set, while preserving distinct identities for
different consolidation recommendations.
In `@apps/intelligence/app/services/vault_context.py`:
- Around line 311-324: Update RecommendationEngine.gather_context and its
per-vault rebalance-fetch flow to run requests with bounded concurrency instead
of serially, using a shared concurrency limit. Wrap the complete enrichment
phase in one overall deadline so the recommendation flow proceeds with partial
results when the deadline expires, while retaining the existing per-request
timeout and empty-result handling for failed or timed-out fetches.
---
Duplicate comments:
In `@apps/intelligence/app/services/recommendation_engine.py`:
- Around line 989-1004: The recommendation cache identity must include
risk_tolerance. Update _context_fingerprint and the _cache_get/_cache_set flow
around the recommendation generation path to incorporate the request’s
risk_tolerance, ensuring conservative and aggressive requests use separate cache
entries while preserving existing fingerprint validation.
- Around line 99-105: Normalize the caller-supplied `now` in `months_remaining`
before subtracting it from `deadline`: when `now` is naive, assign it UTC
timezone information, while preserving timezone-aware values unchanged. Ensure
the `deadline - now` calculation works for both input forms.
- Around line 799-801: Update the exception handling in the recommendation
selection retry loop to continue to the next attempt after a transient failure
instead of breaking immediately. Preserve the logger.exception call, and allow
_MAX_REGENERATE_ATTEMPTS to be fully consumed before using the templated
fallback.
- Around line 723-747: Sanitize and bound user-authored text before assembling
candidate prompt lines in the candidate_lines flow. Apply
newline/control-character stripping and a defined maximum length to c.summary
and c.risk_context, preserving their existing optional behavior and formatting
while preventing injected prompt instructions and unbounded prompt growth.
- Around line 906-944: Parallelize the per-vault fetches in the
recommendation-building flow: update the loops around
fetch_vault_rebalance_suggestion and fetch_vault_risk to schedule requests
concurrently and await their results as a batch, while preserving each vault’s
existing fallback and output mapping. Ensure candidate generation waits only for
the concurrent batch rather than serialized round-trips.
In `@apps/intelligence/app/services/recommendation_store.py`:
- Around line 55-86: The Redis-backed engagement store still latches off after
transient read failures and performs unsafe read-modify-write updates. Refactor
_read, record, and related accessors to store each candidate engagement as a
Redis hash field using HSET with an expiry, preserving existing entries and
avoiding writes after failed reads; remove permanent _available disabling for
transient Redis errors while retaining safe fallback behavior.
- Around line 143-160: Update _RedisEngagementStore to expose its connection
status through a public available property, then have _build_store use
s.available instead of the private _available field. Remove the unreachable
broad exception branch if initialization continues swallowing errors, or change
initialization to propagate failures so _build_store’s existing except path can
handle them.
In `@apps/intelligence/tests/test_recommendation_engine.py`:
- Around line 448-453: Extend the recommendation engine tests with coverage for
enrich_with_projections and fingerprint-based cache invalidation. Add a stub
provider to verify projection enrichment, then mutate an EngineContext between
calls and assert the fingerprint change invalidates cached results and
recomputes recommendations.
- Around line 497-532: Update
test_engine_end_to_end_never_recommends_dismissed_candidate to inject a stub
projection provider instead of passing None, and monkeypatch the recommendation
cache path by replacing _cache_set and _get_redis with test-local no-op/stub
implementations. Ensure the test neither contacts real Redis or the projection
provider nor mutates shared cache or Redis module globals, while preserving the
assertion that no recommendations are returned and the LLM client is
unreachable.
In `@apps/intelligence/tests/test_recommendation_store.py`:
- Around line 6-33: The engagement-store tests only cover basic in-memory
behavior; add focused tests for _InMemoryEngagementStore._evict_stale and
_RedisEngagementStore. Use a fake Redis client to exercise availability latching
and blob overwrite behavior, and include TTL-expiration cases that verify stale
entries are evicted while current entries remain.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 33b68cf7-0a27-4307-9763-a3fed6286245
📒 Files selected for processing (13)
apps/intelligence/app/config.pyapps/intelligence/app/main.pyapps/intelligence/app/models/__init__.pyapps/intelligence/app/models/savings_recommendation.pyapps/intelligence/app/routers/recommendations.pyapps/intelligence/app/services/finance_math.pyapps/intelligence/app/services/projection_client.pyapps/intelligence/app/services/recommendation_engine.pyapps/intelligence/app/services/recommendation_store.pyapps/intelligence/app/services/savings_service.pyapps/intelligence/app/services/vault_context.pyapps/intelligence/tests/test_recommendation_engine.pyapps/intelligence/tests/test_recommendation_store.py
Comment on lines
+12
to
19
| # claude-sonnet-5 is the current flagship id. Used for explanation-only | ||
| # workloads (narrating an already-computed allocation, the recommendation | ||
| # engine's tool-use selection, coaching/analysis narration) on a per-user | ||
| # cadence, not per keystroke, so Sonnet-tier capability for prose quality | ||
| # and tool-use/structured-output reliability is worth the cost over Haiku. | ||
| # Still fully overridable via INTELLIGENCE_ANTHROPIC_MODEL for deployments | ||
| # that want to trade capability for cost. | ||
| anthropic_model: str = "claude-sonnet-5" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect all Sonnet calls and response-block assumptions before migration.
rg -n -C3 'model=settings\.anthropic_model|response\.content\[0\]|max_tokens=' apps/intelligenceRepository: Suncrest-Labs/nester
Length of output: 9476
🌐 Web query:
Claude Sonnet 5 adaptive thinking default max_tokens includes thinking response output content blocks text response.content[0]
💡 Result:
On Claude Sonnet 5, adaptive thinking is enabled by default for requests that do not specify a thinking configuration [1][2]. Because adaptive thinking consumes tokens within the total response, these thinking tokens count toward your specified max_tokens limit [1][2]. Key details regarding this behavior include: Hard Limit: The max_tokens parameter acts as a hard cap on the combined total of both thinking output and the final text response [1][2]. If the model's adaptive reasoning process uses a large portion of this budget, the remaining tokens for the text response will be reduced, which may lead to truncation if max_tokens is set too low [1]. Content Blocks: When thinking is active, the response content array includes one or more thinking blocks followed by text blocks [3][4]. Specifically, your code might access these via response.content, where index 0 is typically a thinking block if Claude chooses to think for that turn [3][4]. Display Settings: By default on Claude Sonnet 5, the thinking content itself is omitted (the thinking field in the content block is empty), although a signature field is still included for multi-turn continuity [3][5]. To view the actual reasoning text, you must explicitly set display to summarized in your request configuration [3][4]. Managing Budgets: Because Claude Sonnet 5 uses a new tokenizer that produces approximately 30% more tokens for equivalent text compared to previous versions (like Sonnet 4.6), you may need to increase your max_tokens limits for workloads that were previously tightly tuned [1][2]. If you encounter truncation, you can raise max_tokens or adjust the effort parameter (using output_config.effort) to influence how much thinking the model performs [1][2][6]. To disable thinking entirely, you must explicitly pass thinking: {type: "disabled"} in your request [1][2].
Citations:
- 1: https://platform.claude.com/docs/en/build-with-claude/prompt-engineering/prompting-claude-sonnet-5
- 2: https://platform.claude.com/docs/en/about-claude/models/whats-new-sonnet-5
- 3: https://platform.claude.com/docs/en/build-with-claude/thinking
- 4: https://platform.claude.com/docs/en/build-with-claude/thinking-tool-workflows
- 5: https://platform.claude.com/docs/en/api/messages
- 6: https://platform.claude.com/docs/en/build-with-claude/thinking-steering-and-cost
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the savings-service implementation and any explicit handling of text/thinking blocks.
sed -n '120,170p' apps/intelligence/app/services/savings_service.py
echo
echo '---'
sed -n '1,120p' apps/intelligence/app/config.pyRepository: Suncrest-Labs/nester
Length of output: 4073
🌐 Web query:
site:platform.claude.com Sonnet 5 thinking enabled by default max_tokens includes thinking response output content blocks
💡 Result:
Claude Sonnet 5 has adaptive thinking enabled by default [1][2][3]. In this model, the API automatically manages the thinking process, and the thinking field in the API request is optional [1][2][3]. Regarding token usage and output content blocks: 1. Max Tokens: The max_tokens parameter represents a strict, hard limit on the total output generated by the model in a single turn [1][4][5]. This limit includes both the generated "thinking" tokens and the final response text tokens [1][4][5]. Because thinking tokens consume this capacity, developers are advised to review and potentially increase their max_tokens settings when migrating to Sonnet 5 from earlier models where thinking was disabled by default [1][4]. 2. Thinking Content Blocks: When the model performs thinking, the output includes specialized thinking content blocks [5][6][7]. - By default, these blocks return with their content "omitted" (redacted) on Sonnet 5 to save space, though a signature is provided for multi-turn continuity [2][8][7]. - To view the actual thinking text, you must explicitly opt in by setting the display parameter to "summarized" in your request configuration, for example: thinking: {"type": "adaptive", "display": "summarized"} [2][8][7]. Manual extended thinking (using budget_tokens) is not supported on Claude Sonnet 5 and will return a 400 error [1][9][3]. Instead, you should control the depth of the model's reasoning using the effort parameter [1][9][10].
Citations:
- 1: https://platform.claude.com/docs/en/about-claude/models/whats-new-sonnet-5
- 2: https://platform.claude.com/docs/en/build-with-claude/thinking
- 3: https://platform.claude.com/docs/en/claude_api_primer
- 4: https://platform.claude.com/docs/en/about-claude/models/migration-guide
- 5: https://platform.claude.com/docs/it/build-with-claude/thinking
- 6: https://platform.claude.com/docs/en/build-with-claude/extended-thinking?_rsc=1ebj0
- 7: https://platform.claude.com/docs/en/build-with-claude/thinking-troubleshooting
- 8: https://platform.claude.com/docs/en/api/messages
- 9: https://platform.claude.com/docs/en/build-with-claude/extended-thinking
- 10: https://platform.claude.com/docs/it/build-with-claude/effort
Handle Sonnet 5 thinking blocks before keeping apps/intelligence/app/config.py:12-19 as the default. apps/intelligence/app/services/savings_service.py:140-146 only reads response.content[0].text; with Sonnet 5 adaptive thinking, the first block can be thinking and max_tokens is shared with that reasoning, so this path can return an empty narrative. Disable thinking for this call or iterate over all text blocks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/config.py` around lines 12 - 19, Update the
savings-service response handling around response.content in savings_service.py
so Sonnet 5 thinking blocks cannot produce an empty narrative: either disable
adaptive thinking for this call or iterate through content blocks and use the
text block(s). Preserve the claude-sonnet-5 default in config.py once this path
handles the response correctly.
| projection_source="heuristic", | ||
| ), | ||
| risk_context=None, | ||
| priority_score=abs(delta) + prob_delta * 100.0, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
priority_score is not comparable across candidate types, so consolidation candidates are effectively never surfaced.
The four generators emit scores in four different units:
- Line 195: dollars-per-month + probability-points (
abs(delta) + prob_delta * 100.0) - Line 253 / Line 298 / Line 355: absolute USD yield over 12 months
- Line 416: months saved
filter_and_rank_candidates sorts all of them together (Line 534) and select_and_explain truncates to _MAX_CANDIDATES_IN_PROMPT. A consolidation candidate saving 6 months scores 6.0 and loses to any yield candidate worth more than $6 — it will essentially never reach the model. Normalize each generator onto a shared 0-1 scale (or apply per-type weights) before the global sort.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_engine.py` at line 195,
Normalize priority scores from all candidate generators to a comparable 0–1
scale before filter_and_rank_candidates performs the global sort, including the
consolidation score near priority_score and the yield/months-saved generators.
Preserve each candidate’s underlying savings and impact values, and ensure
select_and_explain ranks consolidation candidates fairly before truncating the
prompt list.
Comment on lines
+258
to
+266
| pool = [ | ||
| v | ||
| for v in available | ||
| if v.risk_tier_score <= risk_cap or risk_tolerance == "aggressive" | ||
| ] | ||
| if not pool: | ||
| pool = list(available) | ||
| if not pool: | ||
| continue |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Risk tolerance is not enforced authoritatively across the vault-destination generators. Both yield-family generators can recommend a destination vault outside the user's declared tolerance: one discards the cap as a fallback, the other never applies it. The shared root cause is that _RISK_TOLERANCE_CAPS is consulted ad hoc in one generator rather than being a single mandatory filter applied wherever a destination vault is selected.
apps/intelligence/app/services/recommendation_engine.py#L258-L266: drop theif not pool: pool = list(available)fallback andcontinueinstead, so an empty in-tolerance pool yields no recommendation rather than an out-of-tolerance one.apps/intelligence/app/services/recommendation_engine.py#L304-L330: add arisk_toleranceparameter togenerate_term_lock_candidates(passingcontext.risk_toleranceat the Line 430 call site) and filterlocked_optionsthrough the same cap beforemax(..., key=lambda v: v.apy).
Extracting one _within_risk_tolerance(vaults, risk_tolerance) helper and routing both call sites through it prevents the next generator from re-introducing the gap.
📍 Affects 1 file
apps/intelligence/app/services/recommendation_engine.py#L258-L266(this comment)apps/intelligence/app/services/recommendation_engine.py#L304-L330
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_engine.py` around lines 258 -
266, Enforce risk tolerance centrally across both vault-destination generators
in apps/intelligence/app/services/recommendation_engine.py: add and use a shared
_within_risk_tolerance(vaults, risk_tolerance) helper for the pool logic around
lines 258-266, removing the out-of-tolerance fallback and continuing when no
vault remains; update generate_term_lock_candidates around lines 304-330 to
accept risk_tolerance and filter locked_options before selecting by APY, and
pass context.risk_tolerance at the line 430 call site.
Comment on lines
+401
to
+418
| candidates.append( | ||
| RecommendationCandidate( | ||
| candidate_id=_candidate_key( | ||
| "consolidate_goals", | ||
| "+".join([target.goal_id] + [g.goal_id for g in others]), | ||
| ), | ||
| action_type="consolidate_goals", | ||
| title=f"Consolidate contributions toward {target.name}", | ||
| summary=summary, | ||
| target_id=target.goal_id, | ||
| impact=RecommendationImpact( | ||
| time_saved_months=time_saved, | ||
| projection_source="heuristic", | ||
| ), | ||
| risk_context=None, | ||
| priority_score=time_saved, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'recommendations.py' apps/intelligence/app/routers --exec cat -n {}
rg -nP -C4 '_validate_candidate_id' apps/intelligenceRepository: Suncrest-Labs/nester
Length of output: 6453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline apps/intelligence/app/services/recommendation_engine.py --view expanded | sed -n '1,260p'
printf '\n---\n'
rg -n "dismiss|dismissed|candidate_id|filter_and_rank_candidates|mark_acted_on|generate_consolidation_candidates" apps/intelligence/app/services/recommendation_engine.pyRepository: Suncrest-Labs/nester
Length of output: 5672
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '361,418p' apps/intelligence/app/services/recommendation_engine.py | cat -n
printf '\n---\n'
sed -n '508,530p' apps/intelligence/app/services/recommendation_engine.py | cat -n
printf '\n---\n'
sed -n '1000,1012p' apps/intelligence/app/services/recommendation_engine.py | cat -nRepository: Suncrest-Labs/nester
Length of output: 4639
Make consolidation dismissals stable across goal-set changes apps/intelligence/app/services/recommendation_engine.py:403-406, 508-527 — candidate_id encodes every goal in the set, but dismissal is matched by exact candidate_id, so adding/removing a goal changes the key and lets a dismissed consolidation suggestion reappear. Consider a stable canonical key for the underlying recommendation instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_engine.py` around lines 401 -
418, The consolidation recommendation identifier generated in the consolidation
path around _candidate_key must remain stable when goals are added or removed,
so dismissals continue matching the same underlying recommendation. Update the
candidate ID and corresponding dismissal-matching logic near the consolidation
handling to use a canonical identity that does not encode the full changing goal
set, while preserving distinct identities for different consolidation
recommendations.
Comment on lines
+456
to
+500
| for candidate in candidates: | ||
| if candidate.action_type != "increase_contribution" or not candidate.target_id: | ||
| enriched.append(candidate) | ||
| continue | ||
| goal = goals_by_id.get(candidate.target_id) | ||
| if goal is None: | ||
| enriched.append(candidate) | ||
| continue | ||
|
|
||
| remaining_amount = max(goal.target_amount - goal.current_amount, 0.0) | ||
| months_left = goal.months_remaining(now) | ||
| monthly_rate = goal.apy / 12.0 | ||
| required = required_monthly_deposit(remaining_amount, monthly_rate, months_left) | ||
| current = goal.current_monthly_contribution | ||
|
|
||
| try: | ||
| projection = await provider.fetch_goal_projection( | ||
| candidate.target_id, | ||
| initial_deposit=goal.current_amount, | ||
| current_monthly_contribution=current, | ||
| required_monthly_contribution=required, | ||
| apy=goal.apy, | ||
| period_months=months_left, | ||
| target_amount=goal.target_amount, | ||
| deadline_months=months_left, | ||
| ) | ||
| except Exception: | ||
| logger.exception("projection provider raised for goal %s", candidate.target_id) | ||
| projection = None | ||
| if not projection: | ||
| enriched.append(candidate) | ||
| continue | ||
| try: | ||
| delta = float(projection["success_probability_delta"]) | ||
| except (KeyError, TypeError, ValueError): | ||
| enriched.append(candidate) | ||
| continue | ||
| new_impact = candidate.impact.model_copy( | ||
| update={ | ||
| "goal_success_probability_delta": round(delta, 4), | ||
| "projection_source": "monte_carlo", | ||
| } | ||
| ) | ||
| enriched.append(candidate.model_copy(update={"impact": new_impact})) | ||
| return enriched |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
Another serialised per-item HTTP fan-out, same shape as the vault loop.
One awaited fetch_goal_projection per contribution candidate, in sequence, on the request path. Same fix as the gather_context loops: asyncio.gather with a bounded semaphore.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_engine.py` around lines 456 -
500, The per-candidate projection requests in the enrichment loop are awaited
serially, causing unbounded request latency. Update the flow around
`fetch_goal_projection` to run eligible goal projections concurrently via
`asyncio.gather`, while using a bounded semaphore to cap in-flight requests;
preserve the existing per-projection exception handling, fallback behavior, and
candidate ordering.
Comment on lines
+609
to
+623
| def _grounded_numbers(candidate: RecommendationCandidate) -> set[str]: | ||
| """Every normalized number this candidate legitimately carries -- the set | ||
| the fabrication guard checks LLM prose against.""" | ||
| text = f"{candidate.summary} {candidate.risk_context or ''}" | ||
| numbers = extract_numbers(text) | ||
| for value in ( | ||
| candidate.impact.goal_success_probability_delta, | ||
| candidate.impact.additional_yield_usdc, | ||
| candidate.impact.time_saved_months, | ||
| ): | ||
| if value is None: | ||
| continue | ||
| for rendering in (value, abs(value), value * 100, abs(value) * 100): | ||
| numbers.update(extract_numbers(str(round(rendering, 2)))) | ||
| return numbers |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The value * 100 renderings widen the fabrication guard enough to admit order-of-magnitude errors.
Line 621 grounds four renderings per impact figure. The ×100 forms exist for percentage-style probability deltas, but they are added unconditionally — including for additional_yield_usdc and time_saved_months. For a candidate with additional_yield_usdc = 0.2, the grounded set contains both 0.2 and 20.0, so the model can claim "$20 more" for a 20-cent gain and pass validation.
Restrict the ×100 expansion to goal_success_probability_delta, which is the only fraction-valued field:
🔒 Proposed fix
- for value in (
- candidate.impact.goal_success_probability_delta,
- candidate.impact.additional_yield_usdc,
- candidate.impact.time_saved_months,
- ):
- if value is None:
- continue
- for rendering in (value, abs(value), value * 100, abs(value) * 100):
- numbers.update(extract_numbers(str(round(rendering, 2))))
+ prob = candidate.impact.goal_success_probability_delta
+ if prob is not None:
+ for rendering in (prob, abs(prob), prob * 100, abs(prob) * 100):
+ numbers.update(extract_numbers(str(round(rendering, 2))))
+ for value in (
+ candidate.impact.additional_yield_usdc,
+ candidate.impact.time_saved_months,
+ ):
+ if value is None:
+ continue
+ for rendering in (value, abs(value)):
+ numbers.update(extract_numbers(str(round(rendering, 2))))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _grounded_numbers(candidate: RecommendationCandidate) -> set[str]: | |
| """Every normalized number this candidate legitimately carries -- the set | |
| the fabrication guard checks LLM prose against.""" | |
| text = f"{candidate.summary} {candidate.risk_context or ''}" | |
| numbers = extract_numbers(text) | |
| for value in ( | |
| candidate.impact.goal_success_probability_delta, | |
| candidate.impact.additional_yield_usdc, | |
| candidate.impact.time_saved_months, | |
| ): | |
| if value is None: | |
| continue | |
| for rendering in (value, abs(value), value * 100, abs(value) * 100): | |
| numbers.update(extract_numbers(str(round(rendering, 2)))) | |
| return numbers | |
| def _grounded_numbers(candidate: RecommendationCandidate) -> set[str]: | |
| """Every normalized number this candidate legitimately carries -- the set | |
| the fabrication guard checks LLM prose against.""" | |
| text = f"{candidate.summary} {candidate.risk_context or ''}" | |
| numbers = extract_numbers(text) | |
| prob = candidate.impact.goal_success_probability_delta | |
| if prob is not None: | |
| for rendering in (prob, abs(prob), prob * 100, abs(prob) * 100): | |
| numbers.update(extract_numbers(str(round(rendering, 2)))) | |
| for value in ( | |
| candidate.impact.additional_yield_usdc, | |
| candidate.impact.time_saved_months, | |
| ): | |
| if value is None: | |
| continue | |
| for rendering in (value, abs(value)): | |
| numbers.update(extract_numbers(str(round(rendering, 2)))) | |
| return numbers |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_engine.py` around lines 609 -
623, The _grounded_numbers function currently adds ×100 renderings for every
impact field, allowing fabricated order-of-magnitude values. Keep the direct and
absolute renderings for all values, but restrict the value * 100 and abs(value)
* 100 expansions to candidate.impact.goal_success_probability_delta only; do not
apply them to additional_yield_usdc or time_saved_months.
Comment on lines
+814
to
+827
| def _get_redis() -> Any: | ||
| global _redis_client, _redis_available | ||
| if _redis_client is not None: | ||
| return _redis_client if _redis_available else None | ||
| try: | ||
| import redis as _redis | ||
|
|
||
| _redis_client = _redis.from_url(settings.redis_url, decode_responses=True) | ||
| _redis_client.ping() | ||
| _redis_available = True | ||
| except Exception as exc: | ||
| logger.warning("recommendation cache: redis unavailable (%s), using in-memory", exc) | ||
| _redis_available = False | ||
| return _redis_client if _redis_available else None |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
A failed ping latches the cache into in-memory mode for the process lifetime.
If from_url succeeds but ping() raises (Redis slow, restarting, auth blip), _redis_client is left non-None with _redis_available False. Line 816 then short-circuits on every subsequent call and returns None forever — Redis is never retried even after it recovers, so every worker silently diverges onto per-process caches.
Reset the client on failure so the next call re-attempts, or add a retry-after timestamp:
🐛 Proposed fix
except Exception as exc:
logger.warning("recommendation cache: redis unavailable (%s), using in-memory", exc)
+ _redis_client = None
_redis_available = False
return _redis_client if _redis_available else None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _get_redis() -> Any: | |
| global _redis_client, _redis_available | |
| if _redis_client is not None: | |
| return _redis_client if _redis_available else None | |
| try: | |
| import redis as _redis | |
| _redis_client = _redis.from_url(settings.redis_url, decode_responses=True) | |
| _redis_client.ping() | |
| _redis_available = True | |
| except Exception as exc: | |
| logger.warning("recommendation cache: redis unavailable (%s), using in-memory", exc) | |
| _redis_available = False | |
| return _redis_client if _redis_available else None | |
| def _get_redis() -> Any: | |
| global _redis_client, _redis_available | |
| if _redis_client is not None: | |
| return _redis_client if _redis_available else None | |
| try: | |
| import redis as _redis | |
| _redis_client = _redis.from_url(settings.redis_url, decode_responses=True) | |
| _redis_client.ping() | |
| _redis_available = True | |
| except Exception as exc: | |
| logger.warning("recommendation cache: redis unavailable (%s), using in-memory", exc) | |
| _redis_client = None | |
| _redis_available = False | |
| return _redis_client if _redis_available else None |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_engine.py` around lines 814 -
827, Update _get_redis so any failed connection or ping clears _redis_client
along with setting _redis_available to False, allowing subsequent calls to retry
Redis instead of being short-circuited by the initial non-None client. Preserve
the existing in-memory fallback and warning behavior.
Comment on lines
+840
to
+855
| entry = _mem_reco_cache.get(user_id) | ||
| if entry and time.monotonic() < entry[1]: | ||
| return entry[0] | ||
| return None | ||
|
|
||
|
|
||
| def _cache_set(user_id: str, payload: dict[str, Any]) -> None: | ||
| key = _RECO_KEY_PREFIX + user_id | ||
| r = _get_redis() | ||
| if r is not None: | ||
| try: | ||
| r.setex(key, _RECO_CACHE_TTL, json.dumps(payload)) | ||
| return | ||
| except Exception as exc: | ||
| logger.warning("recommendation cache redis set failed: %s", exc) | ||
| _mem_reco_cache[user_id] = (payload, time.monotonic() + _RECO_CACHE_TTL) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
_mem_reco_cache grows without bound — expired entries are never removed.
Line 841 checks expiry on read but nothing deletes stale entries, and Line 855 inserts unconditionally. In the Redis-unavailable path (which Line 816 can make permanent, see the _get_redis comment) every distinct user_id seen by the worker retains a full serialised SavingsRecommendationSet forever. Contrast with _InMemoryEngagementStore._evict_stale, which does sweep.
Bound it — evict on read miss, or use a size-capped OrderedDict/cachetools.TTLCache:
🧹 Minimal fix
def _cache_set(user_id: str, payload: dict[str, Any]) -> None:
key = _RECO_KEY_PREFIX + user_id
r = _get_redis()
if r is not None:
try:
r.setex(key, _RECO_CACHE_TTL, json.dumps(payload))
return
except Exception as exc:
logger.warning("recommendation cache redis set failed: %s", exc)
+ now = time.monotonic()
+ if len(_mem_reco_cache) > 1000:
+ for stale in [k for k, (_, exp) in _mem_reco_cache.items() if exp <= now]:
+ _mem_reco_cache.pop(stale, None)
- _mem_reco_cache[user_id] = (payload, time.monotonic() + _RECO_CACHE_TTL)
+ _mem_reco_cache[user_id] = (payload, now + _RECO_CACHE_TTL)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| entry = _mem_reco_cache.get(user_id) | |
| if entry and time.monotonic() < entry[1]: | |
| return entry[0] | |
| return None | |
| def _cache_set(user_id: str, payload: dict[str, Any]) -> None: | |
| key = _RECO_KEY_PREFIX + user_id | |
| r = _get_redis() | |
| if r is not None: | |
| try: | |
| r.setex(key, _RECO_CACHE_TTL, json.dumps(payload)) | |
| return | |
| except Exception as exc: | |
| logger.warning("recommendation cache redis set failed: %s", exc) | |
| _mem_reco_cache[user_id] = (payload, time.monotonic() + _RECO_CACHE_TTL) | |
| entry = _mem_reco_cache.get(user_id) | |
| if entry and time.monotonic() < entry[1]: | |
| return entry[0] | |
| return None | |
| def _cache_set(user_id: str, payload: dict[str, Any]) -> None: | |
| key = _RECO_KEY_PREFIX + user_id | |
| r = _get_redis() | |
| if r is not None: | |
| try: | |
| r.setex(key, _RECO_CACHE_TTL, json.dumps(payload)) | |
| return | |
| except Exception as exc: | |
| logger.warning("recommendation cache redis set failed: %s", exc) | |
| now = time.monotonic() | |
| if len(_mem_reco_cache) > 1000: | |
| for stale in [k for k, (_, exp) in _mem_reco_cache.items() if exp <= now]: | |
| _mem_reco_cache.pop(stale, None) | |
| _mem_reco_cache[user_id] = (payload, now + _RECO_CACHE_TTL) |
🧰 Tools
🪛 ast-grep (0.44.1)
[info] 850-850: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_engine.py` around lines 840 -
855, Update the in-memory recommendation cache flow around _cache_set and its
read helper to remove expired entries instead of retaining them indefinitely.
When a lookup finds a stale _mem_reco_cache entry, delete it before returning
None, and preserve the existing TTL behavior for valid entries and the Redis
path.
Comment on lines
+878
to
+884
| def _parse_deadline(raw: Any) -> datetime: | ||
| if isinstance(raw, datetime): | ||
| return raw if raw.tzinfo else raw.replace(tzinfo=timezone.utc) | ||
| text = str(raw) | ||
| if text.endswith("Z"): | ||
| text = text[:-1] + "+00:00" | ||
| return datetime.fromisoformat(text) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
_parse_deadline returns naive datetimes from the string branch, which crashes the consolidation sort.
Line 880 normalizes a datetime input to UTC, but Line 884 returns datetime.fromisoformat(text) unnormalized. Any deadline lacking an offset ("2026-06-01", "2026-06-01T00:00:00") yields a naive value.
GoalContext.months_remaining re-normalizes defensively (Lines 101-103) so that path survives, but generate_consolidation_candidates Line 372 does sorted(active, key=lambda g: g.deadline) on the raw values. One naive deadline mixed with one aware deadline raises TypeError: can't compare offset-naive and offset-aware datetimes and takes down the whole generate_for_user request.
🐛 Proposed fix
def _parse_deadline(raw: Any) -> datetime:
if isinstance(raw, datetime):
return raw if raw.tzinfo else raw.replace(tzinfo=timezone.utc)
text = str(raw)
if text.endswith("Z"):
text = text[:-1] + "+00:00"
- return datetime.fromisoformat(text)
+ parsed = datetime.fromisoformat(text)
+ return parsed if parsed.tzinfo else parsed.replace(tzinfo=timezone.utc)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _parse_deadline(raw: Any) -> datetime: | |
| if isinstance(raw, datetime): | |
| return raw if raw.tzinfo else raw.replace(tzinfo=timezone.utc) | |
| text = str(raw) | |
| if text.endswith("Z"): | |
| text = text[:-1] + "+00:00" | |
| return datetime.fromisoformat(text) | |
| def _parse_deadline(raw: Any) -> datetime: | |
| if isinstance(raw, datetime): | |
| return raw if raw.tzinfo else raw.replace(tzinfo=timezone.utc) | |
| text = str(raw) | |
| if text.endswith("Z"): | |
| text = text[:-1] + "+00:00" | |
| parsed = datetime.fromisoformat(text) | |
| return parsed if parsed.tzinfo else parsed.replace(tzinfo=timezone.utc) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/recommendation_engine.py` around lines 878 -
884, Normalize the string-parsing branch of _parse_deadline to return a
timezone-aware UTC datetime, matching the existing datetime-input behavior.
After datetime.fromisoformat(text), attach timezone.utc when the parsed value
has no tzinfo while preserving any supplied offset.
Comment on lines
+311
to
+324
| try: | ||
| async with aiohttp.ClientSession() as session: | ||
| async with session.get( | ||
| url, headers=headers, timeout=aiohttp.ClientTimeout(total=5) | ||
| ) as response: | ||
| if response.status != 200: | ||
| logger.warning( | ||
| f"Failed to fetch rebalance suggestion for vault {vault_id}: " | ||
| f"{response.status}" | ||
| ) | ||
| return {} | ||
| payload = await response.json() | ||
| data = payload.get("data", payload) if isinstance(payload, dict) else payload | ||
| return dict(data) if isinstance(data, dict) else {} |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Bound rebalance-fetch latency across vaults.
RecommendationEngine.gather_context awaits this five-second request once per vault, serially. A user with N vaults can wait up to roughly 5 × N seconds before recommendations proceed. Fetch with bounded concurrency and apply one overall enrichment deadline.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/intelligence/app/services/vault_context.py` around lines 311 - 324,
Update RecommendationEngine.gather_context and its per-vault rebalance-fetch
flow to run requests with bounded concurrency instead of serially, using a
shared concurrency limit. Wrap the complete enrichment phase in one overall
deadline so the recommendation flow proceeds with partial results when the
deadline expires, while retaining the existing per-request timeout and
empty-result handling for failed or timed-out fetches.
Merged
cyb3ralee
pushed a commit
to cyb3ralee/nester
that referenced
this pull request
Jul 30, 2026
…ed in user data (Suncrest-Labs#897) * feat(intelligence): personalized savings recommendation engine grounded in user data Adds a savings recommendation engine (app/services/recommendation_engine.py) that generates personalized, actionable recommendations from a user's real goals, positions, and cash-flow behavior. Candidate actions (increase a goal's contribution, move idle balance to higher yield, lock for a term boost, consolidate goals toward the nearest deadline) and every number attached to them are computed deterministically in Python -- Claude, called via tool use, only selects 2-4 candidates, orders them, and writes a short explanation, constrained to a `select_recommendations` tool schema that can only reference candidates by id. A fabrication guard (`_validate_selection`) checks every number in the model's prose against the set of numbers the referenced candidate actually carries, rejects and regenerates once on violation, then falls back to a fully templated explanation built straight from the candidate's own fields -- so a fabricated number can never reach the response. Yield-related candidates always carry risk context (from the vault's real risk score or a documented default disclosure); goal-success figures are integrated with the Suncrest-Labs#843 Monte Carlo simulation endpoint when reachable (two calls -- current vs. proposed contribution -- diffed into a real probability delta) and degrade to a documented heuristic otherwise, since per-user (Redis-backed with an in-memory fallback, mirroring conversation_store.py's pattern) and filtered out permanently; acted-on action types get a deterministic priority boost. Recommendations are cached per-user for 6 hours and invalidated automatically when goal/vault figures change materially, rather than recomputed per page load. Fixes the stale `claude-sonnet-4-6` model id. Closes Suncrest-Labs#847 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(intelligence): correct Suncrest-Labs#843 simulation contract, mypy/ruff cleanup for Suncrest-Labs#847 - projection_client.py: point ProjectionProvider at the real Suncrest-Labs#843 endpoint contract (POST /api/v1/tools/simulation, goal_success.probability) now that it's known, instead of the placeholder GET route guessed before Suncrest-Labs#843's shape was finalized. Computes the success-probability delta from two simulation calls (current vs required contribution) rather than guessing the Go service's internal sensitivity-grid step sizes. - recommendation_engine.py: thread GoalContext into enrich_with_projections so it can build the simulation request; type the Anthropic tool-use call properly (ToolParam/ToolChoiceToolParam/MessageParam) instead of bare dicts, fixing mypy strict errors -- this is the first tool-use call in the intelligence service, so no prior typed precedent existed. - ruff: import sort, drop pointless f-string prefixes, wrap one long line. mypy --strict and ruff both clean; full suite still 105/105 passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore: retrigger CI (no functional change) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
0xDeon
added a commit
that referenced
this pull request
Aug 10, 2026
* chore: add CodeRabbit AI review config (auto-review PRs targeting dev/main)
* fix(ci): unblock Rust and Go pipelines (#798)
Bump ethnum 1.5.0 -> 1.5.3 in packages/contracts so the contract crate
compiles on the current stable toolchain (rustc 1.97.1). ethnum 1.5.0 fails
with error[E0512] transmuting () into TryFromIntError, which no longer share
a size; the crate fixed it in 1.5.1.
Bump apps/api to Go 1.25.12 and golang.org/x/text v0.39.0 to clear the two
govulncheck findings not in the allowlist: GO-2026-5856 (crypto/tls, fixed
in go1.25.12) and GO-2026-5970 (x/text, fixed in v0.39.0).
* Branch to solve issue#788 (#797)
* test(contracts): add negative authorization coverage
* test: tighten negative authorization assertions
---------
Co-authored-by: Deon <110722148+0xDeon@users.noreply.github.com>
* feat(api): envelope encryption, key versioning & rotation for account cipher (#799)
* feat(crypto): version-tagged envelope cipher for account numbers
Replace the single-key AccountCipher with a multi-version AES-256-GCM
cipher. Encrypt seals with the active key and returns a CipherEnvelope
carrying the key version; Decrypt resolves the key by the ciphertext's
version and fails with ErrUnknownKeyVersion when it is not registered.
NewAccountCipher is retained (registers the key as v1) for backward
compatibility. Fingerprints use a stable pepper independent of the active
key so the uniqueness index survives rotation.
* test(crypto): cover active-key encrypt, cross-version decrypt, unknown version, fingerprint stability
* feat(config): add AccountCipherConfig accessor and versioned key set type
* feat(config): parse ACCOUNT_CIPHER_KEYS/ACTIVE_KEY with legacy single-key fallback
ACCOUNT_CIPHER_KEYS (comma-separated version:base64 pairs) plus
ACCOUNT_CIPHER_ACTIVE_KEY take precedence; when unset, the existing
BANK_ACCOUNT_ENCRYPTION_KEY is registered as v1 so single-key
deployments keep working. Validates active version membership and pair
format at startup.
* test(config): cover multi-key parsing, legacy fallback, and validation errors
* docs(config): document account cipher key set and rotation env vars in .env.example
* feat(db): add bank_accounts.key_version column defaulting existing rows to v1
New column records which key sealed each row so rotation never rewrites
history. Indexed so the rotation tool can cheaply find un-rotated rows.
* feat(db): down migration dropping key_version column and index
* feat(bankaccount): thread key version through Repository Create/GetByID
* feat(repo): persist key_version on bank account insert
* feat(repo): return key_version from GetByID and add rotation Store methods
GetByID now yields the stored key version so callers can decrypt with the
right key. CountPending/ScanPending/UpdateCipher implement rotation.Store
for the rotation tool; UpdateCipher leaves the fingerprint untouched so the
uniqueness index is unaffected.
* feat(rotation): idempotent, resumable batch key-rotation engine
Rotator scans rows not on the active key version, decrypts each with its
recorded version, re-seals with the active key, and commits per row. A
second run finds nothing (idempotent); an interrupted run resumes from the
remainder. Logs only counts and row IDs, never plaintext, keys, or
ciphertext.
* test(rotation): re-encrypt to active, idempotency, resume-after-interrupt, no data loss
* feat(service): seal new accounts with the active key envelope
* feat(service): decrypt saved accounts by their stored key version
ResolveForSettlement reconstructs the CipherEnvelope from the stored
ciphertext and key version; SetDefault/Remove absorb the extra GetByID
return value.
* test(service): update in-memory repo mock for key-versioned signatures
* test(service): new writes use active key; legacy row decrypts after key added
* feat(cmd): rotate_keys CLI to re-encrypt accounts onto the active key
Loads the same key config as the API, refuses to run when no cipher is
configured, and drives the rotation engine over the bank_accounts store
with -batch-size and -timeout flags.
* feat(api): wire the multi-key account cipher from AccountCipherConfig
* docs(security): key versioning model, env format, and 5-step rotation runbook
* fix(crypto): require explicit fingerprint pepper when no v1 key is configured
Defaulting the fingerprint pepper to the active key let it change on every
rotation (e.g. v2->v3) and silently break blind-index uniqueness. Fail closed
with ErrFingerprintKeyRequired instead; cover the no-v1 active-key rotation
case. (CodeRabbit)
* fix(config): fail closed on empty keyset, over-long versions, and v1-less sets
- ACCOUNT_CIPHER_KEYS that parses to zero entries now errors instead of
silently disabling the cipher, and an active version absent from the set is
always rejected.
- Reject key versions longer than 32 chars (bank_accounts.key_version is
VARCHAR(32)) before they fail at the DB boundary.
- Require ACCOUNT_CIPHER_FINGERPRINT_KEY when the key set has no v1. (CodeRabbit)
* fix(db): guard key_version rollback and build its index concurrently
- 057 down aborts if any row is on a non-v1 key, since dropping key_version
would make rotated ciphertext undecryptable.
- Move the index into 058 using CREATE INDEX CONCURRENTLY so a large
bank_accounts table is not write-locked during deploy. (CodeRabbit)
* docs: clarify that a v1-less key set must set an explicit fingerprint pepper (CodeRabbit)
* test(service): consolidate key-versioning scenarios into a table-driven test (CodeRabbit)
* docs: clarify no active-key fallback; config-load failure vs constructor sentinel (CodeRabbit)
* feat(api): distributed rate limiting with strict route limits (#800)
* feat(api): add distributed rate limiting with strict route limits
Extend the existing in-memory rate limiter with a dual-mode backend: a
Redis fixed-window counter for cross-instance enforcement, falling back
to the in-memory token bucket when REDIS_ADDR is unset.
- Add Limiter interface + NewLimiter factory (Redis or in-memory)
- Global per-IP limiter now excludes /health*, /readyz, /metrics
- Strict per-IP limiter on POST /auth/challenge and /auth/verify
(credential stuffing) and strict per-user limiter on
POST /settlements (settlement spam)
- New RATELIMIT_AUTH_* and RATELIMIT_SETTLEMENT_* config knobs + .env
- Redis limiter fails open on outage so it never blocks live traffic
- Table-driven tests: under/over limit, 429 + Retry-After, window
reset, per-IP and per-user isolation, memory fallback, and a
Redis integration test guarded by REDIS_ADDR
* fix(api): address CodeRabbit review on rate limiting
- CORS: move cors middleware outermost so 429 responses from the global
and auth-route limiters still carry Access-Control-Allow-Origin and
stay readable to browser clients
- Redis: bound each limiter round-trip with a 75ms timeout so a slow
(not just down) Redis fails fast into fail-open instead of adding
multi-second latency to every request; log fail-open events
- Proxy-aware client IP: add RATELIMIT_TRUSTED_PROXY_COUNT (default 0).
When set, derive the client IP from X-Forwarded-For counting hops from
the right, so traffic behind a load balancer keys off the real client
instead of collapsing onto the proxy address, without letting clients
spoof past the trusted-proxy boundary
- Tests: proxy-aware keying + spoof resistance, and config default /
override / negative-validation for the new knob
* fix(api): reject sub-millisecond rate-limit windows
The Redis limiter converts the window to whole milliseconds for PEXPIRE,
so a positive but sub-1ms window (e.g. 500us) truncates to 0, expiring
the counter immediately and silently disabling enforcement. Reject
global/auth/settlement windows below 1ms at config load, with a
regression test.
* feat: core backend + AI primitives — job queue, harvest engine, portfolio valuation, RAG grounding (#824 #845 #832 #852) (#876)
* feat(api): durable async job queue (#824)
PostgreSQL-backed job queue with FOR UPDATE SKIP LOCKED dequeue, lease-based
visibility timeout with crash recovery, exponential backoff + full jitter,
dead-letter queue, per-job-type concurrency limits, idempotent enqueue, and
graceful drain on shutdown. Queue-depth/DLQ/latency metrics and correlation-ID
propagation included. Worker pool wired into the API with config knobs.
* feat(api): yield harvest orchestration engine (#845)
Cadence + event-triggered engine that applies the economic gate (harvest iff
accrued yield > gas fee + margin), defers under network congestion, and submits
harvests as idempotent, window-deduplicated jobs on the #824 queue. Includes a
gas oracle abstraction, vault/user/service adapters, an idempotent job handler,
and an owner-scoped harvest-status API (pending yield, threshold, estimated next
harvest). Pure decision core and engine fully unit-tested.
* feat(api): real-time portfolio valuation service (#832)
Stroop-exact aggregation of positions, pending deposits, accrued yield, goal
allocations, and claimable rewards with a structured per-vault/per-goal
breakdown (principal vs yield, locked vs flexible, settled vs pending,
claimable). Multi-asset oracle pricing with confidence propagation, per-user
cache with event-driven invalidation on confirmed transactions, and WebSocket
push of refreshed valuations. Pure aggregator, cache, and service unit-tested.
* feat(intelligence): RAG grounding for Prometheus AI (#852)
Structured retrieval layer that routes queries to the right user-scoped data
sources (positions, goals, transactions, yield landscape) without embeddings,
assembling only the minimal context needed with citations. Grounding rules
force the model to answer solely from retrieved context, cite it, and refuse
when data is missing; post-generation numeric validation flags any figure not
present in the context to catch hallucinations. Strict per-user isolation: scope
is fixed by the JWT subject and cannot be widened by prompt injection. Wired into
streaming chat (and WebSocket chat via the shared path). Fully unit-tested.
* feat(api): savings goal archive-on-delete, amount/name validation, yield cache warming (#874)
* fix(savingsgoal): soft-archive goals on DELETE instead of hard-delete (#685)
Replace the permanent DELETE with an UPDATE that stamps archived_at and
sets status to 'archived'. Adds migration 059 to introduce the
archived_at column. Already-archived goals surface as ErrGoalNotFound
(404) so callers get a sensible response without a 500. Adds two unit
tests via sqlmock asserting the soft-delete and already-archived cases.
* feat(yield): warm DeFiLlama Stellar cache on service startup (#667)
* fix(savingsgoal): validate target_amount and goal name (#692 #681)
#692 — Add savingsgoal.ErrInvalidAmount (defined in the savingsgoal
domain, not imported from vault) and update validateSavingsGoalInput to
return it when target_amount is zero, negative, or below MinTargetAmount
(0.01). Handler writeError now maps ErrInvalidAmount to 400 Bad Request,
fixing the 500 that was returned when vault.ErrInvalidAmount was not
recognised.
#681 — Add validateGoalName capping name at MaxGoalNameLength (100
chars) consistent with the savings_goals.name column width. Called on
both Create and Update paths so over-long names return a 400 instead of
a DB error.
* test(savingsgoal): cover amount and name validation cases (#692 #681)
* feat(contracts): add reentrancy guard and callee allowlist framework (#811) (#879)
Introduce shared temporary-storage reentrancy guards and callee allowlists in libs/common, apply them across vault, treasury, and allocation strategy fund-moving paths, and add hostile mocks with adversarial integration tests plus documented resource costs.
* chore(security): fix IDOR vulnerabilities and harden JWT configuration (#872)
* chore(security): fix IDOR vulnerabilities and harden JWT configuration
Addresses highest-priority findings from security assessment (Issue #589):
Fixed:
- Added ownership validation for vault retrieval endpoints (GET /vaults/{id}, GET /vaults/{id}/allocations)
- Added ownership validation for transaction creation (POST /transactions)
- Added ownership validation for transaction retrieval (GET /transactions/{hash})
- Hardened Intelligence service by preventing production startup without JWT secret via Pydantic model_validator
Documentation:
- Added docs/security/threat-model.md (assets, trust boundaries, entry points, threat actors, controls)
- Added docs/security/pentest-report-v1.md (11 findings with evidence, impact, root cause, remediation, verification)
Authorization was implemented at the handler level rather than the service layer because the service methods are shared by numerous trusted internal system components (scheduler, rebalance, TVL, projections). Refactoring those interfaces would have expanded scope considerably and increased regression risk.
Closes #589
* style: fix ruff line length in test_config.py
* fix: return 404 for missing vault in transaction ownership check
* chore: address CodeRabbit review comments
- Remove dead var _ = decimal.Zero
- Fix function name extractClientIP -> clientIP in evidence
- Update Go test result from manual to ALL PASS (CI confirmed)
- Correct deposit flow wording (price_per_share is not user-supplied)
- Fix WebSocket nil-authenticator mitigation claim (no nil check exists)
* fix(api): resolve duplicate 059 migration prefix on dev
Two migrations shared the prefix 059. `059_create_jobs` landed first in
#876; `059_add_savings_goal_archived_at` landed eight minutes later in
#874 and collided with it.
The consequence was worse than a lint failure. golang-migrate refuses to
load a directory containing duplicate versions, so migrations could not
run at all past 058, and the migration-prefix guard in the API (Go)
workflow failed on every pull request touching the Go API — six open PRs
were red through no fault of their authors.
Renumber the later arrival to 060 and leave `059_create_jobs` in place,
since it merged first and is the version any environment already sitting
at 59 will have applied. Renumbering it instead would have desynced those
environments. `059_add_savings_goal_archived_at` has never been applied
anywhere, because the collision prevented golang-migrate from loading the
directory in the first place, so moving it is safe.
The only in-repo reference to either filename is in
job_repository_integration_test.go, which points at `059_create_jobs.up.sql`
and is unaffected.
Open PRs claiming 060 will need to rebase and renumber.
* feat(api): feature flags, server-side exports, replica routing, API versioning (#882)
Implements four platform capabilities:
Feature flags (#838) — internal/flags/
- Boolean kill-switch, deterministic percentage rollout, cohort and
typed-value flags stored in Postgres (migration 060)
- Percentage membership is hash-based and stable: a user in at 10% stays
in at 20%
- In-process cache with TTL backstop and pub/sub invalidation channel so
changes propagate across instances within seconds
- Kill switches fail SAFE: evaluator returns the registered safe position
when the flag service is unreachable, never fail-open
- Secret guard rejects secret-marked names from the flag store
- Every change goes through a required AuditRecorder
Server-side exports (#839) — internal/export/
- Transaction-history CSV generated from the ledger source of truth with a
stable, documented column schema
- Reconciliation invariant: exported movements must sum to the ledger's
net change per asset or the export errors instead of delivering a wrong
document
- Exports above a documented row threshold route to the durable job queue
- HMAC-signed, time-limited, ownership-verified download tokens; another
user cannot fetch someone's export
Read-replica routing (#841) — internal/db/router.go
- Explicit Read/Write paths declared at call sites, never inferred from SQL
- Read-your-writes: users are pinned to the primary for a bounded window
after writing (Pinner interface; in-memory impl, Redis-ready)
- Unhealthy or lag-exceeding replicas are routed around automatically with
primary fallback; transactions always use the primary
- Per-role pool stats exposed for metrics; Close drains all pools
API versioning (#842) — internal/server/versioning.go, docs/api-versioning.md
- Uniform URL-path versioning with versioned route groups
- Deprecated versions emit Deprecation, Sunset and successor Link headers
on every response; retired versions return 410 Gone with guidance
- Unversioned requests route to a pinned default, not 'latest'
- Per-version usage counting so retirement is data-driven
All packages fully unit-tested (33 tests).
* feat(intelligence): yield optimization engine with constraint-based allocation strategies (#889)
Adds a deterministic, constraint-based yield optimizer to the intelligence
service (#848). Given candidate yield sources and hard constraints
(diversification cap, liquidity floor, lock-horizon fit, risk ceiling,
deposit caps, source status), app.services.yield_optimizer.optimize()
solves a concave-quadratic risk-adjusted-return objective with
scipy.optimize.minimize (SLSQP) and returns per-source weights (fraction
and basis points), expected yield, aggregate risk, and a diversification
index. Infeasible constraint sets are reported explicitly via
infeasibility_reasons and never silently relaxed.
The optimizer is a pure, synchronous, dependency-free function, kept
separate from app.services.yield_explanation, which has Claude narrate an
already-computed result in plain language and validates (via the existing
extract_numbers/normalize_number helpers from retrieval.py) that no number
absent from the result appears in the explanation, falling back to a
deterministic template otherwise.
Also wires a new POST /intelligence/yield-optimization endpoint, adds
scipy/numpy to requirements.txt, and fixes config.py's stale
anthropic_model default (claude-sonnet-4-6 -> claude-sonnet-5).
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* chore(security): create load and stress testing plan for vault API and real-time balance endpoints (#891)
Co-authored-by: felladaniel36-hash <felladaniel36@gmail.com>
* feat(api): scheduler leader election for safe multi-instance background jobs (#894)
Adds Postgres-advisory-lock leader election (internal/scheduler/leadership.go)
gating all five scheduler background job loops (rebalancer, recurring
deposits, APY deviation alerts, goal deadline reminders, protocol health
checks) so exactly one instance runs them at a time, with automatic failover
bounded by a 3s heartbeat interval and an execution-time leadership recheck
immediately before every money-moving/notification action to guard against
split-brain during failover.
Also fixes a real latent bug the recurring-deposit job had: its transaction
hash was derived from the schedule ID alone (constant across every
occurrence of a recurring schedule), so only a schedule's first-ever
occurrence could ever be recorded — every later occurrence hit
vault_transactions' unique transaction_hash constraint and retried forever.
The hash now folds in the occurrence timestamp, and the deposit-recording
step is routed through the existing durable job queue (internal/domain/
jobqueue) with a per-occurrence idempotency key, mirroring the harvest
engine's enqueue pattern, for at-least-once safety.
Wires the three job loops that existed but were never started in main.go
(the rebalance-decision Scheduler remains unwired pending real on-chain
RebalanceSubmitter/YieldFetcher adapters — a pre-existing gap, not a
regression), and exposes current leader/instance/since via a new
GET /api/v1/admin/scheduler/leadership endpoint.
Closes #846
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* feat: AI savings coaching, AI rebalance engine, PWA offline support, i18n framework (#896)
Closes #112, #110, #790, #789
- Savings goal AI coaching (#112): on-demand GET /api/v1/users/savings-goals/{id}/coaching
endpoint plus a weekly GoalCoachingScheduler background job, both backed by the
existing intelligence /intelligence/coaching endpoint. Progress tracking, on-track
status, and required-deposit math already existed in the savingsgoal domain/service;
this closes the remaining AI-coaching gap in the issue.
- AI rebalancing engine (#110): new risk_model.py (Sharpe-ratio inspired
risk_adjusted_score, per-protocol risk factors) and rebalance_engine.py in
apps/intelligence, combining live DeFiLlama APY data with a cached-baseline
fallback and Claude-generated rationale. New POST /vaults/{id}/rebalance/suggest
and /execute endpoints (execute builds an unsigned Stellar transaction via
stellar-sdk), proxied through the Go API at the same paths.
- PWA installability + offline handling (#790): web app manifest, hand-rolled
service worker (app-shell precache, network-first navigation with an /offline
fallback, API requests never cached), and an offline guard on the offramp
withdraw action. The online/offline hook, banner, and deposit/withdraw guards
already existed; this fills the manifest/SW/offline-route/offramp gaps.
- i18n framework (#789): locale provider + en/fr message catalogs + a shared
formatCurrency/formatNumber/formatDate helper (Intl-based), wired into
dashboard/savings/offramp/settings screens, replacing several ad-hoc
formatCurrency implementations. Locale persists to localStorage and is
selectable from Settings > Preferences.
Verified: go build/vet/test + golangci-lint clean; pytest (84 tests) + ruff +
mypy --strict clean; vitest (83 tests) + tsc --noEmit + eslint clean.
Co-authored-by: Chidimj <Chidimj@users.noreply.github.com>
* feat(contracts): granular RBAC, autonomous circuit breaker, vault factory, referral program (#900)
Closes #820, #817, #816, #818
- access_control: granular Role enum (Guardian, Upgrader, Attester,
FeeManager, RebalanceKeeper, Treasurer, VaultCreator), generalised
two-step role transfer (transfer_role/accept_role/cancel_role_transfer),
time-bounded grants (grant_role_until), bounded on-chain enumeration
(get_role_members, role_expires_at). Guardian can pause/halt but never
unpause/upgrade/withdraw.
- vault: autonomous staged circuit breaker (breaker.rs) with independently
configurable trip conditions (share-price move, yield sanity, withdrawal
velocity with anti-griefing margin, source failure), graded severity
(Normal/Throttled/DepositsHalted/FullHalt), staged cooled-down recovery
gated to Admin/Upgrader, and an emergency withdrawal path that works at
every severity. Guardian-only pause/halt entrypoints added.
- vault_factory: new contract deploying vaults from a governed WASM hash
via the Soroban deployer, atomic deploy+init, deterministic address
prediction, O(1) is_nester_vault registry, bounded pagination, timelocked
WASM-hash governance, deprecate_vault.
- referral: new standalone contract for a trustless referral program.
Rewards accrue from the protocol's performance-fee slice (never the
referred user's own yield), gated by minimum deposit/tenure, capped per
referrer and by a global budget that halts accrual without clawback.
Vault is the sole trusted caller, mirroring the existing
treasury.receive_fees pattern.
- Narrower roles wired into treasury (Treasurer), yield_registry
(Attester), and allocation_strategy/vault (RebalanceKeeper, FeeManager)
alongside existing Admin/Operator checks.
- EVENTS.md, SECURITY.md, and the contracts README document the new role
model, Guardian asymmetry, and breaker/factory/referral event surface.
All contracts build to wasm32-unknown-unknown; full workspace test suite
and clippy (-D warnings) pass clean.
* feat(intelligence): add sourced market context signals (#892)
* feat(intelligence): add sourced market context signals
* fix(intelligence): satisfy market context lint
* fix(intelligence): type extraction client boundary
* fix(intelligence): harden signal provenance and batching
* feat(security): add adaptive API abuse protection (#893)
* feat(security): add adaptive abuse protection
* fix(security): harden adaptive abuse state
* feat(intelligence): personalized savings recommendation engine grounded in user data (#897)
* feat(intelligence): personalized savings recommendation engine grounded in user data
Adds a savings recommendation engine (app/services/recommendation_engine.py)
that generates personalized, actionable recommendations from a user's real
goals, positions, and cash-flow behavior. Candidate actions (increase a
goal's contribution, move idle balance to higher yield, lock for a term
boost, consolidate goals toward the nearest deadline) and every number
attached to them are computed deterministically in Python -- Claude, called
via tool use, only selects 2-4 candidates, orders them, and writes a short
explanation, constrained to a `select_recommendations` tool schema that can
only reference candidates by id. A fabrication guard
(`_validate_selection`) checks every number in the model's prose against the
set of numbers the referenced candidate actually carries, rejects and
regenerates once on violation, then falls back to a fully templated
explanation built straight from the candidate's own fields -- so a fabricated
number can never reach the response.
Yield-related candidates always carry risk context (from the vault's real
risk score or a documented default disclosure); goal-success figures are
integrated with the #843 Monte Carlo simulation endpoint when reachable
(two calls -- current vs. proposed contribution -- diffed into a real
probability delta) and degrade to a documented heuristic otherwise, since
per-user (Redis-backed with an in-memory fallback, mirroring
conversation_store.py's pattern) and filtered out permanently; acted-on
action types get a deterministic priority boost. Recommendations are cached
per-user for 6 hours and invalidated automatically when goal/vault figures
change materially, rather than recomputed per page load. Fixes the stale
`claude-sonnet-4-6` model id.
Closes #847
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(intelligence): correct #843 simulation contract, mypy/ruff cleanup for #847
- projection_client.py: point ProjectionProvider at the real #843 endpoint
contract (POST /api/v1/tools/simulation, goal_success.probability) now
that it's known, instead of the placeholder GET route guessed before
#843's shape was finalized. Computes the success-probability delta from
two simulation calls (current vs required contribution) rather than
guessing the Go service's internal sensitivity-grid step sizes.
- recommendation_engine.py: thread GoalContext into enrich_with_projections
so it can build the simulation request; type the Anthropic tool-use call
properly (ToolParam/ToolChoiceToolParam/MessageParam) instead of bare
dicts, fixing mypy strict errors -- this is the first tool-use call in
the intelligence service, so no prior typed precedent existed.
- ruff: import sort, drop pointless f-string prefixes, wrap one long line.
mypy --strict and ruff both clean; full suite still 105/105 passing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* chore: retrigger CI (no functional change)
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* feat(intelligence): backend plumbing for periodic financial insight digests (#898)
Adds the Go-side data plumbing for #859's periodic digest: a digest_cadence
notification preference (off/weekly/monthly, opt-out respected), a
user_digests cache/audit table for one-generation-per-period, a
digest-ledger source endpoint exposing deterministic period deposit/yield/
streak facts for the intelligence service to narrate via the relay, and a
leader-elected daily scheduler job that generates and delivers digests
through the existing notification dispatcher.
This PR covers the Go backend groundwork only. The intelligence-service
narrative generation (grounded LLM prompt, zero-save honesty handling,
attention items, Redis caching), frontend insights card, and test coverage
described in #859's acceptance criteria are not yet implemented — tracked
as follow-up. #865, #864, and #856 are referenced per this repo's issue
numbering but have no implementation in this branch.
Note: this environment has no Go toolchain available, so these changes are
reviewed manually but not compiled or test-run locally.
* feat(vault): fair-ordering emergency queue, tiered fees, slippage-safe rebalance, penalty escrow (#901)
Implements four vault contract features plus their backend indexing:
- #814: fair-ordering emergency withdrawal queue (queue.rs) so paused-vault
exits are served in request order instead of first-caller-wins.
- #813: duration/size-tiered fee schedule (performance, exit, management)
replacing the flat-rate config, with a continuous tenure curve superseding
the old binary min-lock gate.
- #810: slippage-safe multi-hop rebalance split into plan/execute steps
(rebalance.rs) with per-leg minimum-out enforcement.
- #805: early-exit penalty escrow with depositor/treasury split distribution
instead of penalties vanishing into thin air.
Backend: migrations for the four new event-sourced tables, Stellar event
indexer wiring for all seven new on-chain events (including the previously
unhandled rebalance-completed event), and read-only history endpoints under
/api/v1/vaults/{id}/.
Co-authored-by: dslegacy <dslegacy@users.noreply.github.com>
* feat(contracts): on-chain savings goal registry with milestone attestation (#903)
* feat(contracts): on-chain savings goal registry with milestone attestation
Adds a savings_goal Soroban contract recording goal ownership, target,
deadline, and progress trustlessly, with an idempotent 25/50/75/100%
milestone bitmask and bounded multi-contributor accounting. The registry
never custodies funds — only the vault does. Vaults are validated against
the deployed vault_factory at goal creation.
Backend: onchain_goal_id/onchain_status columns and model fields, repo
read/write wiring, and a bitmask<->milestone translation helper aligned
with the contract's semantics so the existing notifier can treat an
on-chain attestation as equivalent to a notified milestone.
* fix: renumber duplicate/colliding migrations to match landed dev sequence
* test(api): add unit tests for YieldHarvest model (#962)
Co-authored-by: opascal221-design <opascal221-design@users.noreply.github.com>
* feat(dapp): savings goal and vault progress visualization with locked-position support
Add rich progress visualization for savings goals and vaults:
- Segmented progress bar distinguishing locked vs flexible portions
- Principal vs earned yield composition breakdown
- Maturity timeline for locked positions with boost badges and unlock dates
- Probabilistic projection band (confidence interval + success probability)
- Constructive at-risk messaging when goal is off track
- Multi-asset vault composition donut (reuses existing recharts pattern)
- Celebration/completion state, encouraging empty state
- Respects reduced-motion preferences via existing useReducedMotion hook
- All states: empty, in-progress-with-locks, at-risk, completed
- Backward compatible: falls back to simple progress bar when rich data absent
Closes #869
* test(api): add unit tests for YieldHarvest model (#962)
Co-authored-by: opascal221-design <opascal221-design@users.noreply.github.com>
* fix: remove unused variable and import
- Remove unused startKYC variable from rotate_keys/main.go (go vet error).
- Remove unused time import from backfill_kyc_encryption/main.go (go vet error).
* fix(apysnapshot): add unique constraint and idempotent upsert (#963)
Adds DB-level uniqueness on (protocol_slug, captured_at) via migration
069, and updates Upsert to use an explicit ON CONFLICT target so duplicate
oracle reports for the same protocol+timestamp are silently ignored.
* feat: session hardening, Prometheus tool-use, nudge engine, unified list search (#967)
* fix(ci): unblock Rust and Go pipelines
Bump ethnum 1.5.0 -> 1.5.3 in packages/contracts so the contract crate
compiles on the current stable toolchain (rustc 1.97.1). ethnum 1.5.0 fails
with error[E0512] transmuting () into TryFromIntError, which no longer share
a size; the crate fixed it in 1.5.1.
Bump apps/api to Go 1.25.12 and golang.org/x/text v0.39.0 to clear the two
govulncheck findings not in the allowlist: GO-2026-5856 (crypto/tls, fixed
in go1.25.12) and GO-2026-5970 (x/text, fixed in v0.39.0).
* chore: remove internal audit and decision report files from repo root
* feat(intelligence): add prompt-injection and output-safety guardrails for chat/analyze (#875)
* feat(intelligence): add prompt-injection and output-safety guardrails
Claude calls in the chat and analyze paths had no defense against prompt
injection or system-prompt extraction, and recommendation output wasn't
schema-enforced. Add input screening (regex-based, logs request_id + a
non-reversible fingerprint, never raw content), a hardened system prompt
with an explicit trust boundary and tagged untrusted-content wrapping,
deterministic history/message bounding, and output post-processing that
strips leaked system-prompt text and enforces a non-model-controlled
disclaimer on /analyze and related endpoints.
* fix(intelligence): close remaining guardrail gaps from review
- Validate inbound X-Request-Id against a bounded safe charset before
trusting it in state/headers/logs, falling back to a fresh UUID
otherwise (prevents log/header injection via a client-supplied header).
- Fix the chat streaming leak-redaction buffer to retain a sanitized
lookback tail on flush instead of resetting to empty, so a
system-prompt marker split across two deltas is still caught.
- Wrap the remaining unwrapped context data interpolated into the
recommend/vault and analyze prompts (positions, vault/user context
lines) in the same trust-boundary tags used elsewhere.
- Sanitize the few model-derived output fields that were missed:
confidence_reason/data_freshness in Recommendation, insight card
action.label/href, and deposit schedule note.
* feat(api/pkg): add keyset cursor and list query grammar parsing
* feat(api/vault): add full-text search and advanced list filtering
* feat(api/settlement): implement memo search and filter updates
* feat(api/savingsgoal): implement search, list filters and repository updates
* feat(api/activity): introduce activity domain, repository, handlers and filter
* feat(dapp/history): update history page to support list filtering and search
* feat(db): add migration for session family rotation and tracking
* feat(api): implement session domain models, repositories, and config
* feat(api): add auth services for token rotation, revocation, and anomaly detection
* feat(api): add session-aware auth middleware, HTTP handlers, and wire main API
* feat(ws): disconnect active WebSocket connections on session revocation
* feat(frontend): implement automatic token refresh and auth provider state
* feat(frontend): add active sessions UI management in settings
* ci: add concurrency groups with cancel-in-progress across workflows
* test(api/savingsgoal): implement ListPaginated mock in template handler tests
* db(migrations): add schemas for user timezone, activity events, nudge log, and preferences
Add DB migration files:
- 057: User timezone column on users table
- 058: Activity events table for tracking user logins and interactions
- 059: Nudge dispatch log table for dispatch history and outcome tracking
- 060: Nudges enabled preference flag
* feat(domain): define smart nudge catalog, user signals, anti-fatigue rules, and intelligence DTOs
Introduce core domain primitives for smart savings nudges:
- Nudge catalog, trigger condition evaluation, priority ranking, and anti-fatigue limits
- User activity, engagement heuristics, responsive timing window, and user segmentation
- Intelligence request/response DTOs for AI copy generation
- User model update for timezone preferences
* feat(repo): add data access for user timezones, activity events, nudge history, and goals
Implement Postgres repository methods for:
- User profile updates supporting timezone
- Recording and querying user activity events
- Logging nudge dispatches, checking anti-fatigue thresholds, and tracking conversion outcomes
- Fetching active savings goals for nudge evaluation
* feat(intelligence): add AI nudge copy generation endpoint with numeric grounding guardrails
Add FastAPI endpoint and AI services for dynamic push copy generation:
- Generate personalized nudge copy via Anthropic Claude model integration
- Validate numeric grounding in guardrails to prevent hallucinated currency figures
- Register /intelligence/nudges route in main FastAPI application
* feat(service): implement nudge engine orchestration, copy generation, and outcome tracking
Add core service logic for smart savings nudges:
- Composite copy generator (static templates fallback + LLM generated copy)
- Prometheus client method for generating nudge copy
- Nudge notifier adapter and milestone-to-nudge milestone mapper
- Nudge outcome service for recording deposits, goal completions, and return visits
- Core NudgeEngineService evaluating rules, user signals, ranking, and anti-fatigue limits
- Register EventSavingsNudge in notifications package
* feat(auth,savings): integrate timezone capture, activity tracking, and nudge outcome hooks
Hook user actions into nudge signals and outcome tracking:
- Return userID from Auth.VerifyAndIssue to record user timezone, login activity event, and return visit outcome
- Attach OutcomeRecorder to SavingsGoalService to track goal completion outcomes
* feat(scheduler,cmd): replace legacy reminder job with periodic nudge engine and wire main app
Wire up the smart savings nudge engine:
- Replace legacy goal deadline reminder job with background NudgeEngineJob
- Initialize repositories, services, and nudge notification dispatcher in main.go
- Trigger nudge evaluation and outcome tracking on completed transaction deposits
* refactor(api): extract audit entry model to domain layer to prevent import cycle
* feat(api): add jti claim to access tokens for unique token minting
* db(migrations): add 057_create_tool_invocations for tool audit logging
* feat(api): add tool audit domain, repository, service, handlers, and proxy routes
* feat(intelligence): implement Prometheus tool execution loop, tool registry, cost governor, and audit client
* feat(dapp): add interactive tool confirmation flow to Prometheus chatbot UI
* style: format code and sort imports across Python intelligence service and Go test files
* fix(api): add timezone field to UpdateProfileInput in UserService
Extend UpdateProfileInput struct with Timezone field to enable clean profile updates from auth handler during wallet verification.
* test(api): add unit test coverage for nudge rules, signals, and outcome recording
Add unit tests covering:
- Anti-fatigue cooldown limits and cap checks
- Static copy template formatting and facts mapping
- Priority scoring and ranking for candidate nudges
- Responsive window signal calculations
- Nudge outcome recorder (deposit, goal completion, return visit tracking)
* fix(intelligence): enforce strict numeric grounding on percentage values and add tests
Update validate_numeric_grounding guardrail:
- Treat percentage values (e.g., '8%') as fact-grounded regardless of digit count to prevent APY mismatches
- Add unit test suite for numeric grounding validation across dollar amounts, Naira figures, percentages, and prose integers
* feat(api): migrate refresh tokens to httpOnly secure cookies
* feat(frontend): adapt API client and auth store for httpOnly refresh cookies
* fix(intelligence): harden input screening against nested boundary tags
* fix(cmd,usersignal): fix vault lookup method in txPoller and remove unused import
Fix vault lookup in main.go transaction poller callback from GetByID to GetVault, and remove unused time import from usersignal interfaces.
* style(intelligence): format Python nudge models, router, and services
Clean up import order and apply ruff/black formatting across Python intelligence service nudge endpoints and functions.
* style: format code and sort imports across Python intelligence service and Go test files
* refactor(api/migrations): renumber search & activity migrations to 061-064
* refactor(intelligence): add strict type hints and defensive checks to tool handlers
* test(api): update auth_service_test for 3-tuple return from VerifyAndIssue
Update unit test assertions in auth_service_test.go to match the updated VerifyAndIssue signature returning (token, userID, err).
* style(intelligence): format Pydantic schema in nudge models
Format blank lines around Pydantic classes in nudge.py according to PEP 8 standards.
* refactor(intelligence): add strict type hints and defensive checks to tool handlers
* fix(intelligence): remove duplicate Any import
* feat(intelligence): secure nudge copy router with JWT auth and strong typing
Update nudge copy endpoint contract:
- Switch route authentication dependency from API key to JWT verification (verify_jwt)
- Update generate_nudge_copy service function to return strongly typed NudgeCopyResponse Pydantic models
* fix(intelligence): remove unused type ignores and add missing kwargs type
* fix(intelligence): source rebalance rationale model from settings
---------
Co-authored-by: 0xDeon <oluwadamilare_daniel@outlook.com>
Co-authored-by: G-ELM <alfygodwin@gmail.com>
* Feat/issues 943 944 945 946 (#969)
* test(api): add unit tests for protocoltvl model
- Add coverage for TVL delta computation
- Add tests for negative and zero TVL edge cases
- Test 24h change percentage calculations
* test(api): add unit tests for tvl model
- Add coverage for aggregation across protocols
- Test zero and negative TVL edge cases
- Test precision handling for USDC formatting
* feat(api): add vault capacity limits and soft-cap warnings
- Add SoftCapacity and CapacityWarningPct fields to Vault model
- Implement GetCapacityStatus() for API exposure
- Implement CanAcceptDeposit() to gate deposits at capacity
- Add ErrCapacityExceeded error type
- Add comprehensive tests for capacity status and gating
* feat(api): add harvest dry-run/simulation mode
- Add SimulateHarvest() method to harvest engine
- Returns expected gas cost and net yield without execution
- Integrates with existing gas estimation in gas.go
- Useful for user-facing harvest preview features
---------
Co-authored-by: meloball9993 <starmeloball9993@gmail.com>
* test(api): unit tests for apysnapshot model validation (#975)
apps/api/internal/domain/apysnapshot/model.go had no test coverage.
Adds model_test.go covering:
- Validate(): required protocol slug, non-negative APY/TVL, non-zero
capture timestamp
- ByCapturedAt: chronological ordering of a snapshot slice
- DuplicateTimestamps: detecting repeated captured_at values within a
protocol's snapshot history, which the (protocol_slug, captured_at)
unique constraint should otherwise prevent from reaching storage
- error message assertions for ErrProtocolNotFound and
ErrDuplicateSnapshot
Validate, ByCapturedAt, and DuplicateTimestamps are small additions to
model.go needed to give the requested validation/ordering/duplicate
tests something concrete to exercise at the domain layer, independent
of the Postgres repository.
Co-authored-by: AdaBliss <295242925+AdaBliss@users.noreply.github.com>
* fix: correct computePctChange for negative TVL values and fix precision test expectation (#977)
- computePctChange now treats negative current as zero and returns 0 for
negative prior (avoid division-by-zero with negative denominator)
- Fix TestPrecisionHandling expected value: StringFixed(2) rounds half-up,
so 1234.567890 -> 1234.57, not 1234.56
* feat(dapp): market sentiment component historical trend view (#978)
components/ai/marketSentiment.tsx showed current sentiment only. Adds a
small 7/30 day sparkline so users see the trend, not just a
point-in-time read.
- app/services/sentiment_history.py: records each successfully computed
sentiment (signal + confidence) with a timestamp, backed by Redis
when available (same pattern as coingecko.py's cache) with an
in-memory fallback, retaining 30 days of points
- wire recording into prometheus.get_market_sentiment on its success
path
- new endpoint GET /api/v1/market/sentiment/history?days=7|30 in
analyze.py, clamped to [1, 30]
- dapp: intelligence.getMarketSentimentHistory(days) client method and
a SentimentSparkline component in marketSentiment.tsx rendering an
inline SVG confidence trend line with a 7d/30d toggle, colored by the
most recent point's signal, with a graceful "not enough history yet"
state when fewer than 2 points are available
Co-authored-by: AdaBliss <295242925+AdaBliss@users.noreply.github.com>
* feat(api): per-vault configurable harvest frequency (#974)
The harvest engine previously evaluated every vault on a single global
tick interval, so a small vault and a large one paid the same harvest
cadence regardless of their gas-cost tradeoffs. Vaults can now be
configured for daily or weekly harvesting.
- add harvest_frequency and last_harvested_at columns to vaults
(migration 080), defaulting new vaults to daily
- add vault.ParseHarvestFrequency and a Repository.UpdateHarvestFrequency
method
- gate the harvest engine's tick, TriggerVault and status/simulation paths
on a new DueForHarvest check alongside the existing economic gate, and
record last_harvested_at whenever a harvest is applied
- add a PATCH /api/v1/vaults/{id}/harvest-frequency endpoint, restricted
to the vault owner
Complements the harvest engine from #845.
Co-authored-by: AdaBliss <295242925+AdaBliss@users.noreply.github.com>
* feat(contracts): add timelock-governed upgrade framework for Soroban contracts (#959)
* feat(contracts): implement secure timelock-governed upgrade framework
* fix(ci): build contract packages explicitly for WASM target
* fix(ci): strip CR/LF from extracted package name to fix WASM build loop
* fix(ci): use cargo metadata to enumerate workspace-member contracts for WASM build
* fix(api): fix TVL negative edge cases and precision truncation
- protocoltvl: add computePctChange() — clamps negative current to 0,
returns 0 for negative/zero prior (undefined %). Add model_test.go.
- tvl: add FormatUSD() (truncate-2) and FormatUSDC() (truncate-6) to
prevent rounding-up of displayed balances. Use them in tvl service.
Add model_test.go covering TestPrecisionHandling.
* fix(api): move computePctChange to model_test.go to avoid redeclaration
---------
Co-authored-by: Hamfit <opefawazademolar@gmail.com>
* feat(api): Monte Carlo savings forecasting engine (#890)
* feat(api): Monte Carlo savings forecasting engine
Upgrade savings projections from a single deterministic point estimate to
a Monte Carlo forecast: thousands of randomized paths over the horizon
varying yield (grounded in a vault's real historical APY volatility) and
contribution behavior (grounded in the user's own active savings
schedule, with a documented new-user prior), reporting a P10/P50/P90
band and a goal-success probability plus a deposit/deadline sensitivity
grid whose "more deposit never lowers success probability" guarantee is
an exact structural property (common random numbers), not statistical.
- internal/domain/projection/simulation.go: pure Monte Carlo engine
(RunMonteCarloSimulation, SensitivityGrid, DeriveSeed, MeanStdDev) and
supporting types, carried over from a prior session and left
unmodified except for adding SimulationOutput.ContributionSource.
- internal/service/projection_simulation.go: SimulateVaultProjection
resolves real APY history/schedule data, derives a stable seed, and
caches results in a small in-process TTL cache (5 min window).
- internal/handler/projection_handler.go: new authenticated
POST /api/v1/tools/simulation endpoint.
- cmd/api/main.go: wires the savings goal/schedule repos into
ProjectionService.
- internal/domain/projection/README.md: documents every distributional
assumption (yield model, contribution/skip model + new-user prior,
path count rationale, RNG seeding/caching scheme).
- calculator_test.go: percentile stability across runs with the same
seed, zero-volatility collapse to the deterministic projection,
goal-success probability against a hand-computed case, and
sensitivity-grid deposit monotonicity.
- Frontend: lib/api/projection.ts gains typed simulation types/client;
savings-calculator.tsx renders the P10/P90 band + P50 line and a
goal-success probability tile alongside the existing deterministic
chart.
Closes #843
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(api): bound Monte Carlo simulation horizon to fix CodeQL memory-exhaustion alerts
CodeQL flagged two high-severity findings on this PR: make([][]float64,
months) and make([]PercentileTimelinePoint, months) in
RunMonteCarloSimulation size their allocation directly off the caller-
supplied PeriodMonths, with no upper bound. A caller (or a bug upstream)
supplying an extreme period_months value would drive an unbounded
allocation before any other check fires -- a memory-exhaustion DoS vector.
Adds MaxPeriodMonths (50 years) and:
- SimulationInput.Validate rejects PeriodMonths/DeadlineMonths beyond it
with a new ErrPeriodTooLong, so a caller gets a clear error instead of
a silently truncated result.
- RunMonteCarloSimulation also clamps to MaxPeriodMonths directly at the
allocation site, as defense in depth for any caller that reaches it
without going through Validate first.
Adds regression tests for both: TestSimulationInput_Validate_RejectsExcessivePeriod
and TestRunMonteCarloSimulation_ClampsExcessivePeriodMonths (the latter
passing quickly without an OOM is itself the assertion for a
2-billion-month input).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(api): use min() in make() calls to satisfy CodeQL flow analysis
* fix(api): guard against excessive PeriodMonths with early return instead of clamping
CodeQL's taint tracking for 'Slice memory allocation with excessive size
value' could not verify the defensive clamp (months = MaxPeriodMonths)
as a sufficient bounds check. Replacing with a guard clause
(months <= 0 || months > MaxPeriodMonths -> early return) makes the
invariant explicit: the make([]T, months) calls are only reachable when
months is already within [1, MaxPeriodMonths].
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* feat: add reconciliation engine foundation (#887)
Co-authored-by: kitWarse <278602811+kitWarse@users.noreply.github.com>
* feat: add time-series rollup store (#885)
Co-authored-by: kitWarse <278602811+kitWarse@users.noreply.github.com>
* fix: address review feedback - useMemo dependency, unused import and vars
* feat(api): yield APY snapshot anomaly flagging before ingestion
apysnapshot/model.go stores APY snapshots straight from the DeFiLlama
poller with no sanity check, so a bad upstream reading (oracle glitch,
scraping error, a genuinely manipulated pool) flows straight into vault
APY history and user-facing yield figures. This adds a guard that flags
implausible jumps before a snapshot is persisted, complementing the
oracle aggregation/failover work in #830.
- add apysnapshot.DetectAnomalousJump: compares a new snapshot's APY to
the protocol's most recent prior reading and flags moves of more than
AnomalyJumpMultiplier (3x) in either direction, skipping near-zero
baselines (< 0.5%) where large percentage swings are normal noise
- add Flagged/FlagReason fields to APYSnapshot, persisted via migration
081 (apy_snapshots.flagged, flag_reason)
- wire the guard into APYService.poll via a new flagIfAnomalous step
that looks up the most recent snapshot within a 48h lookback window
and flags (does not reject) the incoming snapshot, so a genuine
market dislocation doesn't starve history/oracle failover of data
* feat: Claude rate-limit handling, retrieval tests, defillama staleness guard, AI opt-out
- #928: apps/intelligence/app/services/prometheus.py's stream_chat caught
every Claude error identically (generic "trouble connecting" message).
Added a specific anthropic.APIStatusError handler that distinguishes
429 (rate-limited) / 529 (overloaded) with a clearer "receiving a lot
of requests, try again shortly" message, while other API status errors
and non-API exceptions keep the existing generic fallback. Neither
chat.py nor ws_chat.py ever surfaced a raw 500 for this — stream_chat
already caught everything — but the message didn't call out the
specific, actionable rate-limit/overload case.
- #930: added tests/test_retrieval_relevance.py. retrieval.py's
"relevance filtering" is intent-based section gating (route_query ->
which of GOALS/TRANSACTIONS/YIELD_LANDSCAPE/POSITIONS get fetched),
not numeric relevance scoring — tests confirm sections NOT matched by
the query's intents are never even fetched (not just absent from
output), plus the existing empty-result fallback behavior.
tests/test_retrieval.py (from #852) already covered routing and basic
empty-fallback; this fills the specific "sections excluded, not just
empty" gap #930 asks for.
- #931: apps/intelligence/app/services/defillama.py already had a TTL
cache; added the staleness guard the issue is actually about — every
successful fetch also writes a long-TTL (24h) "last known good" copy,
and a live-fetch failure after the short-TTL entry has expired now
serves that stale copy instead of an empty list, so a DefiLlama
outage degrades to slightly-stale yield data instead of no data.
- #935: apps/api/internal/service/goal_coaching_scheduler.go's weekly
AI goal-coaching job iterated every active goal and called the
intelligence service unconditionally — no opt-out check at all,
unlike nudge_engine_service.go's existing NudgesEnabled gate for
generic nudges. Added the same nudge.PreferenceChecker gate before
any intelligence-service call, and added an explicit
ai_insights_enabled field to CoachingRequest (both Go and the
intelligence service's Pydantic model) so the intelligence service
also refuses to generate content when told a user opted out —
enforcement independent of the caller, not just "trust the API
already checked." Defaults to enabled so on-demand (user-initiated)
coaching requests, which opt-out doesn't apply to, are unaffected.
Verification notes:
- Python (apps/intelligence): full suite run locally in a fresh uv venv
— 212 passed, including all new/changed tests.
- Go (apps/api): no Go toolchain was available in the environment this
was authored in, so main.go / goal_coaching_scheduler.go / model.go
and their test changes could not be compiled or run locally —
reviewed by hand for signature/interface consistency (nudgeHistoryRepo
already implements nudge.PreferenceChecker; the *T-vs-T receiver on
the new recordingGoalCoachingClient test double satisfies the
GoalCoachingClient interface). Please confirm via CI or a local
`go build ./... && go test ./...` before merging.
Closes #928
Closes #930
Closes #931
Closes #935
* feat(api): typed GetOrCompute cache layer with single-flight and stale-while-revalidate (#827)
Adds a generic Redis-backed cache (internal/cache) sitting in front of any
compute function: in-process single-flight collapses concurrent same-key
misses to one compute regardless of Redis, a best-effort cross-process Redis
lock reduces duplicate work across instances, TTLs are jittered to avoid
synchronized expiry, and soft/hard TTLs enable serve-stale-while-revalidate
(a stale value is returned immediately while a background refresh runs).
Namespace-scoped Invalidate targets a single key; a nil Redis client
degrades the cache to in-process-only behavior rather than failing.
closes #827
* feat(api): horizontally-scalable WebSocket layer with Redis pub/sub fan-out (#828)
Extends the WebSocket hub so events reach connected clients regardless of
which API instance holds their socket or produced the event: each instance
publishes broadcast events to Redis pub/sub and re-injects events received
from other instances into its own local delivery path, skipping its own
echoed publishes via an origin-instance tag. Per-topic Redis subscriptions
are reference-counted against local subscriber counts so an instance only
subscribes to channels its own clients actually need, and are released on
the last local unsubscribe or on graceful shutdown.
Adds Redis-backed presence tracking with a heartbeat-refreshed TTL so a
crashed instance's presence entries self-expire rather than lingering.
Adds per-IP connection limits (429 on exceeding the configured cap) and
keeps the existing slow-client backpressure (disconnect on a full send
buffer) intact. All of this degrades to the pre-existing single-instance
in-process-only behavior when no Redis client is configured (nil-safe
throughout, matching the codebase's existing dual-mode convention for
middleware.NewLimiter).
Tests include a real two-Redis-sharing two-Hub cross-instance delivery test,
an own-event-not-double-delivered test, a reconnect-moves-subscriptions
test, a cross-instance presence test, a slow-client disconnect test, and a
per-IP limit rejection test — all passing against a real Redis instance.
closes #828
* feat(api): multi-channel notification service with categories, preferences, dedup and delivery tracking (#829)
Adds a suppressibility Category (safety/transactional/promotional) per
EventType: safety notifications always bypass preference and rate-limit
checks (a breaker trip must never be silently opted out of), promotional
fully honors opt-out, transactional sits in between. Preferences can now be
resolved per-category via an optional CategoryPreferenceStore seam (a
Postgres-backed GetForCategory/SetCategoryOverride is added to
NotificationRepository, storing overrides in a new category_overrides JSONB
column added by migration 069) while stores that only implement the
existing flat PreferenceStore keep working unchanged.
Adds dedup (in-memory and Redis-backed, SET-NX-EX) and per-user-per-category
rate limiting (reusing middleware.NewLimiter's existing dual-mode Redis/
in-process pattern) — both suppress a Send while still persisting the
notification with a recorded SuppressedReason, so a suppressed message is
auditable rather than silently dropped. A suppressed or delivered
notification's outcome is tracked per channel (Delivered/Error/IsFallback)
via a new optional DeliveryOutcomeRecorder seam, with a Push/Email failure
falling back to WebSocket delivery (deduped against a WebSocket delivery
already in that event's normal channel matrix). Failed Email/Push
deliveries enqueue a durable retry job through the existing job queue
(jobqueue.Client); the job handler redelivers via the specific channel that
failed. Dispatcher.Stats() exposes per-category attempted/delivered/failed/
suppressed counts for a metrics endpoint.
Also fixes the stale "TODO: Fix interface implementation" in main.go that
had left NewWebSocketChannel commented out — WebSocketHub's PushToUser
signature already matched Hub's once #828's hub.go changes landed, so
in-app websocket delivery through the notification dispatcher is now
actually wired, not just persisted-and-discarded.
Deliberately deferred (disclosed rather than silently skipped): HTTP
handler/frontend surface for editing category preferences (the existing
flat-preference handler/settings page is unchanged); migrating
goal_milestone_notifier's own notified_milestones dedup onto the new
generic Deduplicator (that table is a permanent, non-windowed,
correctness-sensitive dedup — migrating it is exactly the kind of
unreviewed, regression-risk change this PR intentionally avoids); real SMTP/
push provider integrations (the existing MailSender/PushSender seams and
their Noop/Recording implementations are unchanged).
Also fixes CI: the api job's Redis service only exported REDIS_URL, but
every Redis-backed test (existing internal/cache tests included) skips via
REDIS_ADDR per the established convention, so these tests have been
silently skipping in CI. Sets REDIS_ADDR alongside REDIS_URL.
closes #829
* fix(ci): export REDIS_ADDR alongside REDIS_URL so Redis-backed tests actually run
The api job's redis service was only exposed to tests via REDIS_URL, but
every Redis-backed test in this codebase (internal/cache, internal/
middleware's rate limiter, and this PR's internal/ws and internal/
notifications tests) skips via t.Skip when REDIS_ADDR specifically is
unset. That means these tests have been silently skipping in CI even
though a real Redis service was running right next to them the whole time.
* feat(api): oracle aggregation layer with multi-source consensus and failover (#830)
Adds Aggregate: queries every healthy registered source for a data type in
parallel (each bounded by a per-source timeout so one slow source cannot
stall the result), then reconciles responses via median-with-deviation-band
outlier rejection — a source more than MaxDeviationBPS from the pre-filter
median is discarded before the final median is recomputed from survivors,
so a single bad or manipulated print cannot move the consensus. Returns
Unavailable only when zero sources respond; a lone responding source below
MinAgreeingSources still produces a value (preserving the existing
priority-failover availability guarantee) but with reduced Confidence
rather than a blind pass-through, so a caller that needs full consensus can
gate on Confidence instead of merely on "a value came back".
Adds HealthTracker: per-source consecutive-failure count, last error, and
an exponential backoff window (5s base, doubling per consecutive failure,
capped at 5m) during which a source is skipped rather than queried on every
request; a success clears the failure history immediately.
Wires this into RateService.fetchXLM (internal/oracle/service.go),
replacing the previous "try providers in priority order, first success
wins" loop with real two-source (Horizon, DeFiLlama) consensus — the
existing XLM sanity-bounds check is kept as a second, independent defense
against every source being corrupted in the same direction, which
deviation-band rejection alone can't catch. ExchangeRate gains Confidence
and SourcesUsed fields (empty/zero for rates that don't go through the
aggregator, e.g. the fixed USDC/USD peg) and a MeetsConfidenceThreshold
helper for downstream consumers to gate on. All existing service_test.go
cases pass unchanged, including the priority-style single-surviving-source
assertions (SourceName() reports a lone source's own name, matching the
prior Source field behavior exactly, and only joins names when more than
one source genuinely agreed).
Deliberately deferred (disclosed rather than silently skipped): a second
independent source for TVL and for the DeFiLlama-sourced portions of the
APY pipeline (apy_service.go / apy_refresh.go) — those currently have only
one real external provider each in this codebase, and standing up a second
genuine external data provider integration is out of scope here; migrating
risk_service.go and the on-chain attestation signer (a separate contracts
repo) onto Confidence gating. One added cost worth flagging: because
Aggregate queries every healthy source in parallel rather than stopping at
the first success, DeFiLlama is now called on every XLM/USD refresh even
when Horizon succeeds, not only as a fallback.
Tests cover: agreeing sources produce the correct median consensus; a
wildly-off outlier is rejected without moving the consensus; all-but-one
source down yields a value with reduced (not zero, not full) confidence
rather than Unavailable; every source down is Unavailable; a slow source
times out without stalling the result; a source is skipped once unhealthy
and re-probed after its backoff window elapses; backoff grows with
consecutive failures; a success clears failure history; confidence decay
for staleness reaches exactly half at maxAge.
closes #830
* feat(intelligence): add per-user AI tone/style preference (#927)
* feat(intelligence): add explainability trace for AI-suggested actions (#925)
* feat(api): add soft-delete with recovery window for savings goals (#924)
* fix(api): prevent duplicate deadline reminders across timezones (#923)
* feat: contribution limits, admin goal templates, and calculator export
- api: add optional min/max per-contribution limits on savings goals,
validated at deposit time in DepositSplit (savingsgoal/model.go)
- api: let admins publish/edit/remove curated savings goal templates via
domain/admin, growing the catalog beyond the pre-built #778 defaults
without a redeploy
- dapp: add CSV/PDF export to the savings calculator, reusing the existing
lib/export utilities
Closes #918
Closes #919
Closes #922
* fix(api): restore soft-delete code erased by merge 9c30de1 (#1000)
* fix(api): restore soft-delete code erased by merge 9c30de1 (#994)
Merge 9c30de1 (via PR #985) resolved conflicts in the savings g…
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
Closes #847
Adds a savings recommendation engine that generates personalized, actionable recommendations grounded in the user's real goals, positions, and cash-flow behavior. Every number is computed deterministically in Python; Claude only selects, orders, and explains.
Architecture
app/services/recommendation_engine.py): four candidate generators, each a pure function over real fetched data —generate_contribution_candidates(goal behind schedule → required monthly deposit via the standard amortization formula, now shared withSavingsServicevia the newapp/services/finance_math.py),generate_yield_move_candidates(prefers the Go API's own already-computed rebalance suggestion viavault_rebalance_service.go'sGET /api/v1/vaults/{id}/rebalance-suggestion, falls back to comparing against the best available vault within the user's risk tolerance),generate_term_lock_candidates,generate_consolidation_candidates(goal-snowball toward the nearest deadline).select_and_explaingives Claude the candidates (with their pre-computed numbers) and aselect_recommendationstool schema constrained to referencing only providedcandidate_ids. Claude picks 2-4, orders them, and writes a short explanation._validate_selectionextracts every number in the model's explanation and checks it against the exact set of numbers the referenced candidate legitimately carries (_grounded_numbers, checked across multiple roundings/representations). Any unsupported number triggers one regeneration attempt with the violation fed back to the model; persistent fabrication falls back to a fully templated explanation built directly from the candidate's own fields — the response can never contain an invented figure, proven inTestFabricationGuardincluding a full mocked-Anthropic-client end-to-end test of the reject→regenerate→fallback path.move_to_higher_yield,lock_for_term_boost) always carry arisk_contextstring — either genuinely computed (naming the target vault's real risk score) or, if none was set, a documented default risk disclosure filled in by_ensure_risk_context— never silent. Projections are always framed as probabilities via the standard non-adviceSTANDARD_DISCLAIMERon every response, never as guarantees.app/services/projection_client.py): calls feat(api): Monte Carlo savings forecasting engine with probabilistic goal projections #843's realPOST /api/v1/tools/simulationendpoint twice per contribution candidate (once at the current monthly contribution, once at the required one) and diffs the twogoal_success.probabilityvalues into a real, Monte-Carlo-computed success-probability delta. feat(api): Monte Carlo savings forecasting engine with probabilistic goal projections #843 isn't merged todevyet, so this degrades safely to the heuristic estimate already computed on any failure (network error, 404, unexpected shape) — never blocks, never invents a number.app/services/recommendation_store.py): mirrorsconversation_store.py's exact Redis-with-in-memory-fallback pattern. Dismissed candidates are filtered out permanently (filter_and_rank_candidates); action types the user has previously acted on get a deterministic 1.25x priority boost — retrieved context feeding deterministic ranking, never an opaque model._context_fingerprint, hashing goal targets/balances/deadlines and vault balances/APYs) rather than regenerated on every page load;refresh=trueforces regeneration.claude-sonnet-4-6default toclaude-sonnet-5, documented, still overridable viaINTELLIGENCE_ANTHROPIC_MODEL.Endpoints (
app/routers/recommendations.py, mounted under/intelligence)GET /savings-recommendations?risk_tolerance=&refresh=— the recommendation set.POST /savings-recommendations/{candidate_id}/dismissPOST /savings-recommendations/{candidate_id}/acted-onTests (
tests/test_recommendation_engine.py,tests/test_recommendation_store.py)Covers all required scenarios: candidate-generation correctness against known inputs for all four generator types, the fabrication guard (grounded numbers pass, invented numbers rejected, unknown candidate ids rejected, full mocked-client regenerate-then-fallback flow), risk context always attached to yield candidates, and dismissed candidates never re-recommended (scoped per-user) while acted-on types get boosted not filtered.
Note on test execution: this sandbox's system Python is 3.10; the project targets
>=3.12(apps/intelligence/pyproject.toml) and an existing, unrelated file (conversation_store.py, untouched by this PR) usesdatetime.UTC(3.11+), so the full suite cannot execute here regardless of this change — confirmed by the same import error occurring on pre-existing, unmodified test files. Verified instead viapython3 -m compileall(clean, no syntax errors acrossapp/andtests/) and a full manual read-through of every new/changed file. CI (which runs on the project's real Python version) should be the authoritative test run for this PR.Summary by CodeRabbit
New Features
/intelligencepath.Bug Fixes
Tests