Skip to content

fix: costs timeframes and surface limit reset settings#3709

Merged
joeyorlando merged 4 commits intomainfrom
codex/fix-costs-date-range
Apr 11, 2026
Merged

fix: costs timeframes and surface limit reset settings#3709
joeyorlando merged 4 commits intomainfrom
codex/fix-costs-date-range

Conversation

@joeyorlando
Copy link
Copy Markdown
Contributor

@joeyorlando joeyorlando commented Apr 10, 2026

Summary

  • Fix /llm/costs so custom timeframes are sent to the statistics API instead of falling back to all-time data.
  • Initialize the custom timeframe dialog from the currently selected custom range so edits are consistent.
  • Rework the Teams, Agents, LLM Proxies, and Models breakdowns so charts are followed by bounded, scrollable detail tables with sticky headers.
  • Add a visible cleanup-interval hint on /llm/limits that shows the active reset cadence and links to LLM settings.
  • Add regression tests for the custom timeframe query behavior and the limits-page settings hint.

Root Cause

The costs page converted every custom:* timeframe to all before invoking the statistics queries, so future custom ranges continued showing all-time data instead of an empty range. Separately, the limit reset cadence lived only under LLM settings, which made it easy to miss when working from the limits page.

User Impact

  • Custom date ranges on /llm/costs now drive the charts and summaries correctly, including future ranges.
  • Long cost breakdown tables are easier to scan without overwhelming the chart layout.
  • Users can discover and navigate to the limit reset interval setting directly from /llm/limits.

Validation

  • pnpm install
  • pnpm --filter @frontend test -- 'src/app/llm/(costs)/costs/page.test.tsx' --run
  • pnpm --filter @frontend test -- 'src/app/llm/(costs)/limits/page.test.tsx' --run
  • pnpm --filter @frontend type-check
  • pnpm exec biome check 'frontend/src/app/llm/(costs)/costs/page.tsx' 'frontend/src/app/llm/(costs)/costs/page.test.tsx'
  • pnpm exec biome check 'frontend/src/app/llm/(costs)/limits/page.tsx' 'frontend/src/app/llm/(costs)/limits/page.test.tsx'

@joeyorlando joeyorlando changed the title [codex] Fix cost statistics custom timeframe fix: cost statistics custom timeframe Apr 10, 2026
@joeyorlando joeyorlando marked this pull request as ready for review April 10, 2026 18:37
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 10, 2026

Claude finished @joeyorlando's task in 3m 56s —— View job


PR Review: fix: cost statistics custom timeframe

  • Read CLAUDE.md for repo conventions
  • Examine changed files (costs/page.tsx, costs/page.test.tsx, limits/page.tsx, limits/page.test.tsx)
  • Analyze logic changes for correctness
  • Check tests coverage and quality
  • Post review feedback

Overall this is a clean, focused fix. The root cause (costs page downgrading every custom:* to "all" before issuing the query) is correctly identified and the smallest possible change addresses it. The ancillary UX improvements (scrollable tables, cleanup-interval hint) are well-scoped and come with tests. LGTM with a few small observations.

Core fix — looks correct

platform/frontend/src/app/llm/(costs)/costs/page.tsx:104-115 — removing the local currentTimeframe = timeframe.startsWith("custom:") ? "all" : timeframe and passing timeframe straight through to useTeamStatistics / useProfileStatistics / useModelStatistics / useCostSavingsStatistics is exactly the right fix. The backend StatisticsTimeFrameSchema already accepts custom: template literals (platform/shared/statistics.ts:3-8), so the queries will work as intended.

Initializing customFrom / customTo from the URL-derived timeframe in the hydration effect (costs/page.tsx:120-138) is also a nice follow-up: previously the "Edit" dialog would reopen with empty dates even when the URL said otherwise. parseCustomTimeframe (costs/page.tsx:1100-1122) correctly guards against malformed values via Number.isNaN(fromDate.getTime()).

Small observations / nits

  1. getTimeframeDisplay duplicates the parsing logiccosts/page.tsx:178-196 re-implements timeframe.replace("custom:", "").split("_") and new Date(...) inline instead of reusing parseCustomTimeframe. Not a blocker, but you could shrink it to:

    const customRange = parseCustomTimeframe(tf);
    if (customRange) {
      const { from, to } = customRange;
      const hasCustomTime = from.getHours() !== 0 || from.getMinutes() !== 0 || to.getHours() !== 23 || to.getMinutes() !== 59;
      return hasCustomTime ? `${format(from, "MMM d, HH:mm")} - ${format(to, "MMM d, HH:mm")}` : `${format(from, "MMM d")} - ${format(to, "MMM d")}`;
    }

    Fix this →

  2. formatTimestamp doesn't know about short custom rangescosts/page.tsx:218-227 only switches to HH:mm formatting for timeframe === "1h" || "24h". For a custom range < 24h, it still uses MMM d on the axis. Pre-existing behavior (not introduced by this PR), but worth a future pass since custom ranges are now actually reachable.

  3. Redundant as LimitCleanupInterval cast in limits/page.tsx — At limits/page.tsx:308-311:

    const cleanupIntervalLabel =
      CLEANUP_INTERVAL_LABELS[
        (organization?.limitCleanupInterval as LimitCleanupInterval) ?? "1h"
      ];

    Per the generated API types (shared/hey-api/clients/api/types.gen.ts:34752), limitCleanupInterval is already typed as '1h' | '12h' | '24h' | '1w' | '1m' (non-null) on the org response, so both the cast and the ?? "1h" fallback are strictly unnecessary. This mirrors the existing pattern in settings/llm/page.tsx, so it's fine for consistency — just noting that the cast is load-bearing only because the value is being read as unknown-ish here.

  4. Repeated bg-card sticky top-0 z-10 on every TableHead — The sticky-header treatment is applied on each of the ~20 <TableHead> cells across the four tables. Consider extracting a small StickyTableHead component (or composing via a <TableRow className="[&>th]:bg-card [&>th]:sticky [&>th]:top-0 [&>th]:z-10"> wrapper) to cut the repetition. Again, nit — not a blocker.

Tests

  • costs/page.test.tsx directly asserts the bug fix: it sets a custom:* timeframe via useSearchParams, mocks the statistics hooks, and verifies both that the last call used the custom timeframe and that no call ever used "all" (expect(...mock.calls.some(...)).toBe(false) at costs/page.test.tsx:94-99). This is a well-targeted regression test.
  • limits/page.test.tsx verifies the cleanup-interval hint text and link, exercising the CLEANUP_INTERVAL_LABELS["1m"]"Every month" mapping and the /settings/llm link.
  • Both tests follow CLAUDE.md guidance about behavior-focused assertions over implementation details.

Minor test suggestion: you could extend the costs test to also assert that when the URL has no timeframe param, hooks are called with the default ("1h"), which would lock in the current default. Not required though.

Verdict

The fix is targeted, the regression test is exactly what's needed to prevent future reintroduction, and the supporting UI changes (bounded scrollable tables, limits-page hint) are each independently reasonable. Nothing blocking from my side.

--- • Branch: codex/fix-costs-date-range

@joeyorlando joeyorlando changed the title fix: cost statistics custom timeframe [codex] Fix costs timeframes and surface limit reset settings Apr 11, 2026
@joeyorlando joeyorlando changed the title [codex] Fix costs timeframes and surface limit reset settings fix: costs timeframes and surface limit reset settings Apr 11, 2026
@joeyorlando joeyorlando merged commit 6e4154b into main Apr 11, 2026
40 of 41 checks passed
@joeyorlando joeyorlando deleted the codex/fix-costs-date-range branch April 11, 2026 03:32
joeyorlando added a commit that referenced this pull request Apr 11, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.2.10](platform-v1.2.9...platform-v1.2.10)
(2026-04-11)


### Features

* make posthog analytics configurable
([#3707](#3707))
([7ae9101](7ae9101))


### Bug Fixes

* `/llm/costs` table scrolling
([#3722](#3722))
([6a42ba8](6a42ba8))
* apply MCP OAuth lifetime for gateway slugs
([#3711](#3711))
([362aaec](362aaec))
* Bedrock tool name encoding
([#3706](#3706))
([0e2c2d1](0e2c2d1))
* costs timeframes and surface limit reset settings
([#3709](#3709))
([6e4154b](6e4154b))
* jira oauth discovery overrides
([#3721](#3721))
([2c4cf8f](2c4cf8f))
* OIDC discovery trusted origins for IdP registration
([#3714](#3714))
([adb5f5e](adb5f5e))
* preserve shared chat agents on fork
([#3715](#3715))
([252edfc](252edfc))
* reranker model dropdown labels
([#3704](#3704))
([ebd1c8a](ebd1c8a))
* session logs loading state
([#3712](#3712))
([ffba126](ffba126))


### Miscellaneous Chores

* **ci:** add ID-JAG MCP e2e test
([#3702](#3702))
([1a5078a](1a5078a))
* **deps:** bump next from 16.1.7 to 16.2.3 in /platform/frontend
([#3708](#3708))
([d47967c](d47967c))
* use neutral token prefixes with legacy support
([#3719](#3719))
([db5929c](db5929c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: archestra-ci[bot] <222894074+archestra-ci[bot]@users.noreply.github.com>
Co-authored-by: Joey Orlando <joey@archestra.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant