Skip to content

Portfolio upgrade: reconciliation evidence and reliability - #1

Merged
Etherlabs-dev merged 4 commits into
mainfrom
portfolio-upgrade/multi-processor-reconciliation
Aug 9, 2026
Merged

Etherlabs-dev merged 4 commits into
mainfrom
portfolio-upgrade/multi-processor-reconciliation

Conversation

@Etherlabs-dev

@Etherlabs-dev Etherlabs-dev commented Aug 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

This PR upgrades the repository into a tested financial-systems reference implementation while preserving explicit evidence labels. It does not claim a live client deployment, business savings, production throughput, or accounting acceptance.

Engineering changes

  • Added a Python 3.11 package and FastAPI boundary for exact-money normalization, eligibility, scoring, ambiguity handling, refunds, partial refunds, bounded splits, discrepancies, fingerprints, cursors, duplicate events, and accounting idempotency.
  • Updated the n8n ingestion, normalization, and reconciliation artifacts to delegate deterministic financial logic to the service.
  • Removed the workflow behavior that converted match confidence into fictional accounting dollars; accounting jobs now require an approved balanced payload and idempotency key.
  • Hardened PostgreSQL uniqueness, currency, hash, checkpoint, replay, and balanced-journal constraints; corrected the invalid PostgreSQL array seed and removed the self-referential ledger link.
  • Added Ruff, pinned dependencies, 38 automated tests, Docker Compose, GitHub Actions, workflow/secret validation, and a synthetic benchmark artifact.

Verified evidence

  • 38/38 tests pass against PostgreSQL 16.
  • All schema objects and 12 seed CSV files load successfully.
  • All eight workflow JSON files import successfully in n8n 2.33.7.
  • Docker image build and Docker Compose test runner pass in GitHub Actions.
  • Python/PostgreSQL, Docker, and n8n CI jobs pass for both push and pull-request runs.
  • Synthetic benchmark: 6 expected matches, 6 correct, 0 false, 7 deliberately unresolved, 0.6641 ms on the recorded local environment.
  • Dependency audit reports no known vulnerabilities in auditable third-party packages.

Remaining production debt

  • Wire real processor pagination responses to durable raw-event and compare-and-swap checkpoint writes.
  • Add webhook signature verification, retries, rate-limit/dead-letter behavior, observability, exception ownership, and durable service adapters.
  • Calibrate tolerances using real processor statements and validate accounting output against the target chart of accounts.

@Etherlabs-dev
Etherlabs-dev marked this pull request as ready for review August 9, 2026 11:20

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1eaa7fc8a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread sql/load_seed.sql
@@ -0,0 +1,13 @@
\set ON_ERROR_STOP on
\copy processors FROM 'sql/seed/processors.csv' WITH (FORMAT csv, HEADER true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Reset identity sequences after loading explicit seed IDs

The seed CSVs provide explicit IDs to every identity-backed table, but COPY does not advance the associated PostgreSQL sequences. After the documented schema/seed setup, the first ordinary insert without an ID (for example a new raw_events, recon_runs, or qb_sync_jobs row) therefore generates ID 1 and collides with the seeded row. Advance each affected sequence to the table maximum after loading the CSVs.

Useful? React with 👍 / 👎.

Comment on lines +291 to +292
if len(valid_groups) == 1:
group = valid_groups[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Treat split groups as ambiguous across all target records

A group is accepted when it is unique only for the current right record, without checking whether the same left group also reconciles another right record. For example, left amounts 40 and 60 with two separate right amounts of 100 produce an arbitrary match to whichever right ID sorts first and an exception for the other, even though neither target is distinguishable. This is a false financial match; split candidates need global collision/ambiguity resolution before any group is committed.

Useful? React with 👍 / 👎.

Comment thread sql/schema.sql
stats JSONB NOT NULL DEFAULT '{}'::jsonb,
created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
CHECK (time_window_end >= time_window_start),
UNIQUE (input_fingerprint)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Make reconciliation-run creation replay-safe

The new unique fingerprint constraint conflicts with the Create Recon Run node in n8n/workflows/06_reconcile_match_engine.json, which still performs an unconditional insert. If a run is retried after its run row was created but before matches or exceptions finished writing, the retry stops on this constraint and can never resume the partial run. The workflow should upsert or retrieve the existing run by fingerprint before continuing with its idempotent child writes.

Useful? React with 👍 / 👎.

Comment on lines +247 to +248
if candidate.left_id in matched_left or candidate.right_id in matched_right:
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Try the next eligible candidate after a preferred target is taken

When two left records prefer the same right record and their proposal scores differ by more than ambiguity_delta, the higher-scoring proposal claims the target and the other proposal is simply skipped here. Even if the skipped left record has a second candidate above minimum_score, that candidate is never considered, so a valid two-pair assignment becomes one match plus two discrepancies. Resolve the one-to-one assignment globally or retry each contender's remaining ranked candidates.

Useful? React with 👍 / 👎.

Comment on lines +168 to +169
"reference": record.reference or "",
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include partial-refund inputs in the run fingerprint

The serialized fingerprint omits original_gross_amount, although _match_classification uses that field to decide between partial_refund and refund_or_reversal. Consequently, otherwise identical refund inputs with and without an original gross amount produce different reconciliation output but the same fingerprint, causing the unique recon_runs.input_fingerprint constraint to conflate semantically different runs. Include every input field that can affect matching or classification.

Useful? React with 👍 / 👎.

@Etherlabs-dev
Etherlabs-dev merged commit 2f9397f into main Aug 9, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant