Skip to content

feat: persist proposed reconciliation matches - #39

Merged
seonghobae merged 30 commits into
feat/reconciliation-run-apifrom
feat/reconciliation-match-command
Aug 30, 2026
Merged

feat: persist proposed reconciliation matches#39
seonghobae merged 30 commits into
feat/reconciliation-run-apifrom
feat/reconciliation-match-command

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Current exact-head state

  • Recorded stacked base branch: feat/reconciliation-run-api@66f75b986e8c54e5dcaf554e6c18f2d4ebdd0d8e (merged/closed PR feat: open reconciliation runs from immutable evidence #36 branch tip).
  • Exact PR head: 134ecb528cee30ba840dabf6d99d1a2bb3539779.
  • Open / Ready / mergeable against its recorded base, but not merge-ready and not eligible to bypass its stack.
  • The recorded base is stale relative to the live parent lineage: current PR feat: preserve exact bank statement balances #35 head d9572aee616706445f1c46d3a2c6795c13b251c6 is 37 commits ahead of 66f75b9...; comparing current feat: preserve exact bank statement balances #35 to this head is diverged with merge base 56381ee3b877a9f18d3a28de6ba508efb5f0e362, 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...:

  1. Journal reconciliation amount is derived from the cash chart account bound by the run's bank-account assignment rather than journal-wide totals; whole-journal debit/credit totals remain a separate balanced-journal check, while CRDT/DBIT enforces the assigned cash-line side.
  2. Source admission is snapshot-bound: statement booking/value timestamps are bounded by bank_cutoff_at, statement recorded_at and journal posted_at by knowledge_cutoff_at, and journal accounting date by book_cutoff_at.
  3. Match creation locks the reconciliation_run row before evaluating status and retains that serialization boundary through candidate/match/allocation/command persistence.
  4. Command evidence is database-bound to its exact candidate→match chain, exact one-per-side allocation totals and candidate amounts. Migration 0021 performs the forward cross-bank provenance repair for installations that previously applied migration 0019.
  5. Reconciliation candidate/allocation persistence is widened to numeric(38,6), while conservation aggregates remain unconstrained numeric so 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.
  6. Exact idempotent retries return stored immutable evidence before mutable live-state validation; reuse of the key with changed command/source evidence conflicts.

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:

  1. Concurrent allocation can escape command evidence freeze. Under PostgreSQL READ COMMITTED, an allocation INSERT can acquire its command-scoped snapshot before concurrent reconciliation_match_command evidence 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.
  2. Terminal matches can gain false command provenance. enforce_reconciliation_match_command_allocations validates identity and amounts but does not require the parent match to be proposed when command evidence is first attached. Direct SQL can therefore attach proposed-match command provenance to already approved/rejected/superseded legacy decisions. Require proposed at 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 33282389285 completed success on exact head 134ecb528cee30ba840dabf6d99d1a2bb3539779. Repository-owned exact-head SAST (99179641376), security (99179641304), and independently resolved live-base dependency diff (99179641233) also completed success.

Integrated-head attestations job 99180115265 is skipped and therefore non-passing. Formal reviews on the current history are COMMENTED; there is no qualifying independent current-head APPROVED review. 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.

@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d15c8dbe-bc3d-4c30-a0f4-0f995dd6d2b9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@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.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@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.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@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.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head evidence update: the Strix failures on AIP #39/#38 are reproducible at the hosted sidecar boundary as invalid_stream_options: stream_options.include_usage=true is not supported on /v1/chat/completions. I prepared the root-cause fix upstream in contextual-orchestrator #914, current head 3db6b77c, with provider usage forwarding, usage-only SSE capture/order, stale-state clearing, and structured passthrough fail-closed behavior. Local verification is 2529 passed; static checks pass. The upstream Strix run still provisions main b216451…, so it reproduces the old sidecar behavior until #914 is protected-merged. No bypass or AIP merge was performed.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@codex address that feedback

Work only on this PR's existing branch and current exact head 06c02cc9037802287dce9b3e126f92750811385e. Verify the current-head relational-integrity finding against source before changing anything. Test-first: add a real PostgreSQL RED with two candidate/match pairs in one reconciliation run, then attempt to persist reconciliation_match_command using candidate A together with match B whose reconciliation_candidate_id is candidate C; the insert must fail at the database boundary. Implement the narrow DB-owned repair so one composite relationship proves tenant + run + match + candidate are the same chain, preferably a candidate-inclusive unique key on reconciliation_match plus a four-column FK from reconciliation_match_command (or an equivalent relational constraint). Preserve existing tenant RLS, append-only command evidence, idempotency, exact-decimal behavior, and the evidence-only/no-approval/no-close/no-posting authority boundary. Update canonical DATA_MODEL/ERD/ADR/OPERABILITY/CHANGELOG only where the implemented constraint changes those contracts. Rerun the focused PostgreSQL RED first, then the full real-PostgreSQL suite, exact 100% owned statement/branch coverage and public docstrings, repository contracts, SAST/security, package/SBOM/provenance. Do not reuse predecessor-head evidence and do not modify other repositories.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review/fix evidence (2026-08-29):

  • Defect: reconciliation_match_command had independent candidate and match foreign keys, so valid candidate A plus valid match B (where B referenced candidate C) could be persisted as false provenance.
  • TDD: the RED regression is preserved in tests/test_reconciliation_match_command_chain_red.py; PostgreSQL 18.6 rejects the cross-pair with ForeignKeyViolation.
  • Fix at head c77cf1396daa454defd9f97c7f779718200314e6: candidate-inclusive unique key and four-column composite FK in migration 0020_reconciliation_match_command_evidence.sql, with ADR/DATA_MODEL/ERD/standards traceability and CHANGELOG updates.
  • Verification: related 21 tests OK; full suite 557 tests OK; fresh coverage 5545/5545 statements and 2022/2022 branches (100%); repository validator, compileall, changed Ruff, and diff-check OK.

Hosted protected gates still decide merge eligibility. No approval, bypass, force-push, or merge was performed.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Base freshness follow-up: pushed 8602b55ea4416028106ae4ad5633711b2d372cc5 after the hosted dependency gate proved the PR was based on stale feat/reconciliation-run-api (56eb12c). The current base is now included without force-push; final local evidence on this exact branch is 568 PostgreSQL 18.6 tests OK, 5545/5545 statements and 2022/2022 branches, validator, compileall, changed Ruff, and diff-check all passing.

Hosted Checks are being regenerated for 8602b55; prior-head results are superseded. Merge remains protected by current-head required checks and qualifying review approval.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 0 new potential issues.

Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 0 new potential issues.

Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 0 new potential issues.

Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae merged commit 734d1e7 into feat/reconciliation-run-api Aug 30, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant