Compare commits
3
Commits
3bf9b77c6c
...
432e6f851a
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
432e6f851a | ||
|
|
63fba3670c | ||
|
|
31d1c11dcf |
@@ -0,0 +1,59 @@
|
|||||||
|
{
|
||||||
|
"schema": "contract-preservation-inventory/v1",
|
||||||
|
"issue": 12,
|
||||||
|
"pull_request": 13,
|
||||||
|
"review_id": 66,
|
||||||
|
"reviewed_head": "3bf9b77c6cc167a4e2a8706271bdf8e848d2c4fe",
|
||||||
|
"source_revision": "31d1c11dcf397ddf6a23e98d6d9062f96bab0194",
|
||||||
|
"contracts": [
|
||||||
|
{
|
||||||
|
"contract": "last_balance is an exact integer or NULL-compatible unknown value; boolean and floating-point coercion are invalid while zero and signed integer text remain valid",
|
||||||
|
"status": "preserved",
|
||||||
|
"preserved_by": [
|
||||||
|
"test_update_rejects_bool_or_float_dated_balance_without_mutation",
|
||||||
|
"test_update_rejects_empty_undated_balance_without_mutation",
|
||||||
|
"test_update_preserves_zero_and_signed_integer_balance_semantics"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"contract": "invalid account updates return validation_error before changing account cache or valuation history",
|
||||||
|
"status": "preserved",
|
||||||
|
"preserved_by": [
|
||||||
|
"test_update_rejects_bool_or_float_dated_balance_without_mutation",
|
||||||
|
"test_update_rejects_empty_undated_balance_without_mutation"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"contract": "a historical dated candidate is append-only and cannot roll back a newer current cache",
|
||||||
|
"status": "preserved",
|
||||||
|
"preserved_by": [
|
||||||
|
"test_historical_account_update_preserves_current_and_adds_one_candidate",
|
||||||
|
"test_late_import_current_balance_is_authoritative_by_evidence_basis_date"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"contract": "unknown basis remains distinct from last_synced_at and an undated valid balance write creates no dated candidate",
|
||||||
|
"status": "preserved",
|
||||||
|
"preserved_by": [
|
||||||
|
"test_account_write_sync_timestamp_does_not_infer_balance_basis_or_candidate",
|
||||||
|
"test_unknown_legacy_balance_basis_is_not_reported_as_valuation_date"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"contract": "Review 65 create validation, actual zero, valid import recovery, public consumer, valuation selection, snapshot replay, read-only, migration, privacy, authentication, and menu semantics remain intact",
|
||||||
|
"status": "preserved",
|
||||||
|
"preserved_by": [
|
||||||
|
"review66-focused-regressions",
|
||||||
|
"review66-full-suite",
|
||||||
|
"review66-ruff",
|
||||||
|
"review66-compileall",
|
||||||
|
"review66-diff-check"
|
||||||
|
]
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"missing": 0,
|
||||||
|
"ambiguous": 0,
|
||||||
|
"limitations": [
|
||||||
|
"Four fixture-dependent PDF tests remain skipped because private KB Bank and Toss Securities fixtures are unavailable."
|
||||||
|
]
|
||||||
|
}
|
||||||
File diff suppressed because one or more lines are too long
@@ -0,0 +1,64 @@
|
|||||||
|
{
|
||||||
|
"environment": ["os", "kernel", "architecture", "python", "uv"],
|
||||||
|
"command_env_allowlist": ["PATH", "HOME", "TMPDIR", "LANG", "VIRTUAL_ENV", "UV_CACHE_DIR"],
|
||||||
|
"secret_env_names": ["GITEA_TOKEN"],
|
||||||
|
"forbidden_regexes": [],
|
||||||
|
"hash_paths": [
|
||||||
|
"jarca_ledger_mcp/application/accounts_write_services.py",
|
||||||
|
"jarca_ledger_mcp/application/account_valuation_service.py",
|
||||||
|
"tests/test_import_current_balance_authority.py",
|
||||||
|
"tests/test_account_basis_public_consumers.py",
|
||||||
|
"tests/test_issue_12_review63_accounts.py",
|
||||||
|
"tests/test_account_valuation_edge_contracts.py",
|
||||||
|
"tests/test_account_valuations.py",
|
||||||
|
"tests/test_issue_12_regressions.py",
|
||||||
|
"docs/data-model.md",
|
||||||
|
"docs/interface-contract.md",
|
||||||
|
"docs/requirements/req-003_account-valuation-history.md"
|
||||||
|
],
|
||||||
|
"commands": [
|
||||||
|
{
|
||||||
|
"id": "review66-focused-regressions",
|
||||||
|
"purpose": "Run Review 66 and preserved account authority regressions",
|
||||||
|
"argv": ["uv", "run", "--frozen", "--extra", "dev", "--extra", "dashboard", "pytest", "-o", "addopts=", "-q", "tests/test_import_current_balance_authority.py", "tests/test_account_basis_public_consumers.py", "tests/test_issue_12_review63_accounts.py", "tests/test_account_valuation_edge_contracts.py", "tests/test_account_valuations.py", "tests/test_issue_12_regressions.py"],
|
||||||
|
"expected_exit_code": 0,
|
||||||
|
"max_output_bytes": 65536,
|
||||||
|
"claims": ["review66-findings-green"]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "review66-full-suite",
|
||||||
|
"purpose": "Run the canonical full repository suite at the functional source revision",
|
||||||
|
"argv": ["uv", "run", "--frozen", "--extra", "dev", "--extra", "dashboard", "pytest", "-o", "addopts=", "-q"],
|
||||||
|
"expected_exit_code": 0,
|
||||||
|
"max_output_bytes": 65536,
|
||||||
|
"claims": ["review66-full-suite"]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "review66-ruff",
|
||||||
|
"purpose": "Run repository lint checks",
|
||||||
|
"argv": ["uv", "run", "--frozen", "--extra", "dev", "--extra", "dashboard", "ruff", "check", "."],
|
||||||
|
"expected_exit_code": 0,
|
||||||
|
"max_output_bytes": 65536,
|
||||||
|
"claims": ["review66-ruff"]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "review66-compileall",
|
||||||
|
"purpose": "Compile package, dashboard, tests, and scripts",
|
||||||
|
"argv": ["uv", "run", "--frozen", "--extra", "dev", "--extra", "dashboard", "python", "-m", "compileall", "-q", "jarca_ledger_mcp", "dashboard", "tests", "scripts"],
|
||||||
|
"expected_exit_code": 0,
|
||||||
|
"max_output_bytes": 65536,
|
||||||
|
"claims": ["review66-compileall"]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "review66-diff-check",
|
||||||
|
"purpose": "Check the complete pull-request diff for whitespace errors",
|
||||||
|
"argv": ["git", "diff", "--check", "main...HEAD"],
|
||||||
|
"expected_exit_code": 0,
|
||||||
|
"max_output_bytes": 65536,
|
||||||
|
"claims": ["review66-diff-check"]
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"limitations": [
|
||||||
|
"Four PDF fixture tests are skipped because private KB Bank and Toss Securities PDF fixtures are unavailable."
|
||||||
|
]
|
||||||
|
}
|
||||||
@@ -0,0 +1,24 @@
|
|||||||
|
{
|
||||||
|
"schema": "review-finding-matrix/v1",
|
||||||
|
"issue": 12,
|
||||||
|
"pull_request": 13,
|
||||||
|
"review_id": 66,
|
||||||
|
"reviewed_head": "3bf9b77c6cc167a4e2a8706271bdf8e848d2c4fe",
|
||||||
|
"source_revision": "31d1c11dcf397ddf6a23e98d6d9062f96bab0194",
|
||||||
|
"contract_sha256": "6d86b591b022fcc722611bfec6d24e54a64216ab6f7c4098f357e25420218818",
|
||||||
|
"findings": [
|
||||||
|
{
|
||||||
|
"id": "C05-account-update-balance-authority",
|
||||||
|
"classification": "valid_fix",
|
||||||
|
"status": "resolved",
|
||||||
|
"production": "jarca_ledger_mcp/application/accounts_write_services.py",
|
||||||
|
"regressions": [
|
||||||
|
"test_update_rejects_bool_or_float_dated_balance_without_mutation",
|
||||||
|
"test_update_rejects_empty_undated_balance_without_mutation",
|
||||||
|
"test_update_preserves_zero_and_signed_integer_balance_semantics"
|
||||||
|
],
|
||||||
|
"claim": "account updates reject boolean, floating-point, empty, whitespace, and non-integer balance values before account or valuation mutation while preserving zero, signed integer text, dated candidates, historical cache authority, and unknown-basis semantics"
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"green_evidence": "evidence-manifest.json"
|
||||||
|
}
|
||||||
@@ -0,0 +1,23 @@
|
|||||||
|
{
|
||||||
|
"schema": "review-red-evidence/v1",
|
||||||
|
"issue": 12,
|
||||||
|
"pull_request": 13,
|
||||||
|
"review_id": 66,
|
||||||
|
"reviewed_head": "3bf9b77c6cc167a4e2a8706271bdf8e848d2c4fe",
|
||||||
|
"contract_sha256": "6d86b591b022fcc722611bfec6d24e54a64216ab6f7c4098f357e25420218818",
|
||||||
|
"reproductions": [
|
||||||
|
{
|
||||||
|
"id": "C05-account-update-balance-authority",
|
||||||
|
"command": "uv run --frozen --extra dev --extra dashboard pytest -o addopts= -q tests/test_import_current_balance_authority.py::test_update_rejects_bool_or_float_dated_balance_without_mutation tests/test_import_current_balance_authority.py::test_update_rejects_empty_undated_balance_without_mutation",
|
||||||
|
"exit_code": 1,
|
||||||
|
"observed": "2 failed: dated bool and float values returned success after coercion and appended candidates; an undated empty string returned success and replaced the integer cache"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "C05-exact-head-synthetic-probe",
|
||||||
|
"command": "uv run --frozen --extra dev --extra dashboard python <synthetic temporary-DB update probe>",
|
||||||
|
"exit_code": 0,
|
||||||
|
"observed": "True, False, 1.2, and -1.2 were persisted as 1, 0, 1, and -1 with dated candidates; an undated empty string was persisted and cleared the basis; all invalid requests mutated state"
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"sensitive_data": "synthetic temporary databases only; no credentials or real financial data"
|
||||||
|
}
|
||||||
@@ -48,8 +48,15 @@ def update_ledger_account(account_id: str, db_path: str | None = None, **fields:
|
|||||||
if current is None: return fail("not_found", "account not found")
|
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
|
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
|
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 or fields.get("last_balance") == ""):
|
if basis_update and not balance_update:
|
||||||
return fail("validation_error", "last_balance_basis_date requires last_balance")
|
return fail("validation_error", "last_balance_basis_date requires last_balance")
|
||||||
|
normalized_balance = None
|
||||||
|
if balance_update:
|
||||||
|
try:
|
||||||
|
from .account_valuation_service import _strict_int
|
||||||
|
normalized_balance = _strict_int(fields["last_balance"], "last_balance", allow_none=False)
|
||||||
|
except (ValueError, TypeError) as e:
|
||||||
|
return fail("validation_error", str(e))
|
||||||
normalized_basis = None
|
normalized_basis = None
|
||||||
if basis_update:
|
if basis_update:
|
||||||
try:
|
try:
|
||||||
@@ -64,7 +71,7 @@ def update_ledger_account(account_id: str, db_path: str | None = None, **fields:
|
|||||||
if k == "statement_status": v = normalize_statement_status(str(v))
|
if k == "statement_status": v = normalize_statement_status(str(v))
|
||||||
if k == "liquidity_bucket": v = normalize_liquidity_bucket(str(v))
|
if k == "liquidity_bucket": v = normalize_liquidity_bucket(str(v))
|
||||||
if k == "active": v = int(bool(v))
|
if k == "active": v = int(bool(v))
|
||||||
if k == "last_balance" and v != "": v = int(v)
|
if k == "last_balance": v = normalized_balance
|
||||||
if k == "currency": v = str(v).upper()
|
if k == "currency": v = str(v).upper()
|
||||||
if k == "last_balance_basis_date": v = normalized_basis
|
if k == "last_balance_basis_date": v = normalized_basis
|
||||||
except Exception as e: return fail("validation_error", str(e))
|
except Exception as e: return fail("validation_error", str(e))
|
||||||
@@ -83,7 +90,7 @@ def update_ledger_account(account_id: str, db_path: str | None = None, **fields:
|
|||||||
stored_date = str(current["last_balance_basis_date"] or "")[:10]
|
stored_date = str(current["last_balance_basis_date"] or "")[:10]
|
||||||
incoming_date = normalized_basis if basis_update else ""
|
incoming_date = normalized_basis if basis_update else ""
|
||||||
historical = bool(balance_update and incoming_date and stored_date and incoming_date < stored_date)
|
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_balance = normalized_balance if balance_update else current["last_balance"]
|
||||||
if historical:
|
if historical:
|
||||||
kept=[(u,p) for u,p in zip(updates,params) if not u.startswith(("last_balance=","last_balance_basis_date=","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]
|
updates=[u for u,_ in kept]; params=[p for _,p in kept]
|
||||||
|
|||||||
@@ -121,6 +121,78 @@ def test_create_rejects_whitespace_or_non_integer_dated_balance_without_mutation
|
|||||||
assert service.list_account_valuations(baseline["account"]["id"], db_path=path)["valuations"] == before_valuations
|
assert service.list_account_valuations(baseline["account"]["id"], db_path=path)["valuations"] == before_valuations
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_rejects_bool_or_float_dated_balance_without_mutation(tmp_path):
|
||||||
|
path = str(tmp_path / "ledger.sqlite3")
|
||||||
|
account = service.create_ledger_account(
|
||||||
|
"baseline",
|
||||||
|
last_balance=900,
|
||||||
|
last_balance_basis_date="2026-07-01",
|
||||||
|
db_path=path,
|
||||||
|
)["account"]["id"]
|
||||||
|
before = service.list_ledger_accounts(active_only=False, db_path=path)["accounts"][0]
|
||||||
|
before_valuations = service.list_account_valuations(account, db_path=path)["valuations"]
|
||||||
|
|
||||||
|
for invalid_balance in (True, False, 1.2, -1.2):
|
||||||
|
rejected = service.update_ledger_account(
|
||||||
|
account,
|
||||||
|
last_balance=invalid_balance,
|
||||||
|
last_balance_basis_date="2026-07-02",
|
||||||
|
db_path=path,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert rejected["success"] is False
|
||||||
|
assert rejected["error_code"] == "validation_error"
|
||||||
|
assert service.list_ledger_accounts(active_only=False, db_path=path)["accounts"][0] == before
|
||||||
|
assert service.list_account_valuations(account, db_path=path)["valuations"] == before_valuations
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_rejects_empty_undated_balance_without_mutation(tmp_path):
|
||||||
|
path = str(tmp_path / "ledger.sqlite3")
|
||||||
|
account = service.create_ledger_account(
|
||||||
|
"baseline",
|
||||||
|
last_balance=900,
|
||||||
|
last_balance_basis_date="2026-07-01",
|
||||||
|
db_path=path,
|
||||||
|
)["account"]["id"]
|
||||||
|
before = service.list_ledger_accounts(active_only=False, db_path=path)["accounts"][0]
|
||||||
|
before_valuations = service.list_account_valuations(account, db_path=path)["valuations"]
|
||||||
|
|
||||||
|
rejected = service.update_ledger_account(account, last_balance="", db_path=path)
|
||||||
|
|
||||||
|
assert rejected["success"] is False
|
||||||
|
assert rejected["error_code"] == "validation_error"
|
||||||
|
assert service.list_ledger_accounts(active_only=False, db_path=path)["accounts"][0] == before
|
||||||
|
assert service.list_account_valuations(account, db_path=path)["valuations"] == before_valuations
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_preserves_zero_and_signed_integer_balance_semantics(tmp_path):
|
||||||
|
path = str(tmp_path / "ledger.sqlite3")
|
||||||
|
account = service.create_ledger_account("baseline", db_path=path)["account"]["id"]
|
||||||
|
|
||||||
|
zero = service.update_ledger_account(
|
||||||
|
account,
|
||||||
|
last_balance=" +0 ",
|
||||||
|
last_balance_basis_date="2026-07-01",
|
||||||
|
db_path=path,
|
||||||
|
)
|
||||||
|
signed = service.update_ledger_account(
|
||||||
|
account,
|
||||||
|
last_balance="-12",
|
||||||
|
last_balance_basis_date="2026-07-02",
|
||||||
|
db_path=path,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert zero["success"] is True
|
||||||
|
assert zero["account"]["last_balance"] == 0
|
||||||
|
assert signed["success"] is True
|
||||||
|
assert signed["account"]["last_balance"] == -12
|
||||||
|
assert signed["account"]["last_balance_basis_date"] == "2026-07-02"
|
||||||
|
assert [(item["basis_date"], item["amount"]) for item in service.list_account_valuations(account, db_path=path)["valuations"]] == [
|
||||||
|
("2026-07-01", 0),
|
||||||
|
("2026-07-02", -12),
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
def test_zero_dated_balance_and_valid_import_recovery(tmp_path, monkeypatch):
|
def test_zero_dated_balance_and_valid_import_recovery(tmp_path, monkeypatch):
|
||||||
path = str(tmp_path / "ledger.sqlite3")
|
path = str(tmp_path / "ledger.sqlite3")
|
||||||
zero = service.create_ledger_account(
|
zero = service.create_ledger_account(
|
||||||
|
|||||||
Reference in New Issue
Block a user