From 46393a28d3873e8fd2dc5058529af1feb4eb8181 Mon Sep 17 00:00:00 2001 From: OneTwo3D IMS Date: Mon, 22 Jun 2026 22:02:52 +0000 Subject: [PATCH 1/4] fix(accounting): reversal-aware deferred-revenue true-up for partially-refunded orders (scjz.68) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- lib/connectors/quickbooks/daily-sync.ts | 43 +++++++++++++++++-- lib/connectors/xero/daily-sync.ts | 43 +++++++++++++++++-- lib/domain/accounting/revenue-recognition.ts | 40 +++++++++++++++++ .../accounting/revenue-recognition.test.ts | 30 +++++++++++++ 4 files changed, 150 insertions(+), 6 deletions(-) diff --git a/lib/connectors/quickbooks/daily-sync.ts b/lib/connectors/quickbooks/daily-sync.ts index 6dfa85e7..96d210ef 100644 --- a/lib/connectors/quickbooks/daily-sync.ts +++ b/lib/connectors/quickbooks/daily-sync.ts @@ -34,7 +34,7 @@ import { import { addMoney, roundQuantity, subtractMoney, toDecimal, type Decimal } from '@/lib/domain/math/decimal' import { GL_BASE_PRECISION, roundToGlPrecisionNumber } from '@/lib/domain/math/precision-policy' import { calculateCoverageByLine, requirementsMapToRows } from '@/lib/products/fulfillment-coverage' -import { isFullyShippedTerminalStatus, recognizeShipmentRevenue } from '@/lib/domain/accounting/revenue-recognition' +import { isFullyShippedTerminalStatus, recognizeShipmentRevenue, shouldTrueUpPartiallyRefundedDeferral, extractUnearnedReversalDebit } from '@/lib/domain/accounting/revenue-recognition' import { recreateJournaledDateFilter } from '@/lib/domain/accounting/daily-batch-retention' import { expandFulfillmentRequirementsDecimal, loadFulfillmentProductGraph } from '@/lib/products/kit-fulfillment' @@ -761,6 +761,34 @@ export async function runDailyBatchSync(): Promise<{ }), ]) + // scjz.68: per-order posted UNEARNED_REV_REVERSAL (deferral reversed by refunds of + // unshipped lines), so the deferred-revenue true-up below is reversal-aware and the + // PARTIALLY_REFUNDED true-up only fires once no material unshipped value remains. + const orderRefunds = await tx.salesOrderRefund.findMany({ + where: { orderId: { in: orderIds } }, + select: { id: true, orderId: true }, + }) + const refundIdToOrderId = new Map(orderRefunds.map((refund) => [refund.id, refund.orderId])) + const postedUnearnedReversalByOrder = new Map() + if (orderRefunds.length > 0) { + const unearnedReversalLogs = await tx.accountingSyncLog.findMany({ + where: { + connector: QBO_CONNECTOR, + type: 'UNEARNED_REV_REVERSAL', + referenceType: 'SalesOrderRefund', + referenceId: { in: orderRefunds.map((refund) => refund.id) }, + status: { in: ['PENDING', 'PROCESSING', 'SYNCED'] }, + }, + select: { referenceId: true, payload: true }, + }) + for (const log of unearnedReversalLogs) { + const reversalOrderId = refundIdToOrderId.get(log.referenceId) + if (!reversalOrderId) continue + const amount = extractUnearnedReversalDebit(log.payload, settings.quickbooks_unearned_revenue_account) + postedUnearnedReversalByOrder.set(reversalOrderId, (postedUnearnedReversalByOrder.get(reversalOrderId) ?? 0) + amount) + } + } + const referencedCostLayerIds = Array.from(new Set( orderAllocations.flatMap((allocation) => ( parseCostLayerSnapshot(allocation.costLayerSnapshot).map((entry) => entry.costLayerId) @@ -835,7 +863,10 @@ export async function runDailyBatchSync(): Promise<{ const recognizedPreviously = firstShipment.order.shipments.reduce((sum, shipment) => ( shipment.shipmentJournalDate ? sum + Number(shipment.revenueRecognizedAmount ?? 0) : sum ), 0) - const remainingDeferred = round2(Math.max(0, deferredBase - recognizedPreviously)) + // scjz.68: subtract deferral already reversed by refunds of unshipped lines, so a + // PARTIALLY_REFUNDED order's remaining deferred reflects only still-unshipped value. + const postedUnearnedReversal = postedUnearnedReversalByOrder.get(orderId) ?? 0 + const remainingDeferred = round2(Math.max(0, deferredBase - recognizedPreviously - postedUnearnedReversal)) let runningRevenue = 0 for (let index = 0; index < orderShipments.length; index++) { @@ -863,7 +894,13 @@ export async function runDailyBatchSync(): Promise<{ remainingDeferred, runningRevenue, isFinalShipmentOfFullyShippedTerminalOrder: - isFullyShippedTerminalStatus(firstShipment.order.status) && index === orderShipments.length - 1, + index === orderShipments.length - 1 && ( + isFullyShippedTerminalStatus(firstShipment.order.status) || + // 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)) + ), }) const shipmentSnapshotsForLines = shipment.lines.map((line) => ( diff --git a/lib/connectors/xero/daily-sync.ts b/lib/connectors/xero/daily-sync.ts index 2070420e..884e96d9 100644 --- a/lib/connectors/xero/daily-sync.ts +++ b/lib/connectors/xero/daily-sync.ts @@ -39,7 +39,7 @@ import { GL_BASE_PRECISION, roundToGlPrecisionNumber } from '@/lib/domain/math/p import { buildInventoryReconciliationSweepJournal, loadInventoryGlReconciliation } from '@/lib/domain/accounting/inventory-gl-reconciliation' import { recreateJournaledDateFilter } from '@/lib/domain/accounting/daily-batch-retention' import { calculateCoverageByLine, requirementsMapToRows } from '@/lib/products/fulfillment-coverage' -import { isFullyShippedTerminalStatus, recognizeShipmentRevenue } from '@/lib/domain/accounting/revenue-recognition' +import { isFullyShippedTerminalStatus, recognizeShipmentRevenue, shouldTrueUpPartiallyRefundedDeferral, extractUnearnedReversalDebit } from '@/lib/domain/accounting/revenue-recognition' import { expandFulfillmentRequirementsDecimal, loadFulfillmentProductGraph } from '@/lib/products/kit-fulfillment' type MutableLayer = { @@ -850,6 +850,34 @@ export async function runDailyBatchSync(): Promise { }), ]) + // scjz.68: per-order posted UNEARNED_REV_REVERSAL (deferral reversed by refunds of + // unshipped lines), so the deferred-revenue true-up below is reversal-aware and the + // PARTIALLY_REFUNDED true-up only fires once no material unshipped value remains. + const orderRefunds = await tx.salesOrderRefund.findMany({ + where: { orderId: { in: orderIds } }, + select: { id: true, orderId: true }, + }) + const refundIdToOrderId = new Map(orderRefunds.map((refund) => [refund.id, refund.orderId])) + const postedUnearnedReversalByOrder = new Map() + if (orderRefunds.length > 0) { + const unearnedReversalLogs = await tx.accountingSyncLog.findMany({ + where: { + connector: XERO_CONNECTOR, + type: 'UNEARNED_REV_REVERSAL', + referenceType: 'SalesOrderRefund', + referenceId: { in: orderRefunds.map((refund) => refund.id) }, + status: { in: ['PENDING', 'PROCESSING', 'SYNCED'] }, + }, + select: { referenceId: true, payload: true }, + }) + for (const log of unearnedReversalLogs) { + const reversalOrderId = refundIdToOrderId.get(log.referenceId) + if (!reversalOrderId) continue + const amount = extractUnearnedReversalDebit(log.payload, settings.xero_unearned_revenue_account) + postedUnearnedReversalByOrder.set(reversalOrderId, (postedUnearnedReversalByOrder.get(reversalOrderId) ?? 0) + amount) + } + } + const referencedCostLayerIds = Array.from(new Set( orderAllocations.flatMap((allocation) => ( parseCostLayerSnapshot(allocation.costLayerSnapshot).map((entry) => entry.costLayerId) @@ -930,7 +958,10 @@ export async function runDailyBatchSync(): Promise { const recognizedPreviously = firstShipment.order.shipments.reduce((sum, shipment) => ( shipment.shipmentJournalDate ? sum + Number(shipment.revenueRecognizedAmount ?? 0) : sum ), 0) - const remainingDeferred = round2(Math.max(0, deferredBase - recognizedPreviously)) + // scjz.68: subtract deferral already reversed by refunds of unshipped lines, so a + // PARTIALLY_REFUNDED order's remaining deferred reflects only still-unshipped value. + const postedUnearnedReversal = postedUnearnedReversalByOrder.get(orderId) ?? 0 + const remainingDeferred = round2(Math.max(0, deferredBase - recognizedPreviously - postedUnearnedReversal)) let runningRevenue = 0 for (let index = 0; index < orderShipments.length; index++) { @@ -958,7 +989,13 @@ export async function runDailyBatchSync(): Promise { remainingDeferred, runningRevenue, isFinalShipmentOfFullyShippedTerminalOrder: - isFullyShippedTerminalStatus(firstShipment.order.status) && index === orderShipments.length - 1, + index === orderShipments.length - 1 && ( + isFullyShippedTerminalStatus(firstShipment.order.status) || + // 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)) + ), }) // COGS: prefer immutable shipment-line snapshots when present. diff --git a/lib/domain/accounting/revenue-recognition.ts b/lib/domain/accounting/revenue-recognition.ts index 74834110..2e8a34a6 100644 --- a/lib/domain/accounting/revenue-recognition.ts +++ b/lib/domain/accounting/revenue-recognition.ts @@ -31,6 +31,46 @@ export function isFullyShippedTerminalStatus(status: SalesOrderStatus | string): return FULLY_SHIPPED_TERMINAL_STATUS_SET.has(status) } +/** + * scjz.68: largest remaining (reversal-aware) deferred revenue still treated as pure + * rounding stranding — small enough to true up safely. The terminal-status true-up + * (scjz.41) clears at-most-a-few-pence of rounding drift; this caps the PARTIALLY_REFUNDED + * true-up to the same scale so it can never recognize a unit's worth of unshipped value. + */ +export const DEFERRED_TRUEUP_STRANDING_TOLERANCE = 0.05 + +/** + * scjz.68: whether a PARTIALLY_REFUNDED order should true up its remaining deferred + * revenue on its final batch shipment. + * + * PARTIALLY_REFUNDED is NOT a fully-shipped status (the refund workflow can set it from + * pre-shipment states by refunding unshipped lines), so — unlike SHIPPED/COMPLETED/ + * DELIVERED — we can't assume every unit has shipped. But once the already-posted + * UNEARNED_REV_REVERSAL is subtracted from the deferred base, the remaining + * (reversal-aware) deferred revenue IS exactly the value of units that are neither + * recognized-on-shipment nor refunded — i.e. still genuinely unshipped. When that is + * only rounding stranding (<= tolerance) the order is effectively fully shipped net of + * refunds and the remainder is safe to recognize; a material remainder means real + * unshipped value, so leave it deferred (recognizing it would over-debit unearned revenue). + */ +export function shouldTrueUpPartiallyRefundedDeferral(reversalAwareRemainingDeferred: number): boolean { + return reversalAwareRemainingDeferred <= DEFERRED_TRUEUP_STRANDING_TOLERANCE +} + +/** + * scjz.68: sum the debit posted to the unearned-revenue account in an + * UNEARNED_REV_REVERSAL journal payload (DR unearned / CR sales). This is the amount of + * deferred revenue a refund of unshipped lines already reversed, which the daily-batch + * true-up must subtract from the deferred base so it never re-recognizes it. + */ +export function extractUnearnedReversalDebit(payload: unknown, unearnedAccountCode: string): number { + const lines = (payload as { lines?: Array<{ accountCode?: string; debit?: number }> } | null)?.lines + if (!Array.isArray(lines)) return 0 + return lines.reduce((sum, line) => ( + line.accountCode === unearnedAccountCode ? sum + (Number(line.debit) || 0) : sum + ), 0) +} + function round2(value: number): number { return Math.round(value * 100) / 100 } diff --git a/tests/domain/accounting/revenue-recognition.test.ts b/tests/domain/accounting/revenue-recognition.test.ts index f4037ceb..b91b37fa 100644 --- a/tests/domain/accounting/revenue-recognition.test.ts +++ b/tests/domain/accounting/revenue-recognition.test.ts @@ -5,8 +5,38 @@ import { FULLY_SHIPPED_TERMINAL_STATUSES, isFullyShippedTerminalStatus, recognizeShipmentRevenue, + shouldTrueUpPartiallyRefundedDeferral, + extractUnearnedReversalDebit, + DEFERRED_TRUEUP_STRANDING_TOLERANCE, } from '@/lib/domain/accounting/revenue-recognition' +test('scjz.68: PARTIALLY_REFUNDED true-up fires only for rounding-scale remainders', () => { + // Reversal-aware remainder this small = no material unshipped value left -> safe to clear. + assert.equal(shouldTrueUpPartiallyRefundedDeferral(0), true) + assert.equal(shouldTrueUpPartiallyRefundedDeferral(0.04), true) + assert.equal(shouldTrueUpPartiallyRefundedDeferral(DEFERRED_TRUEUP_STRANDING_TOLERANCE), true) + // A material remainder = real unshipped value still deferred -> do NOT recognize. + assert.equal(shouldTrueUpPartiallyRefundedDeferral(0.06), false) + assert.equal(shouldTrueUpPartiallyRefundedDeferral(50), false) +}) + +test('scjz.68: PARTIALLY_REFUNDED is NOT a fully-shipped terminal status', () => { + // It must go through the reversal-aware remainder gate, not the unconditional status true-up. + assert.equal(isFullyShippedTerminalStatus('PARTIALLY_REFUNDED'), false) +}) + +test('scjz.68: extractUnearnedReversalDebit sums the unearned-account debit only', () => { + const payload = { lines: [ + { accountCode: 'UNEARNED', debit: 10 }, + { accountCode: 'SALES', credit: 10 }, + { accountCode: 'UNEARNED', debit: 2.5 }, + ] } + assert.equal(extractUnearnedReversalDebit(payload, 'UNEARNED'), 12.5) + assert.equal(extractUnearnedReversalDebit(payload, 'SALES'), 0) + assert.equal(extractUnearnedReversalDebit(null, 'UNEARNED'), 0) + assert.equal(extractUnearnedReversalDebit({}, 'UNEARNED'), 0) +}) + test('fully-shipped terminal statuses true up deferred revenue', () => { // These reach a terminal post-shipment state, so the final shipment must // recognize all remaining deferred revenue (scjz.41). From 6c018db53b5e500315d3d15f7ab54dd7f8960493 Mon Sep 17 00:00:00 2001 From: OneTwo3D IMS Date: Mon, 22 Jun 2026 22:10:54 +0000 Subject: [PATCH 2/4] fix(accounting): gate .68 true-up on post-batch residual + align preview (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) --- app/actions/xero-daily-batch.ts | 27 +++++++++++-- lib/connectors/quickbooks/daily-sync.ts | 37 +++++------------ lib/connectors/xero/daily-sync.ts | 43 +++++++------------- lib/domain/accounting/deferred-reversal.ts | 47 ++++++++++++++++++++++ 4 files changed, 94 insertions(+), 60 deletions(-) create mode 100644 lib/domain/accounting/deferred-reversal.ts diff --git a/app/actions/xero-daily-batch.ts b/app/actions/xero-daily-batch.ts index bf439d16..608d65ee 100644 --- a/app/actions/xero-daily-batch.ts +++ b/app/actions/xero-daily-batch.ts @@ -6,7 +6,8 @@ import { getSalesOrderReference } from '@/lib/sales-order-display' import { parseCostLayerSnapshot, sumCostLayerSnapshot } from '@/lib/cost-layer-snapshots' import { calculateCoverageByLine, requirementsMapToRows } from '@/lib/products/fulfillment-coverage' import { expandFulfillmentRequirementsDecimal, loadFulfillmentProductGraph } from '@/lib/products/kit-fulfillment' -import { isFullyShippedTerminalStatus, recognizeShipmentRevenue } from '@/lib/domain/accounting/revenue-recognition' +import { isFullyShippedTerminalStatus, recognizeShipmentRevenue, shouldTrueUpPartiallyRefundedDeferral } from '@/lib/domain/accounting/revenue-recognition' +import { loadPostedUnearnedReversalByOrder } from '@/lib/domain/accounting/deferred-reversal' import { addMoney, multiplyMoney, @@ -16,6 +17,7 @@ import { type Decimal, type DecimalInput, } from '@/lib/domain/math/decimal' +import { getXeroSettings } from '@/lib/connectors/xero/settings' /** * Preview & history for the Xero daily batch sub-ledger. @@ -295,10 +297,19 @@ async function computePreview(): Promise { bShipmentsByOrder.set(shipment.orderId, existing) } + // scjz.68: mirror the posting path's reversal-aware deferred true-up so the preview + // matches what actually posts (scjz.69) for PARTIALLY_REFUNDED orders. + const bSettings = await getXeroSettings() + const previewPostedUnearnedReversalByOrder = await loadPostedUnearnedReversalByOrder(db, { + orderIds: Array.from(bShipmentsByOrder.keys()), + connector: 'xero', + unearnedAccountCode: bSettings.xero_unearned_revenue_account, + }) + // Compute per-shipment results grouped by order, then emit in the original // (createdAt asc) order so the displayed list and the 200-cap are stable. const bShipmentResults = new Map() - for (const [, orderShipments] of bShipmentsByOrder) { + for (const [orderId, orderShipments] of bShipmentsByOrder) { const firstShipment = orderShipments[0] const order = firstShipment.order const deferredBase = Number(order.unearnedRevenueAmount ?? order.totalBase) @@ -315,7 +326,9 @@ async function computePreview(): Promise { const recognizedPreviously = order.shipments.reduce((sum, shipment) => ( shipment.shipmentJournalDate ? sum + Number(shipment.revenueRecognizedAmount ?? 0) : sum ), 0) - const remainingDeferred = round2(Math.max(0, deferredBase - recognizedPreviously)) + // scjz.68: reversal-aware, matching the posting path. + const postedUnearnedReversal = previewPostedUnearnedReversalByOrder.get(orderId) ?? 0 + const remainingDeferred = round2(Math.max(0, deferredBase - recognizedPreviously - postedUnearnedReversal)) let runningRevenue = 0 for (let index = 0; index < orderShipments.length; index++) { @@ -338,12 +351,18 @@ async function computePreview(): Promise { const proportionalRevenue = orderLineTotal > 0 ? round2((shipmentLineValue / orderLineTotal) * deferredBase) : 0 + // scjz.68: gate the PARTIALLY_REFUNDED true-up on the residual after this batch's + // proportional recognition (matching the posting path). + const residualAfterProportional = round2(Math.max(0, remainingDeferred - runningRevenue - proportionalRevenue)) const revenueProportion = recognizeShipmentRevenue({ proportionalRevenue, remainingDeferred, runningRevenue, isFinalShipmentOfFullyShippedTerminalOrder: - isFullyShippedTerminalStatus(order.status) && index === orderShipments.length - 1, + index === orderShipments.length - 1 && ( + isFullyShippedTerminalStatus(order.status) || + (order.status === 'PARTIALLY_REFUNDED' && shouldTrueUpPartiallyRefundedDeferral(residualAfterProportional)) + ), }) runningRevenue += revenueProportion diff --git a/lib/connectors/quickbooks/daily-sync.ts b/lib/connectors/quickbooks/daily-sync.ts index 96d210ef..7121a23f 100644 --- a/lib/connectors/quickbooks/daily-sync.ts +++ b/lib/connectors/quickbooks/daily-sync.ts @@ -34,7 +34,8 @@ import { import { addMoney, roundQuantity, subtractMoney, toDecimal, type Decimal } from '@/lib/domain/math/decimal' import { GL_BASE_PRECISION, roundToGlPrecisionNumber } from '@/lib/domain/math/precision-policy' import { calculateCoverageByLine, requirementsMapToRows } from '@/lib/products/fulfillment-coverage' -import { isFullyShippedTerminalStatus, recognizeShipmentRevenue, shouldTrueUpPartiallyRefundedDeferral, extractUnearnedReversalDebit } from '@/lib/domain/accounting/revenue-recognition' +import { isFullyShippedTerminalStatus, recognizeShipmentRevenue, shouldTrueUpPartiallyRefundedDeferral } from '@/lib/domain/accounting/revenue-recognition' +import { loadPostedUnearnedReversalByOrder } from '@/lib/domain/accounting/deferred-reversal' import { recreateJournaledDateFilter } from '@/lib/domain/accounting/daily-batch-retention' import { expandFulfillmentRequirementsDecimal, loadFulfillmentProductGraph } from '@/lib/products/kit-fulfillment' @@ -764,30 +765,11 @@ export async function runDailyBatchSync(): Promise<{ // scjz.68: per-order posted UNEARNED_REV_REVERSAL (deferral reversed by refunds of // unshipped lines), so the deferred-revenue true-up below is reversal-aware and the // PARTIALLY_REFUNDED true-up only fires once no material unshipped value remains. - const orderRefunds = await tx.salesOrderRefund.findMany({ - where: { orderId: { in: orderIds } }, - select: { id: true, orderId: true }, + const postedUnearnedReversalByOrder = await loadPostedUnearnedReversalByOrder(tx, { + orderIds, + connector: QBO_CONNECTOR, + unearnedAccountCode: settings.quickbooks_unearned_revenue_account, }) - const refundIdToOrderId = new Map(orderRefunds.map((refund) => [refund.id, refund.orderId])) - const postedUnearnedReversalByOrder = new Map() - if (orderRefunds.length > 0) { - const unearnedReversalLogs = await tx.accountingSyncLog.findMany({ - where: { - connector: QBO_CONNECTOR, - type: 'UNEARNED_REV_REVERSAL', - referenceType: 'SalesOrderRefund', - referenceId: { in: orderRefunds.map((refund) => refund.id) }, - status: { in: ['PENDING', 'PROCESSING', 'SYNCED'] }, - }, - select: { referenceId: true, payload: true }, - }) - for (const log of unearnedReversalLogs) { - const reversalOrderId = refundIdToOrderId.get(log.referenceId) - if (!reversalOrderId) continue - const amount = extractUnearnedReversalDebit(log.payload, settings.quickbooks_unearned_revenue_account) - postedUnearnedReversalByOrder.set(reversalOrderId, (postedUnearnedReversalByOrder.get(reversalOrderId) ?? 0) + amount) - } - } const referencedCostLayerIds = Array.from(new Set( orderAllocations.flatMap((allocation) => ( @@ -889,6 +871,8 @@ export async function runDailyBatchSync(): Promise<{ const proportionalRevenue = orderLineTotal > 0 ? round2((shipmentLineValue / orderLineTotal) * deferredBase) : 0 + // scjz.68: gate on the residual AFTER this batch's proportional recognition (see Xero). + const residualAfterProportional = round2(Math.max(0, remainingDeferred - runningRevenue - proportionalRevenue)) const revenueProportion = recognizeShipmentRevenue({ proportionalRevenue, remainingDeferred, @@ -896,10 +880,7 @@ export async function runDailyBatchSync(): Promise<{ isFinalShipmentOfFullyShippedTerminalOrder: index === orderShipments.length - 1 && ( isFullyShippedTerminalStatus(firstShipment.order.status) || - // 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)) + (firstShipment.order.status === 'PARTIALLY_REFUNDED' && shouldTrueUpPartiallyRefundedDeferral(residualAfterProportional)) ), }) diff --git a/lib/connectors/xero/daily-sync.ts b/lib/connectors/xero/daily-sync.ts index 884e96d9..36a88a67 100644 --- a/lib/connectors/xero/daily-sync.ts +++ b/lib/connectors/xero/daily-sync.ts @@ -39,7 +39,8 @@ import { GL_BASE_PRECISION, roundToGlPrecisionNumber } from '@/lib/domain/math/p import { buildInventoryReconciliationSweepJournal, loadInventoryGlReconciliation } from '@/lib/domain/accounting/inventory-gl-reconciliation' import { recreateJournaledDateFilter } from '@/lib/domain/accounting/daily-batch-retention' import { calculateCoverageByLine, requirementsMapToRows } from '@/lib/products/fulfillment-coverage' -import { isFullyShippedTerminalStatus, recognizeShipmentRevenue, shouldTrueUpPartiallyRefundedDeferral, extractUnearnedReversalDebit } from '@/lib/domain/accounting/revenue-recognition' +import { isFullyShippedTerminalStatus, recognizeShipmentRevenue, shouldTrueUpPartiallyRefundedDeferral } from '@/lib/domain/accounting/revenue-recognition' +import { loadPostedUnearnedReversalByOrder } from '@/lib/domain/accounting/deferred-reversal' import { expandFulfillmentRequirementsDecimal, loadFulfillmentProductGraph } from '@/lib/products/kit-fulfillment' type MutableLayer = { @@ -853,30 +854,11 @@ export async function runDailyBatchSync(): Promise { // scjz.68: per-order posted UNEARNED_REV_REVERSAL (deferral reversed by refunds of // unshipped lines), so the deferred-revenue true-up below is reversal-aware and the // PARTIALLY_REFUNDED true-up only fires once no material unshipped value remains. - const orderRefunds = await tx.salesOrderRefund.findMany({ - where: { orderId: { in: orderIds } }, - select: { id: true, orderId: true }, + const postedUnearnedReversalByOrder = await loadPostedUnearnedReversalByOrder(tx, { + orderIds, + connector: XERO_CONNECTOR, + unearnedAccountCode: settings.xero_unearned_revenue_account, }) - const refundIdToOrderId = new Map(orderRefunds.map((refund) => [refund.id, refund.orderId])) - const postedUnearnedReversalByOrder = new Map() - if (orderRefunds.length > 0) { - const unearnedReversalLogs = await tx.accountingSyncLog.findMany({ - where: { - connector: XERO_CONNECTOR, - type: 'UNEARNED_REV_REVERSAL', - referenceType: 'SalesOrderRefund', - referenceId: { in: orderRefunds.map((refund) => refund.id) }, - status: { in: ['PENDING', 'PROCESSING', 'SYNCED'] }, - }, - select: { referenceId: true, payload: true }, - }) - for (const log of unearnedReversalLogs) { - const reversalOrderId = refundIdToOrderId.get(log.referenceId) - if (!reversalOrderId) continue - const amount = extractUnearnedReversalDebit(log.payload, settings.xero_unearned_revenue_account) - postedUnearnedReversalByOrder.set(reversalOrderId, (postedUnearnedReversalByOrder.get(reversalOrderId) ?? 0) + amount) - } - } const referencedCostLayerIds = Array.from(new Set( orderAllocations.flatMap((allocation) => ( @@ -984,6 +966,11 @@ export async function runDailyBatchSync(): Promise { const proportionalRevenue = orderLineTotal > 0 ? round2((shipmentLineValue / orderLineTotal) * deferredBase) : 0 + // scjz.68: the PARTIALLY_REFUNDED true-up gate must look at what remains AFTER this + // batch's proportional recognition (the still-unshipped value), not the pre-batch + // remainder which still includes the current shipments — otherwise a final batch that + // leaves only a penny of stranding wouldn't true it up. + const residualAfterProportional = round2(Math.max(0, remainingDeferred - runningRevenue - proportionalRevenue)) const revenueProportion = recognizeShipmentRevenue({ proportionalRevenue, remainingDeferred, @@ -991,10 +978,10 @@ export async function runDailyBatchSync(): Promise { isFinalShipmentOfFullyShippedTerminalOrder: index === orderShipments.length - 1 && ( isFullyShippedTerminalStatus(firstShipment.order.status) || - // 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)) + // a PARTIALLY_REFUNDED order isn't guaranteed fully shipped, but once the deferred + // base is reversal-aware, a residual this small means no material unshipped value + // is left — safe to clear the rounding stranding. + (firstShipment.order.status === 'PARTIALLY_REFUNDED' && shouldTrueUpPartiallyRefundedDeferral(residualAfterProportional)) ), }) diff --git a/lib/domain/accounting/deferred-reversal.ts b/lib/domain/accounting/deferred-reversal.ts new file mode 100644 index 00000000..74d24fbb --- /dev/null +++ b/lib/domain/accounting/deferred-reversal.ts @@ -0,0 +1,47 @@ +import type { PrismaClient } from '@/app/generated/prisma/client' +import { extractUnearnedReversalDebit } from './revenue-recognition' + +/** Both the live transaction client and the top-level db client satisfy this. */ +type DeferredReversalClient = Pick + +/** + * scjz.68: sum the already-posted UNEARNED_REV_REVERSAL (refund-of-unshipped-lines + * deferral reversal) per order, so the daily-batch deferred-revenue true-up — and its + * preview — can be reversal-aware and never re-recognize what a refund already reversed. + * Shared by the Xero + QuickBooks daily syncs and the Xero daily-batch preview so all + * three agree on the remaining deferred balance. + */ +export async function loadPostedUnearnedReversalByOrder( + client: DeferredReversalClient, + opts: { orderIds: string[]; connector: string; unearnedAccountCode: string }, +): Promise> { + const byOrder = new Map() + if (opts.orderIds.length === 0) return byOrder + + const refunds = await client.salesOrderRefund.findMany({ + where: { orderId: { in: opts.orderIds } }, + select: { id: true, orderId: true }, + }) + if (refunds.length === 0) return byOrder + + const refundIdToOrderId = new Map(refunds.map((refund) => [refund.id, refund.orderId])) + const logs = await client.accountingSyncLog.findMany({ + where: { + connector: opts.connector, + type: 'UNEARNED_REV_REVERSAL', + referenceType: 'SalesOrderRefund', + referenceId: { in: refunds.map((refund) => refund.id) }, + status: { in: ['PENDING', 'PROCESSING', 'SYNCED'] }, + }, + select: { referenceId: true, payload: true }, + }) + for (const log of logs) { + const orderId = refundIdToOrderId.get(log.referenceId) + if (!orderId) continue + byOrder.set( + orderId, + (byOrder.get(orderId) ?? 0) + extractUnearnedReversalDebit(log.payload, opts.unearnedAccountCode), + ) + } + return byOrder +} From 52d54348d736f4926907e83d722f25bfc88b1a5a Mon Sep 17 00:00:00 2001 From: OneTwo3D IMS Date: Mon, 22 Jun 2026 23:05:33 +0000 Subject: [PATCH 3/4] fix(accounting): gate .68 true-up on per-line full-shipment-net-of-refunds (Codex P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- app/actions/xero-daily-batch.ts | 15 ++-- lib/connectors/quickbooks/daily-sync.ts | 12 +-- lib/connectors/xero/daily-sync.ts | 20 +++-- lib/domain/accounting/deferred-reversal.ts | 75 +++++++++++++++++++ lib/domain/accounting/revenue-recognition.ts | 26 ------- .../accounting/deferred-reversal.test.ts | 54 +++++++++++++ .../accounting/revenue-recognition.test.ts | 12 --- 7 files changed, 153 insertions(+), 61 deletions(-) create mode 100644 tests/domain/accounting/deferred-reversal.test.ts diff --git a/app/actions/xero-daily-batch.ts b/app/actions/xero-daily-batch.ts index 608d65ee..a8b59a7e 100644 --- a/app/actions/xero-daily-batch.ts +++ b/app/actions/xero-daily-batch.ts @@ -6,8 +6,8 @@ import { getSalesOrderReference } from '@/lib/sales-order-display' import { parseCostLayerSnapshot, sumCostLayerSnapshot } from '@/lib/cost-layer-snapshots' import { calculateCoverageByLine, requirementsMapToRows } from '@/lib/products/fulfillment-coverage' import { expandFulfillmentRequirementsDecimal, loadFulfillmentProductGraph } from '@/lib/products/kit-fulfillment' -import { isFullyShippedTerminalStatus, recognizeShipmentRevenue, shouldTrueUpPartiallyRefundedDeferral } from '@/lib/domain/accounting/revenue-recognition' -import { loadPostedUnearnedReversalByOrder } from '@/lib/domain/accounting/deferred-reversal' +import { isFullyShippedTerminalStatus, recognizeShipmentRevenue } from '@/lib/domain/accounting/revenue-recognition' +import { loadPostedUnearnedReversalByOrder, loadFullyShippedNetOfRefundsOrderIds } from '@/lib/domain/accounting/deferred-reversal' import { addMoney, multiplyMoney, @@ -300,11 +300,13 @@ async function computePreview(): Promise { // scjz.68: mirror the posting path's reversal-aware deferred true-up so the preview // matches what actually posts (scjz.69) for PARTIALLY_REFUNDED orders. const bSettings = await getXeroSettings() + const previewOrderIds = Array.from(bShipmentsByOrder.keys()) const previewPostedUnearnedReversalByOrder = await loadPostedUnearnedReversalByOrder(db, { - orderIds: Array.from(bShipmentsByOrder.keys()), + orderIds: previewOrderIds, connector: 'xero', unearnedAccountCode: bSettings.xero_unearned_revenue_account, }) + const previewFullyShippedNetOfRefundsOrderIds = await loadFullyShippedNetOfRefundsOrderIds(db, previewOrderIds) // Compute per-shipment results grouped by order, then emit in the original // (createdAt asc) order so the displayed list and the 200-cap are stable. @@ -351,9 +353,8 @@ async function computePreview(): Promise { const proportionalRevenue = orderLineTotal > 0 ? round2((shipmentLineValue / orderLineTotal) * deferredBase) : 0 - // scjz.68: gate the PARTIALLY_REFUNDED true-up on the residual after this batch's - // proportional recognition (matching the posting path). - const residualAfterProportional = round2(Math.max(0, remainingDeferred - runningRevenue - proportionalRevenue)) + // scjz.68: mirror the posting path — true up a PARTIALLY_REFUNDED order only when it + // is fully shipped net of refunds. const revenueProportion = recognizeShipmentRevenue({ proportionalRevenue, remainingDeferred, @@ -361,7 +362,7 @@ async function computePreview(): Promise { isFinalShipmentOfFullyShippedTerminalOrder: index === orderShipments.length - 1 && ( isFullyShippedTerminalStatus(order.status) || - (order.status === 'PARTIALLY_REFUNDED' && shouldTrueUpPartiallyRefundedDeferral(residualAfterProportional)) + (order.status === 'PARTIALLY_REFUNDED' && previewFullyShippedNetOfRefundsOrderIds.has(orderId)) ), }) runningRevenue += revenueProportion diff --git a/lib/connectors/quickbooks/daily-sync.ts b/lib/connectors/quickbooks/daily-sync.ts index 7121a23f..ed13076a 100644 --- a/lib/connectors/quickbooks/daily-sync.ts +++ b/lib/connectors/quickbooks/daily-sync.ts @@ -34,8 +34,8 @@ import { import { addMoney, roundQuantity, subtractMoney, toDecimal, type Decimal } from '@/lib/domain/math/decimal' import { GL_BASE_PRECISION, roundToGlPrecisionNumber } from '@/lib/domain/math/precision-policy' import { calculateCoverageByLine, requirementsMapToRows } from '@/lib/products/fulfillment-coverage' -import { isFullyShippedTerminalStatus, recognizeShipmentRevenue, shouldTrueUpPartiallyRefundedDeferral } from '@/lib/domain/accounting/revenue-recognition' -import { loadPostedUnearnedReversalByOrder } from '@/lib/domain/accounting/deferred-reversal' +import { isFullyShippedTerminalStatus, recognizeShipmentRevenue } from '@/lib/domain/accounting/revenue-recognition' +import { loadPostedUnearnedReversalByOrder, loadFullyShippedNetOfRefundsOrderIds } from '@/lib/domain/accounting/deferred-reversal' import { recreateJournaledDateFilter } from '@/lib/domain/accounting/daily-batch-retention' import { expandFulfillmentRequirementsDecimal, loadFulfillmentProductGraph } from '@/lib/products/kit-fulfillment' @@ -770,6 +770,8 @@ export async function runDailyBatchSync(): Promise<{ connector: QBO_CONNECTOR, unearnedAccountCode: settings.quickbooks_unearned_revenue_account, }) + // scjz.68: PARTIALLY_REFUNDED orders that are nonetheless fully shipped net of refunds. + const fullyShippedNetOfRefundsOrderIds = await loadFullyShippedNetOfRefundsOrderIds(tx, orderIds) const referencedCostLayerIds = Array.from(new Set( orderAllocations.flatMap((allocation) => ( @@ -871,8 +873,6 @@ export async function runDailyBatchSync(): Promise<{ const proportionalRevenue = orderLineTotal > 0 ? round2((shipmentLineValue / orderLineTotal) * deferredBase) : 0 - // scjz.68: gate on the residual AFTER this batch's proportional recognition (see Xero). - const residualAfterProportional = round2(Math.max(0, remainingDeferred - runningRevenue - proportionalRevenue)) const revenueProportion = recognizeShipmentRevenue({ proportionalRevenue, remainingDeferred, @@ -880,7 +880,9 @@ export async function runDailyBatchSync(): Promise<{ isFinalShipmentOfFullyShippedTerminalOrder: index === orderShipments.length - 1 && ( isFullyShippedTerminalStatus(firstShipment.order.status) || - (firstShipment.order.status === 'PARTIALLY_REFUNDED' && shouldTrueUpPartiallyRefundedDeferral(residualAfterProportional)) + // scjz.68: see Xero — true up a PARTIALLY_REFUNDED order only when it is fully + // shipped net of refunds. + (firstShipment.order.status === 'PARTIALLY_REFUNDED' && fullyShippedNetOfRefundsOrderIds.has(orderId)) ), }) diff --git a/lib/connectors/xero/daily-sync.ts b/lib/connectors/xero/daily-sync.ts index 36a88a67..1bc98465 100644 --- a/lib/connectors/xero/daily-sync.ts +++ b/lib/connectors/xero/daily-sync.ts @@ -39,8 +39,8 @@ import { GL_BASE_PRECISION, roundToGlPrecisionNumber } from '@/lib/domain/math/p import { buildInventoryReconciliationSweepJournal, loadInventoryGlReconciliation } from '@/lib/domain/accounting/inventory-gl-reconciliation' import { recreateJournaledDateFilter } from '@/lib/domain/accounting/daily-batch-retention' import { calculateCoverageByLine, requirementsMapToRows } from '@/lib/products/fulfillment-coverage' -import { isFullyShippedTerminalStatus, recognizeShipmentRevenue, shouldTrueUpPartiallyRefundedDeferral } from '@/lib/domain/accounting/revenue-recognition' -import { loadPostedUnearnedReversalByOrder } from '@/lib/domain/accounting/deferred-reversal' +import { isFullyShippedTerminalStatus, recognizeShipmentRevenue } from '@/lib/domain/accounting/revenue-recognition' +import { loadPostedUnearnedReversalByOrder, loadFullyShippedNetOfRefundsOrderIds } from '@/lib/domain/accounting/deferred-reversal' import { expandFulfillmentRequirementsDecimal, loadFulfillmentProductGraph } from '@/lib/products/kit-fulfillment' type MutableLayer = { @@ -859,6 +859,9 @@ export async function runDailyBatchSync(): Promise { connector: XERO_CONNECTOR, unearnedAccountCode: settings.xero_unearned_revenue_account, }) + // scjz.68: PARTIALLY_REFUNDED orders that are nonetheless fully shipped net of refunds + // — eligible for the final deferred-revenue true-up. + const fullyShippedNetOfRefundsOrderIds = await loadFullyShippedNetOfRefundsOrderIds(tx, orderIds) const referencedCostLayerIds = Array.from(new Set( orderAllocations.flatMap((allocation) => ( @@ -966,11 +969,6 @@ export async function runDailyBatchSync(): Promise { const proportionalRevenue = orderLineTotal > 0 ? round2((shipmentLineValue / orderLineTotal) * deferredBase) : 0 - // scjz.68: the PARTIALLY_REFUNDED true-up gate must look at what remains AFTER this - // batch's proportional recognition (the still-unshipped value), not the pre-batch - // remainder which still includes the current shipments — otherwise a final batch that - // leaves only a penny of stranding wouldn't true it up. - const residualAfterProportional = round2(Math.max(0, remainingDeferred - runningRevenue - proportionalRevenue)) const revenueProportion = recognizeShipmentRevenue({ proportionalRevenue, remainingDeferred, @@ -978,10 +976,10 @@ export async function runDailyBatchSync(): Promise { isFinalShipmentOfFullyShippedTerminalOrder: index === orderShipments.length - 1 && ( isFullyShippedTerminalStatus(firstShipment.order.status) || - // a PARTIALLY_REFUNDED order isn't guaranteed fully shipped, but once the deferred - // base is reversal-aware, a residual this small means no material unshipped value - // is left — safe to clear the rounding stranding. - (firstShipment.order.status === 'PARTIALLY_REFUNDED' && shouldTrueUpPartiallyRefundedDeferral(residualAfterProportional)) + // scjz.68: a PARTIALLY_REFUNDED order isn't a fully-shipped status, but can be + // genuinely fully shipped net of refunds — then its remaining (reversal-aware) + // deferred revenue, incl. the order-level shipping share, is earned and trued up. + (firstShipment.order.status === 'PARTIALLY_REFUNDED' && fullyShippedNetOfRefundsOrderIds.has(orderId)) ), }) diff --git a/lib/domain/accounting/deferred-reversal.ts b/lib/domain/accounting/deferred-reversal.ts index 74d24fbb..02e26b36 100644 --- a/lib/domain/accounting/deferred-reversal.ts +++ b/lib/domain/accounting/deferred-reversal.ts @@ -1,9 +1,84 @@ import type { PrismaClient } from '@/app/generated/prisma/client' +import { parseCostLayerSnapshot } from '@/lib/cost-layer-snapshots' import { extractUnearnedReversalDebit } from './revenue-recognition' /** Both the live transaction client and the top-level db client satisfy this. */ type DeferredReversalClient = Pick +type FullShipClient = Pick + +// Engine-scale qty tolerance (matches the 6dp FIFO engine) so fractional rounding on a +// fully-shipped line isn't mistaken for a remaining unshipped sliver. +const FULL_SHIP_QTY_TOLERANCE = 1e-6 + +/** + * scjz.68: order IDs whose every SHIPPABLE product line is fully shipped once + * refunded-while-unshipped units are excluded — i.e. nothing more will ship, so the + * remaining deferred revenue (incl. the order-level shipping share) is safe to true up. + * + * Not used for SHIPPED/COMPLETED/DELIVERED orders (their status already guarantees this); + * it lets PARTIALLY_REFUNDED orders — which can sit at that status while genuinely fully + * shipped — qualify for the final true-up without the residual-threshold heuristic, which + * couldn't tell unshipped product value apart from unrecognized shipping. + * + * Per line: shipped qty (all shipments) + refunded-while-UNSHIPPED qty must cover the + * ordered qty. A refund counts as unshipped only when its cost snapshot reversed an + * allocation (no `shipment`-source entry) — a return of a shipped unit does not reduce the + * ship obligation, and a refund with no snapshot is treated conservatively as not-unshipped. + * Non-shippable lines (no product, or NON_INVENTORY service/fee) never ship and are ignored. + */ +export async function loadFullyShippedNetOfRefundsOrderIds( + client: FullShipClient, + orderIds: string[], +): Promise> { + const result = new Set() + if (orderIds.length === 0) return result + + const [orderLines, shipmentLines, refundLines] = await Promise.all([ + client.salesOrderLine.findMany({ + where: { orderId: { in: orderIds } }, + select: { id: true, orderId: true, qty: true, productId: true, product: { select: { type: true } } }, + }), + client.shipmentLine.findMany({ + where: { shipment: { orderId: { in: orderIds } } }, + select: { lineId: true, qty: true }, + }), + client.salesOrderRefundLine.findMany({ + where: { refund: { orderId: { in: orderIds } } }, + select: { salesOrderLineId: true, qty: true, costLayerSnapshot: true }, + }), + ]) + + const shippedByLine = new Map() + for (const sl of shipmentLines) { + shippedByLine.set(sl.lineId, (shippedByLine.get(sl.lineId) ?? 0) + Number(sl.qty)) + } + const refundedUnshippedByLine = new Map() + for (const rl of refundLines) { + if (!rl.salesOrderLineId) continue + const entries = parseCostLayerSnapshot(rl.costLayerSnapshot) + const refundedWhileUnshipped = entries.length > 0 && !entries.some((entry) => entry.source === 'shipment') + if (!refundedWhileUnshipped) continue + refundedUnshippedByLine.set(rl.salesOrderLineId, (refundedUnshippedByLine.get(rl.salesOrderLineId) ?? 0) + Number(rl.qty)) + } + + const linesByOrder = new Map>() + for (const line of orderLines) { + const shippable = !!line.productId && line.product?.type !== 'NON_INVENTORY' + const arr = linesByOrder.get(line.orderId) ?? [] + arr.push({ id: line.id, qty: Number(line.qty), shippable }) + linesByOrder.set(line.orderId, arr) + } + for (const [orderId, lines] of linesByOrder) { + const fullyShipped = lines.every((line) => ( + !line.shippable || + (shippedByLine.get(line.id) ?? 0) + (refundedUnshippedByLine.get(line.id) ?? 0) + FULL_SHIP_QTY_TOLERANCE >= line.qty + )) + if (fullyShipped) result.add(orderId) + } + return result +} + /** * scjz.68: sum the already-posted UNEARNED_REV_REVERSAL (refund-of-unshipped-lines * deferral reversal) per order, so the daily-batch deferred-revenue true-up — and its diff --git a/lib/domain/accounting/revenue-recognition.ts b/lib/domain/accounting/revenue-recognition.ts index 2e8a34a6..f75ab1d6 100644 --- a/lib/domain/accounting/revenue-recognition.ts +++ b/lib/domain/accounting/revenue-recognition.ts @@ -31,32 +31,6 @@ export function isFullyShippedTerminalStatus(status: SalesOrderStatus | string): return FULLY_SHIPPED_TERMINAL_STATUS_SET.has(status) } -/** - * scjz.68: largest remaining (reversal-aware) deferred revenue still treated as pure - * rounding stranding — small enough to true up safely. The terminal-status true-up - * (scjz.41) clears at-most-a-few-pence of rounding drift; this caps the PARTIALLY_REFUNDED - * true-up to the same scale so it can never recognize a unit's worth of unshipped value. - */ -export const DEFERRED_TRUEUP_STRANDING_TOLERANCE = 0.05 - -/** - * scjz.68: whether a PARTIALLY_REFUNDED order should true up its remaining deferred - * revenue on its final batch shipment. - * - * PARTIALLY_REFUNDED is NOT a fully-shipped status (the refund workflow can set it from - * pre-shipment states by refunding unshipped lines), so — unlike SHIPPED/COMPLETED/ - * DELIVERED — we can't assume every unit has shipped. But once the already-posted - * UNEARNED_REV_REVERSAL is subtracted from the deferred base, the remaining - * (reversal-aware) deferred revenue IS exactly the value of units that are neither - * recognized-on-shipment nor refunded — i.e. still genuinely unshipped. When that is - * only rounding stranding (<= tolerance) the order is effectively fully shipped net of - * refunds and the remainder is safe to recognize; a material remainder means real - * unshipped value, so leave it deferred (recognizing it would over-debit unearned revenue). - */ -export function shouldTrueUpPartiallyRefundedDeferral(reversalAwareRemainingDeferred: number): boolean { - return reversalAwareRemainingDeferred <= DEFERRED_TRUEUP_STRANDING_TOLERANCE -} - /** * scjz.68: sum the debit posted to the unearned-revenue account in an * UNEARNED_REV_REVERSAL journal payload (DR unearned / CR sales). This is the amount of diff --git a/tests/domain/accounting/deferred-reversal.test.ts b/tests/domain/accounting/deferred-reversal.test.ts new file mode 100644 index 00000000..d214f1e4 --- /dev/null +++ b/tests/domain/accounting/deferred-reversal.test.ts @@ -0,0 +1,54 @@ +import assert from 'node:assert/strict' +import test from 'node:test' + +import { loadFullyShippedNetOfRefundsOrderIds } from '@/lib/domain/accounting/deferred-reversal' + +type OrderLine = { id: string; orderId: string; qty: number; productId: string | null; product: { type: string } | null } +type ShipLine = { lineId: string; qty: number } +type RefundLine = { salesOrderLineId: string | null; qty: number; costLayerSnapshot: unknown } + +function mockClient(input: { orderLines: OrderLine[]; shipmentLines: ShipLine[]; refundLines: RefundLine[] }) { + return { + salesOrderLine: { findMany: async () => input.orderLines }, + shipmentLine: { findMany: async () => input.shipmentLines }, + salesOrderRefundLine: { findMany: async () => input.refundLines }, + } as never +} + +const alloc = (qty: number) => [{ costLayerId: 'cl', qty, unitCostBase: 1, source: 'allocation' as const }] +const shipped = (qty: number) => [{ costLayerId: 'cl', qty, unitCostBase: 1, source: 'shipment' as const }] + +test('scjz.68: loadFullyShippedNetOfRefundsOrderIds — fully shipped, refunded-unshipped, returns, services', async () => { + const result = await loadFullyShippedNetOfRefundsOrderIds(mockClient({ + orderLines: [ + { id: 'L1', orderId: 'fully', qty: 2, productId: 'p', product: { type: 'SIMPLE' } }, + { id: 'L2', orderId: 'refunded', qty: 2, productId: 'p', product: { type: 'SIMPLE' } }, + { id: 'L3', orderId: 'partial', qty: 2, productId: 'p', product: { type: 'SIMPLE' } }, + { id: 'L4', orderId: 'return', qty: 2, productId: 'p', product: { type: 'SIMPLE' } }, + { id: 'L5', orderId: 'service', qty: 1, productId: 'svc', product: { type: 'NON_INVENTORY' } }, + { id: 'L6', orderId: 'service', qty: 1, productId: 'p', product: { type: 'SIMPLE' } }, + ], + shipmentLines: [ + { lineId: 'L1', qty: 2 }, // fully shipped + { lineId: 'L2', qty: 1 }, // 1 shipped, 1 will be refunded-unshipped + { lineId: 'L3', qty: 1 }, // 1 shipped, 1 NOT accounted for + { lineId: 'L4', qty: 1 }, // 1 shipped, 1 unshipped; the refund below is a RETURN of a shipped unit + { lineId: 'L6', qty: 1 }, // the shippable line of the service order + ], + refundLines: [ + { salesOrderLineId: 'L2', qty: 1, costLayerSnapshot: alloc(1) }, // unshipped refund -> covers the gap + { salesOrderLineId: 'L4', qty: 1, costLayerSnapshot: shipped(1) }, // return of a shipped unit -> does NOT cover the unshipped one + ], + }), ['fully', 'refunded', 'partial', 'return', 'service']) + + assert.equal(result.has('fully'), true) + assert.equal(result.has('refunded'), true) // 1 shipped + 1 refunded-unshipped >= 2 + assert.equal(result.has('partial'), false) // 1 shipped, 1 unaccounted + assert.equal(result.has('return'), false) // return doesn't reduce the ship obligation + assert.equal(result.has('service'), true) // non-inventory line ignored; shippable line fully shipped +}) + +test('scjz.68: empty order list returns empty set', async () => { + const result = await loadFullyShippedNetOfRefundsOrderIds(mockClient({ orderLines: [], shipmentLines: [], refundLines: [] }), []) + assert.equal(result.size, 0) +}) diff --git a/tests/domain/accounting/revenue-recognition.test.ts b/tests/domain/accounting/revenue-recognition.test.ts index b91b37fa..9987053e 100644 --- a/tests/domain/accounting/revenue-recognition.test.ts +++ b/tests/domain/accounting/revenue-recognition.test.ts @@ -5,21 +5,9 @@ import { FULLY_SHIPPED_TERMINAL_STATUSES, isFullyShippedTerminalStatus, recognizeShipmentRevenue, - shouldTrueUpPartiallyRefundedDeferral, extractUnearnedReversalDebit, - DEFERRED_TRUEUP_STRANDING_TOLERANCE, } from '@/lib/domain/accounting/revenue-recognition' -test('scjz.68: PARTIALLY_REFUNDED true-up fires only for rounding-scale remainders', () => { - // Reversal-aware remainder this small = no material unshipped value left -> safe to clear. - assert.equal(shouldTrueUpPartiallyRefundedDeferral(0), true) - assert.equal(shouldTrueUpPartiallyRefundedDeferral(0.04), true) - assert.equal(shouldTrueUpPartiallyRefundedDeferral(DEFERRED_TRUEUP_STRANDING_TOLERANCE), true) - // A material remainder = real unshipped value still deferred -> do NOT recognize. - assert.equal(shouldTrueUpPartiallyRefundedDeferral(0.06), false) - assert.equal(shouldTrueUpPartiallyRefundedDeferral(50), false) -}) - test('scjz.68: PARTIALLY_REFUNDED is NOT a fully-shipped terminal status', () => { // It must go through the reversal-aware remainder gate, not the unconditional status true-up. assert.equal(isFullyShippedTerminalStatus('PARTIALLY_REFUNDED'), false) From 29836f8acc370a51b5db3dca90cea28cc4909182 Mon Sep 17 00:00:00 2001 From: OneTwo3D IMS Date: Mon, 22 Jun 2026 23:11:53 +0000 Subject: [PATCH 4/4] =?UTF-8?q?fix(accounting):=20.68=20full-shipment=20ga?= =?UTF-8?q?te=20=E2=80=94=20dispatched-only,=20kit-safe,=20mixed-refund-aw?= =?UTF-8?q?are=20(Codex=20P2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- lib/domain/accounting/deferred-reversal.ts | 25 +++++++++++++------ .../accounting/deferred-reversal.test.ts | 12 +++++++-- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/lib/domain/accounting/deferred-reversal.ts b/lib/domain/accounting/deferred-reversal.ts index 02e26b36..e81dbe0a 100644 --- a/lib/domain/accounting/deferred-reversal.ts +++ b/lib/domain/accounting/deferred-reversal.ts @@ -39,8 +39,10 @@ export async function loadFullyShippedNetOfRefundsOrderIds( where: { orderId: { in: orderIds } }, select: { id: true, orderId: true, qty: true, productId: true, product: { select: { type: true } } }, }), + // Only DISPATCHED (SHIPPED) shipments count as shipped — a PENDING/PICKING/PACKED + // shipment hasn't gone out yet (Codex). client.shipmentLine.findMany({ - where: { shipment: { orderId: { in: orderIds } } }, + where: { shipment: { orderId: { in: orderIds }, status: 'SHIPPED' } }, select: { lineId: true, qty: true }, }), client.salesOrderRefundLine.findMany({ @@ -56,20 +58,27 @@ export async function loadFullyShippedNetOfRefundsOrderIds( const refundedUnshippedByLine = new Map() for (const rl of refundLines) { if (!rl.salesOrderLineId) continue - const entries = parseCostLayerSnapshot(rl.costLayerSnapshot) - const refundedWhileUnshipped = entries.length > 0 && !entries.some((entry) => entry.source === 'shipment') - if (!refundedWhileUnshipped) continue - refundedUnshippedByLine.set(rl.salesOrderLineId, (refundedUnshippedByLine.get(rl.salesOrderLineId) ?? 0) + Number(rl.qty)) + // Count only the allocation-source (unshipped) QTY — a refund line can mix shipped + // (returned) and unshipped units; returns don't reduce the ship obligation (Codex). + const allocationQty = parseCostLayerSnapshot(rl.costLayerSnapshot) + .reduce((sum, entry) => (entry.source === 'allocation' ? sum + Number(entry.qty) : sum), 0) + if (allocationQty <= 0) continue + refundedUnshippedByLine.set(rl.salesOrderLineId, (refundedUnshippedByLine.get(rl.salesOrderLineId) ?? 0) + allocationQty) } - const linesByOrder = new Map>() + const linesByOrder = new Map>() for (const line of orderLines) { - const shippable = !!line.productId && line.product?.type !== 'NON_INVENTORY' + const type = line.product?.type + const shippable = !!line.productId && type !== 'NON_INVENTORY' + // KIT/BOM lines ship at component granularity, so a raw parent-line qty sum can't tell + // full shipment safely. Don't risk a false-positive true-up — leave such orders deferred. + const componentBacked = type === 'KIT' || type === 'BOM' const arr = linesByOrder.get(line.orderId) ?? [] - arr.push({ id: line.id, qty: Number(line.qty), shippable }) + arr.push({ id: line.id, qty: Number(line.qty), shippable, componentBacked }) linesByOrder.set(line.orderId, arr) } for (const [orderId, lines] of linesByOrder) { + if (lines.some((line) => line.componentBacked)) continue const fullyShipped = lines.every((line) => ( !line.shippable || (shippedByLine.get(line.id) ?? 0) + (refundedUnshippedByLine.get(line.id) ?? 0) + FULL_SHIP_QTY_TOLERANCE >= line.qty diff --git a/tests/domain/accounting/deferred-reversal.test.ts b/tests/domain/accounting/deferred-reversal.test.ts index d214f1e4..cb7cffc5 100644 --- a/tests/domain/accounting/deferred-reversal.test.ts +++ b/tests/domain/accounting/deferred-reversal.test.ts @@ -18,7 +18,7 @@ function mockClient(input: { orderLines: OrderLine[]; shipmentLines: ShipLine[]; const alloc = (qty: number) => [{ costLayerId: 'cl', qty, unitCostBase: 1, source: 'allocation' as const }] const shipped = (qty: number) => [{ costLayerId: 'cl', qty, unitCostBase: 1, source: 'shipment' as const }] -test('scjz.68: loadFullyShippedNetOfRefundsOrderIds — fully shipped, refunded-unshipped, returns, services', async () => { +test('scjz.68: loadFullyShippedNetOfRefundsOrderIds — full shipment, refunds, returns, services, kits, mixed', async () => { const result = await loadFullyShippedNetOfRefundsOrderIds(mockClient({ orderLines: [ { id: 'L1', orderId: 'fully', qty: 2, productId: 'p', product: { type: 'SIMPLE' } }, @@ -27,6 +27,8 @@ test('scjz.68: loadFullyShippedNetOfRefundsOrderIds — fully shipped, refunded- { id: 'L4', orderId: 'return', qty: 2, productId: 'p', product: { type: 'SIMPLE' } }, { id: 'L5', orderId: 'service', qty: 1, productId: 'svc', product: { type: 'NON_INVENTORY' } }, { id: 'L6', orderId: 'service', qty: 1, productId: 'p', product: { type: 'SIMPLE' } }, + { id: 'L7', orderId: 'kit', qty: 1, productId: 'k', product: { type: 'KIT' } }, + { id: 'L8', orderId: 'mixed', qty: 2, productId: 'p', product: { type: 'SIMPLE' } }, ], shipmentLines: [ { lineId: 'L1', qty: 2 }, // fully shipped @@ -34,18 +36,24 @@ test('scjz.68: loadFullyShippedNetOfRefundsOrderIds — fully shipped, refunded- { lineId: 'L3', qty: 1 }, // 1 shipped, 1 NOT accounted for { lineId: 'L4', qty: 1 }, // 1 shipped, 1 unshipped; the refund below is a RETURN of a shipped unit { lineId: 'L6', qty: 1 }, // the shippable line of the service order + { lineId: 'L7', qty: 5 }, // kit shipped at component granularity — must NOT be trusted + { lineId: 'L8', qty: 1 }, // 1 shipped; mixed refund below covers the other 1 (alloc portion) ], refundLines: [ { salesOrderLineId: 'L2', qty: 1, costLayerSnapshot: alloc(1) }, // unshipped refund -> covers the gap { salesOrderLineId: 'L4', qty: 1, costLayerSnapshot: shipped(1) }, // return of a shipped unit -> does NOT cover the unshipped one + // mixed refund line: 1 unshipped (allocation) + 1 return (shipment) — only the alloc 1 counts. + { salesOrderLineId: 'L8', qty: 2, costLayerSnapshot: [...alloc(1), ...shipped(1)] }, ], - }), ['fully', 'refunded', 'partial', 'return', 'service']) + }), ['fully', 'refunded', 'partial', 'return', 'service', 'kit', 'mixed']) assert.equal(result.has('fully'), true) assert.equal(result.has('refunded'), true) // 1 shipped + 1 refunded-unshipped >= 2 assert.equal(result.has('partial'), false) // 1 shipped, 1 unaccounted assert.equal(result.has('return'), false) // return doesn't reduce the ship obligation assert.equal(result.has('service'), true) // non-inventory line ignored; shippable line fully shipped + assert.equal(result.has('kit'), false) // KIT line -> conservatively skipped + assert.equal(result.has('mixed'), true) // 1 shipped + alloc 1 (mixed refund) >= 2 }) test('scjz.68: empty order list returns empty set', async () => {