[jayzed80]

Fixes #3963

Root cause

amordegrc in crates/functions/src/financial/depreciation.rs:136 rounded each period's charge (.round() at lines 154 and 162 on main) and then subtracted the rounded charge from the book value (book -= dep). The rounding error from each period was carried into every later period. For =AMORDEGRC(1000,DATE(2023,1,1),DATE(2023,6,30),0,2,0.1,1):

• main: book 1000 − 123 = 877, − 219 = 658, × 0.25 = 164.5 → 165 (Excel: 164)
• unrounded: 1000 − 123.29 = 876.71, − 219.18 = 657.53, × 0.25 = 164.38 → 164

Period 4 shows the same drift: main gives 93, Excel gives 92.

Fix

The first-period charge and each later charge stay unrounded in the book value and remaining-value calculation. Only the returned charge is rounded. The final-two-periods split ((book * 0.5).round()) is unchanged.

Tests

New test financial::tests::amordegrc_does_not_accumulate_rounding checks the issue's Excel values: periods 0, 1, 2, 4 → 123, 219, 164, 92. The existing documented example (2400, …, period 1 → 776) still passes.

Before the fix:

test financial::tests::amordegrc_does_not_accumulate_rounding ... FAILED
panicked at crates\functions\src\financial\tests.rs:147:5:
got 165, expected 164

After the fix:

test financial::tests::amordegrc_does_not_accumulate_rounding ... ok
test financial::tests::depreciation ... ok

Checks run (Windows)

• cargo fmt -p gridcraft-functions -- --check: clean
• cargo clippy -j 4 -p gridcraft-functions --all-targets -- -D warnings: clean
• cargo test -j 4 -p gridcraft-functions: 181 passed, 1 failed. The failure is engineering::tests::complex_numbers (...186548i vs ...186547i, a last-digit floating-point difference in complex-number text on Windows). That code is in a file this change does not touch.

Not run

cargo xtask ci (wasm, layers, assets) and the full workspace test suite were not run locally. Only the gridcraft-functions crate changed.

Overlap

The fixes for #3964 and #3965 (separate PRs) change the same AMORDEGRC function in crates/functions/src/financial/depreciation.rs. Whichever merges second may need a small rebase.

Summary by CodeRabbit

• Bug Fixes
  • Improved AMORDEGRC depreciation calculations by preventing intermediate rounding from affecting later period charges. Returned charges are rounded, helping preserve accuracy across the depreciation schedule.
  • Added coverage for charges across multiple periods, including the initial period, using a zero salvage value and a 10% rate.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

amordegrc now carries unrounded depreciation charges through book-value calculations and rounds charges when returning results. A regression test checks the results for periods 0, 1, 2, and 4.

Changes: AMORDEGRC calculation

Layer / File(s)	Summary
Depreciation rounding and regression test crates/functions/src/financial/depreciation.rs, crates/functions/src/financial/tests.rs	The calculation retains unrounded charges when updating book value and rounds returned charges. The regression test checks charges of 123, 219, 164, and 92 for periods 0, 1, 2, and 4.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: aiongg

Merge Risk: Minimal · up to 7c425

AMORDEGRC now carries unrounded charges through the schedule, so later periods should match Excel for the cases in the linked issue. No merge-blocking defect was found. Behavior for nonzero salvage values has not been checked against Excel, so a spot check there would be useful but should not block the merge.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: AMORDEGRC book-value calculations now carry unrounded charges, and the issue number is included. The raw summary and objectives confirm that the change alters how book…
Linked Issues check		Issue #3963 requires AMORDEGRC later-period charges to match Excel: periods 0, 1, 2, and 4 for the example should return 123, 219, 164, and 92. The root cause is that each rounded charge was subtracte…
Out of Scope Changes check		Comparing the merge base to the reviewed head changes only crates/functions/src/financial/depreciation.rs (+6/-4) and crates/functions/src/financial/tests.rs (+10/-0). Both changes implement the AMORD…

Full details: Title check

Explanation

The title names the main change: AMORDEGRC book-value calculations now carry unrounded charges, and the issue number is included. The raw summary and objectives confirm that the change alters how book value is tracked and rounds only the returned charge, so the title matches the changeset.

Full details: Linked Issues check

Explanation

Issue #3963 requires AMORDEGRC later-period charges to match Excel: periods 0, 1, 2, and 4 for the example should return 123, 219, 164, and 92. The root cause is that each rounded charge was subtracted from book value. The diff in crates/functions/src/financial/depreciation.rs keeps the period-0 charge and each loop charge unrounded in book, rest, and the book update. Only the returned value is rounded (dep.round() at the period-0 return and at the final num_val). The final-two-periods split is unchanged. Hand check using the rate and coefficient implied by the example (r = 0.25): period 0 = round(123.29) = 123; book after period 1 = 657.53 after 219.18 charge, so period 2 = round(164.38) = 164; period 4 = round(92.47) = 92. These match the issue. The new test amordegrc_does_not_accumulate_rounding in crates/functions/src/financial/tests.rs asserts periods 0, 1, 2, and 4 against these values. The existing depreciation test is unchanged. The PR does not run the new test in its own output beyond the author's report, so the test pass is reported by the author rather than observed here. The code evidence supports compliance.

Full details: Out of Scope Changes check

Explanation

Comparing the merge base to the reviewed head changes only crates/functions/src/financial/depreciation.rs (+6/-4) and crates/functions/src/financial/tests.rs (+10/-0). Both changes implement the AMORDEGRC rounding fix and its regression test for #3963. The added code comments reference #3963. No unrelated files or behavior changes appear in the diff. The PR description notes an overlap with #3964 and #3965, which is a merge-coordination concern and not out-of-scope code in this PR.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: storytold/gridcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: c0ee120
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: b5f30969-f25f-4cdc-ac1c-30130159540d

Commits

Reviewing files that changed from the base of the PR and between c0ee120 and 7c42537.

Files selected for processing: 2

• crates/functions/src/financial/depreciation.rs
• crates/functions/src/financial/tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.