fix(accounting): reversal-aware deferred-revenue true-up for partially-refunded orders (scjz.68)#347
Open
OneTwo3D wants to merge 4 commits into
Open
fix(accounting): reversal-aware deferred-revenue true-up for partially-refunded orders (scjz.68)#347OneTwo3D wants to merge 4 commits into
OneTwo3D wants to merge 4 commits into
Conversation
…y-refunded orders (scjz.68) The Group-B daily-batch deferred-revenue true-up (scjz.41) excluded PARTIALLY_REFUNDED orders: such an order already posts an UNEARNED_REV_REVERSAL (refund of unshipped lines) that remainingDeferred didn't subtract, so truing up would over-recognize. Two changes, both Xero + QBO: - remainingDeferred is now reversal-aware: subtract the posted UNEARNED_REV_REVERSAL (unearned-account debit) for the order's refunds. Harmless for orders without such reversals (terminal-status orders); correct for partially-refunded ones. - PARTIALLY_REFUNDED orders now true up on the final shipment ONLY when the reversal-aware remainder is rounding-scale (<= GBP0.05) — which means no material unshipped value remains (the remainder IS the unshipped-and-unrefunded value). A material remainder is left deferred, so this can never over-debit unearned revenue. Pure helpers (shouldTrueUpPartiallyRefundedDeferral, extractUnearnedReversalDebit) unit-tested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46393a28d3
ℹ️ 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".
| // scjz.68: a PARTIALLY_REFUNDED order isn't guaranteed fully shipped, but once | ||
| // the deferred base is reversal-aware, a remainder this small means no material | ||
| // unshipped value is left — safe to clear the rounding stranding. | ||
| (firstShipment.order.status === 'PARTIALLY_REFUNDED' && shouldTrueUpPartiallyRefundedDeferral(remainingDeferred)) |
There was a problem hiding this comment.
For a PARTIALLY_REFUNDED order that already has a posted shipment plus an unshipped refund, remainingDeferred still includes the value of the shipment being processed in this batch. For example, with three equal units deferred at £100, one unit previously recognized (£33.33), one unit refunded via a £33.33 UNEARNED_REV_REVERSAL, and the last unit shipping now, remainingDeferred is £33.34, so this condition is false and the final shipment recognizes only the £33.33 proportional slice, leaving £0.01 permanently deferred. Apply the tolerance to the residual after this shipment's proportional recognition rather than to the pre-shipment remainder; the QuickBooks mirror has the same condition.
Useful? React with 👍 / 👎.
…iew (Codex P2) P2-1: the PARTIALLY_REFUNDED true-up gate used the pre-batch remainingDeferred (which still includes the current batch's shipments), so a final batch leaving only a penny wouldn't true it up. Gate on the residual AFTER this batch's proportional recognition (remainingDeferred - runningRevenue - proportionalRevenue) in both connectors. P2-2: the Xero daily-batch PREVIEW still used the non-reversal-aware logic, so operators would see a different Group B revenue than what posts for partially-refunded orders. Apply the same reversal-aware remainingDeferred + residual gate to the preview. Also extracts the posted-UNEARNED_REV_REVERSAL prefetch into a shared loadPostedUnearnedReversalByOrder helper used by both connectors and the preview. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…funds (Codex P2) The residual-tolerance gate couldn't distinguish unshipped product value from unrecognized order-level shipping (the deferred base includes shipping, but proportional recognition only covers product lines) — so a PARTIALLY_REFUNDED order with unrefunded shipping on a refunded line stranded real shipping revenue. Replace the heuristic with a proper determination: a PARTIALLY_REFUNDED order trues up only when every SHIPPABLE line is fully shipped once refunded-while-unshipped units are counted (loadFullyShippedNetOfRefundsOrderIds). - Returns (refund of a shipped unit, shipment-source snapshot) don't reduce the ship obligation; only allocation-source (unshipped) refunds do; no-snapshot refunds are treated conservatively as not-unshipped. - Non-shippable lines (no product / NON_INVENTORY) are ignored. remainingDeferred stays reversal-aware; the true-up then recognizes the remaining deferred incl. the shipping share. Applied to both connectors + the preview; helper unit-tested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mixed-refund-aware (Codex P2) - Count only DISPATCHED (status SHIPPED) shipment lines as shipped; PENDING/PICKING/PACKED shipments haven't gone out. - KIT/BOM lines ship at component granularity, so a raw parent-line qty sum can't safely determine full shipment — conservatively exclude orders with such lines from the true-up set (leave deferred rather than risk a false-positive over-recognition). - Count the allocation-source (unshipped) QTY within each refund line's snapshot, so a refund that mixes returned (shipped) and unshipped units only credits the unshipped portion toward the ship obligation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Problem
The Group-B daily-batch deferred-revenue true-up (scjz.41) excluded
PARTIALLY_REFUNDEDorders: such an order already posts anUNEARNED_REV_REVERSAL(refund of unshipped lines) thatremainingDeferreddidn't subtract, so truing up would over-recognize.Fix (Xero + QBO)
remainingDeferredis reversal-aware: subtract the postedUNEARNED_REV_REVERSAL(unearned-account debit) for the order's refunds. No-op for terminal-status orders without such reversals; correct for partially-refunded ones.Pure helpers unit-tested; safety bounded to £0.05 so a logic error can't cause a material misstatement.
🤖 Generated with Claude Code