Skip to content

Commit ae8c418

Browse files
authored
fix: merge monthly counts into existing state instead of overwriting (#538)
* fix: merge monthly counts into existing state instead of overwriting When updateMonthlyAnalytics() stored fresh API results, it replaced the entire monthlyMergedCounts/monthlyClosedCounts/monthlyOpenedCounts objects. If the GitHub Search API returned only recent months (due to pagination limits, caching, or incomplete results), all historical months were lost. Now fresh API data is merged into existing state: months present in the API response are updated, months only in existing state are preserved. Fixes #537 * style: fix prettier formatting * ci: add pre-commit hook for prettier format check
1 parent fd3ed0d commit ae8c418

File tree

3 files changed

+74
-5
lines changed

3 files changed

+74
-5
lines changed

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
"prepare": "[ -n \"$CI\" ] || simple-git-hooks"
2121
},
2222
"simple-git-hooks": {
23+
"pre-commit": "pnpm format:check",
2324
"commit-msg": "./scripts/commit-msg.sh \"$1\""
2425
},
2526
"keywords": [

packages/core/src/commands/dashboard-data.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@
33
*/
44

55
import { describe, it, expect } from 'vitest';
6-
import { buildDashboardStats, computePRsByRepo, computeTopRepos, getMonthlyData } from './dashboard-data.js';
6+
import {
7+
buildDashboardStats,
8+
computePRsByRepo,
9+
computeTopRepos,
10+
getMonthlyData,
11+
mergeMonthlyCounts,
12+
} from './dashboard-data.js';
713
import type { DailyDigest, AgentState, ShelvedPRRef } from '../core/types.js';
814

915
function makeDigest(overrides: Partial<DailyDigest> = {}): DailyDigest {
@@ -276,3 +282,46 @@ describe('getMonthlyData', () => {
276282
expect(result.monthlyOpened).toEqual({});
277283
});
278284
});
285+
286+
describe('mergeMonthlyCounts', () => {
287+
it('preserves existing months not in fresh data', () => {
288+
const existing = { '2025-06': 9, '2025-07': 26, '2026-01': 11 };
289+
const fresh = { '2026-02': 14, '2026-03': 4 };
290+
const result = mergeMonthlyCounts(existing, fresh);
291+
expect(result).toEqual({
292+
'2025-06': 9,
293+
'2025-07': 26,
294+
'2026-01': 11,
295+
'2026-02': 14,
296+
'2026-03': 4,
297+
});
298+
});
299+
300+
it('updates existing months when fresh data has them', () => {
301+
const existing = { '2026-02': 10, '2026-03': 2 };
302+
const fresh = { '2026-02': 14, '2026-03': 4 };
303+
const result = mergeMonthlyCounts(existing, fresh);
304+
expect(result).toEqual({ '2026-02': 14, '2026-03': 4 });
305+
});
306+
307+
it('returns copy of existing when fresh is empty', () => {
308+
const existing = { '2025-06': 9, '2025-07': 26 };
309+
const result = mergeMonthlyCounts(existing, {});
310+
expect(result).toEqual(existing);
311+
expect(result).not.toBe(existing);
312+
});
313+
314+
it('returns fresh data when existing is empty', () => {
315+
const fresh = { '2026-02': 14, '2026-03': 4 };
316+
const result = mergeMonthlyCounts({}, fresh);
317+
expect(result).toEqual(fresh);
318+
});
319+
320+
it('does not mutate inputs', () => {
321+
const existing = { '2026-01': 5 };
322+
const fresh = { '2026-02': 10 };
323+
mergeMonthlyCounts(existing, fresh);
324+
expect(existing).toEqual({ '2026-01': 5 });
325+
expect(fresh).toEqual({ '2026-02': 10 });
326+
});
327+
});

packages/core/src/commands/dashboard-data.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,28 @@ export function buildDashboardStats(digest: DailyDigest, state: Readonly<AgentSt
3737
};
3838
}
3939

40+
/**
41+
* Merge fresh API counts into existing stored counts.
42+
* Months present in the fresh data are updated; months only in the existing data are preserved.
43+
* This prevents historical data loss when the API returns incomplete results
44+
* (e.g. due to pagination limits or transient failures).
45+
*/
46+
export function mergeMonthlyCounts(
47+
existing: Record<string, number>,
48+
fresh: Record<string, number>,
49+
): Record<string, number> {
50+
const merged = { ...existing };
51+
for (const [month, count] of Object.entries(fresh)) {
52+
merged[month] = count;
53+
}
54+
return merged;
55+
}
56+
4057
/**
4158
* Persist monthly chart analytics (merged, closed, opened) to state.
4259
* Each metric is isolated so partial failures don't produce inconsistent state.
43-
* Skips overwriting when data is empty to avoid wiping chart data on transient API failures.
60+
* Fresh API results are merged into existing data so historical months are preserved.
61+
* Skips updating when fresh data is empty to avoid wiping chart data on transient API failures.
4462
*/
4563
export function updateMonthlyAnalytics(
4664
prs: Array<{ createdAt?: string }>,
@@ -50,17 +68,18 @@ export function updateMonthlyAnalytics(
5068
openedFromClosed: Record<string, number>,
5169
): void {
5270
const stateManager = getStateManager();
71+
const state = stateManager.getState();
5372

5473
try {
5574
if (Object.keys(monthlyCounts).length > 0) {
56-
stateManager.setMonthlyMergedCounts(monthlyCounts);
75+
stateManager.setMonthlyMergedCounts(mergeMonthlyCounts(state.monthlyMergedCounts || {}, monthlyCounts));
5776
}
5877
} catch (error) {
5978
warn(MODULE, `Failed to store monthly merged counts: ${errorMessage(error)}`);
6079
}
6180
try {
6281
if (Object.keys(monthlyClosedCounts).length > 0) {
63-
stateManager.setMonthlyClosedCounts(monthlyClosedCounts);
82+
stateManager.setMonthlyClosedCounts(mergeMonthlyCounts(state.monthlyClosedCounts || {}, monthlyClosedCounts));
6483
}
6584
} catch (error) {
6685
warn(MODULE, `Failed to store monthly closed counts: ${errorMessage(error)}`);
@@ -77,7 +96,7 @@ export function updateMonthlyAnalytics(
7796
}
7897
}
7998
if (Object.keys(combinedOpenedCounts).length > 0) {
80-
stateManager.setMonthlyOpenedCounts(combinedOpenedCounts);
99+
stateManager.setMonthlyOpenedCounts(mergeMonthlyCounts(state.monthlyOpenedCounts || {}, combinedOpenedCounts));
81100
}
82101
} catch (error) {
83102
warn(MODULE, `Failed to store monthly opened counts: ${errorMessage(error)}`);

0 commit comments

Comments
 (0)