fix(ux): customer-facing copy audit — hide implementation boundaries, add next-action guidance - #1034
fix(ux): customer-facing copy audit — hide implementation boundaries, add next-action guidance#1034seonghobae wants to merge 31 commits into
Conversation
… add next-action guidance - Remove internal-boundary jargon from user-visible strings (bridge, transcription/separation pipeline wording, raw enum values in tooltips and metric cards). - Give every error and pending state a next action (retry / re-pick file / check link), keeping regex-compatible prefixes for existing flows. - Localize previously hardcoded English UI strings in Workspace stem player, export buttons, song timeline summary, and GrooveMap states; add matching keys to ko/en locales (key sets stay in sync). - Display-only change: no behavior, permission, IPC, or storage changes. Verified: tsc --noEmit clean; vitest 196/196 with 100% statements, branches, functions, and lines coverage.
|
Warning Review limit reachedNext included review available in 10 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent Exact-head owner repair on existing branch Apply
Run focused desktop tests first, then full desktop Vitest + exact 100% statements/branches/functions/lines, TypeScript, i18n key-parity test, and canonical quickcheck/required exact-head workflows. Do not weaken gates, alter unrelated dependencies, or claim success from predecessor/queued/rate-limited review evidence. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent Continue only on existing Apply systematic-debugging + TDD + verification-before-completion to the remaining deterministic desktop failures from predecessor exact release job The remaining first-causal contracts are current-source-verifiable test drift, not permission to roll back the buyer-safety localization:
Commit only to this canonical branch. Do not resolve review state or claim readiness until the unchanged successor head has terminal focused/full evidence. |
| function localAudioSelectionMessage(error: unknown): string { | ||
| if (error instanceof Error) { | ||
| const messageKey = SAFE_LOCAL_AUDIO_MESSAGE_KEYS.get(error.message); | ||
| if (messageKey) { | ||
| return analysisMessage(messageKey); | ||
| } | ||
| } | ||
| return analysisMessage("unsupportedLocalAudio"); | ||
| } |
There was a problem hiding this comment.
📝 Info: Local-audio error mapping stays fail-closed
localAudioSelectionMessage maps the native allowlist strings to distinct localized keys and falls back to unsupportedLocalAudio for any unknown error, so raw bridge detail is never surfaced. This preserves the prior SAFE_LOCAL_AUDIO_MESSAGES behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| } catch { | ||
| return { | ||
| ok: false, | ||
| error: { | ||
| code: "invalid_request", | ||
| message | ||
| message: analysisMessage("youtubeImportFailed") | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
📝 Info: Redacted YouTube error detail is intentional
importYoutubeUrl now discards the caught bridge message and returns youtubeImportFailed, hiding details like age restriction. This matches the PR's stated intent to hide implementation boundaries.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /** Resolve a buyer-visible analysis message in the currently selected locale. */ | ||
| function analysisMessage(key: TranslationKey): string { | ||
| return createTranslator(detectPreferredLocale())(key); | ||
| } |
There was a problem hiding this comment.
📝 Info: Storage/analysis errors resolve locale at throw time
scoreMessage (features/score/scoreStorage.ts:22) and analysisMessage (lib/analysis.ts:61) re-resolve locale from navigator on each error, while UI components fix the translator once at mount via useMemo. navigator.language is stable within a session so output stays consistent; the window-undefined storage test still resolves English.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
📝 Info: Localized import-failure copy survives safeErrorDetail redaction
The bridge-failure branch wraps youtubeImportFailed in safeErrorDetail (App.tsx:461), capped at 220 chars. Both locale strings are shorter and contain no URL/path/secret patterns, so redaction and truncation leave them intact.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (!normalizedUrl) { | ||
| setSelectionError(t("youtubeImportFailed")); | ||
| setSelectionError(t("youtubeLinkGuidance")); | ||
| setSelectionErrorSource("youtube"); | ||
| return; | ||
| } | ||
|
|
||
| if (!isSupportedYoutubeUrl(normalizedUrl)) { | ||
| setSelectionError(t("youtubeImportFailed")); | ||
| setSelectionError(t("youtubeLinkGuidance")); | ||
| setSelectionErrorSource("youtube"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🔴 Stale tests assert removed YouTube rejection copy
handleImportYoutube now shows youtubeLinkGuidance ("Use a standard YouTube video link...") for empty and unsupported URLs. Six existing tests still assert "Failed to import YouTube URL." (App.test.tsx:1204, 1216, 1228, 1240, 1253, 1266). That text no longer renders, so each getByText throws and the tests fail.
Prompt for agents
handleImportYoutube in apps/desktop/src/App.tsx now sets t("youtubeLinkGuidance") for the empty-URL branch and the unsupported-URL branch, replacing the old t("youtubeImportFailed") text. Six tests in apps/desktop/src/App.test.tsx still assert the old copy via screen.getByText(/Failed to import YouTube URL./i): 'rejects empty YouTube URL', 'rejects malformed YouTube URL', 'rejects non-http YouTube URL', 'rejects non-allowlisted YouTube URL intake before invoking the bridge', 'rejects downgraded YouTube URL intake before invoking the bridge', and 'rejects duplicate YouTube video parameters even when one is blank'. Update these assertions to expect the pre-import guidance copy ("Use a standard YouTube video link (youtube.com/watch or youtu.be).") so they match the new behavior, consistent with the new App.youtube-url-guidance.test.tsx.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@opencode-agent Exact-head owner repair on existing canonical branch Apply Run the smallest focused App test proving the six rejection cases and the post-admission failure case, then the repository-prescribed desktop/full verification that is feasible. Commit only the stale-test repair to this existing branch. Resolve |
1 participant
Pull Request
Summary
This PR audits mounted customer-facing copy against the BandScope brand rule: hide implementation boundaries, preserve safe diagnosis, and always give the player a useful next action.
Current exact head:
537d2674a400c3d854102ad01606565c0cf022d7.Buyer-visible changes
en/kodictionaries.not_foundanalysis jobs dedicated retry guidance instead of the misleading “analysis stopped partway through” message.1 sections.Regression coverage added in this branch
Workspace.timeline-summary.test.tsx: one-section English timeline grammar.App.localized-source-error.test.tsx: Korean local-audio browser fallback stays Korean.analysis.localized-errors.test.ts: Korean YouTube/project fallbacks, distinct allowlisted local-audio failure guidance, unknown-error redaction, and browsernot_foundsemantics.scoreStorage.localized-errors.test.ts: Korean desktop-only and malformed-response score errors.These were added regression-first. Hosted RED evidence is not claimed because the successor fixes were pushed before the queued GitHub runners completed those intermediate heads.
Security / trust boundary
Current verification state
The exact-head repository workflows for
537d2674a400c3d854102ad01606565c0cf022d7have been dispatched but are currently queued. No predecessor-head test, coverage, security, release, or review evidence is being counted as success for this head.Before merge this head still requires the repository ruleset’s current exact-head CI/security/SAST/SBOM/build/release/coverage/review gates and a qualifying independent non-author approval.
Dependency and supply chain
Reviewer checklist
develop.