Eliminate error calls in report periods #2457

Xitian9 · 2025-09-12T13:35:23Z

This eliminates the last few error calls that can occur in calculating the report spans of a multi-balance report.

We introduce a newtype wrapper DayPartition for PeriodData Day. By not exporting the constructor, we can ensure that all DayPartitions are well-formed.
splitSpan is moved into Hledger.Data.DayPartition and now produces Maybe DayPartition directly, rather than passing through [DateSpan].

That said, this introduced a fair amount of code churn in order to eliminate the last few error calls. If you feel that the benefit is not worth the cost, let me know.

simonmichael · 2025-09-13T08:56:13Z

I'll hold this one for now as I don't have an open bug that needs it.

If you have time, #2454 would be a great one to look at next for 1.50.1.

simonmichael · 2025-10-02T04:58:14Z

Hola @Xitian9..

This PR has two commits, so I don't understand git log's showing me just one when I check out this branch:

97a0494 (grafted, HEAD -> splitSpan, Xitian9/splitspan, pr/Xitian9/2457) dev!: lib: Refactor splitSpan to return Maybe DayPartition. (Stephen Morgan, Sat Sep 13 09:24)

could you rebase when you have a chance.

Xitian9 · 2025-10-02T12:01:03Z

Rebased.

simonmichael · 2025-10-02T18:16:48Z

hledger-lib/Hledger/Data/DayPartition.hs

+    _ -> d
+
+
+intToDay = ModifiedJulianDay . toInteger


The Modified Julian Day is a standard count of days, with zero being the day 1858-11-17.

I think this is from an earlier PR, but it sounds like it might not fit with our support of all years ? (CE currently, maybe BCE some day)

Yes, this is from an earlier commit, and yes it technically does not support all years.

On a 64-bit system this will support all years between 252 quadrillion BC (well before the development of currency) and 252 quadrillion AD (at which point we'll be more worried about powering our computers since all stars will have become black holes or white dwarfs).

On a 32-bit system, it will support all years between 5 877 751 BC and 5 881 468 AD.

We could extend support to all integers. This would incur a small performance penalty as we'd have to switch from IntMap to Map internally. I personally think the supported date ranges are fine, but happy for you to make a call.

Oh wow. So 0 to 1858 is not a problem then. Great.

simonmichael

AI gave this overview of the latest date types:

Date-related Types Overview

Basic date types (Types.hs:103-123):

EFDay - exact or flexible date (Exact Day | Flex Day)
DateSpan - optional start/end dates (DateSpan (Maybe EFDay) (Maybe EFDay))

Report period types (Types.hs:129-158):

Period - high-level periods (DayPeriod, MonthPeriod, etc.)
Interval - report intervals (Days, Weeks, Months, Quarters, Years, etc.)

Period data structures:

PeriodData a (Types.hs:756) - data for report periods + pre-period
- pdpre :: a - historical/pre-report data
- pdperiods :: IntMap a - data per period (Day as IntMap key)
DayPartition (DayPartition.hs:37) - well-formed time partition
- Newtype wrapping PeriodData Day
- Opaque constructor enforces contiguous spans
- Invariants: non-empty, contiguous, end = start-1 of next

Key functions:

splitSpan :: Interval -> DateSpan -> Maybe DayPartition - split span into
periods
lookupDayPartition :: Day -> DayPartition -> (Maybe Day, Day) - find period
containing day
dayPartitionToDateSpans :: DayPartition -> [DateSpan] - convert to list of
spans

Does it sound about right to you ?

DayPartition is PeriodData with mandatory smart constructors to guarantee invariants. Is it valuable to still have the separate PeriodData type ?

If so we should probably expand the description of both types, making things more approachable.

I have installed this locally and will try to exercise it as well.

simonmichael · 2025-10-02T21:33:25Z

Now also available as https://github.com/simonmichael/hledger/releases/tag/nightly binaries.

Xitian9 · 2025-10-02T22:29:58Z

Yes, I think that description is basically accurate, though I think only splitSpan winds up being a key function in practice.

We would still need the separate PeriodData constructor, as it also holds balance information in the Account data type. I'll have a go at expanding the descriptions.

This allows us to guarantee that the report periods are well-formed and don't contain errors (e.g. empty spans, spans not contiguous, spans not a partition). Note the underlying representation is now for disjoint spans, whereas previously the end date of a span was equal to the start date of the next span, and then was adjusted backwards one day when needed.

This eliminates all error calls from the chain calculating report periods.

simonmichael · 2025-10-08T08:02:04Z

hledger-lib/Hledger/Data/Types.hs

+--
+-- Data is stored in an 'IntMap' for efficiency, where Days are stored as as
+-- Int representing the underlying modified Julian date.
 data PeriodData a = PeriodData {


Would you mind replacing that last line with a mention of how the keys are "days since 1858-whatever".

Hmm, disregard; I am having trouble viewing the latest version of changes with VS Code.

Still, let's spell out how exactly the Int key represents the date.

I'm not sure this is the best place to do that, as that's more or less an internal detail of the Data.Time module. In the unlikely event that they decide to change their internal representation, our documentation would be out of date.

Ok, a pointer to the authoritative doc would be fine then. So a person wanting to understand this type can do so, right ?

simonmichael · 2025-10-08T08:05:41Z

hledger-lib/Hledger/Data/Types.hs

+--
+-- Data is stored in an 'IntMap' for efficiency, where Days are stored as as
+-- Int representing the underlying modified Julian date.
 data PeriodData a = PeriodData {


Still, let's spell out how exactly the Int key represents the date.

simonmichael · 2025-10-08T08:08:44Z

hledger-lib/Hledger/Data/DayPartition.hs

+-- This is a newtype wrapper around 'PeriodData Day', where the start dates are
+-- the keys and the end dates are the values. Spans are stored in inclusive format
+-- [start, end]. Note that this differs from 'DateSpan' which uses [start, end)
+-- format.


Thanks for expanding these descriptions. I think we can do better still; as a person reading this, not familiar with PeriodData (or even if I am), I find it not too clear. "Start dates are keys, end dates are values." I don't see it.

I asked Claude for some docs making it more approachable to contributors. It gave things like this which I found helpful - except for the description of the "pre-period date". Or is this slop ? Apologies if so.

-- | A partition of time into contiguous non-overlapping periods. -- -- Each period is a consecutive span of days with an inclusive start date -- and inclusive end date. Periods must be: -- -- * Contiguous: period N's end date is exactly one day before period N+1's start date -- * Ordered: periods are in chronological order -- * Non-empty: at least one period exists -- -- The constructor is hidden to maintain these invariants. Use 'boundariesToDayPartition' -- or 'splitSpan' to construct. -- -- Internally, a DayPartition wraps 'PeriodData Day', which stores: -- -- * A map of period start dates to period end dates (as IntMap Day for efficiency) -- * A "pre-period" historical date (one day before the first period starts) -- -- ==== Examples -- >>> let Just p = boundariesToMaybeDayPartition [fromGregorian 2024 01 01, fromGregorian 2024 02 01, fromGregorian 2024 03 01] -- >>> dayPartitionToList p -- [(2024-01-01,2024-01-31),(2024-02-01,2024-02-29)] newtype DayPartition = ...

-- Internal invariant checker (not exported) -- Verifies that a DayPartition maintains the required properties. -- All smart constructors should produce values that pass this check. isValidDayPartition :: DayPartition -> Bool

simonmichael · 2025-10-08T08:10:32Z

hledger-lib/Hledger/Data/DayPartition.hs

+--   1. The value stored in pdperiods has at least one key.
+--   2. The value stored in pdpre equals one day before the smallest key in pdperiods.
+--   3. The value stored in each entry of pdperiods equals one day before the
+--      next largest key, except for the value associated to the largest key.


Who needs to follow these rules - a user of this module ? When using which functions ? Would it be better to mention these as invariants of DayPartition, in DayPartition's doc ?

No, users of this module do not need to know these details. The only people who need to know are those who make constructors for DayPartition, as they need to make sure their constructors satisfy these invariants. After that point they should be invisible to users, and can interact with DayPartition through the provided interface.

And it's worth mentioning that that means only people who make commits here in this module need to know (and even not most of them).

simonmichael · 2025-10-10T01:31:13Z

I'm going to merge this and maybe follow up with another pass at haddocks. Thank you!

Xitian9 force-pushed the splitSpan branch 2 times, most recently from 169fdf7 to 97a0494 Compare September 13, 2025 07:24

Xitian9 force-pushed the splitSpan branch from 97a0494 to a3b97ff Compare October 2, 2025 11:55

simonmichael reviewed Oct 2, 2025

View reviewed changes

Xitian9 force-pushed the splitSpan branch from a3b97ff to 7ce4b79 Compare October 3, 2025 22:38

Xitian9 force-pushed the splitSpan branch from 7ce4b79 to 4a185fd Compare October 3, 2025 22:39

dev!: lib: Refactor splitSpan to return Maybe DayPartition.

27c7699

This eliminates all error calls from the chain calculating report periods.

Xitian9 force-pushed the splitSpan branch from 4a185fd to 27c7699 Compare October 3, 2025 23:21

simonmichael reviewed Oct 8, 2025

View reviewed changes

simonmichael merged commit 4e9fa16 into simonmichael:master Oct 10, 2025
1 check passed

Uh oh!

Eliminate error calls in report periods #2457

Eliminate error calls in report periods #2457

Uh oh!

Conversation

Xitian9 commented Sep 12, 2025

Uh oh!

simonmichael commented Sep 13, 2025

Uh oh!

simonmichael commented Oct 2, 2025

Uh oh!

Xitian9 commented Oct 2, 2025

Uh oh!

simonmichael Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonmichael left a comment

Choose a reason for hiding this comment

Uh oh!

simonmichael commented Oct 2, 2025

Uh oh!

Xitian9 commented Oct 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonmichael Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonmichael Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonmichael commented Oct 10, 2025

Uh oh!

Uh oh!

Uh oh!

simonmichael Oct 2, 2025 •

edited

Loading

simonmichael Oct 8, 2025 •

edited

Loading

simonmichael Oct 8, 2025 •

edited

Loading