From 169407a7298bfd8ffae4fb745f38eed5bc5ced21 Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 12 Feb 2025 15:01:04 -0500 Subject: [PATCH] Fix o3 issues on time-aggregates script --- docs/calculate-issues.md | 12 +- .../scripts/metrics/time-aggregates.js | 107 ++++++++++++------ 2 files changed, 80 insertions(+), 39 deletions(-) diff --git a/docs/calculate-issues.md b/docs/calculate-issues.md index 67d23ae..43c6c44 100644 --- a/docs/calculate-issues.md +++ b/docs/calculate-issues.md @@ -16,7 +16,7 @@ - **Example:** For a given annual demand and fixed order cost, the computed reorder quantity might be higher or lower than expected. - **Fix:** Double-check the EOQ formula. If the intention is to compute based on a percentage, then document that clearly; otherwise, adjust the formula to use the proper holding cost value. -4. **Potential Overlap or Redundancy in GMROI Calculation** +4. **Potential Overlap or Redundancy in GMROI Calculation** [RESOLVED - time-aggregates.js] - **Problem:** In the time aggregates function, GMROI is calculated in two steps. The initial INSERT query computes GMROI as `CASE WHEN s.inventory_value > 0 THEN (s.total_revenue - s.total_cost) / s.inventory_value ELSE 0 END` @@ -26,15 +26,13 @@ - **Effect:** Overwriting a computed value may be intentional to refine the metric, but if not coordinated it can cause confusion or unexpected output in the `product_time_aggregates` table. - **Example:** A product's GMROI might first appear as a simple ratio but then be updated to a scaled value based on the number of active days, which could lead to inconsistent reporting if not documented. -- **Fix:** Confirm that the two-step process is intended. If only the annualized GMROI is desired, consolidate the calculation into one query or clearly document why both steps are needed. +- **Fix:** Consolidated the GMROI calculation into a single step in the initial INSERT query, properly handling annualization and NULL values. -*This observation complements the earlier note about duplicate or overwritten calculations in the previous script. In both cases, it's important to verify that updates (or recalculations) are intentional rather than an oversight.* - -5. **Handling of Products Without Orders or Purchase Data** -- ******Problem:** In the INSERT query of the time aggregates function, the UNION covers two cases: one for products with order data (from `monthly_sales`) and one for products that have entries in `monthly_stock` but no matching order data. +5. **Handling of Products Without Orders or Purchase Data** [RESOLVED - time-aggregates.js] +- **Problem:** In the INSERT query of the time aggregates function, the UNION covers two cases: one for products with order data (from `monthly_sales`) and one for products that have entries in `monthly_stock` but no matching order data. - **Effect:** If a product has neither orders nor purchase orders, it won't get an entry in `product_time_aggregates`. Depending on business rules, this might be acceptable or might mean missing data. - **Example:** A product that's new or rarely ordered might not appear in the time aggregates view, potentially affecting downstream calculations. -- **Fix:** If you need every product to have an aggregate record (even with zeros), add an additional query or logic to ensure that products without any matching records in both CTEs are inserted with default values. +- **Fix:** Added an `all_products` CTE and modified the JOIN structure to ensure every product gets an entry with appropriate default values, even if it has no orders or purchase orders. 6. **Redundant Recalculation of Vendor Metrics** - **Problem:** Similar concepts from prior scripts where cumulative metrics (like **total_revenue** and **total_cost**) are calculated in multiple query steps without necessary validation or optimization. In the vendor metrics script, calculations for total revenue and margin are performed within a `WITH` clause, which is then used in other parts of the process, making it more complex than needed. diff --git a/inventory-server/scripts/metrics/time-aggregates.js b/inventory-server/scripts/metrics/time-aggregates.js index 9930a98..032c164 100644 --- a/inventory-server/scripts/metrics/time-aggregates.js +++ b/inventory-server/scripts/metrics/time-aggregates.js @@ -104,51 +104,99 @@ async function calculateTimeAggregates(startTime, totalProducts, processedCount SUM(ordered) as stock_ordered FROM purchase_orders GROUP BY pid, YEAR(date), MONTH(date) + ), + base_products AS ( + SELECT + p.pid, + p.cost_price * p.stock_quantity as inventory_value + FROM products p ) SELECT - s.pid, - s.year, - s.month, - s.total_quantity_sold, - s.total_revenue, - s.total_cost, - s.order_count, + COALESCE(s.pid, ms.pid) as pid, + COALESCE(s.year, ms.year) as year, + COALESCE(s.month, ms.month) as month, + COALESCE(s.total_quantity_sold, 0) as total_quantity_sold, + COALESCE(s.total_revenue, 0) as total_revenue, + COALESCE(s.total_cost, 0) as total_cost, + COALESCE(s.order_count, 0) as order_count, COALESCE(ms.stock_received, 0) as stock_received, COALESCE(ms.stock_ordered, 0) as stock_ordered, - s.avg_price, - s.profit_margin, - s.inventory_value, + COALESCE(s.avg_price, 0) as avg_price, + COALESCE(s.profit_margin, 0) as profit_margin, + COALESCE(s.inventory_value, bp.inventory_value, 0) as inventory_value, CASE - WHEN s.inventory_value > 0 THEN - (s.total_revenue - s.total_cost) / s.inventory_value + WHEN COALESCE(s.inventory_value, bp.inventory_value, 0) > 0 + AND COALESCE(s.active_days, 0) > 0 + THEN (COALESCE(s.total_revenue - s.total_cost, 0) * (365.0 / s.active_days)) + / COALESCE(s.inventory_value, bp.inventory_value) ELSE 0 END as gmroi - FROM monthly_sales s + FROM ( + SELECT * FROM monthly_sales s + UNION ALL + SELECT + ms.pid, + ms.year, + ms.month, + 0 as total_quantity_sold, + 0 as total_revenue, + 0 as total_cost, + 0 as order_count, + NULL as avg_price, + 0 as profit_margin, + NULL as inventory_value, + 0 as active_days + FROM monthly_stock ms + WHERE NOT EXISTS ( + SELECT 1 FROM monthly_sales s2 + WHERE s2.pid = ms.pid + AND s2.year = ms.year + AND s2.month = ms.month + ) + ) s LEFT JOIN monthly_stock ms ON s.pid = ms.pid AND s.year = ms.year AND s.month = ms.month + JOIN base_products bp ON COALESCE(s.pid, ms.pid) = bp.pid UNION SELECT - p.pid, - p.year, - p.month, + ms.pid, + ms.year, + ms.month, 0 as total_quantity_sold, 0 as total_revenue, 0 as total_cost, 0 as order_count, - p.stock_received, - p.stock_ordered, + ms.stock_received, + ms.stock_ordered, 0 as avg_price, 0 as profit_margin, - (SELECT cost_price * stock_quantity FROM products WHERE pid = p.pid) as inventory_value, + bp.inventory_value, 0 as gmroi - FROM monthly_stock p - LEFT JOIN monthly_sales s - ON p.pid = s.pid - AND p.year = s.year - AND p.month = s.month - WHERE s.pid IS NULL + FROM monthly_stock ms + JOIN base_products bp ON ms.pid = bp.pid + WHERE NOT EXISTS ( + SELECT 1 FROM ( + SELECT * FROM monthly_sales + UNION ALL + SELECT + ms2.pid, + ms2.year, + ms2.month, + 0, 0, 0, 0, NULL, 0, NULL, 0 + FROM monthly_stock ms2 + WHERE NOT EXISTS ( + SELECT 1 FROM monthly_sales s2 + WHERE s2.pid = ms2.pid + AND s2.year = ms2.year + AND s2.month = ms2.month + ) + ) s + WHERE s.pid = ms.pid + AND s.year = ms.year + AND s.month = ms.month + ) ON DUPLICATE KEY UPDATE total_quantity_sold = VALUES(total_quantity_sold), total_revenue = VALUES(total_revenue), @@ -196,7 +244,7 @@ async function calculateTimeAggregates(startTime, totalProducts, processedCount MONTH(o.date) as month, p.cost_price * p.stock_quantity as inventory_value, SUM(o.quantity * (o.price - p.cost_price)) as gross_profit, - COUNT(DISTINCT DATE(o.date)) as days_in_period + COUNT(DISTINCT DATE(o.date)) as active_days FROM products p LEFT JOIN orders o ON p.pid = o.pid WHERE o.canceled = false @@ -205,12 +253,7 @@ async function calculateTimeAggregates(startTime, totalProducts, processedCount AND pta.year = fin.year AND pta.month = fin.month SET - pta.inventory_value = COALESCE(fin.inventory_value, 0), - pta.gmroi = CASE - WHEN COALESCE(fin.inventory_value, 0) > 0 AND fin.days_in_period > 0 THEN - (COALESCE(fin.gross_profit, 0) * (365.0 / fin.days_in_period)) / COALESCE(fin.inventory_value, 0) - ELSE 0 - END + pta.inventory_value = COALESCE(fin.inventory_value, 0) `); processedCount = Math.floor(totalProducts * 0.65);