fix: address account valuation review findings

jarca_ledger_mcp/application/account_valuation_service.py

@@ -185,9 +185,13 @@ def register_account_valuation(
             return fail("not_found", "account not found")
         if currency_norm != (account["currency"] or "KRW").upper():
             return fail("validation_error", "valuation currency must match account currency")
-        if (valuation_kind or "balance").strip().lower() in {"loan", "loan_balance"} and str(account["type"]).lower() != "loan":
+        effective_kind = (valuation_kind or "balance").strip().lower()
+        is_loan_account = str(account["type"]).lower() == "loan"
+        if effective_kind in {"loan", "loan_balance"} and not is_loan_account:
             return fail("validation_error", "loan valuation kind requires a loan account")
-        if definition is not None and str(account["type"]).lower() != "loan":
+        if is_loan_account and effective_kind in {"balance", "loan", "loan_balance"} and definition is None:
+            return fail("validation_error", "loan valuation requires representative_liability_definition")
+        if definition is not None and not is_loan_account:
             return fail("validation_error", "representative liability definition requires a loan account")
         seen: set[str] = set()
         if supersedes_id:
@@ -314,16 +318,46 @@ def correct_account_valuation(valuation_id: str, amount: int, correction_reason:
         preserved = {name: fields[name] if name in fields else source[name] for name in (
             "currency", "valuation_kind", "source_type", "source_ref", "verification_status", "memo", "as_of_at", "timezone",
             "cost_basis", "net_contributed", "reported_principal_kind", "reported_principal_label", "profit", "return_rate", "return_basis", "principal", "interest", "fee", "representative_liability_definition")}
+        try:
+            effective = {
+                "amount": _strict_int(amount, "amount", allow_none=False),
+                "account_id": source["account_id"],
+                "basis_date": source["basis_date"],
+                "currency": (preserved["currency"] or "KRW").strip().upper(),
+                "valuation_kind": (preserved["valuation_kind"] or "balance").strip().lower(),
+                "source_type": (preserved["source_type"] or "manual").strip().lower() or "manual",
+                "source_ref": preserved["source_ref"].strip() if isinstance(preserved["source_ref"], str) else "",
+                "verification_status": (preserved["verification_status"] or "unverified").strip().lower(),
+                "memo": preserved["memo"] or "",
+                "as_of_at": preserved["as_of_at"],
+                "timezone": preserved["timezone"],
+                "cost_basis": _strict_int(preserved["cost_basis"], "cost_basis"),
+                "net_contributed": _strict_int(preserved["net_contributed"], "net_contributed"),
+                "reported_principal_kind": preserved["reported_principal_kind"],
+                "reported_principal_label": preserved["reported_principal_label"],
+                "profit": _strict_int(preserved["profit"], "profit"),
+                "return_rate": _strict_float(preserved["return_rate"], "return_rate"),
+                "return_basis": preserved["return_basis"],
+                "principal": _strict_int(preserved["principal"], "principal"),
+                "interest": _strict_int(preserved["interest"], "interest"),
+                "fee": _strict_int(preserved["fee"], "fee"),
+                "representative_liability_definition": preserved["representative_liability_definition"].strip() if preserved["representative_liability_definition"] else None,
+                "supersedes_id": valuation_id,
+                "correction_reason": correction_reason.strip(),
+            }
+        except (TypeError, ValueError, AttributeError) as exc:
+            return fail("validation_error", str(exc))
         if idempotency_key:
             prior = conn.execute("SELECT * FROM account_valuations WHERE account_id=? AND idempotency_key=?", (source["account_id"], idempotency_key)).fetchone()
             if prior:
-                expected = {"amount": amount, "supersedes_id": valuation_id, "correction_reason": correction_reason.strip()}
-                expected.update(preserved)
-                if all(prior[k] == v for k, v in expected.items() if k in prior.keys()):
+                if all(prior[key] == value for key, value in effective.items()):
                     return ok(valuation=_valuation(prior), reused=True)
                 return fail("idempotency_conflict", "idempotency_key already has a different payload", valuation=_valuation(prior))
-        if conn.execute("SELECT 1 FROM account_valuations WHERE supersedes_id=?", (valuation_id,)).fetchone():
-            return fail("correction_conflict", "valuation already has a correction")
+        child = conn.execute("SELECT * FROM account_valuations WHERE supersedes_id=?", (valuation_id,)).fetchone()
+        if child is not None:
+            if all(child[key] == value for key, value in effective.items()):
+                return ok(valuation=_valuation(child), reused=True)
+            return fail("correction_conflict", "correction payload conflicts with existing correction", valuation=_valuation(child))
     return register_account_valuation(
         source["account_id"], source["basis_date"], amount,
         **preserved,

jarca_ledger_mcp/application/accounts_write_services.py

@@ -1,20 +1,20 @@
 from __future__ import annotations
-from .shared import Any, VALID_ACCOUNT_TYPES, db, fail, new_id, normalize_liquidity_bucket, normalize_statement_status, now_iso, ok, row_to_dict
+from .shared import Any, VALID_ACCOUNT_TYPES, db, fail, new_id, normalize_date, normalize_liquidity_bucket, normalize_statement_status, now_iso, ok, row_to_dict
 
 
-def _record_balance_candidate(conn: Any, account: dict[str, Any], balance: Any = None, synced: str | None = None) -> None:
+def _record_balance_candidate(conn: Any, account: dict[str, Any], balance: Any = None, basis_date: str | None = None) -> None:
     balance = account.get("last_balance") if balance is None else balance
-    synced = account.get("last_synced_at") if synced is None else synced
-    if balance is None or not str(synced or ""):
+    basis_date = account.get("last_balance_basis_date") if basis_date is None else basis_date
+    if balance is None or not str(basis_date or ""):
         return
     try:
         from .account_valuation_service import record_account_valuation_candidate
-        record_account_valuation_candidate(conn, account["id"], str(synced), int(balance), account.get("currency", "KRW"), "account_write", account.get("updated_at", ""), f"account-balance:{account['id']}:{str(synced)[:10]}:{balance}", "account balance write candidate")
+        record_account_valuation_candidate(conn, account["id"], str(basis_date), int(balance), account.get("currency", "KRW"), "account_write", account.get("updated_at", ""), f"account-balance:{account['id']}:{str(basis_date)}:{balance}", "account balance write candidate")
     except (ValueError, TypeError):
         return
 
 
-def create_ledger_account(name: str, type: str = "bank", institution: str = "", currency: str = "KRW", active: bool = True, memo: str = "", account_number_masked: str = "", holder_name: str = "", last_balance: int | None = None, last_synced_at: str = "", statement_status: str = "unknown", liquidity_bucket: str = "other", db_path: str | None = None) -> dict[str, Any]:
+def create_ledger_account(name: str, type: str = "bank", institution: str = "", currency: str = "KRW", active: bool = True, memo: str = "", account_number_masked: str = "", holder_name: str = "", last_balance: int | None = None, last_synced_at: str = "", statement_status: str = "unknown", liquidity_bucket: str = "other", db_path: str | None = None, last_balance_basis_date: str = "") -> dict[str, Any]:
     if not name.strip(): return fail("validation_error", "account name is required")
     if type not in VALID_ACCOUNT_TYPES: return fail("validation_error", f"type must be one of {sorted(VALID_ACCOUNT_TYPES)}")
     try:
@@ -22,8 +22,11 @@ def create_ledger_account(name: str, type: str = "bank", institution: str = "",
         liquidity_bucket_norm = normalize_liquidity_bucket(liquidity_bucket)
     except Exception as e: return fail("validation_error", str(e))
     ts = now_iso(); account_id = new_id("acct")
+    try:
+        basis_date = normalize_date(last_balance_basis_date, default_today=False) if last_balance_basis_date else ""
+    except Exception as e: return fail("validation_error", str(e))
     with db(db_path) as conn:
-        conn.execute("INSERT INTO accounts(id,name,type,institution,currency,active,memo,account_number_masked,holder_name,last_balance,last_synced_at,statement_status,liquidity_bucket,created_at,updated_at) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", (account_id,name.strip(),type,institution,currency.upper(),int(active),memo,account_number_masked,holder_name,last_balance,last_synced_at,statement_status_norm,liquidity_bucket_norm,ts,ts))
+        conn.execute("INSERT INTO accounts(id,name,type,institution,currency,active,memo,account_number_masked,holder_name,last_balance,last_balance_basis_date,last_synced_at,statement_status,liquidity_bucket,created_at,updated_at) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", (account_id,name.strip(),type,institution,currency.upper(),int(active),memo,account_number_masked,holder_name,last_balance,basis_date,last_synced_at,statement_status_norm,liquidity_bucket_norm,ts,ts))
         row = conn.execute("SELECT * FROM accounts WHERE id=?", (account_id,)).fetchone()
         account = row_to_dict(row)
         _record_balance_candidate(conn, account)
@@ -31,10 +34,20 @@ def create_ledger_account(name: str, type: str = "bank", institution: str = "",
 
 
 def update_ledger_account(account_id: str, db_path: str | None = None, **fields: Any) -> dict[str, Any]:
-    allowed = {"name","type","institution","currency","active","memo","account_number_masked","holder_name","last_balance","last_synced_at","statement_status","liquidity_bucket"}
+    allowed = {"name","type","institution","currency","active","memo","account_number_masked","holder_name","last_balance","last_balance_basis_date","last_synced_at","statement_status","liquidity_bucket"}
     with db(db_path) as conn:
         current = conn.execute("SELECT * FROM accounts WHERE id=?", (account_id,)).fetchone()
         if current is None: return fail("not_found", "account not found")
+        balance_update = "last_balance" in fields and fields.get("last_balance") is not None
+        basis_update = "last_balance_basis_date" in fields and fields.get("last_balance_basis_date") is not None
+        if basis_update and not balance_update:
+            return fail("validation_error", "last_balance_basis_date requires last_balance")
+        normalized_basis = None
+        if basis_update:
+            try:
+                normalized_basis = normalize_date(fields["last_balance_basis_date"], default_today=False) if fields["last_balance_basis_date"] else ""
+            except Exception as e:
+                return fail("validation_error", str(e))
         updates=[]; params=[]
         for k,v in fields.items():
             if k not in allowed or v is None: continue
@@ -45,21 +58,25 @@ def update_ledger_account(account_id: str, db_path: str | None = None, **fields:
                 if k == "active": v = int(bool(v))
                 if k == "last_balance" and v != "": v = int(v)
                 if k == "currency": v = str(v).upper()
+                if k == "last_balance_basis_date": v = normalized_basis
             except Exception as e: return fail("validation_error", str(e))
             updates.append(f"{k}=?"); params.append(v)
         if not updates: return fail("validation_error", "no fields to update")
-        incoming_date = str(fields.get("last_synced_at") or current["last_synced_at"] or "")[:10]
-        stored_date = str(current["last_synced_at"] or "")[:10]
-        historical = bool(incoming_date and stored_date and incoming_date < stored_date)
+        stored_date = str(current["last_balance_basis_date"] or "")[:10]
+        incoming_date = normalized_basis if basis_update else ""
+        historical = bool(balance_update and incoming_date and stored_date and incoming_date < stored_date)
         candidate_balance = fields.get("last_balance", current["last_balance"])
-        candidate_date = fields.get("last_synced_at", current["last_synced_at"])
         if historical:
-            kept=[(u,p) for u,p in zip(updates,params) if not u.startswith(("last_balance=","last_synced_at="))]
+            kept=[(u,p) for u,p in zip(updates,params) if not u.startswith(("last_balance=","last_balance_basis_date=","last_synced_at="))]
             updates=[u for u,_ in kept]; params=[p for _,p in kept]
+        elif balance_update and not basis_update:
+            # An explicit current-balance write without dated evidence advances
+            # the display cache but keeps its historical basis unknown.
+            updates.append("last_balance_basis_date=?"); params.append("")
         updates.append("updated_at=?"); params.extend([now_iso(), account_id])
         conn.execute(f"UPDATE accounts SET {', '.join(updates)} WHERE id=?", params)
         row = conn.execute("SELECT * FROM accounts WHERE id=?", (account_id,)).fetchone()
         account = row_to_dict(row)
-        if any(k in fields for k in ("last_balance", "last_synced_at")):
-            _record_balance_candidate(conn, account, candidate_balance, candidate_date)
+        if balance_update and basis_update:
+            _record_balance_candidate(conn, account, candidate_balance, incoming_date)
     return ok(account=account)

jarca_ledger_mcp/application/assets_service.py

@@ -137,7 +137,7 @@ def _account_valuation_items(conn: sqlite3.Connection, basis_date: str = "", sel
             "type": account.get("type", ""),
             "currency": (account.get("currency") or "KRW").upper(),
             "liquidity_bucket": bucket,
-            "valuation_date": account.get("last_synced_at", ""),
+            "valuation_date": account.get("last_balance_basis_date", ""),
             "last_synced_at": account.get("last_synced_at", ""),
             "source_note": account.get("memo", ""),
             "memo": account.get("memo", ""),
@@ -259,14 +259,45 @@ def _manual_asset_valuation_items(conn: sqlite3.Connection, active_only: bool =
     return included, excluded
 
 
+def _snapshot_rows_for_run(conn: sqlite3.Connection, run_id: str) -> tuple[list[sqlite3.Row], dict[str, Any]]:
+    """Load one immutable close run and only the rows persisted by that run."""
+    run = conn.execute("SELECT * FROM asset_snapshot_runs WHERE id=?", (run_id,)).fetchone()
+    if not run:
+        return [], {}
+    basis_date = run["basis_date"]
+    rows = conn.execute(
+        "SELECT rowid AS _rowid, * FROM asset_snapshots WHERE basis_date=? AND snapshot_run_id=? ORDER BY created_at ASC, name ASC",
+        (basis_date, run["id"]),
+    ).fetchall()
+    total_rows = conn.execute(
+        "SELECT COUNT(*) FROM asset_snapshots WHERE basis_date=?", (basis_date,)
+    ).fetchone()[0]
+    metadata = {
+        "snapshot_run_id": run["id"],
+        "is_correction": bool(run["correction_of_run_id"]),
+        "correction_of_run_id": run["correction_of_run_id"] or "",
+        "correction_reason": run["correction_reason"] or "",
+        "close_status": run["close_status"],
+        "selection_policy": run["selection_policy"],
+        "approved_by": run["approved_by"],
+        "approved_at": run["approved_at"],
+        "approval_reason": run["approval_reason"],
+        "approval_reference": run["approval_reference"],
+        "applied_evidence": json.loads(run["applied_evidence"] or "[]"),
+        "excluded_evidence": json.loads(run["excluded_evidence"] or "[]"),
+        "carried_evidence": json.loads(run["carried_evidence"] or "[]"),
+        "closed_at": run["created_at"] or "",
+        "total_snapshot_rows_for_basis_date": total_rows,
+    }
+    return rows, metadata
+
+
 def _snapshot_rows_for_latest_run(conn: sqlite3.Connection, basis_date: str) -> tuple[list[sqlite3.Row], dict[str, Any]]:
     rows = conn.execute("SELECT rowid AS _rowid, * FROM asset_snapshots WHERE basis_date=? ORDER BY created_at ASC, name ASC", (basis_date,)).fetchall()
     run = conn.execute("SELECT * FROM asset_snapshot_runs WHERE basis_date=? ORDER BY rowid DESC LIMIT 1", (basis_date,)).fetchone()
     if not run:
         return rows, {"snapshot_run_id": "", "is_correction": False, "correction_of_run_id": "", "correction_reason": "", "total_snapshot_rows_for_basis_date": len(rows)} if rows else {}
-    selected = [row for row in rows if row["snapshot_run_id"] == run["id"]]
-    metadata = {"snapshot_run_id": run["id"], "is_correction": bool(run["correction_of_run_id"]), "correction_of_run_id": run["correction_of_run_id"] or "", "correction_reason": run["correction_reason"] or "", "close_status": run["close_status"], "selection_policy": run["selection_policy"], "approved_by": run["approved_by"], "approved_at": run["approved_at"], "approval_reason": run["approval_reason"], "approval_reference": run["approval_reference"], "applied_evidence": json.loads(run["applied_evidence"] or "[]"), "excluded_evidence": json.loads(run["excluded_evidence"] or "[]"), "carried_evidence": json.loads(run["carried_evidence"] or "[]"), "closed_at": run["created_at"] or "", "total_snapshot_rows_for_basis_date": len(rows)}
-    return selected, metadata
+    return _snapshot_rows_for_run(conn, run["id"])
 
 
 def _snapshot_items(conn: sqlite3.Connection, basis_date: str) -> list[dict[str, Any]]:
@@ -629,8 +660,8 @@ def close_asset_snapshot(basis_date: str = "", source_note: str = "", memo: str
             existing_run = conn.execute("SELECT * FROM asset_snapshot_runs WHERE idempotency_key=?", (idempotency_key,)).fetchone()
             if existing_run:
                 if existing_run["request_fingerprint"] == fingerprint:
-                    replay_rows, replay_meta = _snapshot_rows_for_latest_run(conn, basis)
-                    replay_items = [_public_evidence(item) for item in replay_meta.get("applied_evidence", [])] or _snapshot_items(conn, basis)
+                    replay_rows, replay_meta = _snapshot_rows_for_run(conn, existing_run["id"])
+                    replay_items = [_public_evidence(item) for item in replay_meta.get("applied_evidence", [])]
                     replay_warnings = _closed_warnings(replay_meta, basis, replay_rows)
                     replay_public = _closed_run_metadata(replay_meta)
                     replay_public["warnings"] = replay_warnings

jarca_ledger_mcp/application/imports_service.py

@@ -43,19 +43,10 @@ def _record_import_valuation_candidate(conn: Any, account_id: str, balance: Any,
 def _update_current_balance_if_newer(conn: Any, account_id: str, balance: Any, basis_date: str, synced_at: str) -> None:
     if balance is None or not basis_date:
         return
-    current = conn.execute("SELECT last_synced_at FROM accounts WHERE id=?", (account_id,)).fetchone()
-    latest_evidence = conn.execute(
-        """SELECT basis_date FROM account_valuations
-           WHERE account_id=? AND verification_status IN ('verified', 'unverified')
-           ORDER BY basis_date DESC, recorded_at DESC, id DESC LIMIT 1""",
-        (account_id,),
-    ).fetchone()
-    current_date = max(
-        str(current["last_synced_at"] or "")[:10] if current else "",
-        str(latest_evidence["basis_date"] or "")[:10] if latest_evidence else "",
-    )
+    current = conn.execute("SELECT last_balance_basis_date FROM accounts WHERE id=?", (account_id,)).fetchone()
+    current_date = str(current["last_balance_basis_date"] or "")[:10] if current else ""
     if not current_date or basis_date[:10] >= current_date:
-        conn.execute("UPDATE accounts SET last_balance=?, last_synced_at=?, updated_at=? WHERE id=?", (balance, synced_at, synced_at, account_id))
+        conn.execute("UPDATE accounts SET last_balance=?, last_balance_basis_date=?, last_synced_at=?, updated_at=? WHERE id=?", (balance, basis_date[:10], synced_at, synced_at, account_id))
 
 @read_operation
 def preview_ledger_csv_import(file_path: str, source: str = "generic", db_path: str | None = None) -> dict[str, Any]: