Read a YEARFRAC span the way Excel does, on every basis - #5808
Open
habdelra wants to merge 4 commits into
Open
Conversation
Basis 4 pulls a day-31 back to the 30th at both ends of the span before differencing. The arm reached the day numbers untouched, so neither endpoint moved and the count was a raw 30/360 — a day too many whenever one end fell on the 31st and the other did not. Coverage cases pin a day-31 start, a day-31 end, the span with both where the two shifts cancel, and that day-31 end on basis 0, whose US rule reaches the end date only once the start has landed on the 30th. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes YEARFRAC basis 4 to correctly implement the European 30/360 convention by clamping day-31 to day-30 at both ends of the span before differencing, aligning behavior with Excel-compatible implementations and ensuring downstream financial functions that rely on YEARFRAC (e.g., DISC, PRICEDISC, ACCRINT) inherit the corrected behavior.
Changes:
- Correct
yearFrac(..., basis=4)to clamp start/end day 31 → 30 before computing the 30/360 difference. - Add function-coverage fixture cases covering basis 4 (start=31, end=31, both ends=31) and contrasting with basis 0 behavior.
- Document the upstream divergence and note the fix in changelog and upstream-diffs documentation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/bxl/src/formulajs/dateSerial.ts | Fixes YEARFRAC basis 4 by applying European 30/360 clamping rules before computing the fraction. |
| packages/bxl/tests/unit/fixtures/function-coverage/formula-date.ts | Adds coverage cases that distinguish European 30/360 basis 4 behavior from raw 30/360 and from US 30/360 basis 0. |
| packages/bxl/src/formulajs/UPSTREAM-DIFFS.md | Records the YEARFRAC (basis 4) divergence from upstream and explains the corrected behavior. |
| packages/bxl/CHANGELOG.md | Adds a changelog entry for the basis 4 fix (but includes a small indentation regression to address). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The earlier of the two dates opens the span whichever argument carried it, and a time of day is no part of a day count. Both reach past the sign: bases 0 and 1 read their endpoints asymmetrically, so on a reversed pair basis 0 pulled the wrong end onto the 30th and basis 1 averaged over a year count of zero, answering NaN. Basis 0 also gains the two February clauses it lacked. A span opening on the last day of February counts that February as a whole 30-day month, and closes the same way only when it ends on a February month end too. DISC and PRICEDISC raise #NUM! unless settlement precedes maturity, as the TBILL family already did, and parse their basis rather than coercing it, so a basis that is not a number is an error instead of a silent US 30/360. ACCRINT truncates its frequency and basis, as every other function taking them does. Four Excel-compatible references agree on all of it: LibreOffice's GetYearFrac, Apache POI's YearFracCalculator, ExcelFinancialFunctions' dateDiff360Eu, and David Wheeler's specification derived from Excel itself. Over 800 February-heavy spans across all five bases, none disagrees. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
habdelra
changed the base branch from
cs-12532-bxl-coverage-gate-hardening
to
main
August 18, 2026 16:19
Both sides reached the same reading of the 30/360 conventions from different directions, so the code merged without conflict: the bond schedule's US 30/360 carries the last-day-of-February rules and its European 30/360 does not, which is what YEARFRAC's basis 0 and basis 4 now do. Measured over 3.27 million ordered date pairs, basis 0 and `couponDays360` agree on every one. The two changelog and upstream-diffs conflicts were both additive. The `ACCRINT` entry that said this reading carries into it is corrected: `ACCRINT` counts its own schedule and no longer divides by a year fraction, so `DISC` and `PRICEDISC` are the two that inherit these. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The European 30/360 is `DAYS360`'s European method exactly, so basis 4 counts through the same helper rather than carrying a second copy of the clamp. Basis 0 keeps its own ladder: it reads the bond schedule's rules, including the February clauses `DAYS360` does not carry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
YEARFRACis the day-count primitiveDISCandPRICEDISCdivide by. It read its arguments in four ways Excel does not, so this fixes all four in the one function, and closes three argument-checking holes in the callers.What it read wrong
Basis 4 was a raw 30/360, not the European 30/360 it names. The convention moves a day-31 endpoint onto the 30th at both ends before differencing. The arm reached the day numbers untouched, so neither moved — and the error is a day in whichever direction the 31 sits, since pulling the start back lengthens a span while pulling the end back shortens it.
That last row is the span that agrees either way — 31 at both ends shifts twice and the two shifts cancel — which is every coupon-period span on a month-end schedule.
Basis 0 had no February rule. The US 30/360 counts a February that a span opens on as a whole 30-day month, and closes the same way only when the span also ends on a February month end.
YEARFRAC(DATE(2025, 2, 28), DATE(2026, 9, 12), 0)counted 554 days where it should count 552.The arguments were read in the order given rather than as a span. A start later than the end returned a negative fraction, and because bases 0 and 1 read their two endpoints asymmetrically, the magnitude was wrong too — basis 0 pulled the wrong end onto the 30th, and basis 1 fell into its multi-year branch and averaged over a year count of zero, so
YEARFRAC(DATE(2019, 5, 30), DATE(2018, 6, 29), 1)returnedNaN. That one is worse than a wrong number:NaNsits outside the Excel error model, soISNUMBERanswers false, arithmetic propagates it, and it is not JSON-representable.A time of day counted as part of a day count. A partial day was carried up to a whole one, so
YEARFRAC(TODAY(), NOW(), 1)was a day rather than nothing. This is the shape any call reading the clock arrives in.The callers
DISCandPRICEDISCnow raise#NUM!unless settlement precedes maturity, as theirTBILLsiblings already did — a year fraction measures how long a span is rather than which way it runs, so it cannot report a transposed pair on their behalf. Before, a transposed pair answered a negative discount, a settlement on the maturity date answeredInfinity, andPRICEDISCanswered a price above redemption. Both also parse their basis instead of coercing it, so a non-numeric basis is a#VALUE!rather than a silent US 30/360; andACCRINTtruncates its frequency and basis, as every other function taking them does.What settles the rules
Four independent Excel-compatible implementations agree on every rule here — the two clamps, the February clauses, the swap, and the truncation:
scaddins/source/analysis/analysishelper.cxx,GetYearFracYearFracCalculator, whose algorithm follows Wheeler's spec and whose expectations come from Excel-produced spreadsheetsfsprojects/ExcelFinancialFunctionsdaycountbasis.fs, COM-tested against ExcelYEARFRACspecification, reverse-engineered from a 32.9-million-case cross-product against Excel itself, which states the truncation and the swap outright and notes for basis 4 that February 28th and 29th are not to be touched at allCross-checked against a headless LibreOffice recalc over 800 spans biased toward February and month ends, on all five bases: 0 disagreements, against 405–458 per basis before.
One caution recorded in
UPSTREAM-DIFFS.mdfor whoever runs that differential next: on basis 4 LibreOffice's owngetDiscdivides by the clamped count while itsgetPricediscandgetAccrintmultiply by the unclamped one, soPRICEDISCandACCRINTpart from LibreOffice on basis 4.ExcelFinancialFunctionsuses its clamped counter throughout, which is the shape here.Against main
Merged rather than rebased.
ACCRINT's own work landed on main meanwhile, and the two arrived at the same reading of these conventions from opposite ends: its bond-schedule US 30/360 carries the last-day-of-February rules and its European 30/360 does not, which is exactly what basis 0 and basis 4 do here. The code merged without conflict, and basis 0 agrees with that schedule'scouponDays360on every one of 3,270,403 ordered date pairs — two implementations written independently, landing on the same count.That also means the rule was written out twice, so basis 4 now counts through the
days360helper main introduced rather than keeping its own copy of the clamp. Basis 0 keeps its ladder: unifying it as well would have to movecouponDays360andlastDayOfMonthout offinancial.tsand past their other callers, which is more than a merge should carry.Only the two markdown files conflicted, both additively. The changelog entry claiming this reading carries into
ACCRINTis corrected there —ACCRINTno longer divides by a year fraction at all.Cases
Seventeen coverage cases, and each is mutation-checked — nine mutations, every one caught by exactly the cases meant to catch it:
DAYS360European driftACCRINTuntruncated#VALUE!and fractional-basis casesExpected values are written as the arithmetic they come from (
394 / 360,1000 * 0.1 * (75 / 360)) rather than as decimals, so the derivation is visible and no tolerance is needed. The suite is at 1150 cases across 6 time zones, 829 of 830 functions, and the one remaining known defect isACCRINTon basis 1.The markdown tables in
UPSTREAM-DIFFS.mdre-pad their columns when the pre-commit hook runs prettier over them; the only content changes there are the two new rows and the two notes.