feat: persist proposed reconciliation matches - #39
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@opencode-agent Please perform the required independent review of the current PR HEAD 75ca8ab. Review the exact 1:1 proposed reconciliation-match command, tenant scope, immutable evidence, idempotency, amount invariants, HTTP boundary, and migration chain. Publish the formal current-head review result. |
|
@opencode-agent Re-review the current PR HEAD e38d0d1e7cd9bd9c8cbf9639a326020d07fb9e68. The dependency hash contract now scopes the CPython 3.14 hash assertion to the psycopg-binary stanza. Publish the formal current-head review result. |
|
@opencode-agent Please review current PR HEAD 75732b8. This head includes the proposed-match command and fail-closed CPython 3.14 psycopg-binary stanza validator. |
|
Current-head evidence update: the Strix failures on AIP #39/#38 are reproducible at the hosted sidecar boundary as |
seonghobae
left a comment
There was a problem hiding this comment.
Current-head relational-integrity finding on 06c02cc9037802287dce9b3e126f92750811385e: reconciliation_match_command proves candidate scope and match scope with two independent foreign keys, but it never proves that the stored reconciliation_candidate_id is the candidate actually referenced by the stored reconciliation_match_id. In one tenant/run, candidate A plus match B (whose match points to candidate C) satisfies both foreign keys and becomes immutable command/provenance evidence for a chain that never existed.
Add a real-PostgreSQL RED with two candidate/match pairs in one run and attempt the cross-pair command insert; it must fail at the database boundary. Narrow GREEN: give reconciliation_match a candidate-inclusive unique key and make migration 0020 use one composite foreign key (tenant_account_id, reconciliation_run_id, reconciliation_match_id, reconciliation_candidate_id) to that key, or an equivalent database-owned same-chain constraint. Preserve tenant RLS, idempotency, immutability, and the evidence-only/no-approval/no-posting authority boundary. Rerun exact-head PostgreSQL/full 100% owned statement+branch coverage, repository contracts/public docstrings, SAST/security, package/SBOM/provenance; do not reuse predecessor evidence.
|
@codex address that feedback Work only on this PR's existing branch and current exact head |
|
You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard. |
|
Current-head review/fix evidence (2026-08-29):
Hosted protected gates still decide merge eligibility. No approval, bypass, force-push, or merge was performed. |
|
Base freshness follow-up: pushed Hosted Checks are being regenerated for |
Current exact-head state
feat/reconciliation-run-api@66f75b986e8c54e5dcaf554e6c18f2d4ebdd0d8e(merged/closed PR feat: open reconciliation runs from immutable evidence #36 branch tip).134ecb528cee30ba840dabf6d99d1a2bb3539779.d9572aee616706445f1c46d3a2c6795c13b251c6is 37 commits ahead of66f75b9...; comparing current feat: preserve exact bank statement balances #35 to this head isdivergedwith merge base56381ee3b877a9f18d3a28de6ba508efb5f0e362, this PR 24 commits ahead and 36 commits behind. Restack/reintegration must therefore occur non-destructively only after its parent sequence is eligible; predecessor evidence does not transfer.Buyer-visible scope
This branch persists one tenant-scoped proposed reconciliation candidate/match with exact statement/journal allocation evidence and immutable command/source provenance, and aligns reconciliation monetary persistence with the platform's
numeric(38,6)exact-money domain. The evidence remains non-authoritative: it cannot approve a reconciliation, select a chart account, close a period, reverse/post a journal, or alter accounting policy.Current exact-head accounting controls
Current source on
134ecb5...:CRDT/DBITenforces the assigned cash-line side.bank_cutoff_at, statementrecorded_atand journalposted_atbyknowledge_cutoff_at, and journal accounting date bybook_cutoff_at.reconciliation_runrow before evaluating status and retains that serialization boundary through candidate/match/allocation/command persistence.numeric(38,6), while conservation aggregates remain unconstrainednumericso over-consumption reaches the explicit invariant instead of numeric overflow. Migration 0022 performs its trigger removal/type conversion/function replacement/trigger recreation atomically in one transaction.Current informational review observations for compound-journal cash movement, aggregate precision, and transactional trigger replacement were reverified against this exact source and resolved. Thread resolution is not approval.
Remaining source-real blockers
Two current-head database defects remain unresolved and must be repaired test-first on this branch by its active writer before review threads are resolved:
reconciliation_match_commandevidence commits, block on the parent-row lock, then continue after the command commits while its existing statement snapshot still cannot see the new command row. The freeze must use state visible through the locked parent tuple or an equivalent mechanism that does not depend on a refreshed statement snapshot. Add a real two-connection PostgreSQL regression proving an allocation that starts before command commit cannot later commit into a command-backed exact 1:1 match.enforce_reconciliation_match_command_allocationsvalidates identity and amounts but does not require the parent match to beproposedwhen command evidence is first attached. Direct SQL can therefore attach proposed-match command provenance to already approved/rejected/superseded legacy decisions. Requireproposedat command-evidence insertion while preserving later legitimate status transitions, with direct-SQL regressions for all terminal states.Current exact-head evidence
Accounting Foundation CI run
33282389285completed success on exact head134ecb528cee30ba840dabf6d99d1a2bb3539779. Repository-owned exact-head SAST (99179641376), security (99179641304), and independently resolved live-base dependency diff (99179641233) also completed success.Integrated-head attestationsjob99180115265is skipped and therefore non-passing. Formal reviews on the current history are COMMENTED; there is no qualifying independent current-headAPPROVEDreview. Old reviews, predecessor coverage/package evidence, and status-only/model-only outcomes are not promoted to this head.Merge boundary
Do not merge, enable auto-merge, bypass protection, force-push, destructively rebase, version, tag, or release this stacked PR. First integrate the dependency-root/parent sequence through normal protected policy, repair the two current source-real defects, then non-destructively restack/reintegrate as necessary and revalidate one unchanged successor exact head. Every applicable real-PostgreSQL, exact 100% owned statement/branch coverage, public-docstring/repository-contract, SAST/security, package/SBOM/provenance, review, migration/rollback/operability and integrated-head gate plus qualifying independent approval must pass together before merge.