Repository navigation
feat(activity): add custom date range period - #1005
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1005 +/- ##
==========================================
+ Coverage 61.31% 62.18% +0.86%
==========================================
Files 51 52 +1
Lines 3428 3599 +171
Branches 841 888 +47
==========================================
+ Hits 2102 2238 +136
+ Misses 1311 1277 -34
- Partials 15 84 +69 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export function dateRangeToTimeperiod(range: DateRange, offset: string): TimePeriod { | ||
| return { | ||
| start: get_day_start_with_offset(range.start, offset), | ||
| length: [dateRangeDays(range), 'days'], | ||
| }; |
There was a problem hiding this comment.
A valid custom range can span years. This turns the whole span into a day count, and the desktop loading path sends one request per day, sequentially. Selecting a range such as 2015–2026 therefore requires thousands of requests before the Activity page finishes loading. Monthly bucketing reduces only the later category query; the desktop path also needs a bounded or scalable approach.
Knowledge Base Used: Frontend data integration
There was a problem hiding this comment.
Intended: ActivityWatch/activitywatch#1465 explicitly accepts long ranges being slow rather than capping them. One request per day keeps every request far under the 30s timeout (week-sized chunks were tried and reached 20s on a 1.7 GB DB without being faster overall, see 8ef870d). Month-sized category queries are gone for long ranges (monthly barchart derived from the day results, df66105), the period-usage history is skipped in range mode (9b5d663), and the stacked All time PR adds a progress bar plus a slowness hint.
There was a problem hiding this comment.
That context addresses the concern. Given the explicit product decision to allow long ranges, the measured timeout behavior of day-sized requests, and the later progress/slowness UX in the stacked PR, the sequential per-day loading is an intentional tradeoff rather than an actionable defect in this PR. I’m withdrawing the “Broad ranges stall loading” comment.
| } else if (this.periodLength === 'range') { | ||
| // Invalid range in the URL: fall back to today (a warning is shown) | ||
| return { | ||
| start: get_day_start_with_offset(this._date, settingsStore.startOfDay), | ||
| length: [1, 'day'], | ||
| }; |
There was a problem hiding this comment.
When a range URL is invalid, the page shows today's single-day period, but the previous and next links keep /range/ and generate only a single date. Clicking either link opens another invalid range URL, leaving the warning in place and making those controls unable to browse the fallback period.
Knowledge Base Used: Activity exploration
There was a problem hiding this comment.
Fixed in 9b5d663: the prev/date/next group is hidden while the URL range is invalid; the period buttons still work.
| return _.range(0, 24).map(h => `${(h + hourOffset) % 24}`); | ||
| } else if (resolution.startsWith('day') && count > MAX_DAILY_BUCKETS) { | ||
| // Long custom ranges are bucketed by calendar month (see timeperiodsForBarchart) | ||
| const fmt = new Intl.DateTimeFormat(undefined, { month: 'short', year: 'numeric' }); |
There was a problem hiding this comment.
If a user's selected ActivityWatch language differs from their browser language, the new monthly labels use the browser language, and the new multi-day labels do too because format_date_short receives no locale. The chart can therefore show dates in a different language from the rest of the page.
Knowledge Base Used: Localization
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in 9b5d663: labels now use i18n.locale (moment is already set to it too).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82e762d5de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function dateRangeToTimeperiod(range: DateRange, offset: string): TimePeriod { | ||
| return { | ||
| start: get_day_start_with_offset(range.start, offset), | ||
| length: [dateRangeDays(range), 'days'], | ||
| }; |
There was a problem hiding this comment.
When a user selects a multi-year range, this passes the unbounded dateRangeDays value into periodsForFullDesktopQuery, which expands the range into daily intervals, while queryDesktopPeriods awaits one HTTP request for every interval sequentially. A four-year range therefore creates roughly 1,461 serial fullDesktopQuery requests before the history, editor, and category queries finish, leaving the Activity view loading for many minutes; the range should be bounded or the full-desktop requests should be aggregated into a bounded number of chunks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Intended: ActivityWatch/activitywatch#1465 explicitly accepts long ranges being slow rather than capping them. One request per day keeps every request far under the 30s timeout (week-sized chunks were tried and reached 20s on a 1.7 GB DB without being faster overall, see 8ef870d). Month-sized category queries are gone for long ranges (monthly barchart derived from the day results, df66105), the period-usage history is skipped in range mode (9b5d663), and the stacked All time PR adds a progress bar plus a slowness hint.
🤖 AI code reviewThis PR adds a custom date range period ('range') and an 'all time' period to the Activity view. It introduces URL encoding for ranges as Not safe to merge — 1 P1 openConfidence 3/5 4 findings · ❌ 1 P1 ·
|
| commit | score | findings | engine | when |
|---|---|---|---|---|
82e762d5de9e |
5/5 | 0 | llm | 2026-09-26 15:25 UTC |
8e0cacf41835 |
4/5 | 6 | llm | 2026-09-26 16:41 UTC |
da522bf912d9 |
2/5 | 6 | llm | 2026-09-26 23:42 UTC |
Reviewed 69245506a802 · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 308s · about this reviewer
Maintainer commands
@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df661059c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this.dateRange) { | ||
| return dateRangeToTimeperiod(this.dateRange, settingsStore.startOfDay); |
There was a problem hiding this comment.
For a long custom range, this TimePeriod is also passed to query_active_history, which expands it with timeperiodsAroundTimeperiod and submits every non-future one of 31 equally long intervals. A current one-year range therefore makes the period-usage query scan roughly 16 years of AFK data, while a multi-year range can still time out even though the desktop query is now chunked. Range mode should omit this surrounding-history query or use bounded, fixed-size periods.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9b5d663: range mode skips the period-usage history query (skip_active_history) and hides the bars.
| input.form-control.form-control-sm.activity-dateinput( | ||
| type="date" | ||
| :value="dateRange.end" | ||
| :min="dateRange.start" |
There was a problem hiding this comment.
The custom-range end input has only a minimum, so a user can move the end into the future and then move the start after today as well; parseDateRange accepts the resulting wholly future range and the application issues queries for it. This bypasses the clipping used when range mode is entered and produces an empty, misleading Activity view, so the end date—and transitively the start date—should be capped at the offset-aware current date.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9b5d663: end input has max = today and setRange caps the end at today, so a wholly future range can no longer be built from the UI.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ef870d11d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| previousPeriod: function () { | ||
| if (this.dateRange) { | ||
| return formatDateRange(shiftDateRange(this.dateRange, -1)); | ||
| } | ||
| return moment(this._date) |
There was a problem hiding this comment.
When periodLength is range but the URL range is invalid, dateRange is null, so both arrow methods fall through and return a single YYYY-MM-DD value. The resulting /range/YYYY-MM-DD/... route is still invalid, meaning the arrows keep showing today's fallback data and the warning instead of navigating. Special-case invalid range mode to emit a valid single-day start..end range.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b5d663cc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this.dateRange) { | ||
| return shiftDateRange(this.dateRange, 1).start > today; |
There was a problem hiding this comment.
When the shifted range starts on or before today but ends afterward, nextDisabled remains false and nextPeriod() navigates directly to the untrimmed range. For example, on 2026-09-26, advancing 2026-09-11..2026-09-20 opens 2026-09-21..2026-09-30; the end input then exceeds its own max, and future periods are filtered from queries while the heading and chart still represent the full ten days. The new max and setRange cap do not cover this router-link path, so clip the shifted end to today or disable this navigation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8e0cacf: the shifted end is clipped to today, same as setRange.
| .add(timeperiod.length[0], timeperiod.length[1] as moment.unitOfTime.DurationConstructor); | ||
| const periods: TimePeriod[] = []; | ||
| let cur = start.clone(); | ||
| while (cur.isBefore(end)) { |
There was a problem hiding this comment.
timeperiodsCalendarMonthsOfPeriod, the loop uses while (cur.isBefore(end)) and sets next = cur.clone().startOf('month').add(1, 'month'). For a timeperiod that starts exactly at midnight, this works. But for a timeperiod with a day-start offset like 04:00, the first month period is from 2026-01-15T04:00:00 to 2026-02-01T04:00:00 (clipped to the month boundary). The next cur is 2026-02-01T04:00:00. The loop continues until cur is not before end. If the timeperiod ends exactly at a month boundary, e.g. 2026-03-01T04:00:00, the last month period is from 2026-02-01T04:00:00 to 2026-03-01T04:00:00, and then cur becomes 2026-03-01T04:00:00, which is not before end (equal), so the loop stops. That's correct. However, if the timeperiod length is such that the end is not exactly on a month boundary, the last period is clipped to end. The days calculation uses next.diff(cur, 'days', true) and then Math.round(days). For a period from 2026-01-15T04:00:00 to 2026-02-01T04:00:00, the diff is 17 days exactly, so Math.round(17) is 17. For a period from 2026-01-31T04:00:00 to 2026-02-01T04:00:00, the diff is 1 day, so length 1. This seems correct. But there is a subtle issue: when the timeperiod start is not at midnight, the month boundary is set to startOf('month') which is midnight, then hours are set to the start's hours. For a start at 04:00, the month boundary becomes 2026-02-01T04:00:00. That's correct. However, if the start is at 04:00 and the timeperiod length is 1 day, the end is 2026-01-16T04:00:00. The first month period would be from 2026-01-15T04:00:00 to 2026-02-01T04:00:00 (clipped to end? No, next is 2026-02-01T04:00:00, which is after end, so next = end.clone() becomes 2026-01-16T04:00:00. The days diff is 1, so length 1. That's correct. I don't see a bug.
| return timeperiodsCalendarMonthsOfPeriod({ | ||
| start, | ||
| length: [count, resolution], | ||
| }).map(p => fmt.format(new Date(p.start))); |
There was a problem hiding this comment.
TimelineBarChart.vue, the labels computed for long day ranges uses timeperiodsCalendarMonthsOfPeriod and formats the start date with Intl.DateTimeFormat. However, the timeperiod_start prop is a string, and new Date(p.start) is used. If the timeperiod start has a timezone offset, the date might be off by a day depending on the user's timezone. For example, if p.start is 2026-02-01T04:00:00+00:00, new Date() in a timezone behind UTC might show January 31. This could cause the month label to be wrong. The existing code for day labels uses new Date(start) and sets hours to 12 to avoid timezone issues, but the month labels do not do that. This could show the wrong month for ranges with a non-midnight start offset. The PR description says 'Multi-day barchart labels now show dates instead of 1..N', and the day labels use format_date_short which likely handles timezone. But the month labels use Intl.DateTimeFormat directly on the parsed date, which is timezone-sensitive. This is a potential bug.
|
|
||
| if (this.periodLength === 'range') { | ||
| // The user picked both ends, so show them as picked (end inclusive) | ||
| const range = this.dateRange || { start: this._date, end: this._date }; |
There was a problem hiding this comment.
Activity.vue, the periodReadableRange computed for range mode uses this.dateRange || { start: this._date, end: this._date }. If the range is invalid, this.dateRange is null, and this._date is today's date. So the readable range shows today—today. That's fine. However, the periodLengthTitle computed returns this.periodReadableRange for range mode, which is used in the header. For an invalid range, it shows today—today, which is consistent with the fallback. The warning alert is shown. No issue.
| let periods: string[] = timeperiodsForBarchart(timeperiod).map(timeperiodToStr); | ||
|
|
||
| // Filter out periods that start in the future | ||
| periods = periods.filter(period => new Date(period.split('/')[0]) < new Date()); |
There was a problem hiding this comment.
query_category_time_by_period, the periods are generated by timeperiodsForBarchart and then filtered to remove periods that start in the future. However, for a long range that uses monthly buckets, the last month might start in the past but end in the future (if the range end is today). The query for that month will request data for the entire month, including future days. The server might return data only up to now, or it might return an error. The existing code for day periods also has this issue: if the range ends today, the last day period starts today and is not filtered (since it starts in the past? Actually, new Date(period.split('/')[0]) < new Date() is true for today's start, so it's included). The query for today's period will request the full day, but the server returns data up to now. This is existing behavior. So no new issue.
| ]) as any; | ||
| const byPeriod = categoryByPeriodFromChunks(tp, chunks); | ||
| const months = Object.keys(byPeriod); | ||
| expect(months.map(k => moment(k.split('/')[0]).format('YYYY-MM-DD'))).toEqual([ |
There was a problem hiding this comment.
desktopQuerySplit.test.node.ts, the test 'categoryByPeriodFromChunks sums chunk cat_events per month' uses chunks where each chunk has the same cat_events with durations 10 and 1. It then expects feb[0].duration to be 10 * chunksInFeb. However, mergeEventsByKeys sums durations for the same key, so if each chunk has a 'Work' event with duration 10, the total is 10 * number of chunks. That's correct. But the test does not verify that the month keys are correct for the first and last months. It checks the month start dates, but not the end dates. The categoryByPeriodFromChunks uses timeperiodToStr(month) which includes the end date. The test only checks the start date. This is a minor test gap, but not a defect.
| const tp = dateRangeToTimeperiod({ start: '2026-01-01', end: '2026-12-31' }, '04:00'); | ||
| const periods = timeperiodsCalendarMonthsOfPeriod(tp); | ||
| expect(periods).toHaveLength(12); | ||
| expect(periods.every(p => moment(p.start).format('D HH:mm') === '1 04:00')).toBe(true); |
There was a problem hiding this comment.
timeperiod.test.node.ts, the test 'keeps the day-start offset on month boundaries' uses dateRangeToTimeperiod({ start: '2026-01-01', end: '2026-12-31' }, '04:00') and expects 12 periods, each starting at '1 04:00'. However, the last period is from December 1 to December 31, which is 30 days? Actually, December has 31 days, so the last period is 31 days. The test only checks the start date and time, not the length. The test passes. No issue.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da522bf912
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
|
|
||
| loadEarliestDate: async function () { | ||
| if (this.periodLength !== 'all' || this.earliestDate) return; |
There was a problem hiding this comment.
When backdated events are imported or synced into an existing bucket after the All time view has loaded, this early return preserves the old earliestDate; the module-level earliestDateCache also preserves the same boundary. Consequently, clicking Refresh re-runs queries only from the stale start date and silently omits the newly added historical data until the page is fully reloaded. The forced refresh path should invalidate and reload the cached earliest date before querying.
Useful? React with 👍 / 👎.
|
Reviewed the new Codex comment (22:36) about |
| verbose: true, | ||
| }); | ||
| this.progress_tick(); | ||
| if (data && data[0]) chunks.push(data[0]); |
There was a problem hiding this comment.
❌ P1 — In query_editor, the loop collects chunks but if a chunk request fails (data is falsy), it simply skips that chunk and continues. The final result is merged from the chunks that succeeded, silently omitting the failed period. This produces a partial total (duration, files, languages, projects) that is presented as if it covered the whole period. For example, if the first of three chunks fails, the displayed 'all time' duration is only two-thirds of the real value, with no warning. The Android path explicitly guards against this by returning early on a failed chunk, but the editor path does not. This is a correctness bug for long ranges where a single request can time out.
| const events = chunks | ||
| .filter(([period]) => { | ||
| const chunkStart = new Date(period.split('/')[0]); | ||
| return chunkStart >= start && chunkStart < end; |
There was a problem hiding this comment.
❌ P1 — In categoryByPeriodFromChunks, the chunk filtering uses chunkStart >= start && chunkStart < end where start and end are parsed from the month period string. However, the month period string is generated by timeperiodsCalendarMonthsOfPeriod, which clips the first and last month to the range bounds. For the first month, the period start is the range start (e.g. 2026-01-15), and chunks that start on 2026-01-15 are included. For the last month, the period end is the range end (e.g. 2026-05-14), and chunks that start on 2026-05-14 are included because chunkStart < end (2026-05-14 < 2026-05-14 is false). Wait, let me re-check: the month period for May is start=2026-05-01, end=2026-05-14 (clipped). A chunk starting on 2026-05-14 has chunkStart < end? 2026-05-14 < 2026-05-14 is false, so it is excluded. But the chunk starting on 2026-05-14 covers the day 2026-05-14, which is part of the range. The test uses a range ending 2026-05-14 and expects the last month to include that day, but the test only checks the month keys, not the actual events in the last month. The chunk for 2026-05-14 is excluded, so the last day's category data is missing from the barchart. This is a boundary bug: the last day of a range is dropped from the monthly barchart when the range ends mid-month.
| this.progress_tick(); | ||
| if (!(result && result[0])) { | ||
| // Don't show partial totals as if they covered the whole period | ||
| this.query_window_completed(); |
There was a problem hiding this comment.
query_android, when a chunked query fails (result is falsy), the code calls this.query_window_completed() and returns, but it does not reset this.progress to null. The progress bar in the Activity view for 'all' period will remain stuck at a partial value (done < total) indefinitely, because ensure_loaded only sets this.progress = null after the query completes normally. The user sees a progress bar that never finishes, and subsequent queries may accumulate progress totals incorrectly since progress_add adds to the existing total. This is a real defect in the error path.
| await bucketsStore.ensureLoaded(); | ||
| const hosts = settingsStore.useMultidevice ? bucketsStore.hosts : [host]; | ||
| const key = [hosts.join(','), settingsStore.startOfDay].join('|'); | ||
| if (earliestDateCache.has(key)) return earliestDateCache.get(key); |
There was a problem hiding this comment.
get_earliest_date, the cache key is [hosts.join(','), settingsStore.startOfDay].join('|'). When multidevice is off, hosts is [host], so the key is the host name. When multidevice is on, hosts is bucketsStore.hosts, which is the list of all hosts. However, the cache is never invalidated when new buckets are added or data is imported. If a user has no data for a host and then imports data, the cached null (or a date) will be stale, and the 'all time' range will not update until the page is reloaded. This is a real bug because the cache is module-level and persists across route changes. The comment says 'Cached per host and day-start offset' but there is no invalidation mechanism. The fallback path (on error) is not cached, but the success path is. This means a user who first views 'all time' with no data, then imports data, will still see 'all time' as today..today until they reload the app.
| // Invariant: an event exists at or before `hi`, none before `lo`. | ||
| let hi = new Date(latest[0].timestamp).getTime(); | ||
| let lo = SEARCH_FLOOR.getTime(); | ||
| for (let i = 0; i < 40 && hi - lo > DAY_MS; i++) { |
There was a problem hiding this comment.
earliestEventInBucket, the bisection loop runs up to 40 iterations, but the condition hi - lo > DAY_MS means it stops when the interval is within a day. However, the initial hi is the timestamp of the latest event, and lo is the SEARCH_FLOOR (2000-01-01). If the earliest event is before 2000, the bisection will never find it because lo is the floor. But ActivityWatch has no data before 2000, so this is acceptable. The bigger issue is that the bisection assumes the events API returns the latest event before end with limit=1. If the API returns events in ascending order or does not support the end parameter correctly, the bisection could return a wrong result. The test uses a fake that mimics the expected behavior. This is a dependency contract, but the code is correct for the documented API.
Adds a 'custom range' period to the Activity view, encoded in the URL as /activity/:host/range/YYYY-MM-DD..YYYY-MM-DD (both ends inclusive), so ranges are shareable and existing URLs keep working. - Start/end native date inputs replace the single date input in range mode - Prev/next step by the range length; next is disabled once it would start after today (this also fixes the next button never being disabled, since `today` was never set) - Entering range mode from another period keeps the shown period, clipped to today - Ranges longer than 92 days are bucketed by calendar month in the timeline barchart and the category-by-period query (shared via timeperiodsForBarchart); multi-day barchart labels now show dates Part of ActivityWatch/activitywatch#1465
For ranges long enough to use monthly barchart buckets, split the desktop query into chunks of at most 7 days that never cross a calendar month, and build the monthly category data from those chunk results instead of issuing a month-sized category query per month (10-38s each on a 1.7 GB database, occasionally past the 30s request timeout).
Week-sized chunks were no faster on a 1.7 GB aw-server database and single requests reached 20s, too close to the 30s timeout. Days never cross a month boundary either, so the monthly barchart is still derived from them.
- Skip the period-usage history in range mode and hide its bars: 31 neighbouring ranges can span decades of AFK data - Cap the range end at today - Hide prev/next when the URL range is invalid (they produced invalid links) - Use the app locale for barchart date/month labels - Remove a test file that belongs to the All time PR
* feat(activity): add All time period - New 'all' period (/activity/:host/all/view/...) from the host's first day with data to today, marked with 🐌 plus a slowness hint and a progress bar - Earliest event: metadata.start on aw-server-rust, otherwise a ~15-request bisection with GET /events?end=&limit=1 on aw-server (Python); cached per host - Skips the period-usage history (no neighbouring periods) - Editor and Android/ScreenTime queries are split into bounded chunks and merged client-side instead of one request for the whole span - Long ranges don't keep years of AFK events in reactive state Part of ActivityWatch/activitywatch#1465 * fix(activity): address review on All time - Earliest date covers every bucket type the view queries (editor, browser, stopwatch too) and all hosts when multidevice is on; cache keyed by hosts and day-start offset - Bisection returns a conservative bound (never after the first event) - Earliest-event lookup failure falls back to bucket creation dates instead of blocking the page - Chunked editor/Android queries over-fetch (1000 per chunk) so items outside each chunk's top 100 can still reach the overall top 100 - A failed Android chunk shows no data instead of partial totals - Progress bar counts editor and Android chunks
da522bf to
9260f9e
Compare
There was a problem hiding this comment.
💡 Codex Review
aw-webui/src/views/activity/Activity.vue
Line 830 in 9260f9e
Preserve the full range when switching devices
When the Activity view is in custom-range mode, _date contains only dateRange.start, so using the device selector or an “only” link builds a /range/YYYY-MM-DD/... URL rather than preserving start..end. That route is invalid, causing the destination host to show the warning and today's fallback data instead of the selected range; use the formatted dateRange as the date segment in this mode.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const { merged, chunks } = await queryDesktopPeriods(periods, q, 'fullDesktopQuery', () => | ||
| this.progress_tick() | ||
| ); |
There was a problem hiding this comment.
For custom and All time ranges, this merges daily fullDesktopQuery results even though each result has already been capped to the top 100 apps, titles, domains, URLs, and stopwatch labels. An item that is consistently outside the daily top 100 is therefore absent from every chunk even when its cumulative duration belongs in the overall top 100, producing incorrect rankings for long ranges. The new Android/editor chunking avoids this with CHUNKED_QUERY_LIMIT; the desktop and multidevice queries need equivalent over-fetching before the final merge and limit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same finding as on #1006, where it was scoped out: over-fetching every day's app/title/URL lists across years costs a lot of payload, and an item has to miss the top 100 on every single day to be dropped. Leaving for a follow-up if it shows up in practice.
|
@greptileai review |
|
|
||
| const DAY_MS = 24 * 60 * 60 * 1000; | ||
| // Lower bound for the search; ActivityWatch has no data from before this. | ||
| const SEARCH_FLOOR = new Date('2000-01-01T00:00:00Z'); |
There was a problem hiding this comment.
If an imported event predates 2000, the earliest-event search returns its year-2000 floor rather than the event’s date. “All time” then starts in 2000 and omits that earlier activity.
Knowledge Base Used: Activity exploration
There was a problem hiding this comment.
Fixed in #1008 (stacked on this branch): the search floor is the Unix epoch there.
| const key = [hosts.join(','), settingsStore.startOfDay].join('|'); | ||
| if (earliestDateCache.has(key)) return earliestDateCache.get(key); |
There was a problem hiding this comment.
If older events are imported into an existing bucket, revisiting or refreshing “All time” reuses the previously cached start date. The newly imported activity stays outside the query range and does not appear.
Knowledge Base Used: Frontend data integration
| if ((this.window.available || this.android.available) && !derivedByPeriod) { | ||
| await this.query_category_time_by_period(query_options); | ||
| } | ||
| this.progress = null; |
There was a problem hiding this comment.
If an All time request fails after progress has started, the loading path never clears progress. The progress bar remains at its last value after loading has stopped, making it look as though work is still underway.
This comment has been minimized.
This comment has been minimized.
useMultidevice is gone (#1004): multi-device views are now selected by the route (@ALL or a host list) and loaded by ensure_loaded_multidevice. - ensure_loaded_multidevice follows what the single-device path does for custom ranges and All time: no period-usage history when it's skipped, the monthly barchart derived from the query chunks for long ranges (and no years of active events kept in state), and progress cleared. - get_earliest_date covers the devices the query will include. - The single-device monthly-barchart shortcut no longer checks useMultidevice. - Device-selector links keep the route's date part, so switching devices keeps a custom range (and doesn't add a date to All time).
9260f9e to
6924550
Compare
|
@greptileai review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69245506a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!this.timeperiod) { | ||
| // All time before the earliest date is known; the timeperiod watcher refreshes later | ||
| return; |
There was a problem hiding this comment.
When an All time URL is loaded directly against a Python server, the earliest-date lookup can take multiple requests, and this early return leaves activityStore.query_options as null in the meantime. A saved dashboard containing sunburst_clock or vis_timeline still renders during that lookup; SelectableVisualization.supports_period evaluates isSingleDay, which dereferences activityStore.query_options.timeperiod and throws. Initialize a loading query state before waiting, or guard that consumer until query options exist.
Useful? React with 👍 / 👎.
| const days = Math.max( | ||
| 1, | ||
| Math.round( | ||
| moment(this.timeperiod.start) |
There was a problem hiding this comment.
❌ P1 — In Activity.vue, the setDate function for periodLength === 'range' uses this.timeperiod.start to compute the number of days. However, when the current period is 'all' or 'range', this.timeperiod might be null (for 'all' before earliestDate is known) or a day-length period for invalid range. If the user clicks 'custom range' while in 'all' mode before the earliest date is loaded, this.timeperiod is null, and the code will throw an error when accessing this.timeperiod.start. The setDate function is called from the period buttons, and the 'range' button is always visible. If the user clicks it while the All time period is still loading (earliestDate not yet set), the app will crash. The impact is a runtime error. The fix would be to guard against this.timeperiod being null, or to use a fallback.
There was a problem hiding this comment.
Verified in #1008 (70bb9dc): setDate(_, 'range') now returns early when timeperiod is still null and starts a one-day range from the given date. That's the crash window. Resolving this thread.
| this.progress_tick(); | ||
| if (!(result && result[0])) { | ||
| // Don't show partial totals as if they covered the whole period | ||
| this.query_window_completed(); |
There was a problem hiding this comment.
query_android, when a chunked query fails (e.g. one of the split periods returns no data or an error), the code calls this.query_window_completed() and returns, but it does not reset this.progress to null. The progress bar for All time (or any long Android range) will remain stuck at a partial value, and because progress is not null, subsequent progress_add calls will accumulate on top of the stale total. This leaves the progress bar permanently visible and inaccurate for the rest of the session. The same issue occurs in query_editor if all chunks fail (it calls query_editor_completed with an empty merge, but progress is not reset). The ensure_loaded path normally sets this.progress = null after the queries, but the early return in query_android bypasses that. The fix is to reset this.progress = null before returning in the failure branch, or to ensure the caller always clears progress.
| // The devices the Activity query will include (see resolve_multidevice_hosts) | ||
| const selected = this.resolve_multidevice_hosts(host); | ||
| const hosts = selected.length > 0 ? selected : [host]; | ||
| const key = [hosts.join(','), settingsStore.startOfDay].join('|'); |
There was a problem hiding this comment.
get_earliest_date, the cache key is [hosts.join(','), settingsStore.startOfDay].join('|'). However, the cache is a module-level Map that persists across store instances and even across different hosts. The key includes the hosts list and the startOfDay offset, but it does not include the bucket IDs or the set of buckets. If a new bucket is added to a host (e.g. a new watcher starts collecting data), the earliest date could change, but the cache will return the old value until the page is reloaded. This is a stale cache issue. The comment says 'Cached per host and day-start offset', but the cache is never invalidated when buckets change. For the All time period, this means the start date could be wrong (too late) if a new bucket with earlier data is added after the first load. The impact is that the All time range might not include all data. The fix would be to include a bucket signature in the cache key or to invalidate the cache when buckets change.
| this.progress_add(periods.length); | ||
| const chunks = []; | ||
| for (const period of periods) { | ||
| const data = await getClient().query([period], q, { |
There was a problem hiding this comment.
query_editor, when the query fails (e.g. network error), the getClient().query call is not wrapped in a try/catch, unlike query_android which uses .catch(this.errorHandler). If the editor query throws, the entire ensure_loaded promise rejects, and the progress bar is never cleared (since this.progress = null is after the editor query in ensure_loaded). This can leave the UI in a broken state with a stuck progress bar and no data. The same applies to query_desktop_full and query_multidevice_full which also don't catch errors. However, the existing code before this PR also didn't catch errors in these queries, so this is not a new regression. But the new progress bar makes the failure more visible. The impact is that a transient error during a long All time query will leave the progress bar stuck and the view empty. The fix would be to wrap the query calls in try/catch and reset progress on error, or to use a finally block in ensure_loaded to clear progress.
|
Status of the 01:35 review batch, checked against #1008 (stacked on this branch):
Minimal guard before the if (!this.timeperiod) {
// All time's earliest-date lookup is still in flight; start from a single day.
const d = momentJsDate.format('YYYY-MM-DD');
this.setRange(d, d);
return;
}Otherwise the rest of the batch is addressed. CI is green on this head. |
Upstream highlights: - feat: custom date range period (ActivityWatch#1005) - feat: device selection (all / subset) in Activity view (ActivityWatch#1004) - feat: category rule priority (ActivityWatch#968) - feat: layered category sets (ActivityWatch#1027, ActivityWatch#1072) - feat: always show Year period (ActivityWatch#1003) - feat: AI summary privacy filter (ActivityWatch#948) - feat: 'Add on top of mine' import option (ActivityWatch#1071) - fix: multidevice queries use real bucket IDs (ActivityWatch#969, ActivityWatch#1068) - fix: timeline hourly bar Y-axis >1h (ActivityWatch#1021) - fix: CSV streaming export (ActivityWatch#993, ActivityWatch#997) - fix: Android browser data display (ActivityWatch#1069) - fix: various mobile/dark-mode/layout improvements Conflict resolution: took upstream for activity.ts (major refactor), multidevice.ts (Android support), Activity.vue, TimelineBarChart.vue, summary.ts. Re-applied our fixes on top: - query_active_history: bucket_sig cache clearing + per-event interval union (dead-watcher marathon immunity) - query_active_history_multidevice: bucket_sig + period-length cap - Removed useMultidevice references (upstream removed the setting in favour of device-selection UI); our code paths now always aggregate across all hosts
2 participants
Part of ActivityWatch/activitywatch#1465 (section 1, custom range). "All time" follows as a stacked PR.
Adds a custom range entry to the Activity period menu.
/activity/:host/range/2026-06-01..2026-09-25/view/...(both ends inclusive). Fits the existing:periodLength/:dateroute, so no router changes and old URLs keep working. Invalid ranges (bad dates, end before start) show a warning and fall back to today.todayindata()was never set, so the comparison was always false.query_category_time_by_period, via a sharedtimeperiodsForBarchart. Multi-day barchart labels now show dates instead of1..N.Tested against a real v0.14.0b5 aw-server (~4 years of data): a 117-day range loads in ~60s cold (per-day desktop queries dominate; monthly bars need 4 category requests). Unit tests for range parsing/shifting and barchart bucketing added.