feat(app): ship redesigned notification sounds (phase 1 of #923)#924
Conversation
Replace the 45 inherited OpenCode sound effects with two bundled sounds and point the agent / permission / error defaults at them. This ships PawWork's own sounds without touching notification behavior, which is deferred to the phase 2 refactor tracked in #923 (single three-state control, OS-notification silencing, unified "only when unfocused" gating). - utils/sound.ts: SOUND_OPTIONS reduced from 45 to notify/error - context/settings.tsx: new defaults (agent/permissions -> notify, errors -> error) and withSoundFallback so stale persisted ids fall back to the default instead of going silent for existing users - i18n en/zh: drop the 45 option labels, add notify/error - assets: delete the 45 OpenCode .aac, add notify.aac + error.aac, and record source/license in packages/ui/src/assets/audio/CREDITS.md - snap: add settings-sounds target; update the stale agent-sound e2e assertion Verified: typecheck, 1616 unit tests, lint, and the settings-sounds snap.
|
Warning Review limit reached
More reviews will be available in 47 minutes and 10 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (54)
✨ 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.
Suggested priority: P2 (includes user-path files (packages/app/src/context/settings.tsx, packages/app/src/i18n/en.ts, packages/app/src/i18n/zh.ts, packages/app/src/utils/sound.ts)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
There was a problem hiding this comment.
Code Review
This pull request simplifies the application's notification sounds by reducing the available options to just "notify" and "error". It updates the default settings, localization dictionaries, and sound utilities accordingly, while adding a fallback mechanism to map legacy sound IDs to the new defaults. Additionally, E2E tests and a credits file for the audio assets have been added. One improvement opportunity was identified in the E2E test to make an assertion more precise and robust.
Perf delta summaryComparator: pass
|
defaultSettings was handed to createStore by reference, so reconciling a stale persisted sound id (e.g. the old OpenCode "staplebops-01") mutated defaultSettings.sounds.* in place. withSoundFallback reads those defaults as its fallback, so it ended up "falling back" to the very invalid id it was meant to replace, leaving upgrading users with a silent, unresolvable sound. Clone defaultSettings so it stays a stable source of truth. Tighten the agent-selection assertion to the exact persisted id and add E2E coverage that seeds legacy ids and asserts the dropdowns resolve to the bundled defaults.
Summary
Phase 1 of the notification-sound redesign: replace the 45 inherited OpenCode
.aaceffects with two bundled PawWork sounds (notify.aac,error.aac) and point the agent / permission / error sound defaults at them. Notification behavior is intentionally unchanged here.utils/sound.ts:SOUND_OPTIONSreduced from 45 tonotify/errorcontext/settings.tsx: new defaults (agent + permissions →notify, errors →error) and awithSoundFallbackthat maps any stale persisted id back to the defaulti18nen/zh: drop the 45 option labels, addnotify/error.aac, addnotify.aac+error.aac, record source/license inpackages/ui/src/assets/audio/CREDITS.mdsettings-soundstarget; update the stale agent-sound e2e assertionWhy
The 45 sounds were inherited from OpenCode and don't fit PawWork's tone. This ships PawWork's own sounds now, with low risk, while the larger behavior refactor (single three-state control, OS-notification silencing, unified "only when unfocused" gating) is tracked as phase 2 in #923.
Related Issue
#923 (this is phase 1; phase 2 plan is in the issue's first comment).
Human Review Status
Pending
Review Focus
withSoundFallbackincontext/settings.tsx: existing users have persisted ids likestaplebops-01; confirm they fall back to the new default instead of resolving to a now-missing file (silent sound).Risk Notes
withSoundFallback, so no one ends up with a silent, unresolvable sound..aacremoved; 2 added with provenance recorded inCREDITS.md.How To Verify
Screenshots or Recordings
Visual check via the new
settings-soundssnap target (grid reviewed locally: defaults render correctly and the reduced option set shows None / Notification / Error).Checklist
bug,enhancement,task,documentation.app,ui,platform,harness,ci.P0,P1,P2,P3.Pending,Approved by @<reviewer>, orNot required: <reason>.dev, and my PR title and commit messages use Conventional Commits in English.