feat(events): add summary subcommand; switch date parser to local TZ (FT-1922)#35
Merged
Conversation
BREAKING CHANGE: Date inputs now resolve to local timezone instead of UTC for ISO plain dates (`2024-01-01`), `today`, `yesterday`, and the calendar keywords (`month-start`, etc.). Explicit ISO datetimes with `Z` and epoch ms are unchanged. Set `TZ=UTC` to restore UTC behavior for automation that relied on it. - `2024-01-01` is now local midnight on Jan 1 (was UTC midnight). - `today` is now 00:00 local today (was the current instant). - `yesterday` is now 00:00 local yesterday (was the current instant minus 24h). - Calendar keywords were previously UTC; now local. They were introduced earlier in this branch and never shipped, but flipping here keeps the parser internally consistent.
…matting (FT-1922)
…TZ (FT-1922) Two related fixes: 1. -o json and -o yaml used to short-circuit to the raw API payload, identical to --raw. They now serialize the aggregated, period-rolled, optionally-cumulative data so structured consumers see the same rollup as the table view. --raw still bypasses everything for debugging and raw passthrough. 2. rollUpEvents bucketed timestamps in UTC while formatPeriodCell reads them in local TZ, causing the displayed week to drift by one in non-UTC environments. bucketStart now computes Monday-start weeks (and month/day starts) in local TZ, matching the formatter.
WalkthroughThis pull request introduces a comprehensive events summary feature for the A/B Smartly CLI. It adds a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/events/summary.ts`:
- Around line 264-268: The bar visualization forces a minimum length of 1 which
makes zero totals appear non-zero; update the computation of barLen in the block
that calls pickRowTotal so that when count === 0 barLen becomes 0, otherwise
keep Math.max(1, Math.round((count / max) * BAR_WIDTH)) to preserve visibility
for small non-zero counts; ensure the subsequent bar, colored and lines.push
logic (references: pickRowTotal, BAR_WIDTH, options.noColor) handles an empty
bar string correctly so zeros render without a block.
In `@src/lib/utils/date-parser.ts`:
- Around line 101-110: The thrown Error message for invalid dates (the throw new
Error block referencing dateStr) omits "now" from the list of accepted relative
inputs; update the error string to include "now" among the Relative examples so
the guidance matches supported inputs (i.e., add "now" alongside "yesterday,
today" in the message emitted when parse fails).
- Around line 85-99: The iso date code path using isoDateMatch currently
constructs a Date which can silently normalize invalid calendar dates (e.g.,
2024-02-31); after creating the Date from y, mo, d (variable t and the Date
instance), validate by comparing date.getFullYear() === y, date.getMonth() ===
mo and date.getDate() === d and throw the existing Error if any component
differs. Likewise, for the ISO datetime branch (the
/^\d{4}-\d{2}-\d{2}T[\d:.-]+Z$/ test), extract the numeric components from the
string (year, month, day, hour, minute, second, millisecond as present),
construct a Date and then verify each corresponding
date.getUTCFullYear()/getUTCMonth()/getUTCDate()/getUTCHours()/getUTCMinutes()/getUTCSeconds()/getUTCMilliseconds()
matches the parsed values; if any mismatch occurs, throw the same Invalid date
error instead of accepting the normalized value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67acf30a-7718-4e12-ba45-f5f7bb79841c
📒 Files selected for processing (11)
README.mdpackage.jsonsrc/api-client/api-client.tssrc/commands/events/events.test.tssrc/commands/events/index.tssrc/commands/events/summary.test.tssrc/commands/events/summary.tssrc/core/events/summary.test.tssrc/core/events/summary.tssrc/lib/utils/date-parser.test.tssrc/lib/utils/date-parser.ts
Comment on lines
+264
to
+268
| const count = pickRowTotal(row, options.eventType); | ||
| const barLen = Math.max(1, Math.round((count / max) * BAR_WIDTH)); | ||
| const bar = '█'.repeat(barLen); | ||
| const colored = options.noColor ? bar : chalk.cyan(bar); | ||
| lines.push(`${period} ${'Total'.padEnd(teamWidth)} ${colored} ${count.toLocaleString()}`); |
There was a problem hiding this comment.
Zero totals are rendered as non-zero bars in bar visualisation.
At Line 265, forcing a minimum bar length of 1 makes 0 look non-zero in total mode. This misrepresents the data.
Suggested fix
- const barLen = Math.max(1, Math.round((count / max) * BAR_WIDTH));
+ const barLen = count === 0 ? 0 : Math.max(1, Math.round((count / max) * BAR_WIDTH));
const bar = '█'.repeat(barLen);📝 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
| const count = pickRowTotal(row, options.eventType); | |
| const barLen = Math.max(1, Math.round((count / max) * BAR_WIDTH)); | |
| const bar = '█'.repeat(barLen); | |
| const colored = options.noColor ? bar : chalk.cyan(bar); | |
| lines.push(`${period} ${'Total'.padEnd(teamWidth)} ${colored} ${count.toLocaleString()}`); | |
| const count = pickRowTotal(row, options.eventType); | |
| const barLen = count === 0 ? 0 : Math.max(1, Math.round((count / max) * BAR_WIDTH)); | |
| const bar = '█'.repeat(barLen); | |
| const colored = options.noColor ? bar : chalk.cyan(bar); | |
| lines.push(`${period} ${'Total'.padEnd(teamWidth)} ${colored} ${count.toLocaleString()}`); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/events/summary.ts` around lines 264 - 268, The bar visualization
forces a minimum length of 1 which makes zero totals appear non-zero; update the
computation of barLen in the block that calls pickRowTotal so that when count
=== 0 barLen becomes 0, otherwise keep Math.max(1, Math.round((count / max) *
BAR_WIDTH)) to preserve visibility for small non-zero counts; ensure the
subsequent bar, colored and lines.push logic (references: pickRowTotal,
BAR_WIDTH, options.noColor) handles an empty bar string correctly so zeros
render without a block.
Comment on lines
+85
to
99
| const isoDateMatch = /^(\d{4})-(\d{2})-(\d{2})$/.exec(dateStr); | ||
| if (isoDateMatch) { | ||
| const y = parseInt(isoDateMatch[1]!, 10); | ||
| const mo = parseInt(isoDateMatch[2]!, 10) - 1; | ||
| const d = parseInt(isoDateMatch[3]!, 10); | ||
| const t = new Date(y, mo, d).getTime(); | ||
| if (isNaN(t)) throw new Error(`Invalid date: "${dateStr}" could not be parsed`); | ||
| return t; | ||
| } | ||
|
|
||
| const date = new Date(dateStr); | ||
| if (isNaN(date.getTime())) { | ||
| throw new Error(`Invalid date: "${dateStr}" could not be parsed`); | ||
| if (/^\d{4}-\d{2}-\d{2}T[\d:.-]+Z$/.test(dateStr)) { | ||
| const t = new Date(dateStr).getTime(); | ||
| if (isNaN(t)) throw new Error(`Invalid date: "${dateStr}" could not be parsed`); | ||
| return t; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verifies JS Date normalisation behaviour for invalid calendar dates.
node - <<'NODE'
const d1 = new Date(2024, 1, 31); // Feb 31 (invalid)
console.log('new Date(2024,1,31) =>', d1.toISOString());
const s = '2024-02-31T00:00:00Z';
const d2 = new Date(s);
console.log(`${s} =>`, isNaN(d2.getTime()) ? 'Invalid Date' : d2.toISOString());
NODERepository: absmartly/cli-ts
Length of output: 157
🏁 Script executed:
find . -name "date-parser.ts" -type fRepository: absmartly/cli-ts
Length of output: 91
🏁 Script executed:
if [ -f "src/lib/utils/date-parser.ts" ]; then
wc -l "src/lib/utils/date-parser.ts"
fiRepository: absmartly/cli-ts
Length of output: 93
🏁 Script executed:
if [ -f "src/lib/utils/date-parser.ts" ]; then
sed -n '80,110p' "src/lib/utils/date-parser.ts"
fiRepository: absmartly/cli-ts
Length of output: 1302
Invalid calendar dates are silently accepted and normalised instead of being rejected.
Lines 90 and 96 use JavaScript's Date constructor which normalises impossible dates rather than rejecting them. For example, 2024-02-31 is converted to 2024-03-02 without error. The isNaN() check cannot catch this because getTime() returns a valid timestamp for normalised dates.
To fix, validate that the parsed date components match the input:
Proposed fix
const isoDateMatch = /^(\d{4})-(\d{2})-(\d{2})$/.exec(dateStr);
if (isoDateMatch) {
const y = parseInt(isoDateMatch[1]!, 10);
const mo = parseInt(isoDateMatch[2]!, 10) - 1;
const d = parseInt(isoDateMatch[3]!, 10);
- const t = new Date(y, mo, d).getTime();
- if (isNaN(t)) throw new Error(`Invalid date: "${dateStr}" could not be parsed`);
- return t;
+ const parsed = new Date(y, mo, d);
+ if (
+ parsed.getFullYear() !== y ||
+ parsed.getMonth() !== mo ||
+ parsed.getDate() !== d
+ ) {
+ throw new Error(`Invalid date: "${dateStr}" could not be parsed`);
+ }
+ return parsed.getTime();
}
if (/^\d{4}-\d{2}-\d{2}T[\d:.-]+Z$/.test(dateStr)) {
- const t = new Date(dateStr).getTime();
- if (isNaN(t)) throw new Error(`Invalid date: "${dateStr}" could not be parsed`);
- return t;
+ const parsed = new Date(dateStr);
+ if (isNaN(parsed.getTime()) || parsed.toISOString() !== dateStr) {
+ throw new Error(`Invalid date: "${dateStr}" could not be parsed`);
+ }
+ return parsed.getTime();
}🧰 Tools
🪛 OpenGrep (1.20.0)
[ERROR] 85-85: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/utils/date-parser.ts` around lines 85 - 99, The iso date code path
using isoDateMatch currently constructs a Date which can silently normalize
invalid calendar dates (e.g., 2024-02-31); after creating the Date from y, mo, d
(variable t and the Date instance), validate by comparing date.getFullYear() ===
y, date.getMonth() === mo and date.getDate() === d and throw the existing Error
if any component differs. Likewise, for the ISO datetime branch (the
/^\d{4}-\d{2}-\d{2}T[\d:.-]+Z$/ test), extract the numeric components from the
string (year, month, day, hour, minute, second, millisecond as present),
construct a Date and then verify each corresponding
date.getUTCFullYear()/getUTCMonth()/getUTCDate()/getUTCHours()/getUTCMinutes()/getUTCSeconds()/getUTCMilliseconds()
matches the parsed values; if any mismatch occurs, throw the same Invalid date
error instead of accepting the normalized value.
Comment on lines
+101
to
+110
| throw new Error( | ||
| `Invalid date format: "${dateStr}"\n` + | ||
| `Expected formats:\n` + | ||
| ` - Relative: 7d, 2w, 30d ago, yesterday, today\n` + | ||
| ` - Calendar: month-start, last-month-start, last-month-end, year-start, last-year-start, last-year-end\n` + | ||
| ` - ISO 8601 date: 2024-01-01\n` + | ||
| ` - ISO 8601 datetime: 2024-01-01T00:00:00Z\n` + | ||
| ` - Milliseconds since epoch: 1704067200000\n` + | ||
| `\n` + | ||
| `Relative units: m (minutes), h (hours), d (days), w (weeks), mo (months), y (years)` |
There was a problem hiding this comment.
Error guidance should include now to match accepted inputs.
Line 104 lists relative examples but omits now, which is valid and supported.
💡 Proposed fix
- ` - Relative: 7d, 2w, 30d ago, yesterday, today\n` +
+ ` - Relative: 7d, 2w, 30d ago, yesterday, today, now\n` +📝 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
| throw new Error( | |
| `Invalid date format: "${dateStr}"\n` + | |
| `Expected formats:\n` + | |
| ` - Relative: 7d, 2w, 30d ago, yesterday, today\n` + | |
| ` - Calendar: month-start, last-month-start, last-month-end, year-start, last-year-start, last-year-end\n` + | |
| ` - ISO 8601 date: 2024-01-01\n` + | |
| ` - ISO 8601 datetime: 2024-01-01T00:00:00Z\n` + | |
| ` - Milliseconds since epoch: 1704067200000\n` + | |
| `\n` + | |
| `Relative units: m (minutes), h (hours), d (days), w (weeks), mo (months), y (years)` | |
| throw new Error( | |
| `Invalid date format: "${dateStr}"\n` + | |
| `Expected formats:\n` + | |
| ` - Relative: 7d, 2w, 30d ago, yesterday, today, now\n` + | |
| ` - Calendar: month-start, last-month-start, last-month-end, year-start, last-year-start, last-year-end\n` + | |
| ` - ISO 8601 date: 2024-01-01\n` + | |
| ` - ISO 8601 datetime: 2024-01-01T00:00:00Z\n` + | |
| ` - Milliseconds since epoch: 1704067200000\n` + | |
| `\n` + | |
| `Relative units: m (minutes), h (hours), d (days), w (weeks), mo (months), y (years)` | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/utils/date-parser.ts` around lines 101 - 110, The thrown Error
message for invalid dates (the throw new Error block referencing dateStr) omits
"now" from the list of accepted relative inputs; update the error string to
include "now" among the Relative examples so the guidance matches supported
inputs (i.e., add "now" alongside "yesterday, today" in the message emitted when
parse fails).
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
Add
abs events summaryfor per-team, per-period (day/week/month) event rollups, and switch the shared date parser to local-time semantics across the CLI.JIRA: FT-1922 · bumps version to 1.5.0
New:
abs events summaryWraps
GET /v1/events/summary. Defaults:--period week --group-by total --visualization table --event-type all.Flags:
--from,--to,--event-type all|goal|exposure,--group-by team|total,--period day|week|month,--cumulative,--visualization table|bar,--transpose.Behavior notes:
from > tovalidation client-side before the API call.YYYY-Www(e.g.2026-W19); month asYYYY-MM; day asYYYY-MM-DD.Unowned.--rawpasses the API payload through untouched;-o json|yamlemits the rolled-up data (the same data the table view shows), including team names alongside ids inside each row'steamsmap.--transposeswaps rows and columns in both the table view and the JSON/YAML shape (teams become the outer dimension; aperiods[]array gives ordering and per-period totals).Breaking-but-not-really: date parser now defaults to local TZ
parseDateFlagis shared acrossexperiments,events,activity-feed, etc. Changes:2026-05-01→ local midnight (was UTC midnight).today→ 00:00 local today (was current instant).yesterday→ 00:00 local yesterday (was now - 24h).month-start,last-month-start,last-month-end,year-start,last-year-start,last-year-end— all local.Zand epoch ms unchanged.TZ=UTC abs ….Marked SemVer-major (
feat!:) in the commit body but bumping to 1.5.0 since the package has no external consumers yet.Other changes
getEventsSummarytoAPIClient.src/core/events/summary.ts:getEventsSummary,rollUpEvents(UTC-safe ISO weeks, DST-aware bucket math),aggregateByTeam,applyCumulative.TZ=UTCescape hatch.Test plan
bun test:run— 2358 passed / 4 skippedbun typecheckcleanbun lintcleanbun format --checkcleanabs events summary --helpshows all 7 flags with correct defaultsabs events summary --from 30d --visualization barSummary by CodeRabbit
Release Notes
New Features
events summarysubcommand to display aggregated event analytics by team and time period--group-by,--period,--cumulative,--transpose) for flexible data analysisDocumentation
Chores