fix: re-upload previous day's usage to catch mid-day updates#41
Conversation
|
@jbyoung12 is attempting to deploy a commit to the Pacific Systems Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR modifies the push command's backfill date calculation logic to conditionally include Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/__tests__/commands/push.test.ts (1)
685-703: Consider adding a boundary test for gap = MAX_BACKFILL_DAYS - 1.The critical boundary is at
gap = 6(should includelast_push_date) vsgap = 7(should cap and exclude it). Adding tests for both would strengthen confidence in the boundary condition.🧪 Optional: boundary tests
it("includes last_push_date when gap is MAX_BACKFILL_DAYS - 1 (6 days)", async () => { const sixDaysAgo = daysAgoStr(6); mockLoadConfig.mockReturnValue({ ...fakeConfig, last_push_date: sixDaysAgo }); mockRunCcusageRawAsync.mockResolvedValue("[]"); mockParseCcusageOutput.mockReturnValue({ data: [] }); await pushCommand({}); const [sinceArg] = mockRunCcusageRawAsync.mock.calls[0]!; const expectedSince = sixDaysAgo.replace(/-/g, ""); expect(sinceArg).toBe(expectedSince); // Should include last_push_date }); it("excludes last_push_date when gap equals MAX_BACKFILL_DAYS (7 days)", async () => { const sevenDaysAgo = daysAgoStr(7); mockLoadConfig.mockReturnValue({ ...fakeConfig, last_push_date: sevenDaysAgo }); mockRunCcusageRawAsync.mockResolvedValue("[]"); mockParseCcusageOutput.mockReturnValue({ data: [] }); await pushCommand({}); const [sinceArg] = mockRunCcusageRawAsync.mock.calls[0]!; const expectedSince = daysAgoStr(6).replace(/-/g, ""); // Should be 6 days ago, not 7 expect(sinceArg).toBe(expectedSince); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/__tests__/commands/push.test.ts` around lines 685 - 703, Add two boundary tests around MAX_BACKFILL_DAYS for the push flow: create one test that sets last_push_date to daysAgoStr(MAX_BACKFILL_DAYS - 1) (6 days) and asserts pushCommand calls mockRunCcusageRawAsync with sinceArg equal to that date (formatted without dashes), and a second test that sets last_push_date to daysAgoStr(MAX_BACKFILL_DAYS) (7 days) and asserts sinceArg is capped to daysAgoStr(MAX_BACKFILL_DAYS - 1) instead of the 7-day date; reuse the existing helpers and mocks (pushCommand, mockLoadConfig, mockRunCcusageRawAsync, mockParseCcusageOutput, daysAgoStr) and the same pattern used in the existing test to set up mocks and assert the first argument passed to mockRunCcusageRawAsync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/cli/__tests__/commands/push.test.ts`:
- Around line 685-703: Add two boundary tests around MAX_BACKFILL_DAYS for the
push flow: create one test that sets last_push_date to
daysAgoStr(MAX_BACKFILL_DAYS - 1) (6 days) and asserts pushCommand calls
mockRunCcusageRawAsync with sinceArg equal to that date (formatted without
dashes), and a second test that sets last_push_date to
daysAgoStr(MAX_BACKFILL_DAYS) (7 days) and asserts sinceArg is capped to
daysAgoStr(MAX_BACKFILL_DAYS - 1) instead of the 7-day date; reuse the existing
helpers and mocks (pushCommand, mockLoadConfig, mockRunCcusageRawAsync,
mockParseCcusageOutput, daysAgoStr) and the same pattern used in the existing
test to set up mocks and assert the first argument passed to
mockRunCcusageRawAsync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a42ad60-5154-4b3f-a8bb-eb749a243b4e
📒 Files selected for processing (2)
packages/cli/__tests__/commands/push.test.tspackages/cli/src/commands/push.ts
|
Thanks for catching this @jbyoung12! Great find — the smart sync was silently dropping same-day usage because it never re-fetched Your fix is merged. I applied one additional tweak on main: changed the boundary check from |
Summary
last_push_dateis yesterday, syncs yesterday + today (not just today)Changes
packages/cli/src/commands/push.tsMAX_BACKFILL_DAYS, includeslast_push_datein sync rangeTest plan
straude --dry-run🤖 Generated with Claude Code