Skip to content

[16.0][FIX] stock_available_mrp: avoid infinite recursion on sibling-variant kits#77

Open
alvaro-gmz wants to merge 1 commit into
OCA:16.0from
factorlibre:16.0-fix-stock_available_mrp-kit-recursion
Open

[16.0][FIX] stock_available_mrp: avoid infinite recursion on sibling-variant kits#77
alvaro-gmz wants to merge 1 commit into
OCA:16.0from
factorlibre:16.0-fix-stock_available_mrp-kit-recursion

Conversation

@alvaro-gmz
Copy link
Copy Markdown

Problem

When a phantom BoM uses another variant of its own product.template as a
component, explode_bom_quantities enters an infinite recursion that
eventually raises ValueError: cannot convert float NaN to integer from
math.ceil inside tools.float_round.

Reproducer (covered by the new regression test):

  • One template with two variants: a "unit" variant and a "pack" variant.
  • A phantom BoM bound to the pack variant whose only component is the
    unit variant.

The bug is triggered when reading immediately_usable_qty (or its
downstream computed fields such as immediately_usable_qty_today on
sale.order.line) on the pack variant.

Root cause

explode_bom_quantities walks the BoM tree by calling
first(current_line.product_id.bom_ids) on every component to detect
nested phantom BoMs. product.product.bom_ids is template-wide through
_inherits, so for the unit variant it returns the phantom BoM of its
sibling (the pack) as well. The loop never terminates: each iteration
re-yields the same sibling phantom BoM and multiplies current_qty by
the line quantity. After ~130 iterations the float overflows to inf,
the next arithmetic step produces NaN and math.ceil raises.

The same pattern is silently masked when the template has at least one
normal BoM whose id is lower than the phantom one: first() then
returns the normal BoM and the if sub_bom.type == "phantom" branch is
skipped. The fragility hides behind ordering luck.

Fix

Replace first(.bom_ids) lookups with mrp.bom._bom_find(..., bom_type="phantom"), which honours the variant scope and does not
surface sibling-variant BoMs. The result is cached per (product, bom_type) tuple inside the call to keep the SELECT volume bounded — the
docstring already warned about _bom_find being chatty if called
naively per line.

Test

A TransactionCase reproduces the trap with a minimal template
carrying two variants where one is a phantom kit of the other. It
asserts three things:

  1. explode_bom_quantities(pack) terminates and yields exactly one
    line (unit × 12).
  2. Reading immediately_usable_qty on the pack variant completes
    without ValueError and yields a real number.
  3. explode_bom_quantities(unit) (the component) terminates with an
    empty result instead of recursing on its sibling's BoM.

Compatibility

Behaviour is unchanged for the legitimate cases where the previous
first(.bom_ids) happened to return the right BoM. _bom_find returns
exactly the BoM that applies to the variant, either variant-specific
(product_id = variant) or template-level (product_id = False).
Sibling-variant BoMs are intentionally excluded — surfacing them on a
variant was the bug.

@OCA-git-bot OCA-git-bot added series:16.0 mod:stock_available_mrp Module stock_available_mrp labels May 19, 2026
…t kits

When a phantom BoM uses another variant of its own template as a
component, ``explode_bom_quantities`` followed ``first(.bom_ids)`` on
that component and looped on a sibling phantom BoM (``bom_ids`` is
template-wide via ``_inherits``). After ~130 iterations the accumulated
quantity overflowed to ``inf``, produced ``NaN`` and the final
``math.ceil`` raised ``ValueError: cannot convert float NaN to integer``.

Use ``mrp.bom._bom_find(..., bom_type='phantom')`` so the lookup honours
the variant scope and never surfaces sibling-variant BoMs. Results are
cached per call to avoid the SELECT volume warned about in the docstring.

A regression test reproduces the trap with a minimal template carrying
two variants where one is a phantom kit of the other.
@alvaro-gmz alvaro-gmz force-pushed the 16.0-fix-stock_available_mrp-kit-recursion branch from 62e9883 to 0feac6f Compare May 19, 2026 13:46
Copy link
Copy Markdown

@AdrianaSaiz AdrianaSaiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:stock_available_mrp Module stock_available_mrp series:16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants