mirror of
https://github.com/escalante29/WealthySmart.git
synced 2026-07-17 10:28:47 +02:00
Constraint and query pass: FK delete rules, grouped queries, tiebreakers
- FK rules via migration 7884505b16b3 (verified on dev with a rolled- back probe): transactions/recurring items SET NULL on category delete (this also fixes the 500 that DELETE /categories raised for in-use categories), water readings CASCADE with their receipt. Models declare ondelete to stay in sync. (ARCH-09, BE-11) - compute_actuals_by_source: 3 grouped queries replace 10 single- aggregate round-trips; behavior pinned by the existing cycle suite. (BE-21) - compute_cc_by_category prefetches category names (ARCH-15); agent pension latest-per-fund resolved in SQL (BE-04); municipal water consumption batched into one grouped query (BE-05). - Deterministic pagination: date DESC, id DESC on all transaction listings. (ARCH-12) - New agent-tool tests (session binding, JSON-safe output, batched queries): 64 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
61
backend/alembic/versions/7884505b16b3_fk_delete_rules.py
Normal file
61
backend/alembic/versions/7884505b16b3_fk_delete_rules.py
Normal file
@@ -0,0 +1,61 @@
|
||||
"""fk delete rules
|
||||
|
||||
Revision ID: 7884505b16b3
|
||||
Revises: 7f567c8bafdb
|
||||
Create Date: 2026-06-10 13:50:39.153585
|
||||
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
import sqlmodel
|
||||
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision: str = '7884505b16b3'
|
||||
down_revision: Union[str, Sequence[str], None] = '7f567c8bafdb'
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
"""Add delete rules to the three FKs (review ARCH-09/BE-11).
|
||||
|
||||
Transactions and recurring items outlive their category (SET NULL);
|
||||
water readings die with their receipt (CASCADE). Constraint names match
|
||||
both pre-Alembic databases and fresh baseline builds (Postgres default
|
||||
naming).
|
||||
"""
|
||||
op.drop_constraint("transaction_category_id_fkey", "transaction", type_="foreignkey")
|
||||
op.create_foreign_key(
|
||||
"transaction_category_id_fkey", "transaction", "category",
|
||||
["category_id"], ["id"], ondelete="SET NULL",
|
||||
)
|
||||
op.drop_constraint("recurringitem_category_id_fkey", "recurringitem", type_="foreignkey")
|
||||
op.create_foreign_key(
|
||||
"recurringitem_category_id_fkey", "recurringitem", "category",
|
||||
["category_id"], ["id"], ondelete="SET NULL",
|
||||
)
|
||||
op.drop_constraint("watermeterreading_receipt_id_fkey", "watermeterreading", type_="foreignkey")
|
||||
op.create_foreign_key(
|
||||
"watermeterreading_receipt_id_fkey", "watermeterreading", "municipalreceipt",
|
||||
["receipt_id"], ["id"], ondelete="CASCADE",
|
||||
)
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_constraint("watermeterreading_receipt_id_fkey", "watermeterreading", type_="foreignkey")
|
||||
op.create_foreign_key(
|
||||
"watermeterreading_receipt_id_fkey", "watermeterreading", "municipalreceipt",
|
||||
["receipt_id"], ["id"],
|
||||
)
|
||||
op.drop_constraint("recurringitem_category_id_fkey", "recurringitem", type_="foreignkey")
|
||||
op.create_foreign_key(
|
||||
"recurringitem_category_id_fkey", "recurringitem", "category",
|
||||
["category_id"], ["id"],
|
||||
)
|
||||
op.drop_constraint("transaction_category_id_fkey", "transaction", type_="foreignkey")
|
||||
op.create_foreign_key(
|
||||
"transaction_category_id_fkey", "transaction", "category",
|
||||
["category_id"], ["id"],
|
||||
)
|
||||
@@ -266,16 +266,27 @@ def get_pension_snapshots(
|
||||
) -> list[dict]:
|
||||
"""Pension fund snapshots. Each snapshot covers a period with balances,
|
||||
contributions, returns, fees and the ending balance (saldo_final)."""
|
||||
q = select(PensionSnapshot).order_by(col(PensionSnapshot.period_end).desc())
|
||||
if latest_only:
|
||||
# Latest snapshot per fund resolved in SQL instead of scanning all rows
|
||||
latest = (
|
||||
select(
|
||||
PensionSnapshot.fund.label("fund"),
|
||||
func.max(PensionSnapshot.period_end).label("period_end"),
|
||||
)
|
||||
.group_by(PensionSnapshot.fund)
|
||||
.subquery()
|
||||
)
|
||||
q = select(PensionSnapshot).join(
|
||||
latest,
|
||||
(PensionSnapshot.fund == latest.c.fund)
|
||||
& (PensionSnapshot.period_end == latest.c.period_end),
|
||||
)
|
||||
else:
|
||||
q = select(PensionSnapshot)
|
||||
q = q.order_by(col(PensionSnapshot.period_end).desc())
|
||||
if fund:
|
||||
q = q.where(PensionSnapshot.fund == fund)
|
||||
rows = _s().exec(q).all()
|
||||
if latest_only:
|
||||
seen: dict[str, PensionSnapshot] = {}
|
||||
for r in rows:
|
||||
if r.fund.value not in seen:
|
||||
seen[r.fund.value] = r
|
||||
rows = list(seen.values())
|
||||
return [
|
||||
{
|
||||
"fund": r.fund.value,
|
||||
@@ -322,11 +333,21 @@ def get_municipal_receipts(
|
||||
q = q.where(MunicipalReceipt.account == account)
|
||||
q = q.limit(limit)
|
||||
rows = _s().exec(q).all()
|
||||
# One grouped query for all receipts instead of one per receipt
|
||||
consumption: dict[int, float] = {}
|
||||
ids = [r.id for r in rows if r.id is not None]
|
||||
if ids:
|
||||
grouped = _s().exec(
|
||||
select(
|
||||
WaterMeterReading.receipt_id,
|
||||
func.sum(WaterMeterReading.consumption_m3),
|
||||
)
|
||||
.where(col(WaterMeterReading.receipt_id).in_(ids))
|
||||
.group_by(WaterMeterReading.receipt_id)
|
||||
).all()
|
||||
consumption = {rid: float(total) for rid, total in grouped}
|
||||
out: list[dict] = []
|
||||
for r in rows:
|
||||
readings = _s().exec(
|
||||
select(WaterMeterReading).where(WaterMeterReading.receipt_id == r.id)
|
||||
).all()
|
||||
out.append(
|
||||
{
|
||||
"id": r.id,
|
||||
@@ -338,7 +359,7 @@ def get_municipal_receipts(
|
||||
"interests": float(r.interests),
|
||||
"iva": float(r.iva),
|
||||
"total": float(r.total),
|
||||
"water_consumption_m3": float(sum(w.consumption_m3 for w in readings)),
|
||||
"water_consumption_m3": consumption.get(r.id, 0.0),
|
||||
}
|
||||
)
|
||||
return out
|
||||
|
||||
@@ -31,7 +31,7 @@ def list_salarios(
|
||||
query = (
|
||||
select(Transaction)
|
||||
.where(col(Transaction.transaction_type).in_(SALARIO_TYPES))
|
||||
.order_by(col(Transaction.date).desc())
|
||||
.order_by(col(Transaction.date).desc(), col(Transaction.id).desc())
|
||||
.offset(offset)
|
||||
.limit(limit)
|
||||
)
|
||||
|
||||
@@ -93,7 +93,7 @@ def list_transactions(
|
||||
Transaction.date >= datetime.fromisoformat(start_date),
|
||||
Transaction.date < datetime.fromisoformat(end_date),
|
||||
)
|
||||
query = query.order_by(col(Transaction.date).desc()).offset(offset).limit(limit)
|
||||
query = query.order_by(col(Transaction.date).desc(), col(Transaction.id).desc()).offset(offset).limit(limit)
|
||||
return session.exec(query).all()
|
||||
|
||||
|
||||
@@ -166,7 +166,7 @@ def recent_transactions(
|
||||
query = (
|
||||
select(Transaction)
|
||||
.where(Transaction.source == TransactionSource.CREDIT_CARD)
|
||||
.order_by(col(Transaction.date).desc())
|
||||
.order_by(col(Transaction.date).desc(), col(Transaction.id).desc())
|
||||
.limit(limit)
|
||||
)
|
||||
return session.exec(query).all()
|
||||
|
||||
@@ -157,7 +157,9 @@ class TransactionBase(SQLModel):
|
||||
source: TransactionSource = TransactionSource.CREDIT_CARD
|
||||
bank: Bank = Bank.BAC
|
||||
notes: Optional[str] = None
|
||||
category_id: Optional[int] = Field(default=None, foreign_key="category.id")
|
||||
category_id: Optional[int] = Field(
|
||||
default=None, foreign_key="category.id", ondelete="SET NULL"
|
||||
)
|
||||
deferred_to_next_cycle: bool = Field(
|
||||
default=False, sa_column_kwargs={"server_default": "false"}
|
||||
)
|
||||
@@ -273,7 +275,9 @@ class RecurringItemBase(SQLModel):
|
||||
default=None,
|
||||
sa_column=Column(JSON, nullable=True),
|
||||
)
|
||||
category_id: Optional[int] = Field(default=None, foreign_key="category.id")
|
||||
category_id: Optional[int] = Field(
|
||||
default=None, foreign_key="category.id", ondelete="SET NULL"
|
||||
)
|
||||
is_active: bool = True
|
||||
notes: Optional[str] = None
|
||||
|
||||
@@ -470,7 +474,9 @@ class WaterMeterReadingBase(SQLModel):
|
||||
alcant_sanitario: Money = Field(default=Decimal("0"), max_digits=15, decimal_places=2)
|
||||
iva: Money = Field(default=Decimal("0"), max_digits=15, decimal_places=2)
|
||||
is_historical: bool = False
|
||||
receipt_id: Optional[int] = Field(default=None, foreign_key="municipalreceipt.id")
|
||||
receipt_id: Optional[int] = Field(
|
||||
default=None, foreign_key="municipalreceipt.id", ondelete="CASCADE"
|
||||
)
|
||||
|
||||
|
||||
class WaterMeterReading(WaterMeterReadingBase, table=True):
|
||||
|
||||
@@ -124,72 +124,58 @@ def compute_actuals_by_source(
|
||||
|
||||
amount_crc = get_converted_amount_expr(session)
|
||||
|
||||
def _grouped(*where) -> dict[tuple, tuple[float, int]]:
|
||||
"""{(source, type): (total_crc, count)} for matching transactions."""
|
||||
rows = session.exec(
|
||||
select(
|
||||
Transaction.source,
|
||||
Transaction.transaction_type,
|
||||
func.coalesce(func.sum(amount_crc), 0),
|
||||
func.count(),
|
||||
)
|
||||
.where(*where)
|
||||
.group_by(Transaction.source, Transaction.transaction_type)
|
||||
).all()
|
||||
return {(s, t): (float(total), cnt) for s, t, total, cnt in rows}
|
||||
|
||||
# Three grouped queries replace the previous ten single-aggregate ones.
|
||||
cc_now = _grouped(
|
||||
Transaction.date >= cc_start,
|
||||
Transaction.date < cc_end,
|
||||
Transaction.source == TransactionSource.CREDIT_CARD,
|
||||
Transaction.deferred_to_next_cycle == False, # noqa: E712
|
||||
)
|
||||
cc_deferred = _grouped(
|
||||
Transaction.date >= prev_start,
|
||||
Transaction.date < prev_end,
|
||||
Transaction.source == TransactionSource.CREDIT_CARD,
|
||||
Transaction.deferred_to_next_cycle == True, # noqa: E712
|
||||
)
|
||||
non_cc = _grouped(
|
||||
Transaction.date >= cal_start,
|
||||
Transaction.date < cal_end,
|
||||
Transaction.source != TransactionSource.CREDIT_CARD,
|
||||
)
|
||||
|
||||
results = {}
|
||||
for source in TransactionSource:
|
||||
if source == TransactionSource.CREDIT_CARD:
|
||||
start, end = cc_start, cc_end
|
||||
# Normal transactions in this cycle (not deferred)
|
||||
compra_normal = session.exec(
|
||||
select(func.coalesce(func.sum(amount_crc), 0)).where(
|
||||
Transaction.date >= start,
|
||||
Transaction.date < end,
|
||||
Transaction.source == source,
|
||||
Transaction.transaction_type == TransactionType.COMPRA,
|
||||
Transaction.deferred_to_next_cycle == False, # noqa: E712
|
||||
buckets = (cc_now, cc_deferred)
|
||||
else:
|
||||
buckets = (non_cc,)
|
||||
compra = sum(
|
||||
b.get((source, TransactionType.COMPRA), (0.0, 0))[0] for b in buckets
|
||||
)
|
||||
).one()
|
||||
# Deferred from previous cycle
|
||||
compra_deferred = session.exec(
|
||||
select(func.coalesce(func.sum(amount_crc), 0)).where(
|
||||
Transaction.date >= prev_start,
|
||||
Transaction.date < prev_end,
|
||||
Transaction.source == source,
|
||||
Transaction.transaction_type == TransactionType.COMPRA,
|
||||
Transaction.deferred_to_next_cycle == True, # noqa: E712
|
||||
devolucion = sum(
|
||||
b.get((source, TransactionType.DEVOLUCION), (0.0, 0))[0]
|
||||
for b in buckets
|
||||
)
|
||||
).one()
|
||||
compra = float(compra_normal) + float(compra_deferred)
|
||||
|
||||
dev_normal = session.exec(
|
||||
select(func.coalesce(func.sum(amount_crc), 0)).where(
|
||||
Transaction.date >= start,
|
||||
Transaction.date < end,
|
||||
Transaction.source == source,
|
||||
Transaction.transaction_type == TransactionType.DEVOLUCION,
|
||||
Transaction.deferred_to_next_cycle == False, # noqa: E712
|
||||
count = sum(
|
||||
cnt
|
||||
for b in buckets
|
||||
for (s, t), (_total, cnt) in b.items()
|
||||
if s == source and t not in INCOME_TYPES
|
||||
)
|
||||
).one()
|
||||
dev_deferred = session.exec(
|
||||
select(func.coalesce(func.sum(amount_crc), 0)).where(
|
||||
Transaction.date >= prev_start,
|
||||
Transaction.date < prev_end,
|
||||
Transaction.source == source,
|
||||
Transaction.transaction_type == TransactionType.DEVOLUCION,
|
||||
Transaction.deferred_to_next_cycle == True, # noqa: E712
|
||||
)
|
||||
).one()
|
||||
devolucion = float(dev_normal) + float(dev_deferred)
|
||||
|
||||
count_normal = session.exec(
|
||||
select(func.count()).where(
|
||||
Transaction.date >= start,
|
||||
Transaction.date < end,
|
||||
Transaction.source == source,
|
||||
col(Transaction.transaction_type).notin_(INCOME_TYPES),
|
||||
Transaction.deferred_to_next_cycle == False, # noqa: E712
|
||||
)
|
||||
).one()
|
||||
count_deferred = session.exec(
|
||||
select(func.count()).where(
|
||||
Transaction.date >= prev_start,
|
||||
Transaction.date < prev_end,
|
||||
Transaction.source == source,
|
||||
col(Transaction.transaction_type).notin_(INCOME_TYPES),
|
||||
Transaction.deferred_to_next_cycle == True, # noqa: E712
|
||||
)
|
||||
).one()
|
||||
count = count_normal + count_deferred
|
||||
|
||||
results[source.value] = {
|
||||
"source": source.value,
|
||||
"total_compra": compra,
|
||||
@@ -197,42 +183,6 @@ def compute_actuals_by_source(
|
||||
"net": compra - devolucion,
|
||||
"count": count,
|
||||
}
|
||||
else:
|
||||
# Cash / Transfer: calendar month, no deferred logic
|
||||
compra = session.exec(
|
||||
select(func.coalesce(func.sum(amount_crc), 0)).where(
|
||||
Transaction.date >= cal_start,
|
||||
Transaction.date < cal_end,
|
||||
Transaction.source == source,
|
||||
Transaction.transaction_type == TransactionType.COMPRA,
|
||||
)
|
||||
).one()
|
||||
devolucion = session.exec(
|
||||
select(func.coalesce(func.sum(amount_crc), 0)).where(
|
||||
Transaction.date >= cal_start,
|
||||
Transaction.date < cal_end,
|
||||
Transaction.source == source,
|
||||
Transaction.transaction_type == TransactionType.DEVOLUCION,
|
||||
)
|
||||
).one()
|
||||
count = session.exec(
|
||||
select(func.count()).where(
|
||||
Transaction.date >= cal_start,
|
||||
Transaction.date < cal_end,
|
||||
Transaction.source == source,
|
||||
col(Transaction.transaction_type).notin_(INCOME_TYPES),
|
||||
)
|
||||
).one()
|
||||
|
||||
compra_val = float(compra)
|
||||
devolucion_val = float(devolucion)
|
||||
results[source.value] = {
|
||||
"source": source.value,
|
||||
"total_compra": compra_val,
|
||||
"total_devolucion": devolucion_val,
|
||||
"net": compra_val - devolucion_val,
|
||||
"count": count,
|
||||
}
|
||||
return results
|
||||
|
||||
|
||||
@@ -380,18 +330,17 @@ def compute_cc_by_category(
|
||||
).all()
|
||||
)
|
||||
|
||||
# Resolve category names
|
||||
# Resolve category names in one query instead of one per category
|
||||
from app.models.models import Category
|
||||
|
||||
names = {
|
||||
c.id: c.name for c in session.exec(select(Category)).all()
|
||||
}
|
||||
result = []
|
||||
for cat_id, amount in totals.items():
|
||||
if amount <= 0:
|
||||
continue
|
||||
if cat_id is not None:
|
||||
cat = session.get(Category, cat_id)
|
||||
name = cat.name if cat else "Sin categoría"
|
||||
else:
|
||||
name = "Sin categoría"
|
||||
name = names.get(cat_id, "Sin categoría") if cat_id is not None else "Sin categoría"
|
||||
result.append({"category_name": name, "amount": round(amount, 2)})
|
||||
|
||||
return sorted(result, key=lambda x: x["amount"], reverse=True)
|
||||
|
||||
151
backend/tests/test_agent_tools.py
Normal file
151
backend/tests/test_agent_tools.py
Normal file
@@ -0,0 +1,151 @@
|
||||
"""Agent tool coverage: session binding, JSON-safe output (no Decimal leaks),
|
||||
and the batched queries introduced in Phase 2."""
|
||||
|
||||
import json
|
||||
from datetime import date, datetime
|
||||
from decimal import Decimal
|
||||
|
||||
import pytest
|
||||
|
||||
from app.agent import tools
|
||||
from app.agent.tools import reset_session, set_session
|
||||
from app.models.models import (
|
||||
Account,
|
||||
AccountType,
|
||||
Bank,
|
||||
Currency,
|
||||
MunicipalReceipt,
|
||||
PensionSnapshot,
|
||||
Transaction,
|
||||
WaterMeterReading,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def bound_session(session):
|
||||
token = set_session(session)
|
||||
yield session
|
||||
reset_session(token)
|
||||
|
||||
|
||||
def make_snapshot(fund: Bank, start: date, end: date, saldo: str) -> PensionSnapshot:
|
||||
zero = Decimal("0")
|
||||
return PensionSnapshot(
|
||||
fund=fund,
|
||||
contract_number="C-1",
|
||||
period_start=start,
|
||||
period_end=end,
|
||||
saldo_anterior=zero,
|
||||
aportes=zero,
|
||||
rendimientos=zero,
|
||||
retiros=zero,
|
||||
traslados=zero,
|
||||
comision=zero,
|
||||
correccion=zero,
|
||||
bonificacion=zero,
|
||||
saldo_final=Decimal(saldo),
|
||||
source_filename="t.pdf",
|
||||
)
|
||||
|
||||
|
||||
def make_receipt(period: str) -> MunicipalReceipt:
|
||||
zero = Decimal("0")
|
||||
return MunicipalReceipt(
|
||||
receipt_date=date(2026, 4, 1),
|
||||
due_date=date(2026, 4, 15),
|
||||
period=period,
|
||||
account="A1",
|
||||
finca="F1",
|
||||
holder_name="x",
|
||||
holder_cedula="x",
|
||||
holder_address="x",
|
||||
subtotal=Decimal("100"),
|
||||
interests=zero,
|
||||
iva=Decimal("13"),
|
||||
total=Decimal("113"),
|
||||
source_filename="r.pdf",
|
||||
)
|
||||
|
||||
|
||||
def test_unbound_session_raises(session):
|
||||
with pytest.raises(LookupError):
|
||||
tools.get_accounts()
|
||||
|
||||
|
||||
def test_accounts_output_is_json_safe(bound_session):
|
||||
bound_session.add(
|
||||
Account(
|
||||
bank=Bank.BAC,
|
||||
currency=Currency.CRC,
|
||||
label="Main",
|
||||
balance=Decimal("1234.56"),
|
||||
account_type=AccountType.BANK,
|
||||
)
|
||||
)
|
||||
bound_session.commit()
|
||||
out = tools.get_accounts()
|
||||
json.dumps(out) # would raise on Decimal leakage
|
||||
assert out[0]["balance"] == 1234.56
|
||||
|
||||
|
||||
def test_pension_latest_only_per_fund(bound_session):
|
||||
bound_session.add(
|
||||
make_snapshot(Bank.FCL, date(2026, 1, 1), date(2026, 1, 31), "100")
|
||||
)
|
||||
bound_session.add(
|
||||
make_snapshot(Bank.FCL, date(2026, 2, 1), date(2026, 2, 28), "200")
|
||||
)
|
||||
bound_session.add(
|
||||
make_snapshot(Bank.ROP, date(2026, 1, 1), date(2026, 1, 31), "900")
|
||||
)
|
||||
bound_session.commit()
|
||||
|
||||
out = tools.get_pension_snapshots(latest_only=True)
|
||||
json.dumps(out)
|
||||
assert len(out) == 2
|
||||
by_fund = {o["fund"]: o for o in out}
|
||||
assert by_fund["FCL"]["saldo_final"] == 200.0 # latest period wins
|
||||
assert by_fund["ROP"]["saldo_final"] == 900.0
|
||||
|
||||
full = tools.get_pension_snapshots(latest_only=False)
|
||||
assert len(full) == 3
|
||||
|
||||
|
||||
def test_municipal_receipts_batched_consumption(bound_session):
|
||||
r1, r2 = make_receipt("2026-03"), make_receipt("2026-04")
|
||||
r2.account = "A2"
|
||||
bound_session.add(r1)
|
||||
bound_session.add(r2)
|
||||
bound_session.commit()
|
||||
bound_session.refresh(r1)
|
||||
bound_session.refresh(r2)
|
||||
for rid, m3 in ((r1.id, "10.5"), (r1.id, "4.5"), (r2.id, "7")):
|
||||
bound_session.add(
|
||||
WaterMeterReading(
|
||||
meter_id="7335",
|
||||
period=f"p{m3}",
|
||||
consumption_m3=Decimal(m3),
|
||||
receipt_id=rid,
|
||||
)
|
||||
)
|
||||
bound_session.commit()
|
||||
|
||||
out = tools.get_municipal_receipts()
|
||||
json.dumps(out)
|
||||
by_account = {o["account"]: o for o in out}
|
||||
assert by_account["A1"]["water_consumption_m3"] == 15.0
|
||||
assert by_account["A2"]["water_consumption_m3"] == 7.0
|
||||
|
||||
|
||||
def test_recent_transactions_json_safe(bound_session):
|
||||
bound_session.add(
|
||||
Transaction(
|
||||
amount=Decimal("42.42"),
|
||||
merchant="M",
|
||||
date=datetime(2026, 4, 1),
|
||||
)
|
||||
)
|
||||
bound_session.commit()
|
||||
out = tools.get_recent_transactions()
|
||||
json.dumps(out)
|
||||
assert out[0]["amount"] == 42.42
|
||||
Reference in New Issue
Block a user