feat: Add analytics dashboard with activity insights#33
Merged
Conversation
- Add analytics page with overview stats, storage breakdown, and trends - Implement week-based activity heatmap with navigation and clickable cells - Add timeline gaps detection showing both regular and incognito periods - Include date range filter with custom picker on main timeline page - Add API endpoints for all analytics data (overview, activity, gaps, etc.)
Previously gaps were sorted purely by duration, causing shorter incognito gaps to be pushed below longer overnight gaps. Now interleaves both types so purple (incognito) and yellow (regular) gaps are both visible at the top of the list.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a full Analytics feature: backend analytics API with seven GET endpoints, expanded Pydantic schemas, a React analytics dashboard page, new frontend API client functions and TypeScript types, and homepage changes for URL-driven timeline filtering and navigation to the analytics UI. Changes
Sequence DiagramsequenceDiagram
actor User as User
participant Frontend as "Analytics Dashboard"
participant Client as "API Client"
participant Backend as "Backend API"
participant DB as "Database"
participant FS as "Filesystem"
User->>Frontend: Open analytics page
Frontend->>Client: Parallel fetch (overview, storage, activity, week, gaps, quality, trends)
par Overview
Client->>Backend: GET /analytics/overview
Backend->>DB: Query counts & OCR stats
Backend->>FS: Scan screenshots for sizes
Backend-->>Client: Overview JSON
and Storage
Client->>Backend: GET /analytics/storage?days=...
Backend->>DB: Query daily counts & metadata
Backend->>FS: Validate file sizes & existence
Backend-->>Client: Storage JSON
and Activity
Client->>Backend: GET /analytics/activity?weeks=...
Backend->>DB: Query activity grouped by day/hour
Backend-->>Client: Activity JSON
and ActivityWeek
Client->>Backend: GET /analytics/activity-week?weekOffset=...
Backend->>DB: Query week entries (incl. ids)
Backend-->>Client: ActivityWeek JSON
and Gaps
Client->>Backend: GET /analytics/gaps
Backend->>DB: Query visible timeline
Backend-->>Client: Gaps JSON
and Quality
Client->>Backend: GET /analytics/quality
Backend->>DB: Query compression stats
Backend->>FS: Scan files for distribution
Backend-->>Client: Quality JSON
and Trends
Client->>Backend: GET /analytics/trends?days=...
Backend->>DB: Query daily counts
Backend-->>Client: Trends JSON
end
Client-->>Frontend: All datasets returned
Frontend->>Frontend: Render charts, cards, heatmap, timelines
User->>Frontend: Interact (refresh, navigate, click heatmap)
Frontend->>Client: Fetch updated data as needed
Estimated Code Review Effort🎯 5 (Critical) | ⏱️ ~120 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@api/routes/analytics.py`:
- Around line 466-474: The two unbounded SELECTs fetching all visible and hidden
timestamps should be constrained to avoid loading the entire table: modify both
SQL queries (the SELECT timestamp FROM screenshots WHERE is_hidden = 0 … and the
SELECT timestamp FROM screenshots WHERE is_hidden = 1 … used for incognito
detection) to accept a parameterized lookback window (e.g., WHERE timestamp >=
?) or a configurable LIMIT (e.g., LIMIT ?) and pass those values from a new
config variable (LOOKBACK_DAYS or MAX_SCREENSHOTS); also switch iteration to a
streaming pattern (use cursor.fetchmany(batch_size) or an iterator) when
processing the results in the incognito detection loop to bound memory and
ensure the changes apply to both queries used before the loop around the
incognito detection code.
- Around line 206-232: The code currently queries the 100 most recent
screenshots via cur.execute(...) then stats each Path, so change the query and
approach to actually find the largest files: either (preferred) scan the
screenshots storage directory with os.scandir (or Path.iterdir) to collect file
sizes once, build a mapping from image_path to size, then query the screenshots
rows needed (or join by image_path) and populate largest_files from that
mapping; or if you want a DB-only fix, add a persisted size_bytes column to the
screenshots table (and backfill it) and change the cur.execute(...) to SELECT
id, image_path, timestamp, size_bytes FROM screenshots ORDER BY size_bytes DESC
LIMIT 10 and remove per-row path.stat() calls. Update references to Path and
path.stat() in this block accordingly (largest_files, cur.execute, image_path).
- Around line 53-114: The handler get_overview (and similar async endpoints like
the /storage and /quality handlers) performs synchronous filesystem iteration
and f.stat() calls inside an async def, which blocks the event loop; either make
the endpoint a regular blocking handler (change async def get_overview to def
get_overview) so FastAPI runs it in a threadpool, or keep it async and move all
blocking work (the screenshots_dir iteration, size aggregation and any other
filesystem/db.cursor() blocking calls) into a helper function and call it with
await asyncio.to_thread(helper_function). Locate the filesystem loop in
get_overview (the for f in screenshots_dir.iterdir() / f.stat() / file_sizes
aggregation) and refactor it into a sync helper (e.g., compute_storage_stats)
and replace the inline code with await asyncio.to_thread(compute_storage_stats)
or convert the whole endpoint signature to def get_overview to avoid blocking
the event loop; apply the same pattern to the /storage and /quality endpoints
and any other handlers using db.cursor() synchronously.
- Around line 35-45: The parse_timestamp function currently returns
datetime.now() for wrong-length input and lets int()/datetime() raise unhandled
exceptions for malformed data; instead validate and fail fast: in
parse_timestamp(ts: str) first check length == 12 and ts.isdigit(), then convert
the slices to ints inside a try/except that catches ValueError and TypeError and
attempts datetime(year, month, day, hour, minute, second) inside the same try;
if any step fails raise a clear ValueError (with a short message including the
offending ts) so callers (endpoints) can return a 400 rather than causing a 500.
- Around line 558-563: The gap statistics are being calculated from the
truncated/interleaved list (`interleaved` assigned to `gaps`) so
`total_gap_time`, `longest_gap`, and `avg_gap` reflect only the displayed
subset; preserve the full detected gaps list (e.g., save `original_gaps = gaps`
before you interleave/truncate) and compute `total_gap_time =
sum(g["duration_seconds"] for g in original_gaps)`, `longest_gap =
max((g["duration_seconds"] for g in original_gaps), default=0)`, and `avg_gap =
total_gap_time / len(original_gaps) if original_gaps else 0` using that
preserved list, then proceed to interleave/truncate into `gaps` for display.
In `@web/src/app/analytics/page.tsx`:
- Around line 281-286: formatDateRange is creating Date objects from
weekData.week_start and weekData.week_end using new Date(...), which causes
timezone shifts; change the parsing to treat the stored dates as UTC (e.g., use
a UTC-aware parser like parseISO from date-fns or construct the Date with a 'Z'
suffix/Date.UTC so the day doesn't shift) when creating start and end in the
formatDateRange function to ensure correct month/day rendering for
weekData.week_start and weekData.week_end.
- Around line 261-277: getWeekDates is creating Date objects from a "YYYY-MM-DD"
string which is parsed as UTC and then mixed with local getDate(), causing
one-day shifts in some timezones; fix by parsing weekData.week_start into a
local Date (split the "YYYY-MM-DD" into year, month, day and construct new
Date(year, month-1, day)) and then derive each day by constructing new
Date(baseLocal.getFullYear(), baseLocal.getMonth(), baseLocal.getDate() + i)
(use getFullYear/getMonth/getDate for arithmetic) so labels (label, date) and
dayLabel generation in getWeekDates remain consistent and timezone-safe.
In `@web/src/app/page.tsx`:
- Around line 1633-1660: The "Find Similar" button is misleading because it
builds a date text from selectedImage.timestamp and calls setSearchMode('image')
with a textual query; either change the UI text to reflect a date-based search
(e.g., "Search by date") or implement a real image-based search by passing the
image identifier/path to the search system instead of a formatted date — update
the button label or replace the setQuery(...) call with a call that supplies the
image's unique id/path (e.g., selectedImage.path or selectedImage.id) and/or a
dedicated image-search setter (e.g., setImageSearchSource(selectedImage.path))
while keeping setActiveView('search') and setSearchMode('image') as appropriate.
🧹 Nitpick comments (3)
api/routes/analytics.py (1)
53-53: Missingresponse_modelon all endpoints.Per coding guidelines, FastAPI router endpoints should define
response_model. None of the seven endpoints specify one, which means no response validation or OpenAPI schema documentation for the response body.As per coding guidelines:
api/routes/**/*.py: "FastAPI router pattern: use APIRouter with prefix and tags, define endpoints with response_model and docstrings."Also applies to: 117-118, 274-275, 357-358, 451-452, 574-575, 652-653
web/src/app/analytics/page.tsx (1)
462-496:Promise.allmakes data loading all-or-nothing — one failing endpoint blanks the entire dashboard.If any single analytics endpoint fails (e.g.,
/qualitytimes out), the catch on Line 490 sets a generic error and none of the state is updated. Consider usingPromise.allSettledso partial data can still be displayed.web/src/app/page.tsx (1)
1756-1896: Date Range Picker uses direct DOM access (getElementById) instead of React state.Lines 1861–1867 read form values via
document.getElementByIdand Line 1875 toggles visibility viaclassList. This bypasses React's rendering model, making the component harder to test and maintain, and fragile if IDs collide or during SSR.Use
useRefor controlled state (useState) for the form inputs instead.
Comment on lines
+35
to
+45
| def parse_timestamp(ts: str) -> datetime: | ||
| """Convert YYMMDDHHMMSS to datetime.""" | ||
| if len(ts) != 12: | ||
| return datetime.now() | ||
| year = 2000 + int(ts[0:2]) | ||
| month = int(ts[2:4]) | ||
| day = int(ts[4:6]) | ||
| hour = int(ts[6:8]) | ||
| minute = int(ts[8:10]) | ||
| second = int(ts[10:12]) | ||
| return datetime(year, month, day, hour, minute, second) |
There was a problem hiding this comment.
parse_timestamp silently returns datetime.now() on invalid input and crashes on malformed data.
If ts is 12 characters but contains non-numeric data or invalid date values (e.g., month=13), int() or datetime() will raise an unhandled exception that propagates up to the endpoint, returning a 500. The length check on Line 37 is insufficient.
🛡️ Proposed fix
def parse_timestamp(ts: str) -> datetime:
"""Convert YYMMDDHHMMSS to datetime."""
- if len(ts) != 12:
+ if len(ts) != 12 or not ts.isdigit():
return datetime.now()
- year = 2000 + int(ts[0:2])
- month = int(ts[2:4])
- day = int(ts[4:6])
- hour = int(ts[6:8])
- minute = int(ts[8:10])
- second = int(ts[10:12])
- return datetime(year, month, day, hour, minute, second)
+ try:
+ year = 2000 + int(ts[0:2])
+ month = int(ts[2:4])
+ day = int(ts[4:6])
+ hour = int(ts[6:8])
+ minute = int(ts[8:10])
+ second = int(ts[10:12])
+ return datetime(year, month, day, hour, minute, second)
+ except ValueError:
+ return datetime.now()🤖 Prompt for AI Agents
In `@api/routes/analytics.py` around lines 35 - 45, The parse_timestamp function
currently returns datetime.now() for wrong-length input and lets
int()/datetime() raise unhandled exceptions for malformed data; instead validate
and fail fast: in parse_timestamp(ts: str) first check length == 12 and
ts.isdigit(), then convert the slices to ints inside a try/except that catches
ValueError and TypeError and attempts datetime(year, month, day, hour, minute,
second) inside the same try; if any step fails raise a clear ValueError (with a
short message including the offending ts) so callers (endpoints) can return a
400 rather than causing a 500.
Placeholder image search feature is not ready yet - will be implemented properly with CLIP vector similarity on a separate branch.
- Add try-except to parse_timestamp for malformed data - Validate URL date params (12-digit format, start <= end) - Use bisect for O(log n) incognito gap detection instead of O(n*m) - Batch file size lookups via single directory scan instead of per-row stat
- Convert all analytics endpoints from async to sync (FastAPI runs sync handlers in thread pool, avoiding event loop blocking for I/O) - Add lookback_days param to /gaps endpoint to bound query size - Compute gap statistics from full dataset before interleaving/truncation - Batch file size lookups via single directory scan instead of per-row stat - Parse YYYY-MM-DD dates as local time to avoid UTC timezone shift
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@api/routes/analytics.py`:
- Around line 57-58: Add explicit response models to each analytics endpoint
decorator and include docstrings: update the `@router.get`("/overview") decorator
(and the others: get_storage, get_activity, get_activity_week, get_gaps,
get_quality, get_trends) to include a response_model (either a defined Pydantic
model for that endpoint or at minimum response_model=dict) and add a short
docstring to each function describing the returned schema; ensure the
response_model types are imported and used consistently so FastAPI validates
outgoing data and the OpenAPI docs are generated.
- Line 618: The median calculation uses file_sizes[total_files // 2] which
returns the upper-middle value for even-length lists; update the logic around
median_size to handle even and odd counts: if total_files is odd, keep the
middle element from file_sizes, otherwise compute the median as the average of
the two central elements (the elements at indices total_files // 2 - 1 and
total_files // 2) and assign that average to median_size; ensure file_sizes is
sorted before this calculation and keep variable names file_sizes, total_files,
and median_size unchanged.
In `@web/src/app/analytics/page.tsx`:
- Around line 541-546: The Search nav Link currently uses the same href as the
Timeline link; update the Search Link element (the JSX Link whose children are
"Search") so it points to a distinct route or query (for example change href="/"
to "/search" or "/?mode=search") to avoid duplicating the Timeline route and
ensure Search navigates to the correct view.
- Line 488: The call to getAnalyticsGaps(5, 50) uses a 5-minute threshold which
contradicts the UI tooltip that says "Periods of 30+ minutes without
screenshots"; update the usage to getAnalyticsGaps(30, 50) so the
min_gap_minutes matches the InfoTooltip text (or, alternatively, change the
tooltip string to reflect 5 minutes) — locate the invocation of getAnalyticsGaps
and the InfoTooltip text and make them consistent (prefer changing the first
argument of getAnalyticsGaps to 30 to match the tooltip).
- Around line 116-123: The tooltip toggle <button> in page.tsx renders only "i"
and lacks an accessible label; update the button element (the one using
onMouseEnter/onMouseLeave/onClick and state via isVisible and setIsVisible) to
include a meaningful aria-label (and optionally a title) such as "Show analytics
info" or "Toggle analytics tooltip" so screen readers get context; ensure the
aria-label reflects the visible state if needed (e.g., "Show analytics info" vs
"Hide analytics info") when toggling via setIsVisible.
🧹 Nitpick comments (3)
api/routes/analytics.py (3)
208-216: Reusefile_size_mapinstead of per-rowpath.stat()for compressed file sizes.
file_size_map(built at Line 137) already contains current file sizes from a single directory pass. These per-rowstat()calls on Lines 212–214 duplicate that work.Proposed fix
# Get current compressed file sizes cur.execute("SELECT image_path FROM screenshots WHERE is_compressed = 1") compressed_current_bytes = 0 for row in cur.fetchall(): - path = Path(row[0]) - if path.exists(): - compressed_current_bytes += path.stat().st_size + compressed_current_bytes += file_size_map.get(row[0], 0)
515-519: Overly broad exception suppression hides unexpected failures.
contextlib.suppress(Exception)on Line 517 will silently swallow any exception (e.g.,MemoryError,RecursionError), not just parse failures. Narrow this to the expected exception types.Proposed fix
for (ts,) in hidden_rows: - with contextlib.suppress(Exception): + with contextlib.suppress(ValueError, TypeError): hidden_timestamps.append(parse_timestamp(ts))
95-107: Filesystem scan in/overviewduplicates work done by/storageand/quality.Lines 100–105 perform a full directory scan identical to
/quality(Lines 598–601) and partially overlapping with/storage(Lines 137–142). If this endpoint is called frequently (e.g., dashboard load), this repeated I/O is wasteful.Consider extracting a shared helper (or caching the scan result with a short TTL) to avoid scanning the same directory multiple times per dashboard load.
| avg_size = sum(file_sizes) / total_files | ||
| min_size = file_sizes[0] | ||
| max_size = file_sizes[-1] | ||
| median_size = file_sizes[total_files // 2] |
There was a problem hiding this comment.
Median calculation is off-by-one for even-length arrays.
For an even number of files, the standard median is the average of the two middle values. file_sizes[total_files // 2] picks the upper-middle element, which overestimates by up to half a bucket width.
Proposed fix
- median_size = file_sizes[total_files // 2]
+ mid = total_files // 2
+ median_size = (file_sizes[mid - 1] + file_sizes[mid]) // 2 if total_files % 2 == 0 else file_sizes[mid]🤖 Prompt for AI Agents
In `@api/routes/analytics.py` at line 618, The median calculation uses
file_sizes[total_files // 2] which returns the upper-middle value for
even-length lists; update the logic around median_size to handle even and odd
counts: if total_files is odd, keep the middle element from file_sizes,
otherwise compute the median as the average of the two central elements (the
elements at indices total_files // 2 - 1 and total_files // 2) and assign that
average to median_size; ensure file_sizes is sorted before this calculation and
keep variable names file_sizes, total_files, and median_size unchanged.
Comment on lines
+116
to
+123
| <button | ||
| className="w-4 h-4 rounded-full bg-[#1e1e1e] text-[#555] hover:text-[#8a8a8a] hover:bg-[#2a2a2a] text-[10px] flex items-center justify-center transition-colors" | ||
| onMouseEnter={() => setIsVisible(true)} | ||
| onMouseLeave={() => setIsVisible(false)} | ||
| onClick={() => setIsVisible(!isVisible)} | ||
| > | ||
| i | ||
| </button> |
There was a problem hiding this comment.
Tooltip button lacks an accessible label.
The <button> only renders "i" with no aria-label or title. Screen readers will announce it without context. Add an aria-label for accessibility.
<button
+ aria-label="More information"
className="w-4 h-4 rounded-full ..."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| className="w-4 h-4 rounded-full bg-[#1e1e1e] text-[#555] hover:text-[#8a8a8a] hover:bg-[#2a2a2a] text-[10px] flex items-center justify-center transition-colors" | |
| onMouseEnter={() => setIsVisible(true)} | |
| onMouseLeave={() => setIsVisible(false)} | |
| onClick={() => setIsVisible(!isVisible)} | |
| > | |
| i | |
| </button> | |
| <button | |
| aria-label="More information" | |
| className="w-4 h-4 rounded-full bg-[`#1e1e1e`] text-[`#555`] hover:text-[`#8a8a8a`] hover:bg-[`#2a2a2a`] text-[10px] flex items-center justify-center transition-colors" | |
| onMouseEnter={() => setIsVisible(true)} | |
| onMouseLeave={() => setIsVisible(false)} | |
| onClick={() => setIsVisible(!isVisible)} | |
| > | |
| i | |
| </button> |
🤖 Prompt for AI Agents
In `@web/src/app/analytics/page.tsx` around lines 116 - 123, The tooltip toggle
<button> in page.tsx renders only "i" and lacks an accessible label; update the
button element (the one using onMouseEnter/onMouseLeave/onClick and state via
isVisible and setIsVisible) to include a meaningful aria-label (and optionally a
title) such as "Show analytics info" or "Toggle analytics tooltip" so screen
readers get context; ensure the aria-label reflects the visible state if needed
(e.g., "Show analytics info" vs "Hide analytics info") when toggling via
setIsVisible.
Comment on lines
+541
to
+546
| <Link | ||
| href="/" | ||
| className="px-4 py-2 text-sm font-medium rounded-lg text-[#8a8a8a] hover:text-[#f5f5f5] hover:bg-[#1c1c1c] transition-all" | ||
| > | ||
| Search | ||
| </Link> |
There was a problem hiding this comment.
Search nav link points to the same route as Timeline.
Both "Timeline" (Line 536) and "Search" (Line 541) link to "/". The Search link likely needs a distinct route or query parameter (e.g., /?mode=search or /search).
🤖 Prompt for AI Agents
In `@web/src/app/analytics/page.tsx` around lines 541 - 546, The Search nav Link
currently uses the same href as the Timeline link; update the Search Link
element (the JSX Link whose children are "Search") so it points to a distinct
route or query (for example change href="/" to "/search" or "/?mode=search") to
avoid duplicating the Timeline route and ensure Search navigates to the correct
view.
- Add Pydantic response_model to all 7 analytics endpoints for validation and OpenAPI docs generation - Fix tooltip saying "30+ minutes" when actual threshold is 5 minutes
6 tasks
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
1 participant
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.
Summary
/analytics) with overview stats, storage breakdown, capture trends, and recording quality metricsChanges
api/routes/analytics.py— New analytics API endpoints (overview, storage, activity, activity-week, gaps, quality, trends)api/main.py— Register analytics routerweb/src/app/analytics/page.tsx— Full analytics dashboard UIweb/src/app/page.tsx— Date range filter with custom picker on main timelineweb/src/lib/api.ts— Analytics API client functionsweb/src/types/index.ts— Analytics TypeScript typesTest plan
/analytics— all cards should load with data🤖 Generated with Claude Code
Summary by CodeRabbit