feat(api): add token usage timeseries endpoint for metrics charting - #225
Conversation
Add new /api/projects/{id}/metrics/tokens/timeseries endpoint that
returns token usage data aggregated by time intervals (hour, day, week)
for visualization in the CostDashboard charts.
Changes:
- Add get_token_usage_timeseries() method to MetricsTracker for
bucket-based aggregation with configurable intervals
- Create timeseries API endpoint with date range validation
- Add date filtering support to existing /metrics/costs endpoint
- Add flexible date parsing that accepts both ISO 8601 and yyyy-MM-dd
formats for frontend compatibility
- Add comprehensive TDD test suite (13 new tests)
The endpoint fixes 404 errors when users select date range filters
(last-7-days, last-30-days) in the CostDashboard component.
WalkthroughAdds date-range filtering to project cost calculations and a new token-usage timeseries feature (hour/day/week buckets). API routes and parsing helpers are added/updated, core metrics logic extended for time-based aggregation, and comprehensive endpoint tests were introduced. Changes
Sequence DiagramsequenceDiagram
participant Client
participant API as API Router
participant MT as MetricsTracker
participant DB as Database
Client->>API: GET /api/projects/{id}/metrics/tokens/timeseries?start_date=...&end_date=...&interval=day
API->>API: Parse dates & validate interval
API->>API: Verify project access
API->>MT: get_token_usage_timeseries(project_id, start_date, end_date, interval)
MT->>DB: Fetch usage records filtered by date range
DB-->>MT: Return usage records
MT->>MT: Bucket by hour/day/week, aggregate tokens & costs
MT-->>API: Return timeseries list
API-->>Client: JSON timeseries response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Code Review - PR #225SummaryThis PR implements a missing Critical Issues1. Timezone Handling Inconsistency (codeframe/lib/metrics_tracker.py:536-538)The timestamp parsing logic doesn't preserve timezone information properly. If the database returns a datetime object without timezone info (naive datetime), this code won't add timezone. This can cause timezone-naive/aware comparison errors. Recommendation: Add handling for naive datetimes: if isinstance(timestamp, str):
timestamp = datetime.fromisoformat(timestamp.replace("Z", "+00:00"))
elif timestamp.tzinfo is None:
# Assume UTC for naive datetimes from database
timestamp = timestamp.replace(tzinfo=timezone.utc)2. Missing Timezone in _parse_date() (codeframe/ui/routers/metrics.py:137-166)The helper imports Code Quality Issues3. Redundant Default Handling (codeframe/lib/metrics_tracker.py:594-596)The 4. Inconsistent Error MessagesThe error messages for invalid intervals should reference a shared constant to ensure consistency if updated in the future. Performance Considerations5. Potential N+1 Query PatternWhile the current in-memory aggregation is efficient, for large datasets (100k+ records), this could become memory-intensive. The current decision is acceptable for MVP but should be noted for future optimization with database-level aggregation using SQL GROUP BY. Test Coverage6. Missing Edge Case TestsThe test suite is comprehensive (13 new tests), but missing:
Recommendation: Add at least one test for invalid date format to ensure proper error handling. Security Considerations7. Authorization Checks - Correct ✓The endpoint properly checks project existence, user access via Documentation Quality8. Excellent Docstrings ✓The docstrings are comprehensive and follow Google style with clear parameter descriptions, return value structure, raises section, and usage examples. The timeseries endpoint docstring (lines 176-219) is exemplary. Best Practices Alignment9. Follows Project Conventions ✓
SummaryStrengths:
Must Fix Before Merge:
Should Fix:
Future Enhancements:
VerdictApprove with changes - The implementation is solid and well-tested. Address the critical timezone issues (#1, #2) before merging. The other issues are nice-to-haves that can be addressed in follow-up PRs if needed. Great work on the comprehensive testing and documentation! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @codeframe/lib/metrics_tracker.py:
- Around line 533-559: The loop that parses record["timestamp"] can yield
timezone-naive datetimes which will later cause incorrect bucket keys from
_get_bucket_key; after parsing (the code that uses datetime.fromisoformat on
record["timestamp"]) validate timestamp.tzinfo and either raise a clear
ValueError noting the offending record or normalize naive timestamps to UTC by
setting tzinfo=timezone.utc; update the block that assigns/overwrites timestamp
before calling self._get_bucket_key to perform this check/normalization so all
downstream aggregation uses timezone-aware datetimes.
🧹 Nitpick comments (4)
codeframe/ui/routers/metrics.py (2)
137-167: Consider moving the import to module level and handling edge cases.
- The
from datetime import timezone as tzimport inside the function should be at the module level for consistency and slight performance improvement.- The ISO 8601 parsing branch (line 155) doesn't validate the format—malformed strings like
"TT"would pass the"T" in date_strcheck but fail with an unclear error fromfromisoformat.♻️ Suggested improvement
import logging -from datetime import datetime +from datetime import datetime, timezone from typing import OptionalThen in the function:
def _parse_date(date_str: str) -> datetime: # Try full ISO 8601 format first if "T" in date_str: - return datetime.fromisoformat(date_str.replace("Z", "+00:00")) + try: + return datetime.fromisoformat(date_str.replace("Z", "+00:00")) + except ValueError: + raise ValueError( + f"Invalid date format: '{date_str}'. " + "Use ISO 8601 format (e.g., '2025-01-01T00:00:00Z' or '2025-01-01')" + ) # Try date-only format (yyyy-MM-dd) try: - from datetime import timezone as tz - parsed = datetime.strptime(date_str, "%Y-%m-%d") - return parsed.replace(tzinfo=tz.utc) + return parsed.replace(tzinfo=timezone.utc) except ValueError: raise ValueError( f"Invalid date format: '{date_str}'. " "Use ISO 8601 format (e.g., '2025-01-01T00:00:00Z' or '2025-01-01')" )
216-220: Consider validating that start_date is before end_date.The endpoint doesn't verify that
start_date <= end_date. While the backend may handle this gracefully (returning empty results), an explicit validation with a clear error message would improve the developer experience.♻️ Optional validation
# Parse and validate date parameters try: start_dt = _parse_date(start_date) end_dt = _parse_date(end_date) except ValueError as e: raise HTTPException(status_code=400, detail=str(e)) + # Validate date range + if start_dt > end_dt: + raise HTTPException( + status_code=400, + detail="start_date must be before or equal to end_date" + ) + # Validate intervaltests/api/test_api_metrics.py (2)
417-441: Consider strengthening the aggregation assertion.The test verifies tokens are greater than 0 but doesn't validate the expected total. Given the fixture creates 3 records with known values (3500 input + 1750 output = 5250 total), you could assert the exact sum for stronger validation.
💡 Stronger assertion
# Verify total tokens across all data points matches expected total_input = sum(point["input_tokens"] for point in data) total_output = sum(point["output_tokens"] for point in data) - # We have 3 records: today (1500 tokens), yesterday (750), two days ago (3000) - # Filtering to last 3 days should include all - assert total_input + total_output > 0 + # We have 3 records: 1000+500 + 500+250 + 2000+1000 = 5250 total + assert total_input == 3500 # 1000 + 500 + 2000 + assert total_output == 1750 # 500 + 250 + 1000
25-89: Consider adding a test for invalid date format on the costs endpoint.The
TestProjectCostMetricsDateFilteringclass doesn't include a test for invalid date formats (e.g.,?start_date=invalid). WhileTestProjectTokenMetricsEndpointhas this test, adding one for costs would ensure consistent error handling.💡 Add test for invalid date format
def test_invalid_date_format_returns_400(self, api_client, project_with_token_usage): """Test that invalid date format returns 400 Bad Request.""" project_id, _ = project_with_token_usage response = api_client.get( f"/api/projects/{project_id}/metrics/costs?start_date=invalid-date" ) assert response.status_code == 400
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
codeframe/lib/metrics_tracker.pycodeframe/ui/routers/metrics.pytests/api/test_api_metrics.py
🧰 Additional context used
📓 Path-based instructions (1)
codeframe/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/**/*.py: Use Python 3.11+ for backend development with FastAPI, AsyncAnthropic, SQLite with async support (aiosqlite), and tiktoken for token counting
Use token counting via tiktoken library for token budget management with ~50,000 token limit per conversation
Use asyncio patterns with AsyncAnthropic for async/await in Python backend for concurrent operations
Implement quality gates with multi-stage pre-completion checks (tests → type → coverage → review) and Git + SQLite + context snapshots for project state rollback
Use tiered memory system (HOT/WARM/COLD) with importance scoring using hybrid exponential decay algorithm for context management with 30-50% token reduction
Implement session lifecycle management with auto-save/restore using file-based storage at .codeframe/session_state.json
Files:
codeframe/lib/metrics_tracker.pycodeframe/ui/routers/metrics.py
🧬 Code graph analysis (2)
codeframe/lib/metrics_tracker.py (1)
codeframe/persistence/database.py (1)
get_token_usage(658-660)
tests/api/test_api_metrics.py (1)
tests/api/conftest.py (1)
api_client(67-177)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Frontend Unit Tests
- GitHub Check: Backend Unit Tests
- GitHub Check: E2E Smoke Tests (Chromium)
- GitHub Check: claude-review
🔇 Additional comments (9)
codeframe/ui/routers/metrics.py (2)
170-220: LGTM! Well-structured endpoint with proper validation.The timeseries endpoint correctly:
- Validates required date parameters before project lookup (fail-fast)
- Checks project existence and authorization
- Validates interval against allowed values
- Uses appropriate HTTP status codes (400, 403, 404, 500)
The docstring is comprehensive with clear examples.
275-366: Good integration of date filtering into the existing endpoint.The costs endpoint properly:
- Maintains backward compatibility (dates are optional)
- Reuses the
_parse_datehelper for consistent parsing- Passes parsed dates to the tracker
The approach is clean and consistent with the timeseries endpoint.
codeframe/lib/metrics_tracker.py (3)
205-247: LGTM! Clean extension for date filtering.The method signature and implementation correctly extend the existing functionality with optional date parameters. The filtering is delegated to the database layer, keeping the tracker focused on aggregation logic.
471-528: LGTM! Well-documented timeseries aggregation method.The method includes:
- Clear docstring with examples
- Upfront interval validation
- Proper delegation to database for filtered data
- Empty list return for no records (clean API)
588-596: Week bucket calculation handles edge cases correctly.The ISO week calculation correctly uses
weekday()(0=Monday) and subtracts days to get to Monday. This properly handles week boundaries.tests/api/test_api_metrics.py (4)
374-388: LGTM! Good basic endpoint availability test.The test correctly uses date-only format and verifies the endpoint returns 200.
390-416: LGTM! Comprehensive structure validation.The test properly validates that each data point in the response contains all required fields:
timestamp,input_tokens,output_tokens,total_tokens, andcost_usd.
486-500: Good coverage of required parameter validation.Testing both missing
start_dateand missingend_datescenarios ensures the endpoint properly enforces required parameters.
540-593: LGTM! Thorough date filtering test coverage.The tests properly cover:
- Filtered vs all-time cost comparison
- ISO 8601 format acceptance
- Backward compatibility (no date params)
This ensures the existing API contract is maintained while adding the new functionality.
Code review fixes: - Add timezone handling for naive datetimes from database - Fix import inconsistency in _parse_date (use module-level timezone) - Replace redundant else clause with defensive ValueError in _get_bucket_key - Add 3 edge case tests for invalid date formats (invalid month, day, malformed) All 30 metrics API tests pass.
Code Review - PR #225SummaryThis PR implements the missing Critical Issues1. Timezone Handling Inconsistency (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
codeframe/ui/routers/metrics.py (2)
137-166: Good date parsing implementation with flexible format support.The function correctly handles both ISO 8601 with time and date-only formats, ensuring timezone-aware datetimes are returned. The error handling is functional, though there's a minor inconsistency in error messages between the two paths:
- Date-only format errors (caught at line 161): Custom error message from lines 162-165
- ISO 8601 format errors (line 155): Default
fromisoformaterror messageBoth errors are properly caught by callers, so this doesn't affect functionality.
♻️ Optional: Wrap ISO parsing for consistent error messages
def _parse_date(date_str: str) -> datetime: """Parse a date string in multiple formats. Accepts: - ISO 8601 with time: '2025-01-01T00:00:00Z' - Date only: '2025-01-01' (converts to start of day UTC) Args: date_str: Date string to parse Returns: Parsed datetime object Raises: ValueError: If date format is invalid """ - # Try full ISO 8601 format first - if "T" in date_str: - return datetime.fromisoformat(date_str.replace("Z", "+00:00")) - - # Try date-only format (yyyy-MM-dd) try: + # Try full ISO 8601 format first + if "T" in date_str: + return datetime.fromisoformat(date_str.replace("Z", "+00:00")) + + # Try date-only format (yyyy-MM-dd) parsed = datetime.strptime(date_str, "%Y-%m-%d") return parsed.replace(tzinfo=timezone.utc) except ValueError: raise ValueError( f"Invalid date format: '{date_str}'. " "Use ISO 8601 format (e.g., '2025-01-01T00:00:00Z' or '2025-01-01')" )
168-271: Excellent implementation of the timeseries endpoint.The endpoint is well-structured with:
- Proper validation of required parameters and interval values
- Authorization checks before data access
- Comprehensive error handling with appropriate HTTP status codes
- Clear logging for observability
The implementation correctly delegates to
MetricsTracker.get_token_usage_timeseriesand handles all error cases appropriately.♻️ Optional: Add date range order validation
Consider adding validation to ensure start_date <= end_date for better user feedback:
# Parse and validate date parameters try: start_dt = _parse_date(start_date) end_dt = _parse_date(end_date) except ValueError as e: raise HTTPException(status_code=400, detail=str(e)) + + # Validate date range order + if start_dt > end_dt: + raise HTTPException( + status_code=400, + detail=f"start_date must be before or equal to end_date" + )This provides clearer feedback vs. returning an empty result set.
codeframe/lib/metrics_tracker.py (1)
575-602: Solid bucket key calculation with correct time truncation logic.The helper correctly calculates bucket start times for all three intervals:
- Hour: Truncates to start of hour
- Day: Truncates to start of day
- Week: Truncates to ISO week start (Monday) using
weekday()The defensive
ValueErrorat line 599 is good practice, even though validation inget_token_usage_timeseriesshould prevent reaching this path.♻️ Optional: Add explicit UTC conversion for defensive programming
While the current implementation should be safe given the timezone handling in
get_token_usage_timeseries(lines 539-541), explicitly converting to UTC before formatting would make the code more robust against future changes:else: # This should never be reached due to validation in get_token_usage_timeseries raise ValueError(f"Invalid interval: {interval}") - # Return ISO format with Z suffix for UTC - return bucket_start.strftime("%Y-%m-%dT%H:%M:%SZ") + # Return ISO format with Z suffix for UTC (ensure UTC conversion) + return bucket_start.astimezone(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")This ensures the "Z" suffix always represents actual UTC time, regardless of the input datetime's timezone.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
codeframe/lib/metrics_tracker.pycodeframe/ui/routers/metrics.pytests/api/test_api_metrics.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/api/test_api_metrics.py
🧰 Additional context used
📓 Path-based instructions (1)
codeframe/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/**/*.py: Use Python 3.11+ for backend development with FastAPI, AsyncAnthropic, SQLite with async support (aiosqlite), and tiktoken for token counting
Use token counting via tiktoken library for token budget management with ~50,000 token limit per conversation
Use asyncio patterns with AsyncAnthropic for async/await in Python backend for concurrent operations
Implement quality gates with multi-stage pre-completion checks (tests → type → coverage → review) and Git + SQLite + context snapshots for project state rollback
Use tiered memory system (HOT/WARM/COLD) with importance scoring using hybrid exponential decay algorithm for context management with 30-50% token reduction
Implement session lifecycle management with auto-save/restore using file-based storage at .codeframe/session_state.json
Files:
codeframe/lib/metrics_tracker.pycodeframe/ui/routers/metrics.py
🧬 Code graph analysis (2)
codeframe/lib/metrics_tracker.py (1)
codeframe/persistence/database.py (1)
get_token_usage(658-660)
codeframe/ui/routers/metrics.py (1)
codeframe/lib/metrics_tracker.py (1)
get_token_usage_timeseries(471-573)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Frontend Unit Tests
- GitHub Check: Backend Unit Tests
- GitHub Check: E2E Smoke Tests (Chromium)
🔇 Additional comments (5)
codeframe/ui/routers/metrics.py (2)
13-13: LGTM - Proper import for timezone handling.The
timezoneimport is correctly added to support UTC timezone assignment for date-only strings in the_parse_datehelper function.
273-381: LGTM - Clean integration of date filtering.The cost metrics endpoint has been successfully extended with optional date range filtering while maintaining backward compatibility. The implementation:
- Uses the same
_parse_datehelper for consistency- Maintains the same error handling pattern as the timeseries endpoint
- Properly passes date filters to
MetricsTracker.get_project_costs- Preserves all existing functionality
codeframe/lib/metrics_tracker.py (3)
38-38: LGTM - Required imports for time-based aggregations.The
timedeltaandtimezoneimports support the new bucket key calculations and timezone normalization in the timeseries functionality.
205-311: LGTM - Backward-compatible date filtering extension.The
get_project_costsmethod has been successfully extended to support optional date range filtering while maintaining full backward compatibility. The implementation correctly passes the date filters to the underlying data retrieval and preserves all existing aggregation logic.
471-573: Excellent timeseries aggregation implementation.The method provides robust time-bucketed token usage aggregation with several strengths:
- Proper timezone handling (lines 534-541): Handles string timestamps, naive datetimes (assumes UTC per DB contract), and timezone-aware datetimes
- Defensive validation: Validates interval parameter with clear error messages
- Correct aggregation: Properly accumulates tokens and costs per bucket
- Sorted output: Results are chronologically ordered for charting
The timezone handling at lines 539-541 correctly addresses the concern that database timestamps may be naive, which was noted in the commit messages as review feedback.
1 participant
Summary
/api/projects/{id}/metrics/tokens/timeseriesendpoint that returns token usage data aggregated by time intervals (hour, day, week) for visualization in CostDashboard charts/metrics/costsendpoint for consistency2026-01-01T00:00:00Z) and simple date (2026-01-01) formats for frontend compatibilityProblem
The frontend
CostDashboardcomponent calls a non-existent/api/projects/{project_id}/metrics/tokens/timeseriesendpoint when users select date range filters (last-7-days or last-30-days), causing 404 errors and breaking the date filtering functionality.Solution
Implemented the missing endpoint leveraging the existing
TokenRepository.get_token_usage()method with date filters, then grouping results by timestamp intervals.Changes
codeframe/lib/metrics_tracker.pyget_token_usage_timeseries()method and_get_bucket_key()helper; updatedget_project_costs()to accept date paramscodeframe/ui/routers/metrics.py/tokens/timeseriesendpoint and_parse_date()helper; added date filtering to costs endpointtests/api/test_api_metrics.pyTest plan
uv run pytest tests/api/test_api_metrics.py- 27 tests passuv run pytest tests/api/- 209 tests pass (no regressions)uv run ruff check- All checks passeduv run mypy- No issues foundSummary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.