fix: address PR 36 CodeRabbit follow-ups

This commit is contained in:
2026-03-29 00:05:18 -07:00
parent c5fcd50cc0
commit cb37c68248
7 changed files with 89 additions and 41 deletions

View File

@@ -20,7 +20,7 @@ Priority keys:
| ID | Pri | Status | Area | Title |
| ------ | --- | ------ | -------------- | --------------------------------------------------- |
| SM-013 | P1 | doing | review-followup | Address PR #36 CodeRabbit action items |
| SM-013 | P1 | done | review-followup | Address PR #36 CodeRabbit action items |
## Ready
@@ -241,7 +241,7 @@ Done:
Title: Address PR #36 CodeRabbit action items
Priority: P1
Status: doing
Status: done
Scope:
- `plugins/subminer-workflow/skills/subminer-change-verification/scripts/verify_subminer_change.sh`
@@ -251,7 +251,16 @@ Scope:
- `src/core/services/immersion-tracker/maintenance.ts`
- `src/main/boot/services.ts`
- `src/main/character-dictionary-runtime/zip.test.ts`
Acceptance:
Acceptance:
- fix valid open CodeRabbit findings on PR #36
- add focused regression coverage for behavior changes where practical
- verify touched tests plus typecheck stay green
Done:
- hardened `--artifact-dir` validation in the verification script
- fixed trend aggregation rounding and monthly ratio bucketing
- preserved unwatched anime episodes in episode queries
- restored seconds-based aggregate timestamps in shared maintenance
- fixed the startup refactor compile break by making the predicates local at the call site
- verified with `bun test src/core/services/immersion-tracker/__tests__/query.test.ts src/core/services/immersion-tracker/__tests__/query-split-modules.test.ts` and `bun run typecheck`

View File

@@ -155,29 +155,32 @@ function upsertDailyRollupsForGroups(
CAST(julianday(s.started_at_ms / 1000, 'unixepoch', 'localtime') - 2440587.5 AS INTEGER) AS rollup_day,
s.video_id AS video_id,
COUNT(DISTINCT s.session_id) AS total_sessions,
COALESCE(SUM(sm.max_active_ms), 0) / 60000.0 AS total_active_min,
COALESCE(SUM(sm.max_lines), 0) AS total_lines_seen,
COALESCE(SUM(sm.max_tokens), 0) AS total_tokens_seen,
COALESCE(SUM(sm.max_cards), 0) AS total_cards,
COALESCE(SUM(COALESCE(sm.max_active_ms, s.active_watched_ms)), 0) / 60000.0 AS total_active_min,
COALESCE(SUM(COALESCE(sm.max_lines, s.lines_seen)), 0) AS total_lines_seen,
COALESCE(SUM(COALESCE(sm.max_tokens, s.tokens_seen)), 0) AS total_tokens_seen,
COALESCE(SUM(COALESCE(sm.max_cards, s.cards_mined)), 0) AS total_cards,
CASE
WHEN COALESCE(SUM(sm.max_active_ms), 0) > 0
THEN (COALESCE(SUM(sm.max_cards), 0) * 60.0) / (COALESCE(SUM(sm.max_active_ms), 0) / 60000.0)
WHEN COALESCE(SUM(COALESCE(sm.max_active_ms, s.active_watched_ms)), 0) > 0
THEN (COALESCE(SUM(COALESCE(sm.max_cards, s.cards_mined)), 0) * 60.0)
/ (COALESCE(SUM(COALESCE(sm.max_active_ms, s.active_watched_ms)), 0) / 60000.0)
ELSE NULL
END AS cards_per_hour,
CASE
WHEN COALESCE(SUM(sm.max_active_ms), 0) > 0
THEN COALESCE(SUM(sm.max_tokens), 0) / (COALESCE(SUM(sm.max_active_ms), 0) / 60000.0)
WHEN COALESCE(SUM(COALESCE(sm.max_active_ms, s.active_watched_ms)), 0) > 0
THEN COALESCE(SUM(COALESCE(sm.max_tokens, s.tokens_seen)), 0)
/ (COALESCE(SUM(COALESCE(sm.max_active_ms, s.active_watched_ms)), 0) / 60000.0)
ELSE NULL
END AS tokens_per_min,
CASE
WHEN COALESCE(SUM(sm.max_lookups), 0) > 0
THEN CAST(COALESCE(SUM(sm.max_hits), 0) AS REAL) / CAST(SUM(sm.max_lookups) AS REAL)
WHEN COALESCE(SUM(COALESCE(sm.max_lookups, s.lookup_count)), 0) > 0
THEN CAST(COALESCE(SUM(COALESCE(sm.max_hits, s.lookup_hits)), 0) AS REAL)
/ CAST(COALESCE(SUM(COALESCE(sm.max_lookups, s.lookup_count)), 0) AS REAL)
ELSE NULL
END AS lookup_hit_rate,
? AS CREATED_DATE,
? AS LAST_UPDATE_DATE
FROM imm_sessions s
JOIN (
LEFT JOIN (
SELECT
t.session_id,
MAX(t.active_watched_ms) AS max_active_ms,
@@ -227,14 +230,14 @@ function upsertMonthlyRollupsForGroups(
CAST(strftime('%Y%m', s.started_at_ms / 1000, 'unixepoch', 'localtime') AS INTEGER) AS rollup_month,
s.video_id AS video_id,
COUNT(DISTINCT s.session_id) AS total_sessions,
COALESCE(SUM(sm.max_active_ms), 0) / 60000.0 AS total_active_min,
COALESCE(SUM(sm.max_lines), 0) AS total_lines_seen,
COALESCE(SUM(sm.max_tokens), 0) AS total_tokens_seen,
COALESCE(SUM(sm.max_cards), 0) AS total_cards,
COALESCE(SUM(COALESCE(sm.max_active_ms, s.active_watched_ms)), 0) / 60000.0 AS total_active_min,
COALESCE(SUM(COALESCE(sm.max_lines, s.lines_seen)), 0) AS total_lines_seen,
COALESCE(SUM(COALESCE(sm.max_tokens, s.tokens_seen)), 0) AS total_tokens_seen,
COALESCE(SUM(COALESCE(sm.max_cards, s.cards_mined)), 0) AS total_cards,
? AS CREATED_DATE,
? AS LAST_UPDATE_DATE
FROM imm_sessions s
JOIN (
LEFT JOIN (
SELECT
t.session_id,
MAX(t.active_watched_ms) AS max_active_ms,
@@ -276,7 +279,7 @@ function getAffectedRollupGroups(
FROM imm_session_telemetry t
JOIN imm_sessions s
ON s.session_id = t.session_id
WHERE t.sample_ms > ?
WHERE t.sample_ms >= ?
`,
)
.all(lastRollupSampleMs) as unknown as RollupGroupRow[]

View File

@@ -186,7 +186,7 @@ export function getSimilarWords(db: DatabaseSync, wordId: number, limit = 10): S
headword: string;
reading: string;
} | null;
if (!word) return [];
if (!word || word.headword.trim() === '') return [];
return db
.prepare(
`

View File

@@ -205,7 +205,7 @@ export function getQueryHints(db: DatabaseSync): {
const now = new Date();
const todayLocal = Math.floor(
(now.getTime() / 1000 - now.getTimezoneOffset() * 60) / 86_400,
new Date(now.getFullYear(), now.getMonth(), now.getDate()).getTime() / 86_400_000,
);
const episodesToday =

View File

@@ -272,5 +272,11 @@ export function deleteSessionsByIds(db: DatabaseSync, sessionIds: number[]): voi
}
export function toDbMs(ms: number | bigint): bigint {
return BigInt(Math.trunc(Number(ms)));
if (typeof ms === 'bigint') {
return ms;
}
if (!Number.isFinite(ms)) {
throw new TypeError(`Invalid database timestamp: ${ms}`);
}
return BigInt(Math.trunc(ms));
}

View File

@@ -168,7 +168,7 @@ function buildAggregatedTrendRows(rollups: ImmersionSessionRollupRow[]) {
words: 0,
sessions: 0,
};
existing.activeMin += Math.round(rollup.totalActiveMin);
existing.activeMin += rollup.totalActiveMin;
existing.cards += rollup.totalCards;
existing.words += rollup.totalTokensSeen;
existing.sessions += rollup.totalSessions;
@@ -179,7 +179,7 @@ function buildAggregatedTrendRows(rollups: ImmersionSessionRollupRow[]) {
.sort(([left], [right]) => left - right)
.map(([key, value]) => ({
label: makeTrendLabel(key),
activeMin: value.activeMin,
activeMin: Math.round(value.activeMin),
cards: value.cards,
words: value.words,
sessions: value.sessions,
@@ -243,22 +243,32 @@ function buildSessionSeriesByMonth(
.map(([monthKey, value]) => ({ label: makeTrendLabel(monthKey), value }));
}
function buildLookupsPerHundredWords(sessions: TrendSessionMetricRow[]): TrendChartPoint[] {
const lookupsByDay = new Map<number, number>();
const wordsByDay = new Map<number, number>();
function buildLookupsPerHundredWords(
sessions: TrendSessionMetricRow[],
groupBy: TrendGroupBy,
): TrendChartPoint[] {
const lookupsByBucket = new Map<number, number>();
const wordsByBucket = new Map<number, number>();
for (const session of sessions) {
const epochDay = getLocalEpochDay(session.startedAtMs);
lookupsByDay.set(epochDay, (lookupsByDay.get(epochDay) ?? 0) + session.yomitanLookupCount);
wordsByDay.set(epochDay, (wordsByDay.get(epochDay) ?? 0) + getTrendSessionWordCount(session));
const bucketKey =
groupBy === 'month' ? getLocalMonthKey(session.startedAtMs) : getLocalEpochDay(session.startedAtMs);
lookupsByBucket.set(
bucketKey,
(lookupsByBucket.get(bucketKey) ?? 0) + session.yomitanLookupCount,
);
wordsByBucket.set(
bucketKey,
(wordsByBucket.get(bucketKey) ?? 0) + getTrendSessionWordCount(session),
);
}
return Array.from(lookupsByDay.entries())
return Array.from(lookupsByBucket.entries())
.sort(([left], [right]) => left - right)
.map(([epochDay, lookups]) => {
const words = wordsByDay.get(epochDay) ?? 0;
.map(([bucketKey, lookups]) => {
const words = wordsByBucket.get(bucketKey) ?? 0;
return {
label: dayLabel(epochDay),
label: groupBy === 'month' ? makeTrendLabel(bucketKey) : dayLabel(bucketKey),
value: words > 0 ? +((lookups / words) * 100).toFixed(1) : 0,
};
});
@@ -595,7 +605,7 @@ export function getTrendsDashboard(
const animePerDay = {
episodes: buildEpisodesPerAnimeFromDailyRollups(dailyRollups, titlesByVideoId),
watchTime: buildPerAnimeFromDailyRollups(dailyRollups, titlesByVideoId, (rollup) =>
Math.round(rollup.totalActiveMin),
rollup.totalActiveMin,
),
cards: buildPerAnimeFromDailyRollups(
dailyRollups,
@@ -633,7 +643,7 @@ export function getTrendsDashboard(
),
},
ratios: {
lookupsPerHundred: buildLookupsPerHundredWords(sessions),
lookupsPerHundred: buildLookupsPerHundredWords(sessions, groupBy),
},
animePerDay,
animeCumulative: {

View File

@@ -4480,11 +4480,31 @@ const { runAndApplyStartupState } = composeHeadlessStartupHandlers<
});
runAndApplyStartupState();
if (isAnilistTrackingEnabled(getResolvedConfig())) {
void refreshAnilistClientSecretStateIfEnabled({ force: true });
anilistStateRuntime.refreshRetryQueueState();
const shouldUseMinimalStartup = Boolean(
appState.initialArgs?.texthooker ||
(appState.initialArgs?.stats &&
(appState.initialArgs?.statsCleanup ||
appState.initialArgs?.statsBackground ||
appState.initialArgs?.statsStop)),
);
const shouldSkipHeavyStartup = Boolean(
appState.initialArgs &&
(shouldRunSettingsOnlyStartup(appState.initialArgs) ||
appState.initialArgs.stats ||
appState.initialArgs.dictionary ||
appState.initialArgs.setup),
);
if (!appState.initialArgs || (!shouldUseMinimalStartup && !shouldSkipHeavyStartup)) {
if (isAnilistTrackingEnabled(getResolvedConfig())) {
void refreshAnilistClientSecretStateIfEnabled({ force: true }).catch((error) => {
logger.error('Failed to refresh AniList client secret state during startup', error);
});
anilistStateRuntime.refreshRetryQueueState();
}
void initializeDiscordPresenceService().catch((error) => {
logger.error('Failed to initialize Discord presence service during startup', error);
});
}
void initializeDiscordPresenceService();
const { createMainWindow: createMainWindowHandler, createModalWindow: createModalWindowHandler } =
createOverlayWindowRuntimeHandlers<BrowserWindow>({
createOverlayWindowDeps: {