Repository navigation
docs(aggregator): write the #3161 charges contract rules into the module headers - #5131
Conversation
![developing-today[bot]](https://s.lequ.pw/_assets/images/gravatars/gravatar-user-420.png){kind=avatar}
…ule headers Phase 3 closing truth-up for the charges pipeline contract (design #3161): state the three rules where the pipeline states its philosophy — admission (payment evidence, amount paid) in the produce-charge-candidates header, identity and the token-boundary classification rule in the charges-match header, the shared identity rule on charges-collect sameCharge plus the delivery rule (surfaced means delivered) in the charges-collect header. Comment-only; no behavior change. Charges suites: 68 pass.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 41 minutes. View limit details
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. CodeRabbit Verify
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86fcd5fb79
ℹ️ 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".
| // boundaries; an empty normalized merchant never matches anything, | ||
| // and conflicting known payment methods veto the merge (agreement | ||
| // or absence does not block it). Classification follows the same |
There was a problem hiding this comment.
For equal-amount, same-merchant Plaid and Gmail observations within three days, matchCharges merges the observations even when their card masks conflict: its clustering predicate never compares payment methods. It also allows empty-normalized merchants to merge when their raw descriptors agree. The stricter guarantees documented here apply to charges-collect.ts's sameCharge, not the cross-source matcher. Since this change documents the as-built contract, distinguish these implementations rather than asserting safeguards that the cross-source pipeline does not enforce.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct on both points — verified in the clustering predicate: matchCharges never compares payment methods, and empty-normalized merchants merge when raw descriptors agree exactly. Fixed in 2b82ab4: the header now describes the identity rule in its two as-built forms (cross-source dedupe: cross-source + equal amount/window/normalized merchant, raw-descriptor agreement when normalization is empty; hand-fed merge: token-boundary comparison plus the card-mask veto, which is sameCharge-only).
Codex review on PR #5131: the charges-match header overstated the cross-source dedupe — matchCharges clustering does not compare payment methods (the card-mask veto is sameCharge-only) and empty-normalized merchants merge when their raw descriptors agree exactly. Distinguish the two forms of the identity rule.
Phase 3 closing truth-up for design #3161 ("a charge is evidence — one identity and lifecycle contract for the charges pipeline"), scoped by triage task 8198 and queued as task 8692.
All nine origin issues are closed COMPLETED (#3120, #3121, #2130, #3049, #3123, #3048, #3119, #3122, #3124) and the register row in potential-projects.md already reads "landed". The one outstanding acceptance item was this truth-up: write the design's three rules into the module headers, matching as-built behavior.
scripts/produce-charge-candidates.tsheader gains the admission rule: payment evidence only (declined/failed and amounts merely due excluded viaisNotChargeSubject), and the amount is the amount paid (explicit paid/charged labels outrank a bare total; a bare total counts only as a standalone label, never inside "Subtotal").src/charges-match.tsheader gains the full contract statement: admission, the one identity rule (cross-source provenance, equal amounts, 3-day window, token-boundary merchant identity, empty-normalized never matches, conflicting known payment methods veto), the token-boundary classification corollary against the whole known catalog, and delivery (surfaced means delivered; consumption independent).src/charges-collect.tsheader states the contract as it applies there, andsameChargegains a doc comment stating the shared identity rule in its hand-fed-vs-auto form (token-sequence containment over alias-expanded labels, preserving the METAPAY / "Meta PAYG spill" merge; card-mask veto).Comment-only diff (48 insertions, 0 behavior lines). Charges test suites on the laptop: 68 pass, 0 fail (charges-match, charges-collect, charges-catalog, produce-charge-candidates).
On merge, the landing evidence is posted on #3161 and #3161 closes (with a reference to #1057, the feature of record, which stays closed).
Queue: task 8692 (repo-health-code-pilot), lane code-n.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.