mirror of
https://github.com/escalante29/WealthySmart.git
synced 2026-07-17 11:28:47 +02:00
Error visibility and agent tool cleanup
Exchange-rate fetchers log every source failure instead of silently passing (BE-18); the rate tool exposes fetched_at so staleness is visible (BE-20). Agent: unbound-session failures raise a clear RuntimeError (BE-12); net worth converts via get_crc_multipliers with last-known fallbacks instead of a hardcoded 600 CRC / 1.08 EUR guess, and reports accounts it cannot convert rather than inventing numbers (BE-24); budget tool enforces MIN/MAX_YEAR (BE-16); municipal receipts gain offset paging (BE-17); category analytics prefetches names; the module docstring pins the read-only tool policy (SEC-09). Note: the refresh loop never swallowed CancelledError (BaseException since 3.8) — ARCH-08 was a false positive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,11 @@ Read-only tools exposed to the MAF ChatAgent. Each tool is a thin wrapper
|
|||||||
around existing SQLModel queries / service helpers — they do NOT duplicate
|
around existing SQLModel queries / service helpers — they do NOT duplicate
|
||||||
business logic. The active DB session is resolved via a ContextVar so tool
|
business logic. The active DB session is resolved via a ContextVar so tool
|
||||||
signatures stay clean for the LLM.
|
signatures stay clean for the LLM.
|
||||||
|
|
||||||
|
POLICY: every tool here must stay READ-ONLY. Transaction descriptions reach
|
||||||
|
the model from external emails (indirect prompt-injection surface), so a
|
||||||
|
write-capable tool would let crafted text mutate financial data. Any future
|
||||||
|
write tool requires an explicit user-confirmation step in the UI.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
@@ -28,10 +33,13 @@ from app.models.models import (
|
|||||||
WaterMeterReading,
|
WaterMeterReading,
|
||||||
)
|
)
|
||||||
from app.services.budget_projection import (
|
from app.services.budget_projection import (
|
||||||
|
MAX_YEAR,
|
||||||
|
MIN_YEAR,
|
||||||
compute_monthly_projection,
|
compute_monthly_projection,
|
||||||
compute_yearly_projection_with_cumulative,
|
compute_yearly_projection_with_cumulative,
|
||||||
get_cycle_range,
|
get_cycle_range,
|
||||||
)
|
)
|
||||||
|
from app.services import exchange_rate as fx
|
||||||
from app.services.exchange_rate import (
|
from app.services.exchange_rate import (
|
||||||
get_converted_amount_expr,
|
get_converted_amount_expr,
|
||||||
get_current_rate,
|
get_current_rate,
|
||||||
@@ -49,7 +57,13 @@ def reset_session(token: contextvars.Token) -> None:
|
|||||||
|
|
||||||
|
|
||||||
def _s() -> Session:
|
def _s() -> Session:
|
||||||
|
try:
|
||||||
return _session_ctx.get()
|
return _session_ctx.get()
|
||||||
|
except LookupError as exc:
|
||||||
|
raise RuntimeError(
|
||||||
|
"DB session not bound to agent context — tool called outside an "
|
||||||
|
"HTTP request (the AG-UI middleware binds it per request)"
|
||||||
|
) from exc
|
||||||
|
|
||||||
|
|
||||||
# ─── Tools ──────────────────────────────────────────────────────────────────
|
# ─── Tools ──────────────────────────────────────────────────────────────────
|
||||||
@@ -77,26 +91,31 @@ def get_accounts() -> list[dict]:
|
|||||||
def get_net_worth() -> dict:
|
def get_net_worth() -> dict:
|
||||||
"""Return total assets, liabilities and net worth in CRC (primary currency).
|
"""Return total assets, liabilities and net worth in CRC (primary currency).
|
||||||
USD/EUR balances are converted at the latest exchange rate."""
|
USD/EUR balances are converted at the latest exchange rate."""
|
||||||
accounts = _s().exec(select(Account)).all()
|
session = _s()
|
||||||
rate = get_current_rate(_s())
|
accounts = session.exec(select(Account)).all()
|
||||||
sell = float(rate.sell_rate) if rate else 600.0
|
multipliers = fx.get_crc_multipliers(session)
|
||||||
assets_crc = 0.0
|
assets_crc = 0.0
|
||||||
liabilities_crc = 0.0
|
liabilities_crc = 0.0
|
||||||
|
excluded: list[str] = []
|
||||||
for a in accounts:
|
for a in accounts:
|
||||||
amt = float(a.balance)
|
mult = multipliers.get(a.currency.value)
|
||||||
if a.currency.value == "USD":
|
if mult is None:
|
||||||
amt = float(a.balance) * sell
|
# No live or last-known rate: say so instead of inventing a number
|
||||||
elif a.currency.value == "EUR":
|
excluded.append(f"{a.label} ({a.currency.value})")
|
||||||
amt = float(a.balance) * sell * 1.08 # rough; real conversion is endpoint-side
|
continue
|
||||||
|
amt = float(a.balance) * float(mult)
|
||||||
if a.account_type.value == "LIABILITY":
|
if a.account_type.value == "LIABILITY":
|
||||||
liabilities_crc += amt
|
liabilities_crc += amt
|
||||||
else:
|
else:
|
||||||
assets_crc += amt
|
assets_crc += amt
|
||||||
return {
|
result = {
|
||||||
"assets_crc": round(assets_crc, 2),
|
"assets_crc": round(assets_crc, 2),
|
||||||
"liabilities_crc": round(liabilities_crc, 2),
|
"liabilities_crc": round(liabilities_crc, 2),
|
||||||
"net_crc": round(assets_crc - liabilities_crc, 2),
|
"net_crc": round(assets_crc - liabilities_crc, 2),
|
||||||
}
|
}
|
||||||
|
if excluded:
|
||||||
|
result["excluded_accounts_no_rate"] = excluded
|
||||||
|
return result
|
||||||
|
|
||||||
|
|
||||||
def get_recent_transactions(
|
def get_recent_transactions(
|
||||||
@@ -218,6 +237,8 @@ def get_budget_projection(
|
|||||||
"""Budget projection. If month is omitted, returns the yearly rollup; if
|
"""Budget projection. If month is omitted, returns the yearly rollup; if
|
||||||
given, returns the monthly detail with income items, expense items and
|
given, returns the monthly detail with income items, expense items and
|
||||||
actuals by source."""
|
actuals by source."""
|
||||||
|
if not MIN_YEAR <= year <= MAX_YEAR:
|
||||||
|
return {"error": f"year must be between {MIN_YEAR} and {MAX_YEAR}"}
|
||||||
session = _s()
|
session = _s()
|
||||||
if month is None:
|
if month is None:
|
||||||
months_data = compute_yearly_projection_with_cumulative(session, year)
|
months_data = compute_yearly_projection_with_cumulative(session, year)
|
||||||
@@ -322,6 +343,7 @@ def get_salary_summary() -> dict:
|
|||||||
|
|
||||||
def get_municipal_receipts(
|
def get_municipal_receipts(
|
||||||
limit: Annotated[int, Field(ge=1, le=50)] = 12,
|
limit: Annotated[int, Field(ge=1, le=50)] = 12,
|
||||||
|
offset: Annotated[int, Field(ge=0, description="Skip the N most recent")] = 0,
|
||||||
account: Annotated[
|
account: Annotated[
|
||||||
Optional[str], Field(description="Municipal account/contract id")
|
Optional[str], Field(description="Municipal account/contract id")
|
||||||
] = None,
|
] = None,
|
||||||
@@ -331,7 +353,7 @@ def get_municipal_receipts(
|
|||||||
q = select(MunicipalReceipt).order_by(col(MunicipalReceipt.receipt_date).desc())
|
q = select(MunicipalReceipt).order_by(col(MunicipalReceipt.receipt_date).desc())
|
||||||
if account:
|
if account:
|
||||||
q = q.where(MunicipalReceipt.account == account)
|
q = q.where(MunicipalReceipt.account == account)
|
||||||
q = q.limit(limit)
|
q = q.offset(offset).limit(limit)
|
||||||
rows = _s().exec(q).all()
|
rows = _s().exec(q).all()
|
||||||
# One grouped query for all receipts instead of one per receipt
|
# One grouped query for all receipts instead of one per receipt
|
||||||
consumption: dict[int, float] = {}
|
consumption: dict[int, float] = {}
|
||||||
@@ -387,13 +409,10 @@ def get_analytics_by_category(
|
|||||||
q = q.where(Transaction.date >= start, Transaction.date < end)
|
q = q.where(Transaction.date >= start, Transaction.date < end)
|
||||||
rows = session.exec(q).all()
|
rows = session.exec(q).all()
|
||||||
grand = sum(float(r[1]) for r in rows) or 1.0
|
grand = sum(float(r[1]) for r in rows) or 1.0
|
||||||
|
names = {c.id: c.name for c in session.exec(select(Category)).all()}
|
||||||
out = []
|
out = []
|
||||||
for cat_id, total, count in rows:
|
for cat_id, total, count in rows:
|
||||||
name = "Uncategorized"
|
name = names.get(cat_id, "Uncategorized") if cat_id else "Uncategorized"
|
||||||
if cat_id:
|
|
||||||
cat = session.get(Category, cat_id)
|
|
||||||
if cat:
|
|
||||||
name = cat.name
|
|
||||||
out.append(
|
out.append(
|
||||||
{
|
{
|
||||||
"category_id": cat_id,
|
"category_id": cat_id,
|
||||||
@@ -460,6 +479,7 @@ def get_exchange_rate() -> dict:
|
|||||||
"buy_rate": float(rate.buy_rate),
|
"buy_rate": float(rate.buy_rate),
|
||||||
"sell_rate": float(rate.sell_rate),
|
"sell_rate": float(rate.sell_rate),
|
||||||
"date": rate.date.isoformat(),
|
"date": rate.date.isoformat(),
|
||||||
|
"fetched_at": rate.fetched_at.isoformat() if rate.fetched_at else None,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -61,8 +61,8 @@ def _fetch_bccr_rate(indicator: int, date_str: str) -> float | None:
|
|||||||
for datos in root.iter():
|
for datos in root.iter():
|
||||||
if datos.tag.endswith("NUM_VALOR"):
|
if datos.tag.endswith("NUM_VALOR"):
|
||||||
return float(datos.text.strip().replace(",", "."))
|
return float(datos.text.strip().replace(",", "."))
|
||||||
except Exception:
|
except Exception as e:
|
||||||
pass
|
logger.warning("BCCR rate fetch failed (indicator %s): %s", indicator, e)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
@@ -91,8 +91,8 @@ def _fetch_exchangerate_api() -> tuple[float, float] | None:
|
|||||||
crc = data["rates"].get("CRC")
|
crc = data["rates"].get("CRC")
|
||||||
if crc:
|
if crc:
|
||||||
return _mid_to_buy_sell(float(crc))
|
return _mid_to_buy_sell(float(crc))
|
||||||
except Exception:
|
except Exception as e:
|
||||||
pass
|
logger.warning("ExchangeRate-API fetch failed: %s", e)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
@@ -106,7 +106,8 @@ def _fetch_currency_api() -> tuple[float, float] | None:
|
|||||||
crc = data.get("usd", {}).get("crc")
|
crc = data.get("usd", {}).get("crc")
|
||||||
if crc:
|
if crc:
|
||||||
return _mid_to_buy_sell(float(crc))
|
return _mid_to_buy_sell(float(crc))
|
||||||
except Exception:
|
except Exception as e:
|
||||||
|
logger.warning("currency-api fetch failed (%s): %s", url, e)
|
||||||
continue
|
continue
|
||||||
return None
|
return None
|
||||||
|
|
||||||
@@ -120,8 +121,8 @@ def _fetch_floatrates() -> tuple[float, float] | None:
|
|||||||
crc_data = data.get("crc")
|
crc_data = data.get("crc")
|
||||||
if crc_data and "rate" in crc_data:
|
if crc_data and "rate" in crc_data:
|
||||||
return _mid_to_buy_sell(float(crc_data["rate"]))
|
return _mid_to_buy_sell(float(crc_data["rate"]))
|
||||||
except Exception:
|
except Exception as e:
|
||||||
pass
|
logger.warning("FloatRates fetch failed: %s", e)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
@@ -199,8 +200,8 @@ def _fetch_fiat_crc_mid(code: str) -> float | None:
|
|||||||
x = data["rates"].get(code)
|
x = data["rates"].get(code)
|
||||||
if crc and x:
|
if crc and x:
|
||||||
return float(crc) / float(x)
|
return float(crc) / float(x)
|
||||||
except Exception:
|
except Exception as e:
|
||||||
pass
|
logger.warning("%s/CRC fiat rate fetch failed: %s", code, e)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
@@ -220,8 +221,8 @@ def _fetch_crypto_crc(code: str) -> float | None:
|
|||||||
price = data.get(coin_id, {}).get("crc")
|
price = data.get(coin_id, {}).get("crc")
|
||||||
if price:
|
if price:
|
||||||
return float(price)
|
return float(price)
|
||||||
except Exception:
|
except Exception as e:
|
||||||
pass
|
logger.warning("CoinGecko %s/CRC fetch failed: %s", code, e)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -68,7 +68,7 @@ def make_receipt(period: str) -> MunicipalReceipt:
|
|||||||
|
|
||||||
|
|
||||||
def test_unbound_session_raises(session):
|
def test_unbound_session_raises(session):
|
||||||
with pytest.raises(LookupError):
|
with pytest.raises(RuntimeError, match="not bound to agent context"):
|
||||||
tools.get_accounts()
|
tools.get_accounts()
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user