a window is a count and a unit, so 90d and all answer

The relative half of ParseWindow was a table of four spellings. A window nobody had enumerated — 14d, 90d, all — was an unknown range and a 400, on the surface an operator reads to see where money went. The stores behind these series keep two years, so the door was refusing questions the data could answer. Read the count off the label instead: any <N>h or <N>d resolves, day/week/month still name the common ones, and all reaches back to Horizon, the retention those stores keep. Every label the table admitted resolves to the very same window, so the change only adds. Past the horizon is still a refusal, by name, rather than a window served short: on a series that carries money a quiet zero reads as "nothing was spent".
2026-10-01 00:45:52 +00:00 · 2026-08-13 23:56:10 -07:00
parent 2381673958
commit 1e81bc7b64
2 changed files with 204 additions and 19 deletions
@@ -1,6 +1,7 @@
 package types

 import (
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
@@ -41,12 +42,25 @@ type Window struct {
 	Interval Interval
 }

+// Horizon is how far back a window may reach: the retention the warehouses
+// behind these series keep. "all" resolves to exactly this, so a caller asking
+// for everything is told the real span it got rather than an open bound the
+// store cannot honour.
+const Horizon = 2 * 365 * 24 * time.Hour
+
 // ParseWindow resolves a range request into an absolute window.
 //
-// "24h", "7d" and "30d", with their aliases, are relative to now. "custom"
-// reads start and end, each RFC3339 or unix seconds; end defaults to now and
-// must follow start. An unrecognised label is an error rather than a silent
-// default, so a typo cannot quietly change the window a caller is shown.
+// A relative label is a count and a unit — "24h", "90d", any <N>h or <N>d up to
+// Horizon — so a window nobody enumerated still resolves. "day", "week" and
+// "month" name the common ones, "all" reaches back to Horizon, and "custom"
+// reads start and end, each RFC3339 or unix seconds, end defaulting to now and
+// having to follow start.
+//
+// A label that is neither is an error rather than a silent default, so a typo
+// cannot quietly change the window a caller is shown, and a span past Horizon
+// is refused by its own name rather than served short: on a series that carries
+// money, a window that answers less than it was asked for is worse than one
+// that refuses.
 //
 // now is a parameter so the window is a function of the arguments alone.
 func ParseWindow(label, start, end string, now time.Time) (Window, error) {
@@ -57,14 +71,7 @@ func ParseWindow(label, start, end string, now time.Time) (Window, error) {
 		w.Label = "24h"
 	}

-	switch strings.ToLower(strings.TrimSpace(label)) {
-	case "", "24h", "1d", "day", "today":
-		w.Start, w.End, w.Interval = now.Add(-24*time.Hour), now, Hour
-	case "7d", "week":
-		w.Start, w.End, w.Interval = now.Add(-7*24*time.Hour), now, Day
-	case "30d", "month":
-		w.Start, w.End, w.Interval = now.Add(-30*24*time.Hour), now, Day
-	case "custom":
+	if strings.EqualFold(w.Label, "custom") {
 		from, err := parseTime(start)
 		if err != nil {
 			return Window{}, fmt.Errorf("custom range requires a valid start: %w", err)
@@ -78,17 +85,79 @@ func ParseWindow(label, start, end string, now time.Time) (Window, error) {
 		if !to.After(from) {
 			return Window{}, fmt.Errorf("custom range end must be after start")
 		}
-		w.Start, w.End, w.Interval = from.UTC(), to.UTC(), Hour
-		if w.End.Sub(w.Start) > 48*time.Hour {
-			w.Interval = Day
-		}
-	default:
-		return Window{}, fmt.Errorf("unknown range %q", label)
+		w.Start, w.End = from.UTC(), to.UTC()
+		w.Interval = bucket(w.End.Sub(w.Start))
+		return w, nil
 	}

+	back, err := span(w.Label)
+	if errors.Is(err, errPastHorizon) {
+		return Window{}, fmt.Errorf("range %q reaches past the %d-day horizon; %q is the longest window", label, int(Horizon/(24*time.Hour)), "all")
+	}
+	if err != nil {
+		return Window{}, fmt.Errorf("unknown range %q", label)
+	}
+	w.Start, w.End, w.Interval = now.Add(-back), now, bucket(back)
 	return w, nil
 }

+// The two ways a relative label fails, distinguished because they mean opposite
+// things to a caller: one is a typo, the other is a window the stores behind
+// these series do not go back far enough to answer.
+var (
+	errUnknown     = errors.New("unknown range")
+	errPastHorizon = errors.New("range reaches past the horizon")
+)
+
+// span is how far back a relative label reaches. The named windows are the
+// counts worth a word; every other count is read off the label itself.
+func span(label string) (time.Duration, error) {
+	unit := time.Hour
+	count := 0
+
+	switch strings.ToLower(label) {
+	case "today", "day":
+		count = 24
+	case "week":
+		count, unit = 7, 24*time.Hour
+	case "month":
+		count, unit = 30, 24*time.Hour
+	case "all":
+		return Horizon, nil
+	default:
+		digits, u := label[:len(label)-1], label[len(label)-1]
+		switch u {
+		case 'd', 'D':
+			unit = 24 * time.Hour
+		case 'h', 'H':
+			unit = time.Hour
+		default:
+			return 0, errUnknown
+		}
+		n, err := strconv.Atoi(digits)
+		if err != nil || n <= 0 || digits[0] < '0' || digits[0] > '9' {
+			return 0, errUnknown
+		}
+		count = n
+	}
+
+	// Compared before multiplying, so a count no store could serve is refused
+	// rather than overflowing into a window that looks small.
+	if int64(count) > int64(Horizon/unit) {
+		return 0, errPastHorizon
+	}
+	return time.Duration(count) * unit, nil
+}
+
+// bucket is the interval a span of this width buckets to. One rule for every
+// window, named or counted, so two ranges of the same length cannot disagree.
+func bucket(d time.Duration) Interval {
+	if d > 48*time.Hour {
+		return Day
+	}
+	return Hour
+}
+
 // parseTime accepts RFC3339 or a unix-seconds string.
 func parseTime(s string) (time.Time, error) {
 	s = strings.TrimSpace(s)
@@ -235,7 +235,7 @@ func resolveLegacy(rangeLabel, startStr, endStr string, now time.Time) (start, e
 func TestParseWindowMatchesTheReplacedImplementation(t *testing.T) {
 	labels := []string{
 		"", "   ", "24h", "1d", "day", "today", "7d", "week", "30d", "month",
-		"24H", " 7d ", "custom", "CUSTOM", "yesterday", "1h", "all",
+		"24H", " 7d ", "custom", "CUSTOM", "yesterday",
 	}
 	bounds := []string{
 		"", "   ", "2026-07-01T00:00:00Z", "2026-07-25T12:00:00Z", "2026-07-26T00:00:00Z",
@@ -287,3 +287,119 @@ func legacyStep(interval string) time.Duration {
 	}
 	return time.Hour
 }
+
+// A count and a unit resolve on their own, so a window nobody enumerated still
+// answers. These are the ones a spend view asks for; the point is that the set
+// is not a list anyone has to remember to extend.
+func TestParseWindowReadsTheCount(t *testing.T) {
+	for _, tc := range []struct {
+		label string
+		back  time.Duration
+		want  Interval
+	}{
+		{"1h", time.Hour, Hour},
+		{"6h", 6 * time.Hour, Hour},
+		{"12h", 12 * time.Hour, Hour},
+		{"48h", 48 * time.Hour, Hour},
+		{"49h", 49 * time.Hour, Day},
+		{"14d", 14 * 24 * time.Hour, Day},
+		{"60d", 60 * 24 * time.Hour, Day},
+		{"90d", 90 * 24 * time.Hour, Day},
+		{"180d", 180 * 24 * time.Hour, Day},
+		{"365d", 365 * 24 * time.Hour, Day},
+		{"90D", 90 * 24 * time.Hour, Day},
+		{"  90d  ", 90 * 24 * time.Hour, Day},
+		{"730d", Horizon, Day},
+		{"17520h", Horizon, Day},
+		{"all", Horizon, Day},
+		{"ALL", Horizon, Day},
+	} {
+		t.Run(strings.TrimSpace(tc.label), func(t *testing.T) {
+			w, err := ParseWindow(tc.label, "", "", now)
+			if err != nil {
+				t.Fatalf("ParseWindow(%q) errored: %v", tc.label, err)
+			}
+			if !w.End.Equal(now) {
+				t.Errorf("End = %v, want %v", w.End, now)
+			}
+			if !w.Start.Equal(now.Add(-tc.back)) {
+				t.Errorf("Start = %v, want %v (span %v, want %v)", w.Start, now.Add(-tc.back), w.End.Sub(w.Start), tc.back)
+			}
+			if w.Interval != tc.want {
+				t.Errorf("Interval = %q, want %q", w.Interval, tc.want)
+			}
+		})
+	}
+}
+
+// Past the horizon the answer would be a window the store cannot fill, so it is
+// refused by name. Serving it short would read as a quiet zero on a spend view.
+func TestParseWindowRefusesPastTheHorizon(t *testing.T) {
+	for _, label := range []string{"731d", "1000d", "17521h", "3650d"} {
+		t.Run(label, func(t *testing.T) {
+			w, err := ParseWindow(label, "", "", now)
+			if err == nil {
+				t.Fatalf("want a refusal, got window %+v", w)
+			}
+			want := fmt.Sprintf("range %q reaches past the 730-day horizon; \"all\" is the longest window", label)
+			if err.Error() != want {
+				t.Errorf("error = %q, want %q", err.Error(), want)
+			}
+			if w != (Window{}) {
+				t.Errorf("refused window must be zero, got %+v", w)
+			}
+		})
+	}
+}
+
+// A count is read, not guessed: a unit this grammar does not carry stays an
+// unknown range rather than becoming some nearby window.
+func TestParseWindowStillRefusesNonCounts(t *testing.T) {
+	for _, label := range []string{"1w", "3mo", "0d", "0h", "-5d", "+5d", "d", "h", "lastweek", "1.5d", "d7"} {
+		t.Run(label, func(t *testing.T) {
+			w, err := ParseWindow(label, "", "", now)
+			if err == nil {
+				t.Fatalf("want a refusal, got window %+v", w)
+			}
+			if want := fmt.Sprintf("unknown range %q", label); err.Error() != want {
+				t.Errorf("error = %q, want %q", err.Error(), want)
+			}
+			if w != (Window{}) {
+				t.Errorf("refused window must be zero, got %+v", w)
+			}
+		})
+	}
+}
+
+// The widening is strictly additive: every label the replaced implementation
+// admitted still resolves to the very same window (the oracle test above), and
+// every label this one adds is one that used to be a 400. Nothing quietly
+// changed meaning underneath a caller.
+func TestParseWindowOnlyAddsToWhatWasAdmitted(t *testing.T) {
+	for _, label := range []string{"1h", "6h", "14d", "90d", "180d", "365d", "all"} {
+		t.Run(label, func(t *testing.T) {
+			if _, _, _, err := resolveLegacy(label, "", "", now); err == nil {
+				t.Fatalf("%q was already admitted; it is not part of the widening", label)
+			}
+			if _, err := ParseWindow(label, "", "", now); err != nil {
+				t.Fatalf("ParseWindow(%q) still refuses: %v", label, err)
+			}
+		})
+	}
+}
+
+// Horizon is the retention the warehouses behind these series keep, and "all"
+// has to be exactly it — an "all" longer than the store would report empty
+// buckets as though nothing had been spent in them.
+func TestAllIsTheHorizon(t *testing.T) {
+	w, err := ParseWindow("all", "", "", now)
+	if err != nil {
+		t.Fatalf("errored: %v", err)
+	}
+	if got := w.End.Sub(w.Start); got != Horizon {
+		t.Errorf("all spans %v, want the %v horizon", got, Horizon)
+	}
+	if got, want := int(Horizon/(24*time.Hour)), 730; got != want {
+		t.Errorf("horizon = %d days, want %d", got, want)
+	}
+}