[jayzed80]

Fixes #3964

Root cause

crates/functions/src/financial/depreciation.rs:155-157 (before the fix): for period = 0,
amordegrc returned the rounded first charge year_frac * rate * coef * cost directly, with no
cap. When salvage is high, that charge takes the book value below salvage. AMORLINC in the same
file already caps its first charge at cost - salvage (line 128).

Fix

The period-0 result is dep.min(cost - salvage). Only the period-0 return changes; the book value
used for later periods and the later-period schedule are untouched.

Tests

New test financial::tests::amordegrc_first_period_stops_at_salvage in
crates/functions/src/financial/tests.rs, with the issue's cases:

• AMORDEGRC(1000, 2023-01-01, 2024-01-01, 900, 0, 0.1, 3) → 100 (was 250)
• AMORDEGRC(1000, 2023-01-01, 2023-06-30, 900, 0, 0.1, 1) → 100 (was 123)
• zero-salvage control AMORDEGRC(1000, 2023-01-01, 2024-01-01, 0, 0, 0.1, 3) → 250 (unchanged)

Without the fix:

test financial::tests::amordegrc_first_period_stops_at_salvage ... FAILED
thread 'financial::tests::amordegrc_first_period_stops_at_salvage' panicked at crates\functions\src\financial\tests.rs:144:5:
got 250, expected 100

With the fix:

test financial::tests::amordegrc_first_period_stops_at_salvage ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 181 filtered out

Checks run (Windows)

• cargo fmt -p gridcraft-functions: no changes beyond this diff.
• cargo clippy -p gridcraft-functions --all-targets -- -D warnings: clean.
• cargo test -p gridcraft-functions: 181 passed, 1 failed. The failure is
  engineering::tests::complex_numbers (...0.707106781186548i vs ...0.707106781186547i, last
  digit). It fails the same way with this fix reverted, so it is pre-existing and unrelated
  (a floating-point difference on this platform).

Not run

• cargo xtask ci (full workspace gates, wasm) and the gridcraft-cli eval check were not run;
  relying on CI.
• Later periods when the first charge already exceeds cost - salvage (for example period 1 of the
  issue's first case) are out of scope here, as the issue isolates period 0. They were not compared
  with Excel and are unchanged.

Overlap

The fixes for #3963 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
  • First-period AMORDEGRC depreciation is now limited to the asset’s cost minus its salvage value, preventing depreciation from exceeding the depreciable amount.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

For amordegrc, the period-zero depreciation charge is now capped at cost minus salvage. Regression tests cover full-year and partial-year first periods with high salvage, plus a zero-salvage case.

Changes: AMORDEGRC first-period cap

Layer / File(s)	Summary
Cap first-period depreciation and test cases crates/functions/src/financial/depreciation.rs, crates/functions/src/financial/tests.rs	The period-zero depreciation return is capped at cost minus salvage. Regression tests cover full-year and partial-year periods with high salvage, and a zero-salvage case.

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: aiongg

Merge Risk: Minimal · up to 0b5df

AMORDEGRC's first-period charge is now capped at cost minus salvage, matching the reported Excel results. Later-period scheduling is intentionally unchanged. No actionable merge-blocking risk remains.

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 first-period depreciation is now capped so that book value does not drop below salvage. The changeset caps the period-0 result at cost minus salvage in amord…
Linked Issues check		Issue #3964 requires that AMORDEGRC's period-0 charge not exceed cost minus salvage. The diff at crates/functions/src/financial/depreciation.rs changes the period == 0.0 return from num_val(dep) to nu…
Out of Scope Changes check		The whole-PR diff against the merge base touches two files. depreciation.rs has one changed line in the period-0 branch of amordegrc, with a comment that cites #3964. tests.rs adds one test for the is…

Full details: Title check

Explanation

The title names the main change: AMORDEGRC first-period depreciation is now capped so that book value does not drop below salvage. The changeset caps the period-0 result at cost minus salvage in amordegrc and adds a test for it. The title cites #3964, which matches the linked issue number, though the PR objectives cite #4106 for the PR itself. That mismatch does not change the assessment of the title's content.

Full details: Linked Issues check

Explanation

Issue #3964 requires that AMORDEGRC's period-0 charge not exceed cost minus salvage. The diff at crates/functions/src/financial/depreciation.rs changes the period == 0.0 return from num_val(dep) to num_val(dep.min(cost - salvage)). With cost 1000 and salvage 900, the cap is 100, which matches the Excel results in the issue for both the whole-year and partial-year cases. The zero-salvage control keeps its 250 result, because the cap is 1000 and the charge is 250. The diff also adds amordegrc_first_period_stops_at_salvage in crates/functions/src/financial/tests.rs, which asserts 100, 100, and 250 for these three cases. The later-period book value path is not changed. The issue's coding requirement is met. The test results reported in the PR description were not re-run during this review.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base touches two files. depreciation.rs has one changed line in the period-0 branch of amordegrc, with a comment that cites #3964. tests.rs adds one test for the issue's cases. Both changes implement the linked issue's period-zero cap requirement. No unrelated changes were found. The PR description mentions overlapping work for #3963 and #3965 in separate PRs. Those changes are not in this diff, so they do not affect this assessment.

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: ba4109e2-af21-4977-8407-a86211e0804e

Commits

Reviewing files that changed from the base of the PR and between c0ee120 and 0b5dfd8.

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; 6 remain after this review.