From 5763378ee1f6980693729e63487476ba685f9961 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 27 Jul 2026 13:37:08 +0530 Subject: [PATCH 1/2] fix: pool batch slot values on every run, not only when negative A batch is one valuation pool, so any per-slot value difference within a batch is stale detail from the report's own age slots, not real valuation. The rebalance only ran when consumption had already driven a slot negative, so a batch whose receipts landed at different rates kept a skewed split across age buckets (one bucket free, another double-priced) while the total stayed correct. Drop the negative-slot precondition and always spread a batch's pooled value over its slots in proportion to qty. Redistribution preserves group totals, so buckets still sum to Stock Balance; only the split across ages changes. (cherry picked from commit cedaaa3a000be2f8647bd12a57fa33dd6c6ff349) --- .../stock/report/stock_ageing/stock_ageing.py | 18 +++--- .../report/stock_ageing/test_stock_ageing.py | 62 +++++++++++++++++-- 2 files changed, 65 insertions(+), 15 deletions(-) diff --git a/erpnext/stock/report/stock_ageing/stock_ageing.py b/erpnext/stock/report/stock_ageing/stock_ageing.py index 55a07835436..83ecd4b1fee 100644 --- a/erpnext/stock/report/stock_ageing/stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/stock_ageing.py @@ -325,7 +325,7 @@ class FIFOSlots: del stock_ledger_entries self._recompute_moving_average_slots() - self._rebalance_negative_batch_slots() + self._rebalance_batch_slots() if not self.filters.get("show_warehouse_wise_stock"): # (Item 1, WH 1), (Item 1, WH 2) => (Item 1) @@ -347,14 +347,14 @@ class FIFOSlots: if is_qty_slot(slot): slot[FIFO_VALUE_INDEX] = flt(slot[FIFO_QTY_INDEX] * rate) - def _rebalance_negative_batch_slots(self) -> None: + def _rebalance_batch_slots(self) -> None: for item_dict in self.item_details.values(): if item_dict.get("has_batch_no"): - self._rebalance_negative_batch_slot_values(item_dict["fifo_queue"]) + self._rebalance_batch_slot_values(item_dict["fifo_queue"]) - def _rebalance_negative_batch_slot_values(self, fifo_queue: list) -> None: - """A batch is one valuation pool, so a slot driven negative by consumption - at the pooled rate is stale detail: spread the pool value over its slots.""" + def _rebalance_batch_slot_values(self, fifo_queue: list) -> None: + """A batch is one valuation pool, so per-slot value differences are stale + detail: spread the pool value over its slots in proportion to qty.""" groups = {} for slot in fifo_queue: if is_batch_slot(slot): @@ -362,12 +362,8 @@ class FIFOSlots: groups.setdefault(key, []).append(slot) for slots in groups.values(): - has_negative_slot = any( - flt(slot[BATCH_SLOT_VALUE_INDEX]) < 0 and flt(slot[BATCH_SLOT_QTY_INDEX]) > 0 - for slot in slots - ) total_qty = sum(flt(slot[BATCH_SLOT_QTY_INDEX]) for slot in slots) - if not has_negative_slot or total_qty <= 0: + if total_qty <= 0: continue rate = sum(flt(slot[BATCH_SLOT_VALUE_INDEX]) for slot in slots) / total_qty diff --git a/erpnext/stock/report/stock_ageing/test_stock_ageing.py b/erpnext/stock/report/stock_ageing/test_stock_ageing.py index 640c10e35c3..73bdfd5b419 100644 --- a/erpnext/stock/report/stock_ageing/test_stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/test_stock_ageing.py @@ -565,10 +565,11 @@ class TestStockAgeing(ERPNextTestSuite): ], ) - def test_partial_batch_reco_keeps_existing_slot_values(self): + def test_partial_batch_reco_pools_slot_values(self): """Ledger (same wh, batch B): [+10 @ 100, single-SLE reco >> 12] The reco entry qty (delta 2) does not cover the whole batch, so - stock_value_difference / qty is not the batch rate: skip the rescale.""" + stock_value_difference / qty is not the batch rate: skip the rescale. + The batch total (1400) is untouched, then pooled across both slots.""" from erpnext.stock.doctype.item.test_item import make_item item_code = make_item( @@ -608,11 +609,64 @@ class TestStockAgeing(ERPNextTestSuite): slots = FIFOSlots(self.filters, sle).generate() queue = slots[item_code]["fifo_queue"] + self.assertEqual( + [slot[:4] for slot in queue], + [ + [batch_no, 1, 10.0, "2021-12-01"], + [batch_no, 1, 2.0, "2021-12-01"], + ], + ) + self.assertAlmostEqual(queue[0][4], 1166.67, places=2) + self.assertAlmostEqual(queue[1][4], 233.33, places=2) + + def test_batch_receipts_at_differing_rates_pool_slot_values(self): + """Ledger (same wh, batch B): [+10 @ 0, +10 @ 10] and no issue. + Nothing goes negative, but the batch is one valuation pool, so both + age slots carry the pooled rate instead of their receipt value.""" + from erpnext.stock.doctype.item.test_item import make_item + + item_code = make_item( + "Test Stock Ageing Batch Pool Split", + {"is_stock_item": 1, "has_batch_no": 1, "valuation_method": "FIFO"}, + ).name + + batch_no = "SA-POOL-SPLIT-BATCH" + if not frappe.db.exists("Batch", batch_no): + frappe.get_doc({"doctype": "Batch", "batch_id": batch_no, "item": item_code}).insert( + ignore_permissions=True + ) + frappe.db.set_value("Batch", batch_no, "use_batchwise_valuation", 1) + + def make_sle(posting_date, voucher_no, actual_qty, qty_after, stock_value_difference): + return frappe._dict( + name=item_code, + actual_qty=actual_qty, + qty_after_transaction=qty_after, + stock_value_difference=stock_value_difference, + valuation_rate=abs(stock_value_difference / actual_qty) if actual_qty else 0, + warehouse="WH 1", + posting_date=posting_date, + voucher_type="Stock Entry", + voucher_no=voucher_no, + has_serial_no=False, + has_batch_no=True, + serial_no=None, + batch_no=batch_no, + ) + + sle = [ + make_sle("2021-12-01", "001", 10, 10, 0), + make_sle("2021-12-02", "002", 10, 20, 100), + ] + + slots = FIFOSlots(self.filters, sle).generate() + queue = slots[item_code]["fifo_queue"] + self.assertEqual( queue, [ - [batch_no, 1, 10.0, "2021-12-01", 1000.0], - [batch_no, 1, 2.0, "2021-12-01", 400.0], + [batch_no, 1, 10.0, "2021-12-01", 50.0], + [batch_no, 1, 10.0, "2021-12-01", 50.0], ], ) From 85925680ace587f87cb10770dee5f58e0c263c9e Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 27 Jul 2026 13:42:25 +0530 Subject: [PATCH 2/2] test: assert batch pooling preserves the group total on a repeating rate (cherry picked from commit 545262c5d4c265d5f41fbb84aa2a0764c28c2db3) --- .../report/stock_ageing/test_stock_ageing.py | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/report/stock_ageing/test_stock_ageing.py b/erpnext/stock/report/stock_ageing/test_stock_ageing.py index 73bdfd5b419..39c046fb689 100644 --- a/erpnext/stock/report/stock_ageing/test_stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/test_stock_ageing.py @@ -5,7 +5,13 @@ from unittest.mock import patch import frappe -from erpnext.stock.report.stock_ageing.stock_ageing import FIFOSlots, format_report_data, get_average_age +from erpnext.stock.report.stock_ageing.stock_ageing import ( + BATCH_SLOT_QTY_INDEX, + BATCH_SLOT_VALUE_INDEX, + FIFOSlots, + format_report_data, + get_average_age, +) from erpnext.tests.utils import ERPNextTestSuite @@ -670,6 +676,53 @@ class TestStockAgeing(ERPNextTestSuite): ], ) + def test_batch_pooling_preserves_total_on_repeating_rate(self): + """Ledger (same wh, batch B): [+3 @ 100/3, +6 @ 0, +2 @ 0] + The pooled rate does not terminate, so assert the redistributed + slot values still add back to the batch total.""" + from erpnext.stock.doctype.item.test_item import make_item + + item_code = make_item( + "Test Stock Ageing Batch Pool Residual", + {"is_stock_item": 1, "has_batch_no": 1, "valuation_method": "FIFO"}, + ).name + + batch_no = "SA-POOL-RESIDUAL-BATCH" + if not frappe.db.exists("Batch", batch_no): + frappe.get_doc({"doctype": "Batch", "batch_id": batch_no, "item": item_code}).insert( + ignore_permissions=True + ) + frappe.db.set_value("Batch", batch_no, "use_batchwise_valuation", 1) + + def make_sle(posting_date, voucher_no, actual_qty, qty_after, stock_value_difference): + return frappe._dict( + name=item_code, + actual_qty=actual_qty, + qty_after_transaction=qty_after, + stock_value_difference=stock_value_difference, + valuation_rate=abs(stock_value_difference / actual_qty) if actual_qty else 0, + warehouse="WH 1", + posting_date=posting_date, + voucher_type="Stock Entry", + voucher_no=voucher_no, + has_serial_no=False, + has_batch_no=True, + serial_no=None, + batch_no=batch_no, + ) + + sle = [ + make_sle("2021-12-01", "001", 3, 3, 100), + make_sle("2021-12-02", "002", 6, 9, 0), + make_sle("2021-12-03", "003", 2, 11, 0), + ] + + slots = FIFOSlots(self.filters, sle).generate() + queue = slots[item_code]["fifo_queue"] + + self.assertEqual([slot[BATCH_SLOT_QTY_INDEX] for slot in queue], [3.0, 6.0, 2.0]) + self.assertEqual(sum(slot[BATCH_SLOT_VALUE_INDEX] for slot in queue), 100.0) + def test_batch_issue_at_pooled_rate_keeps_slot_values_positive(self): """Ledger (same wh, batch B): [+10 @ 0, +10 @ 10, -4 @ pooled 5] Consuming the zero-valued head slot at the pooled rate drives it