Repository navigation
fix(quickbooks): don't demote fully corroborated Spreedly matches on name gap - #238
Merged
Merged
Conversation
…name gap Wholesale buyers routinely pay company invoices on personal cards, so the Stripe cardholder and QBO invoice customer legitimately differ (Oct-7 production: Wilhemus Schutten paying the Tropics Cafe invoice). Once both Spreedly legs corroborate — PaymentRefNum plus a linked-invoice DocNumber — the identifiers alone establish which payment this is; the name gap is surfaced as informational cardholder_customer_name_differs evidence instead of demoting to suggested. Non-Spreedly hits and incomplete linkage keep the conservative customer_mismatch demotion. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Reviewer's GuideThe PR narrows a matching-policy bug by allowing only fully corroborated two-leg Spreedly identifiers to override a cardholder/customer name mismatch; all other identity conflicts remain conservatively demoted, with new UI labels, operational documentation, and regression tests. Sequence diagram for fully corroborated Spreedly matchingsequenceDiagram
participant Stripe as Stripe charge
participant Matcher as matchOlliePayment
participant Spreedly as Spreedly identity claim
participant QBO as QBO payment
Stripe->>Matcher: Match identity hit
Matcher->>Spreedly: Validate composite order_id legs
Spreedly-->>Matcher: PaymentRefNum and invoice DocNumber corroborate
Matcher->>QBO: Confirm both legs reference same payment
QBO-->>Matcher: Unique payment match
Matcher->>Matcher: Add spreedly_identity
Matcher->>Matcher: Add cardholder_customer_name_differs if names differ
Matcher-->>Stripe: verified match
Flow diagram for Spreedly identity match confidenceflowchart TD
A[Stripe charge with identity hit] --> B{Spreedly composite claim?}
B -- No --> C{Customer names differ?}
C -- Yes --> D[Suggested: customer_mismatch]
C -- No --> E[Verified]
B -- Yes --> F{Both payment and invoice legs corroborate same QBO payment?}
F -- No --> G[Suggested: spreedly_linkage_unverified]
G --> H{Customer names differ?}
H -- Yes --> I[Add customer_mismatch]
F -- Yes --> J[Verified: spreedly_identity]
J --> K{Customer names differ?}
K -- Yes --> L[Add cardholder_customer_name_differs as informational evidence]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (1 skipped: 1… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary fix: preserving verified Spreedly matches when cardholder and customer names differ. Description check The description includes the required Summary, Changes, Verification, and Risk / deployment notes sections. It explains the bug, implementation, tests, and deployment impact. It omits some template ve… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
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. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="lib/stripe-payouts-common.ts" line_range="637-638" />
<code_context>
const stripeName = identity.customerName?.trim().toLowerCase();
const qboName = hit.payment.customerName?.trim().toLowerCase();
const customerConflict = Boolean(stripeName && qboName && stripeName !== qboName);
- if (customerConflict) {
- evidence.push("customer_mismatch");
- return {
</code_context>
<issue_to_address>
**Unrelated references verify payments**
When a charge contains additional labeled refs matching a candidate’s payment and invoice refs, but its composite Spreedly order or payment values do not, `compareOllieRefs` and the namespace-only leg check accept those hits as corroboration, so the name mismatch remains informational and the wrong QBO payment stays verified and eligible for automatic reclassification.
Compare each leg against the corresponding values in `identity.spreedly` instead of accepting any shared ref in its namespace.
</issue_to_address>
### Comment 2
<location path="lib/stripe-payouts-common.ts" line_range="638" />
<code_context>
}
evidence.push("spreedly_identity");
+ if (customerConflict) {
+ evidence.push("cardholder_customer_name_differs");
+ }
+ } else if (customerConflict) {
</code_context>
<issue_to_address>
**Two payouts can claim one payment**
When two overlapping payout builds both see the same QBO candidate before either has persisted a claiming proposal, and each charge carries identifiers that match it, both builds query `claimedByOthers` before either persists its proposal, so each can match and mark the same QBO payment verified. Each then creates a distinct payout proposal and the automatic pass can execute both; the payment is reclassified only once, while both Stripe transactions are treated as reconciled to it.
Atomically reserve the QBO payment across payout proposals before allowing either proposal to become auto-eligible, or make the executor reject a payment already bound to another payout.
</issue_to_address>
### Comment 3
<location path="lib/stripe-payouts-common.ts" line_range="638" />
<code_context>
}
evidence.push("spreedly_identity");
+ if (customerConflict) {
+ evidence.push("cardholder_customer_name_differs");
+ }
+ } else if (customerConflict) {
</code_context>
<issue_to_address>
**Stale match can auto-apply**
When two builds of the same payout overlap, a linkage change or lookup failure makes the newer build incomplete, and the older fully corroborated build writes its proposal afterward, a slower build with fully corroborated Spreedly data can overwrite a newer same-payout build that found incomplete linkage, because proposal transactions write each build’s computed fields without checking build freshness. Its automatic pass can then execute the stale verified proposal; the executor rechecks the QBO payment’s account and amount, not the Spreedly linkage, so the payment is reclassified using outdated evidence.
Serialize all ledger builds for a payout or attach a monotonic build version and reject proposal writes and automatic execution from stale builds; revalidate the required Spreedly legs before execution.
</issue_to_address>
### Comment 4
<location path="lib/stripe-payouts-common.ts" line_range="637-639" />
<code_context>
const stripeName = identity.customerName?.trim().toLowerCase();
const qboName = hit.payment.customerName?.trim().toLowerCase();
const customerConflict = Boolean(stripeName && qboName && stripeName !== qboName);
- if (customerConflict) {
- evidence.push("customer_mismatch");
- return {
- confidence: "suggested",
- payment: hit.payment,
- candidateIds: [hit.payment.id],
- evidence,
- identityRefs: hit.identityRefs,
- };
- }
// A Spreedly composite claim is held to a stricter standard than a
// lone namespaced ref: BOTH legs must positively corroborate on
</code_context>
<issue_to_address>
**Memo-only references qualify for automatic matching**
When the QBO PaymentRefNum is absent or does not match, but the QBO memo contains the claimed labeled payment reference and a linked invoice carries the claimed order DocNumber, `ollieRefsFromQboPayment` also extracts `paymentNumbers` from the QBO memo, so a memo-only payment reference plus a linked invoice can satisfy the two-leg check. With a cardholder/customer name gap, this branch now keeps the match `verified`, allowing the existing auto-eligibility path to act on a payment whose `PaymentRefNum` did not corroborate the claimed reference.
Require the Spreedly payment leg to match QBO `PaymentRefNum` specifically before treating a name gap as informational; keep memo-derived matches review-only.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 4 findings to address first, and a Spreedly match with both identifier legs corroborated can now remain verified despite a cardholder/customer name mismatch, making it eligible for the executor’s automatic path. If those identifiers do not establish the intended payment, an incorrect QBO reclassification can persist after reverting; this changes the eligibility rule for every such match.
Blocking findings: lib/stripe-payouts-common.ts:638, lib/stripe-payouts-common.ts:638, lib/stripe-payouts-common.ts:638, lib/stripe-payouts-common.ts:639
Sourcery findings on #238: the leg check accepted any namespace hit, so an unrelated labeled ref sharing the order/payment space could satisfy a leg the claim never made; and paymentNumbers merged memo-derived refs with PaymentRefNum, so a memo-only "Ollie Payment #…" mention satisfied the payment leg without PaymentRefNum corroborating. Legs now require the claimed paymentRef in the PaymentRefNum field itself and the claimed orderNumber on a linked-invoice DocNumber. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Merged
8 tasks done
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The October 7 payout exposed a matching-policy bug: the Stripe charge (
txn_3UNHoNC2zsUcVaNLl3PhIrt9, $230.00) carried valid Spreedly metadata00326-1623638and both legs uniquely corroborated QBO Payment 1786 (PaymentRefNum1623638+ linked invoice DocNumber00326), yet the match demoted tosuggestedsolely because cardholder Wilhemus Schutten differs from invoice customer Tropics Cafe — a routine wholesale pattern (owner pays the company invoice on a personal card).Once both Spreedly legs independently corroborate the same payment, the transaction identifiers alone establish identity; the cardholder↔customer name difference is informational, not disqualifying.
Changes
lib/stripe-payouts-common.ts— in the single-identity-hit branch, a fully corroborated Spreedly two-leg match now staysverifiedand surfacescardholder_customer_name_differsevidence. Thecustomer_mismatchdemotion still applies to non-Spreedly identity hits and to Spreedly claims whose linkage is incomplete — names never substitute for identifiers.components/admin-quickbooks-payouts.tsx— labels forcardholder_customer_name_differs,spreedly_identity,spreedly_linkage_unverified(the latter two previously rendered as raw codes).docs/operations/quickbooks.md— documents the evidence hierarchy: names corroborate but never prove identity, and fully corroborated Spreedly identity makes the name gap informational only.Verification
npx tsc --noEmitnpm run lintnpm test— 1241 pass (+2)npm run test:rules— 49 passnpm run check:md-linksnpm run check:react-versionsRisk / deployment notes
Generated with Devin
Summary by Sourcery
Keep fully corroborated Spreedly matches verified despite cardholder-to-customer name differences while preserving strict identifier and linkage validation.
Bug Fixes:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit