From b8b8811a2a50371795a02e4fa65f097c9d9508ac Mon Sep 17 00:00:00 2001 From: gitea-impl-agent Date: Wed, 7 Oct 2026 03:30:23 +0000 Subject: [PATCH] Fix loan valuation definition bypasses --- docs/data-model.md | 4 +- docs/interface-contract.md | 4 +- .../req-003_account-valuation-history.md | 2 +- .../application/account_valuation_service.py | 14 ++- .../test_account_valuation_edge_contracts.py | 105 ++++++++++++++++++ tests/test_issue_12_review59_account.py | 24 ++++ tests/test_valuation_contract_docs.py | 10 ++ 7 files changed, 155 insertions(+), 8 deletions(-) diff --git a/docs/data-model.md b/docs/data-model.md index e293fe8..ca15952 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -118,14 +118,14 @@ ### transactions, import-batches, reconciliation-sessions, review-decisions, schema-meta — 기존 entities -기존 필드와 관계를 보존한다. import는 거래를 기록할 수 있지만 balance valuation candidate는 dated `unverified` row만 추가한다. reconciliation은 calculated balance provenance와 `VALUATION_MANAGED` 상태를 보존하며 investment valuation 때문에 transaction을 생성하지 않는다. `schema_meta.schema_version`은 12다. +기존 필드와 관계를 보존한다. import는 거래를 기록할 수 있지만 balance valuation candidate는 dated `unverified` row만 추가한다. loan cache writes without an explicit liability definition do not append an `account_valuations` candidate; current cache와 transaction import는 그대로 완료하며 정의를 추정하지 않는다. reconciliation은 calculated balance provenance와 `VALUATION_MANAGED` 상태를 보존하며 investment valuation 때문에 transaction을 생성하지 않는다. `schema_meta.schema_version`은 12다. ### 현재 계좌 잔액 캐시와 기준일 - `last_balance` is the current cache value: it is the latest current balance observed by an import or account write, not a historical valuation and not opening evidence. - `last_balance_basis_date` is the basis field for the evidence represented by that cache. It is a date when known; legacy or date-less writes store unknown/NULL and must not be inferred from `last_synced_at` or a valuation row. - `last_synced_at` is the execution/sync timestamp: when the import or account write advanced the cache, not the evidence basis date. -- imports and account writes advance the account cache; they may also append a dated `account_valuations` candidate, but updating the cache does not rewrite historical rows. +- imports and account writes advance the account cache; non-loan writes may also append a dated `account_valuations` candidate. loan cache writes without an explicit liability definition do not append historical evidence, and updating the cache does not rewrite historical rows. - `account_valuations` is the historical authority. A dated valuation remains append-only evidence even when it is older than the current cache; older evidence never rolls back the current cache. ## 관계와 불변조건 diff --git a/docs/interface-contract.md b/docs/interface-contract.md index 86c8d29..210a49a 100644 --- a/docs/interface-contract.md +++ b/docs/interface-contract.md @@ -337,11 +337,11 @@ AppTest는 callback, query mapping, synthetic renderer output과 error/loading e The public contract explicitly supports these exact values: source_type: `manual`, `import`, `account_write`, `reconciliation`, `snapshot`; verification_status: `verified`, `unverified`, `rejected`; valuation_kind: `balance`, `cash_balance`, `total_value`, `loan`, `loan_balance`; representative_liability_definition: `principal_only`, `principal_interest_fees`. These are the currently supported enums, not a delegation to source code. -loan accounts require representative_liability_definition for `loan`, `loan_balance`, and `balance` aliases. legacy valuation-kind aliases remain accepted and map to the canonical purpose; alias acceptance does not broaden the canonical enum set. Invalid values, missing loan definitions, and incompatible definitions return `validation_error` and perform no write. A validation failure returns `validation_error` and performs no write. +loan accounts require representative_liability_definition for `loan`, `loan_balance`, and `balance` aliases. Those are the only accepted valuation kinds for loan accounts; `cash_balance` and `total_value` do not represent a loan liability and are rejected. legacy valuation-kind aliases remain accepted and map to the canonical purpose; alias acceptance does not broaden the canonical enum set. Invalid values, missing loan definitions, and incompatible definitions return `validation_error` and perform no write. A validation failure returns `validation_error` and performs no write. loan valuations map to liabilities and non-loan valuations map to assets. The output mapping preserves the existing envelopes and signatures: `success` remains the outcome field, failures include `error_code` and `error`, and validation or conflict failures do not partially write. -Account current-cache writes keep `last_balance_basis_date` (optional `YYYY-MM-DD`) separate from `last_synced_at` (execution/sync timestamp). `create_ledger_account` keeps the established positional `db_path` argument and adds `last_balance_basis_date` after it as an optional keyword. `update_ledger_account` accepts the same field through its existing keyword-field boundary. A balance with no explicit basis advances only the current display cache, leaves the basis unknown, and creates no historical valuation candidate; an explicit balance+basis pair may advance the cache and append an unverified candidate. Older explicit evidence is retained as history and does not roll back a newer dated cache. +Account current-cache writes keep `last_balance_basis_date` (optional `YYYY-MM-DD`) separate from `last_synced_at` (execution/sync timestamp). `create_ledger_account` keeps the established positional `db_path` argument and adds `last_balance_basis_date` after it as an optional keyword. `update_ledger_account` accepts the same field through its existing keyword-field boundary. A balance with no explicit basis advances only the current display cache, leaves the basis unknown, and creates no historical candidate. A dated older non-loan balance appends historical evidence but cannot roll back a newer current cache. loan cache writes without an explicit liability definition do not append a historical candidate; they preserve current-cache and import behavior without inferring a definition. Import and account writers use deterministic candidate keys. ### shared contract and adapter limits diff --git a/docs/requirements/req-003_account-valuation-history.md b/docs/requirements/req-003_account-valuation-history.md index 815a35c..e4fc057 100644 --- a/docs/requirements/req-003_account-valuation-history.md +++ b/docs/requirements/req-003_account-valuation-history.md @@ -100,7 +100,7 @@ amount와 principal/interest/fee/cost_basis/net_contributed/profit은 exact inte ### REQ-003.05 — current와 historical authority -current latest display와 historical authority를 분리한다. `last_balance`는 opening evidence가 아니라 external current comparison이며, balance_after/opening evidence가 모두 없으면 `INCOMPLETE_CALCULATION`이다. 늦게 도착한 historical row는 current를 과거로 되돌리지 않으며 cannot roll back newer current cache. account/import writes는 real dated evidence가 있을 때만 dated `unverified` candidate를 만든다. +current latest display와 historical authority를 분리한다. `last_balance`는 opening evidence가 아니라 external current comparison이며, balance_after/opening evidence가 모두 없으면 `INCOMPLETE_CALCULATION`이다. 늦게 도착한 historical row는 current를 과거로 되돌리지 않으며 cannot roll back newer current cache. account/import writes는 real dated evidence가 있을 때만 dated `unverified` candidate를 만든다. 대출 cache write는 대표 부채 정의가 없으면 이력 후보를 추가하지 않는다. 이때 current cache와 import 처리는 유지하고 정의를 추정하지 않는다. **영향 범위** - 인터페이스: account update/import candidate, list/select/report diff --git a/jarca_ledger_mcp/application/account_valuation_service.py b/jarca_ledger_mcp/application/account_valuation_service.py index acc3bcd..0dfd7b7 100644 --- a/jarca_ledger_mcp/application/account_valuation_service.py +++ b/jarca_ledger_mcp/application/account_valuation_service.py @@ -92,17 +92,23 @@ def _validate_common(account_id: str, basis_date: str, amount: int, currency: st def record_account_valuation_candidate(conn: sqlite3.Connection, account_id: str, basis_date: str, amount: int, currency: str, source_type: str, source_ref: str, idempotency_key: str, memo: str = "") -> dict[str, Any]: """Append an unverified candidate on a caller-owned transaction.""" - account = conn.execute("SELECT currency FROM accounts WHERE id=?", (account_id,)).fetchone() + account = conn.execute("SELECT type,currency FROM accounts WHERE id=?", (account_id,)).fetchone() if account is None: return fail("not_found", "account not found") if (currency or "KRW").upper() != (account["currency"] or "KRW").upper(): return fail("validation_error", "valuation currency must match account currency") + if str(account["type"]).lower() == "loan": + return fail("validation_error", "loan valuation requires representative_liability_definition") + try: + amount_int = _strict_int(amount, "amount", allow_none=False) + except (TypeError, ValueError) as exc: + return fail("validation_error", str(exc)) basis = normalize_date(basis_date) existing = conn.execute("SELECT * FROM account_valuations WHERE account_id=? AND idempotency_key=?", (account_id, idempotency_key)).fetchone() if existing: incoming = { "basis_date": basis, - "amount": int(amount), + "amount": amount_int, "currency": (currency or "KRW").upper(), "valuation_kind": "balance", "source_type": source_type, @@ -120,7 +126,7 @@ def record_account_valuation_candidate(conn: sqlite3.Connection, account_id: str valuation_id = new_id("valuation") conn.execute("""INSERT INTO account_valuations (id,account_id,basis_date,amount,currency,valuation_kind,source_type,source_ref,verification_status,recorded_at,memo,idempotency_key) - VALUES(?,?,?,?,?,?,?,?,?,?,?,?)""", (valuation_id, account_id, basis, int(amount), (currency or "KRW").upper(), "balance", source_type, source_ref or "", "unverified", now_iso(), memo, idempotency_key)) + VALUES(?,?,?,?,?,?,?,?,?,?,?,?)""", (valuation_id, account_id, basis, amount_int, (currency or "KRW").upper(), "balance", source_type, source_ref or "", "unverified", now_iso(), memo, idempotency_key)) return ok(valuation=_valuation(conn.execute("SELECT * FROM account_valuations WHERE id=?", (valuation_id,)).fetchone()), reused=False) @@ -189,6 +195,8 @@ def register_account_valuation( 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 is_loan_account and effective_kind not in {"balance", "loan", "loan_balance"}: + return fail("validation_error", "loan account valuation kind must represent a liability") 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: diff --git a/tests/test_account_valuation_edge_contracts.py b/tests/test_account_valuation_edge_contracts.py index 4b8e7e3..df8d0b1 100644 --- a/tests/test_account_valuation_edge_contracts.py +++ b/tests/test_account_valuation_edge_contracts.py @@ -1,4 +1,8 @@ +import sqlite3 + from jarca_ledger_mcp import service +from jarca_ledger_mcp.application.account_valuation_service import record_account_valuation_candidate +from jarca_ledger_mcp.application.imports_service import _record_import_valuation_candidate def test_loan_balance_requires_representative_liability_definition(tmp_path): @@ -91,3 +95,104 @@ def test_correction_replays_by_evidence_and_conflicts_on_changed_payload(tmp_pat assert changed["success"] is False assert changed["error_code"] == "correction_conflict" assert service.list_account_valuations(account, db_path=path)["count"] == 2 + + +def test_direct_loan_candidate_requires_liability_definition_and_writes_nothing(tmp_path): + path = str(tmp_path / "ledger.sqlite3") + loan = service.create_ledger_account("대출", type="loan", db_path=path)["account"]["id"] + + with sqlite3.connect(path) as conn: + conn.row_factory = sqlite3.Row + rejected = record_account_valuation_candidate( + conn, loan, "2026-06-30", 100, "KRW", "import", "source", "candidate", "memo", + ) + + assert rejected["success"] is False + assert rejected["error_code"] == "validation_error" + assert service.list_account_valuations(loan, db_path=path)["count"] == 0 + + +def test_import_loan_candidate_requires_liability_definition_and_writes_nothing(tmp_path): + path = str(tmp_path / "ledger.sqlite3") + loan = service.create_ledger_account("대출", type="loan", db_path=path)["account"]["id"] + + with sqlite3.connect(path) as conn: + conn.row_factory = sqlite3.Row + assert _record_import_valuation_candidate(conn, loan, 100, "2026-06-30", "source-hash") is None + + assert service.list_account_valuations(loan, db_path=path)["count"] == 0 + + +def test_dated_loan_create_and_historical_update_preserve_current_cache_without_valuations(tmp_path): + path = str(tmp_path / "ledger.sqlite3") + created = service.create_ledger_account( + "대출", type="loan", last_balance=900, last_synced_at="2026-06-30", + last_balance_basis_date="2026-06-30", db_path=path, + ) + loan = created["account"]["id"] + assert created["account"]["last_balance"] == 900 + assert created["account"]["last_synced_at"] == "2026-06-30" + assert service.list_account_valuations(loan, db_path=path)["count"] == 0 + + updated = service.update_ledger_account( + loan, last_balance=700, last_synced_at="2026-06-01", + last_balance_basis_date="2026-06-01", db_path=path, + ) + + assert updated["account"]["last_balance"] == 900 + assert updated["account"]["last_synced_at"] == "2026-06-30" + assert service.list_account_valuations(loan, db_path=path)["count"] == 0 + + +def test_loan_cash_balance_and_total_value_are_rejected_without_writes(tmp_path): + path = str(tmp_path / "ledger.sqlite3") + loan = service.create_ledger_account("대출", type="loan", db_path=path)["account"]["id"] + + for valuation_kind in ("cash_balance", "total_value"): + rejected = service.register_account_valuation( + loan, "2026-06-30", 100, valuation_kind=valuation_kind, db_path=path, + ) + assert rejected["success"] is False + assert rejected["error_code"] == "validation_error" + + assert service.list_account_valuations(loan, db_path=path)["count"] == 0 + + +def test_non_loan_candidate_still_succeeds_and_reuses_idempotently(tmp_path): + path = str(tmp_path / "ledger.sqlite3") + account = service.create_ledger_account("계좌", db_path=path)["account"]["id"] + + with sqlite3.connect(path) as conn: + conn.row_factory = sqlite3.Row + first = record_account_valuation_candidate( + conn, account, "2026-06-30", 100, "KRW", "import", "source", "candidate", "memo", + ) + replay = record_account_valuation_candidate( + conn, account, "2026-06-30", 100, "KRW", "import", "source", "candidate", "memo", + ) + + assert first["success"] is True + assert first["reused"] is False + assert replay["success"] is True + assert replay["reused"] is True + assert replay["valuation"]["id"] == first["valuation"]["id"] + assert service.list_account_valuations(account, db_path=path)["count"] == 1 + + +def test_candidate_invalid_amount_returns_controlled_error_with_existing_row(tmp_path): + path = str(tmp_path / "ledger.sqlite3") + account = service.create_ledger_account("계좌", db_path=path)["account"]["id"] + + with sqlite3.connect(path) as conn: + conn.row_factory = sqlite3.Row + first = record_account_valuation_candidate( + conn, account, "2026-06-30", 100, "KRW", "import", "source", "candidate", "memo", + ) + rejected = record_account_valuation_candidate( + conn, account, "2026-06-30", "not-an-integer", "KRW", "import", "source", "candidate", "memo", + ) + + assert first["success"] is True + assert rejected["success"] is False + assert rejected["error_code"] == "validation_error" + assert service.list_account_valuations(account, db_path=path)["count"] == 1 diff --git a/tests/test_issue_12_review59_account.py b/tests/test_issue_12_review59_account.py index a1610d3..f5cee3e 100644 --- a/tests/test_issue_12_review59_account.py +++ b/tests/test_issue_12_review59_account.py @@ -104,6 +104,30 @@ def test_import_historical_kb_does_not_replace_newer_current_balance(monkeypatch assert service.list_account_valuations(account, db_path=path)["count"] == 2 +def test_loan_import_preserves_transactions_and_current_cache_without_valuation_candidate(monkeypatch, tmp_path): + path = str(tmp_path / "ledger.sqlite3") + account = service.create_ledger_account("대출", type="loan", db_path=path)["account"]["id"] + monkeypatch.setattr( + imports_service, + "parse_kb_bank_pdf", + lambda *_: _parsed( + _fake_tx("2026-06-30", 700, kind="kb"), + "loan-kb", + {"final_balance": 700, "account_number_masked": ""}, + ), + ) + + result = imports_service.import_kb_bank_pdf("ignored", "pw", account_id=account, db_path=path) + current = service.list_ledger_accounts(active_only=False, db_path=path)["accounts"][0] + + assert result["success"] is True + assert result["inserted_count"] == 1 + assert current["last_balance"] == 700 + assert current["last_balance_basis_date"] == "2026-06-30" + assert service.list_ledger_transactions(account_id=account, db_path=path)["count"] == 1 + assert service.list_account_valuations(account, db_path=path)["count"] == 0 + + def test_import_historical_toss_bank_does_not_replace_newer_current_balance(monkeypatch, tmp_path): path = str(tmp_path / "ledger.sqlite3") account = service.create_ledger_account("Toss", institution="토스뱅크", last_balance=900, last_synced_at="2026-06-30", db_path=path, last_balance_basis_date="2026-06-30")["account"]["id"] diff --git a/tests/test_valuation_contract_docs.py b/tests/test_valuation_contract_docs.py index 8b656ef..c473ee1 100644 --- a/tests/test_valuation_contract_docs.py +++ b/tests/test_valuation_contract_docs.py @@ -41,3 +41,13 @@ def test_interface_contract_documents_public_valuation_enums_validation_and_mapp ) for marker in required_contract: assert marker in interface, marker + + +def test_loan_cache_writers_document_no_inferred_valuation_history() -> None: + data_model = _text("docs/data-model.md") + interface = _text("docs/interface-contract.md") + requirement = _text("docs/requirements/req-003_account-valuation-history.md") + + assert "loan cache writes without an explicit liability definition do not append" in data_model + assert "loan cache writes without an explicit liability definition do not append" in interface + assert "대출 cache write는 대표 부채 정의가 없으면 이력 후보를 추가하지 않는다" in requirement