Repository navigation
Check for Updates: cmd/ctrl-click fetches latest perf-tagged prerelease - #7278
Conversation
Co-authored-by: Orca <help@stably.ai>
📝 WalkthroughWalkthroughThis PR adds perf prerelease support to update checks. A shared 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 35b9a579-bfdb-4703-95d9-60f55caf145e
📒 Files selected for processing (14)
src/main/menu/register-app-menu.test.tssrc/main/menu/register-app-menu.tssrc/main/updater-prerelease-feed.test.tssrc/main/updater-prerelease-feed.tssrc/main/updater.test.tssrc/main/updater.tssrc/main/window/attach-main-window-services.tssrc/preload/api-types.tssrc/renderer/src/components/settings/GeneralUpdateSettingsSection.tsxsrc/renderer/src/components/sidebar/SidebarSettingsHelpMenu.test.tsxsrc/renderer/src/components/sidebar/SidebarSettingsHelpMenu.tsxsrc/renderer/src/lib/update-check-click-options.test.tssrc/renderer/src/lib/update-check-click-options.tssrc/shared/types.ts
Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Orca <help@stably.ai>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/renderer/src/lib/update-check-click-options.ts (1)
10-14: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLocalization regression:
getUpdateCheckHint()bypassestranslate().This helper returns a raw hardcoded English string ("... checks the latest RC; ... checks the latest perf build.") with no
translate()call. A prior review on this exact tooltip text ("LocalizeupdateCheckHint") was already flagged and marked addressed by wrapping it intranslate()with a hash-based key. Centralizing the string construction here reverted that fix — bothGeneralUpdateSettingsSection.tsxandSidebarSettingsHelpMenu.tsxnow render this untranslated string directly as theirtitletooltip, so es/ja/ko/zh users won't get a localized hint.Based on learnings, "use hash-based locale keys with
translate('auto.components.<Dir>.<ComponentFile>.<10-char-hash>', 'Default English text')" for settings/menu components — that requires localizing at the call site, not baking English text into a sharedlib/helper.Consider exposing only the platform-specific modifier labels from this helper and let each consumer build/translate the final sentence:
♻️ Suggested restructuring
-export function getUpdateCheckHint(isMac = isMacShortcutPlatform()): string { - const rcClickLabel = isMac ? '⇧+click' : 'Shift+click' - const perfClickLabel = isMac ? '⌘+click' : 'Ctrl+click' - return `${rcClickLabel} checks the latest RC; ${perfClickLabel} checks the latest perf build.` -} +export function getUpdateCheckHintLabels(isMac = isMacShortcutPlatform()): { + rcClickLabel: string + perfClickLabel: string +} { + return { + rcClickLabel: isMac ? '⇧+click' : 'Shift+click', + perfClickLabel: isMac ? '⌘+click' : 'Ctrl+click' + } +}Then in each consumer (e.g.
GeneralUpdateSettingsSection.tsx):const { rcClickLabel, perfClickLabel } = getUpdateCheckHintLabels() const updateCheckHint = translate( 'auto.components.settings.GeneralUpdateSettingsSection.<new-hash>', '{{value0}} checks the latest RC; {{value1}} checks the latest perf build.', { value0: rcClickLabel, value1: perfClickLabel } )Source: Learnings
🧹 Nitpick comments (1)
src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.tsx (1)
155-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a "why" comment for the pointer-down/onSelect modifier capture.
This two-step capture (record modifiers on
onPointerDown, consume them inonSelect) exists solely because Radix'sDropdownMenuItem.onSelectreceives a genericEventwithoutctrlKey/metaKey/shiftKey, as established in the prior review's investigation. That constraint isn't obvious from the code — a future edit could "simplify" this back to reading modifiers off theonSelectevent and silently reintroduce the bug.As per coding guidelines, "When writing or modifying code driven by a design doc or non-obvious constraint, add a comment explaining why the code behaves the way it does. Keep comments short — one or two lines, capturing only the non-obvious reason."
📝 Suggested comment
+ // Why: Radix's DropdownMenuItem.onSelect only exposes a generic Event + // (no ctrlKey/metaKey/shiftKey), so modifiers must be captured on + // pointerdown and consumed here. const handleCheckForUpdatesPointerDown = (event: React.PointerEvent): void => {Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 437557b3-0331-4687-94ce-709e66e61361
📒 Files selected for processing (7)
src/main/updater.test.tssrc/main/updater.tssrc/renderer/src/components/settings/GeneralUpdateSettingsSection.tsxsrc/renderer/src/components/sidebar/SidebarSettingsHelpMenu.test.tsxsrc/renderer/src/components/sidebar/SidebarSettingsHelpMenu.tsxsrc/renderer/src/lib/update-check-click-options.test.tssrc/renderer/src/lib/update-check-click-options.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.test.tsx
- src/main/updater.ts
…se (stablyai#7278) Co-authored-by: Orca <help@stably.ai>
Summary
vX.Y.Z-rc.N.perf, checks platform manifest readiness through the existing RC feed pinning path, and picks the highest semver match.rc.N.perftags unless the explicit perf modifier is used.Tag scheme
Perf builds use prerelease identifiers
[rc, N, perf], for tags such asv1.4.121-rc.6.perf. In semver,rc.N.perfsorts above its baserc.Nbut belowrc.N+1, so perf builds never outrank the main RC series.Selection behavior
Perf checks enable prerelease handling but add a perf-only release filter before the generic feed is pinned. Regular RC checks include prereleases but skip perf-tagged releases, so Shift-click does not select
.perfbuilds. If no newer ready perf release exists, Orca reports the same no-update/not-available result as the RC path and does not fall back to stable or RC.Verification
pnpm run typecheckpnpm exec oxlint <changed files>pnpm exec vitest run --config config/vitest.config.ts src/main/updater-prerelease-feed.test.ts src/main/updater.test.ts src/main/menu/register-app-menu.test.ts src/renderer/src/components/sidebar/SidebarSettingsHelpMenu.test.tsx src/renderer/src/lib/update-check-click-options.test.tspnpm exec vitest run --config config/vitest.config.ts src/main/updater-prerelease-feed.test.ts