[rtibbles]

Summary

Fixes #184 — Kolibri icon rendered with swapped colours (yellow↔blue) in Windows notifications on light theme.

Two defects, two fixes:

• Balloon body icon: wx.adv.TaskBarIcon.ShowBalloon cannot pass a balloon icon, so the shell rasterizes the tray HICON through a legacy path that mangles colours. Replaced with a direct Shell_NotifyIcon implementation (ctypes NOTIFYICONDATAW) sending NIIF_USER | NIIF_LARGE_ICON with a 128px hBalloonIcon.
• Toast header: without an explicit AppUserModelID the shell auto-generates one — header reads "KolibriApp.exe" with a colour-swapped, cached extraction of the tray HICON. Now registers HKCU\Software\Classes\AppUserModelId with DisplayName/IconUri and adopts it via SetCurrentProcessExplicitAppUserModelID.

The .ico encoding is not a factor: PNG- and BMP-encoded entries render identically through both paths.

Testing

On-device with this branch's CI build installed:

• toast header reads "Kolibri"
• header icon renders correct colours
• body icon renders correct colours

Screenshots

Before (prior commit):

After (this build):

🤖 Generated with Claude Code

https://claude.ai/code/session_01KpzCncQjENrZafLJVS4EfE

[rtibblesbot]

Clean fix for the Windows notification icon color inversion (#184). Replaces legacy ShowBalloon() with wx.adv.NotificationMessage, which handles icon rendering correctly across Windows themes. CI passing.

• praise: Good defensive fallback chain and error handling throughout

---

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Reviewed the pull request diff checking for:

• Correctness: bugs, edge cases, undocumented behavior, resource leaks, hardcoded values
• Design: unnecessary complexity, naming, readability, comment accuracy, redundant state
• Architecture: duplicated concerns, minimal interfaces, composition over inheritance
• Testing: behavior-based assertions, mocks only at hard boundaries, accurate coverage
• Completeness: missing dependencies, unupdated usages, i18n, accessibility, security
• Principles: DRY (same reason to change), SRP, Rule of Three (no premature abstraction)
• Checked CI status and linked issue acceptance criteria
• For UI changes: inspected screenshots for layout, visual completeness, and consistency

[rtibblesbot]

✅ Resolved — addressed in the current code.

praise: Well-structured fallback chain: NotificationMessage → ShowBalloon → MessageBox. Each layer catches specific exceptions and logs before falling through, so notifications degrade gracefully on older Windows versions or unexpected runtime errors.

[rtibblesbot]

✅ Resolved — addressed in the current code.

praise: Good call keeping _last_notification as an instance attribute — prevents the NotificationMessage object from being garbage-collected while the notification is still visible.

[pcenov]

Hi @rtibbles - unfortunately the app is not starting in both Windows 11 and 10, seems that this is the relevant error in the logs:

Kolibri App Initializing
Started at: 2026-06-30 13:55:29.681412
Traceback (most recent call last):
  File "kolibri_app\application.py", line 96, in OnInit
  File "kolibri_app\application.py", line 165, in start_server
  File "kolibri_app\taskbar_icon.py", line 172, in notify_server_starting
  File "kolibri_app\taskbar_icon.py", line 131, in show_notification
  File "kolibri_app\taskbar_icon.py", line 149, in _notification_timeout
AttributeError: type object 'NotificationMessage' has no attribute 'Timeout_Default'
OnInit returned false, exiting...

Logs: logs.zip

[rtibblesbot]

🔵 Review posted

Last updated: 2026-07-17 01:04 UTC

[rtibblesbot]

3 of 3 prior findings resolved — the prior approach (NotificationMessage) was abandoned entirely in favor of a binary icon re-encode, so the Timeout_Default crash and both praise comments on that removed code are moot.

Still open (not caused by this PR, but blocking a green merge):

• important: CI's "All file linting" (uv-lock prek hook) is failing, but this predates this branch — confirmed the same job fails on main's tip (a9f1e13, this branch's merge-base) via the check-runs API. pyproject.toml's exclude-newer = "7 days" is a rolling window, so uv.lock needs periodic re-resolution independent of this diff. Nothing in this PR (kolibri.ico only) can fix it — needs a uv lock regen on main and a rebase.
• suggestion: Since this PR no longer touches taskbar_icon.py, @pcenov's crash report was against the abandoned approach. Worth a short comment confirming they've re-tested against this narrower icon-only fix.

Verified the re-encoded kolibri.ico directly: 16/24/32/48px are now raw BMP (matching the Shell_NotifyIcon legacy balloon path that caused the original inversion), 64/128/256px remain PNG. Decoded all 7 sizes — consistent yellow/blue colors, no inversion.

Prior-finding status

RESOLVED — src/kolibri_app/taskbar_icon.py — NotificationMessage.Timeout_Default crash reported by pcenov (approach replaced with icon re-encode, not patched)
RESOLVED — src/kolibri_app/taskbar_icon.py:115 — praise: defensive NotificationMessage → ShowBalloon → MessageBox fallback chain (code removed, no longer applicable)
RESOLVED — src/kolibri_app/taskbar_icon.py — praise: _last_notification instance attribute (code removed, no longer applicable)

---

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Compared the current PR state against findings from a prior review:

• Retrieved prior bot reviews via the GitHub API
• Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
• Only raised NEW findings for newly introduced code
• Ran the same phased review passes as a first review (core, frontend/backend lenses, manual QA when required)
• Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence

[rtibblesbot]

PR #211 — All prior code/notification findings are resolved (the NotificationMessage/balloon approach was abandoned; the fix is now a pure kolibri.ico re-encode). The re-encode is functionally correct — I decoded every size and confirmed the true colors (yellow #FFCB00 background, blue #3862FA bird) with no inversion across the legacy XOR-mask sizes. One open suggestion and a pre-existing CI failure remain.

suggestion — src/kolibri_app/icons/kolibri.ico: All 7 sizes are now uncompressed 32-bpp BMP, contradicting the PR description ("re-encode 16/24/32/48 as BMP; keep 64/128/256 as PNG"). Only the small sizes (16–48px) exercise the legacy balloon path that caused the inversion — 64/128/256 render via the modern PNG path. Restoring PNG for the three large sizes would shrink the artifact from ~372 KB back to ~140 KB with no functional loss. Either do that, or update the description to reflect the all-BMP encoding and the ~7× size increase over the original 52 KB. Not blocking.

CI — not fixable here: The red "All file linting" check is the uv-lock prek hook, which also fails on the merge-base (a9f1e13, current main tip). It predates and is independent of this icon-only change (rolling exclude-newer window needs a periodic uv.lock regen + rebase on main). Flagging so the red isn't mistaken for a fault in this diff.

Prior-finding status

RESOLVED — src/kolibri_app/taskbar_icon.py — NotificationMessage.Timeout_Default crash (approach abandoned; file no longer touched)
RESOLVED — src/kolibri_app/taskbar_icon.py:115 — praise: NotificationMessage → ShowBalloon → MessageBox fallback chain (code removed)
RESOLVED — src/kolibri_app/taskbar_icon.py — praise: _last_notification instance attribute (code removed)
ACKNOWLEDGED — CI uv-lock linting failure (pre-existing on main, cannot be fixed from this icon-only PR)
UNADDRESSED — process: confirm @pcenov re-tested against the narrower icon-only fix (no code impact)

---

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:

• Retrieved prior bot reviews via the GitHub API
• Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
• Only raised NEW findings for newly introduced code
• Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
• Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence

[rtibblesbot]

PR #211 — delta re-review of the new Shell_NotifyIcon/NOTIFYICONDATAW ctypes path. Struct layout, cleanup, and wndproc subclassing look correct; no blocking issues. All prior findings resolved (the previous wx.adv.TaskBarIcon/NotificationMessage approach was abandoned and rewritten).

Two hardening suggestions inline. CI: only the All file linting (uv-lock prek) hook fails — same rolling exclude-newer window that is red on main's tip, not fixable from this diff. Note: the new hand-rolled tray path is untested on-device — a manual right-click / notification pass on Win10/11 is worth doing before merge.

Prior-finding status

RESOLVED — src/kolibri_app/taskbar_icon.py — NotificationMessage.Timeout_Default crash (file rewritten to Shell_NotifyIcon path)
RESOLVED — src/kolibri_app/taskbar_icon.py:115 — praise: NotificationMessage → ShowBalloon → MessageBox fallback (code removed)
RESOLVED — src/kolibri_app/taskbar_icon.py — praise: _last_notification instance attribute (code removed)
RESOLVED — src/kolibri_app/icons/kolibri.ico — all-BMP re-encode (icon approach abandoned; PR now code-only)
ACKNOWLEDGED — CI uv-lock linting failure (still red on 000c039; same as main tip, not fixable from this diff)
UNADDRESSED — process: on-device retest (crashing approach gone, but new Shell_NotifyIcon path itself untested — see menu-QA note)

---

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:

• Retrieved prior bot reviews via the GitHub API
• Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
• Only raised NEW findings for newly introduced code
• Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
• Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence

[rtibblesbot]

✅ Resolved — addressed in the current code.

suggestion: nid.szInfo is WCHAR * 256 and nid.szInfoTitle is WCHAR * 64; assigning a longer Python string raises ValueError: string too long, which isn't in the caught (OSError, pywintypes.error) tuple below — so it escapes the MessageBox fallback and propagates on the startup path. notify_server_failed builds …Check logs at: {log_path} from KOLIBRI_HOME, and a deep profile path can push past 255 chars. Cheap to harden: truncate (message[:255], title[:63]) or add ValueError to the except clause.

[rtibblesbot]

✅ Resolved — addressed in the current code.

suggestion: The right-click menu is now hand-rolled — SetForegroundWindow(self.hwnd) on the hidden tray frame then frame.PopupMenu(menu), where wx.adv.TaskBarIcon previously managed the popup lifecycle. Worth a manual Win10/11 check that the menu dismisses on click-away and all four items fire; I can't verify this without a Windows session.

[rtibblesbot]

PR #211 — delta re-review. All prior findings resolved or acknowledged; no new issues in the newly changed code. The szInfo/szInfoTitle over-long-string risk is fixed via message[:255]/title[:63] truncation. The hand-rolled struct layout, wndproc subclassing, icon lifecycle, and symmetric teardown all check out.

CI is red only on the pre-existing uv-lock hook (already failing on main tip, not fixable from this diff), so this stays a COMMENT rather than an approval. One item remains for the manual-QA pass, not a code defect: verify on Win10/11 that the right-click menu dismisses on click-away, all four items fire, and the startup/ready balloons render with non-inverted colours before merge.

Prior-finding status

RESOLVED — src/kolibri_app/windows/taskbar_icon.py:255 — szInfo/szInfoTitle over-long-string ValueError (now truncated to 255/63)
ACKNOWLEDGED — src/kolibri_app/windows/taskbar_icon.py:226 — hand-rolled right-click menu + balloon path need on-device manual QA (deferred, not a code defect)
ACKNOWLEDGED — CI uv-lock linting failure (pre-existing on main tip, not fixable from this diff)

---

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:

• Retrieved prior bot reviews via the GitHub API
• Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
• Only raised NEW findings for newly introduced code
• Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
• Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence

[rtibblesbot]

PR #211 — delta re-review of the two new commits (c10de40 rebased the Shell_NotifyIcon/NOTIFYICONDATAW path; a2c6ff9 registers an explicit AppUserModelID and adopts it). AUMID additions are small, well-guarded, and read correctly. No blocking issues. All 4 prior findings resolved/acknowledged.

Two low-severity notes on the new AUMID code (inline). CI passing.

Prior-finding status

RESOLVED — src/kolibri_app/windows/taskbar_icon.py:255 — szInfo/szInfoTitle over-long-string truncation
RESOLVED — src/kolibri_app/taskbar_icon.py — NotificationMessage fallback chain / _last_notification retention (praise)
RESOLVED — src/kolibri_app/taskbar_icon.py — NotificationMessage.Timeout_Default crash (approach abandoned)
ACKNOWLEDGED — src/kolibri_app/windows/taskbar_icon.py:260 — hand-rolled right-click menu + balloon/AUMID header path need on-device manual QA
ACKNOWLEDGED — CI uv-lock linting (pre-existing on main tip, not fixable from this diff)

---

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:

• Retrieved prior bot reviews via the GitHub API
• Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
• Only raised NEW findings for newly introduced code
• Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
• Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence

[rtibblesbot]

suggestion: Whether a classic Shell_NotifyIcon NIF_INFO balloon honors this AppUserModelId\...\IconUri value for its toast header icon depends on shell version — I can't confirm it from code. The balloon body icon is already handled directly via NIIF_USER | hBalloonIcon, so this registry write is harmless if it has no effect. Worth verifying in the on-device QA pass that the header shows the correct, non-inverted icon.

[rtibbles]

Verified on-device (Win11 26200, light theme): with the registered AUMID the balloon toast header shows "Kolibri" and renders the IconUri icon with correct colours. Without it, the header showed "KolibriApp.exe" with a colour-swapped icon extracted from the tray HICON.

[rtibblesbot]

Thanks for verifying — that confirms the registry write is doing real work (correct header name and non-inverted icon). Good to know the difference is that clear-cut.

[rtibblesbot]

nitpick: _resolve_icon_path reimplements files("kolibri_app") / TRAY_ICON_ICO (already in view.py:72 and about_dialog.py:29). The .resolve() is justified here since AUMID IconUri/SetCurrentProcessExplicitAppUserModelID want an absolute path — a shared "absolute path to packaged resource" helper would keep the three call sites in sync. Take-it-or-leave-it.

[rtibbles]

Leaving as-is: the other two call sites don't need absolute paths, so a shared helper would only sync an incidental detail. Happy to revisit if a third absolute-path consumer appears.

[rtibblesbot]

Makes sense — the absolute path is genuinely incidental to the other two sites, so a shared helper would couple them without cause. Agreed on revisiting only if a third absolute-path consumer shows up.

[pcenov]

Hi @rtibbles, I confirm that the color inversion is fixed.

[rtibbles]

Merging based on manual QA confirmation.