[jayzed80]

Fixes #3953

What was wrong

DISC, INTRATE, PRICEDISC, RECEIVED and YIELDDISC divided the day count by year_basis(), which for basis 1 (actual/actual) used the length of the settlement year only (366 in a leap year). Excel uses its YEARFRAC actual/actual year length, which depends on the whole interval. For 2024-03-01 to 2025-03-01, GridCraft returned DISC = 0.050137; Excel returns 0.05.

The change

crates/functions/src/financial/securities.rs: the five functions use year_frac(sys, settlement, maturity, basis) (the same YEARFRAC the app already has) instead of day_count / year_basis. For bases 0, 2, 3 and 4 that is the same value as before (30/360, actual/360, actual/365, European 30/360). Only basis 1 changes. year_basis() in day_count.rs had no other callers and is removed.

Tests

disc_basis_1_across_years_matches_excel checks all five functions against the issue's Excel values for 2024-03-01 to 2025-03-01, plus a multi-year span (2023-01-31 to 2024-03-31, 425 days over an average year of 365.5).

• Without the change (the code reverted, test kept): disc_basis_1_across_years_matches_excel ... FAILED (tests.rs:324).
• With it: passes.
• cargo test -p gridcraft-functions: 182 passed, 0 failed. cargo clippy -p gridcraft-functions --all-targets -- -D warnings: clean.

Summary by CodeRabbit

• Bug Fixes
  • Updated security discount, pricing, yield, interest-rate, and proceeds calculations for basis 1 across one-year and multi-year periods. Multi-year calculations now account for the average length of a year, improving consistency with spreadsheet results. These changes affect calculations spanning different dates and may produce updated values for affected financial functions.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The five discount-security functions now use year_frac for settlement-to-maturity intervals. A new test checks basis-1 results over one-year and multi-year spans.

Changes: Basis 1 discount-security calculations

Layer / File(s)	Summary
Use year fractions for discount-security calculations crates/functions/src/financial/day_count.rs, crates/functions/src/financial/securities.rs, crates/functions/src/financial/tests.rs	The year_basis and disc_frac helpers were removed. DISC, INTRATE, PRICEDISC, RECEIVED, and YIELDDISC now use year_frac. The new test checks one-year and multi-year intervals against expected results.

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: aiongg

Merge Risk: Minimal · up to a1089

This change makes the basis-1 results of DISC, INTRATE, PRICEDISC, RECEIVED, and YIELDDISC match Excel when the interval spans years of different lengths. No concrete merge-blocking risk was found, and a new test covers the changed behavior.

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 actual change: basis 1 of the discount securities functions now uses Excel's actual/actual year length instead of the settlement year's length. The changeset replaces year_basis wi…
Linked Issues check		Issue #3953 asks for Excel-compatible basis 1 results for DISC, INTRATE, PRICEDISC, RECEIVED and YIELDDISC when the settlement-to-maturity interval spans years of different lengths. The diff in crates…
Out of Scope Changes check		All changed files relate to the linked issue. securities.rs changes the five discount-security functions named in #3953. The removal of year_basis from day_count.rs is a consequence of this change. …

Full details: Title check

Explanation

The title names the actual change: basis 1 of the discount securities functions now uses Excel's actual/actual year length instead of the settlement year's length. The changeset replaces year_basis with year_frac in DISC, INTRATE, PRICEDISC, RECEIVED, and YIELDDISC, and adds a test for basis 1 across years. The title is specific and matches the linked issue #3953. It is long at 93 characters, and it includes an issue number, but the main change is clear.

Full details: Linked Issues check

Explanation

Issue #3953 asks for Excel-compatible basis 1 results for DISC, INTRATE, PRICEDISC, RECEIVED and YIELDDISC when the settlement-to-maturity interval spans years of different lengths. The diff in crates/functions/src/financial/securities.rs replaces day_count / year_basis() with year_frac(sys, s, m, b) in all five functions. The private disc_frac helper is removed. year_frac (day_count.rs) handles basis 1 with a whole-interval denominator. For a span within one year it uses 365 or 366 depending on whether Feb 29 falls in the interval. For a multi-year span it uses the average length of the calendar years covered. This matches the YEARFRAC basis 1 rule the issue describes. For the issue example 2024-03-01 to 2025-03-01, the denominator is 365, so DISC gives 0.05. The added test disc_basis_1_across_years_matches_excel [#3953] checks all five functions for this interval and the multi-year interval 2023-01-31 to 2024-03-31 (average year 365.5). Expected values match the derivation from year_frac. Bases 0, 2, 3 and 4 return the same values as before: year_frac uses days360 /360 for basis 0, actual/360 for basis 2, actual/365 for basis 3, and European 30/360 /360 for basis 4. The disc_args check (s < m) keeps the sign of the day count unchanged. The related basis-0 end-of-February behavior (PR #2124) is out of scope and is not required here. No coding requirement is unmet in the reviewed evidence. The test suite result (182 passed) and clippy result were not re-run in this review.

Full details: Out of Scope Changes check

Explanation

All changed files relate to the linked issue. securities.rs changes the five discount-security functions named in #3953. The removal of year_basis from day_count.rs is a consequence of this change. The summary reports no other callers, and the import list in securities.rs no longer references it. tests.rs adds a regression test for #3953 only. No unrelated functions, formatting-only edits, or behavior changes to other basis values were found in the reviewed diffs.

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: 928dec59-bc40-4a65-8f8c-ec972f065834

Commits

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

Files selected for processing: 3

• crates/functions/src/financial/day_count.rs
• crates/functions/src/financial/securities.rs
• crates/functions/src/financial/tests.rs

Files with no reviewable changes: 1

• crates/functions/src/financial/day_count.rs

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