From e8f10fe8a9164f24baecf339ff239cf5a6964d9f Mon Sep 17 00:00:00 2001 From: sudacode Date: Tue, 12 May 2026 02:50:05 -0700 Subject: [PATCH] fix: macOS visible-overlay blur no longer invokes Windows-only blur call - Split win32/darwin branches in handleOverlayWindowBlurred so darwin visible blur returns early without calling onWindowsVisibleOverlayBlur - Add regression test asserting Windows callback stays inactive on macOS visible overlay blur - Close TASK-347 --- ...it-review-round-after-stats-session-fix.md | 33 +++++++++++++++---- src/core/services/overlay-window-input.ts | 5 ++- src/core/services/overlay-window.test.ts | 23 +++++++++++++ 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/backlog/tasks/task-347 - Address-PR-57-CodeRabbit-review-round-after-stats-session-fix.md b/backlog/tasks/task-347 - Address-PR-57-CodeRabbit-review-round-after-stats-session-fix.md index a88e6981..f3e7d685 100644 --- a/backlog/tasks/task-347 - Address-PR-57-CodeRabbit-review-round-after-stats-session-fix.md +++ b/backlog/tasks/task-347 - Address-PR-57-CodeRabbit-review-round-after-stats-session-fix.md @@ -1,11 +1,11 @@ --- id: TASK-347 title: Address PR 57 CodeRabbit review round after stats session fix -status: In Progress +status: Done assignee: - codex created_date: '2026-05-12 07:02' -updated_date: '2026-05-12 07:02' +updated_date: '2026-05-12 09:48' labels: - pr-review - coderabbit @@ -13,6 +13,9 @@ labels: dependencies: [] references: - 'https://github.com/ksyasuda/SubMiner/pull/57' +modified_files: + - src/core/services/overlay-window-input.ts + - src/core/services/overlay-window.test.ts priority: high --- @@ -24,10 +27,10 @@ Assess and address the 2026-05-12 CodeRabbit review on PR #57 plus the current r ## Acceptance Criteria -- [ ] #1 Still-valid latest CodeRabbit findings on PR #57 are fixed or documented as skipped with rationale. -- [ ] #2 CI failure context is inspected and any repo-relevant failing tests or formatting issues are fixed. -- [ ] #3 Regression coverage is added for behavior changes where practical before production edits. -- [ ] #4 Relevant local verification passes. +- [x] #1 Still-valid latest CodeRabbit findings on PR #57 are fixed or documented as skipped with rationale. +- [x] #2 CI failure context is inspected and any repo-relevant failing tests or formatting issues are fixed. +- [x] #3 Regression coverage is added for behavior changes where practical before production edits. +- [x] #4 Relevant local verification passes. ## Implementation Plan @@ -37,4 +40,22 @@ Assess and address the 2026-05-12 CodeRabbit review on PR #57 plus the current r 2. Add or update focused regression tests first for behavior changes: stats token aggregation, fullscreen refresh exit cancellation, Hyprland monitor parse failure, and title-only event filtering. 3. Apply minimal production fixes for still-valid findings, plus the subtitle-render duplicate test coverage item. 4. Run targeted tests first, then format/typecheck and broader relevant gates; update the task with results. + +Address latest CodeRabbit callback-scope finding: add regression coverage that macOS visible-overlay blur does not invoke onWindowsVisibleOverlayBlur, then split win32/darwin blur handling in src/core/services/overlay-window-input.ts. + +## Implementation Notes + + +2026-05-12 09:47: Assessed latest PR #57 CodeRabbit comments via gh. Prior review findings in comments are marked addressed by CodeRabbit; latest still-actionable item was macOS visible-overlay blur invoking the Windows-only blur callback in overlay-window-input.ts. + +Added regression coverage showing macOS visible-overlay blur leaves onWindowsVisibleOverlayBlur inactive; test failed before production change and passed after splitting win32/darwin handling. + +Verification: bun test src/core/services/overlay-window.test.ts; bun run typecheck; git diff --check. PR checks: build-test-audit pass, GitGuardian pass, CodeRabbit pass/skipped, Claude skipped. + + +## Final Summary + + +Addressed the latest CodeRabbit callback-scope review on PR #57. Windows visible-overlay blur still invokes onWindowsVisibleOverlayBlur and returns without restacking; macOS visible-overlay blur now returns without restacking and without invoking the Windows-only callback. Added focused regression coverage and verified targeted tests, typecheck, diff whitespace, and current PR checks. + diff --git a/src/core/services/overlay-window-input.ts b/src/core/services/overlay-window-input.ts index 8edd9a7e..54e0c1b7 100644 --- a/src/core/services/overlay-window-input.ts +++ b/src/core/services/overlay-window-input.ts @@ -70,10 +70,13 @@ export function handleOverlayWindowBlurred(options: { platform?: NodeJS.Platform; }): boolean { const platform = options.platform ?? process.platform; - if ((platform === 'win32' || platform === 'darwin') && options.kind === 'visible') { + if (platform === 'win32' && options.kind === 'visible') { options.onWindowsVisibleOverlayBlur?.(); return false; } + if (platform === 'darwin' && options.kind === 'visible') { + return false; + } if (options.kind === 'visible' && !options.isOverlayVisible(options.kind)) { return false; diff --git a/src/core/services/overlay-window.test.ts b/src/core/services/overlay-window.test.ts index edd7b1bb..b69e4346 100644 --- a/src/core/services/overlay-window.test.ts +++ b/src/core/services/overlay-window.test.ts @@ -166,6 +166,29 @@ test('handleOverlayWindowBlurred skips macOS visible overlay restacking after fo assert.deepEqual(calls, []); }); +test('handleOverlayWindowBlurred leaves Windows callback inactive on macOS visible overlay blur', () => { + const calls: string[] = []; + + const handled = handleOverlayWindowBlurred({ + kind: 'visible', + windowVisible: true, + isOverlayVisible: () => true, + ensureOverlayWindowLevel: () => { + calls.push('ensure-level'); + }, + moveWindowTop: () => { + calls.push('move-top'); + }, + onWindowsVisibleOverlayBlur: () => { + calls.push('windows-visible-blur'); + }, + platform: 'darwin', + }); + + assert.equal(handled, false); + assert.deepEqual(calls, []); +}); + test('handleOverlayWindowBlurred preserves active visible/modal window stacking', () => { const calls: string[] = [];