Skip to content

Expand a cancelled quotient of factorials instead of throwing (#817) - #820

Merged
Rafael-SOWNet merged 1 commit into
masterfrom
fix/expand-factorial-quotient
Aug 8, 2026
Merged

Expand a cancelled quotient of factorials instead of throwing (#817)#820
Rafael-SOWNet merged 1 commit into
masterfrom
fix/expand-factorial-quotient

Conversation

@Rafael-SOWNet

Copy link
Copy Markdown
Collaborator

Closes #817.

What was wrong

Entity.Expand cancels a quotient of factorials before it does anything else, and the cancellation can leave a sum(x + 1)! / x! is x + 1. The result went straight back into SmartExpandOver, which is documented <summary>expr is NEITHER Sumf NOR Minusf</summary> and asserts that it never receives one. So the assertion fired and an AngouriBugException — the library reporting that the library is broken — came out of a public method, on an expression Simplify answered as 1 + x throughout.

case Divf:
    expr = expr.Replace(Patterns.ExpandFactorialDivisions);   // (x + 1)! / x!  ->  x + 1
    if (!(expr is Divf(var dividend, var divisor)))
        return SmartExpandOver(expr, ...);                    // ... which must not take a sum

Thrown rather than returned, so a caller mapping Expand over a corpus loses the whole run instead of one entry.

Why this fix is right

GatherLinearChildrenOverSumAndExpand is the sum-aware entry point, and it already delegates to SmartExpandOver for anything that is not a sum. Calling it here is one identifier and a strict superset of the old behaviour: identical where the cancelled expression is not a sum, and correct instead of fatal where it is.

The other two recursive calls are unaffected — one is handed a product, and the public MathS.SmartExpandOver splits by Sumf.LinearChildren before descending, which is exactly the guard that was missing here.

Measured

On this build, each of these threw AngouriBugException before:

input now
(x + 1)! / x! x + 1
(x + 2)! / x! x ^ 2 + 3 * x + 2
(x + 3)! / x! x ^ 3 + 6 * x ^ 2 + 11 * x + 6
x! / (x + 1)! 1 / (x + 1)
(x + 1)! / x! + y x + 1 + y
((x + 1)! / x!) * (a + b) a * x + b * x + a + b
(x + y + 1)! / (x + y)! x + y + 1

Checked as values, not strings, at a point where the factorials are defined: 5!/4! = 5, 6!/4! = 30, 7!/4! = 210 — and the expanded form agrees with the original at that point in every case.

sin(x)! / x! is unchanged and still comes back as written, which is correct: nothing cancels.

Tests

5808 passing, 0 failed, 14 skipped — the skip this issue was holding is gone. F# 130 passing.

Two regression tests: one asserting the value is preserved, one asserting no factorial survives where the two cancel completely. Both were checked against the unfixed code — five cases fail without the change, including the nested (+ y) and multiplied (* (a + b)) ones, so the tests are not passing vacuously.

Compatibility

BREAKING-CHANGES.md records it: the same call returns a value where it used to throw. Nothing could have depended on the old behaviour — AngouriBugException is not a contract — but the entry belongs there anyway since the answer changed.

🤖 Generated with Claude Code

Expansion cancels a quotient of factorials before anything else, and the
cancellation can leave a sum: (x + 1)! / x! is x + 1. The result went straight
back into SmartExpandOver, which is documented not to take a sum and asserts that
it never gets one -- so the assertion fired and an AngouriBugException came out of
Entity.Expand, on an expression Simplify answered as 1 + x throughout.

Sending the cancelled expression through GatherLinearChildrenOverSumAndExpand
instead is one identifier and a strict superset: for anything that is not a sum it
calls SmartExpandOver, which is what the line did before, and for a sum it splits
and expands the terms rather than throwing.

Measured on this build:

  (x + 1)! / x!               was AngouriBugException, is x + 1
  (x + 2)! / x!               was AngouriBugException, is x ^ 2 + 3 * x + 2
  (x + 3)! / x!               was AngouriBugException, is x ^ 3 + 6 * x ^ 2 + 11 * x + 6
  x! / (x + 1)!               was AngouriBugException, is 1 / (x + 1)
  (x + y + 1)! / (x + y)!     was AngouriBugException, is x + y + 1

Each checked as a value rather than a string, at a point where the factorials are
defined: 5!/4! is 5, 6!/4! is 30, 7!/4! is 210. The regression tests assert those
values and that no factorial survives where the two cancel completely; they fail
on five cases without the fix, including the nested and multiplied ones.

Tests: 5808 passing, 0 failed, 14 skipped -- the skip this issue held is gone.
F# 130. BREAKING-CHANGES.md records the throw becoming a value.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Rafael-SOWNet
Rafael-SOWNet merged commit 74b00a8 into master Aug 8, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expand throws AngouriBugException on a quotient of factorials

1 participant