Skip to content

OTA UI final#1584

Merged
yujonglee merged 7 commits intomainfrom
ota-final-ui
Oct 21, 2025
Merged

OTA UI final#1584
yujonglee merged 7 commits intomainfrom
ota-final-ui

Conversation

@yujonglee
Copy link
Contributor

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 20, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Refactors the OTA update flow by removing a monolithic component and replacing it with a modular ota/ directory (store, task, UI), tightens MenuItem prop types and moves styling helper, updates import/mock mappings, and consolidates transcript words from the same speaker into single segments with updated tests.

Changes

Cohort / File(s) Summary
OTA refactor (new modules)
apps/desktop/src/components/main/sidebar/profile/ota/*
apps/desktop/src/components/main/sidebar/profile/ota/index.tsx, apps/desktop/src/components/main/sidebar/profile/ota/store.ts, apps/desktop/src/components/main/sidebar/profile/ota/task.ts
Removes the previous monolithic OTA component and introduces a modular OTA implementation: XState-based updateStore, useOTA and checkForUpdate task integration with Tauri updater, and a new UpdateChecker UI component rendering state-specific menu entries.
OTA component removal
apps/desktop/src/components/main/sidebar/profile/ota.tsx
Deleted: previously exported checkForUpdate and UpdateChecker (monolithic implementation).
MenuItem typing & styling
apps/desktop/src/components/main/sidebar/profile/shared.tsx
Replaces clsx with cn from @hypr/utils and tightens MenuItem props: icon and suffixIcon now typed as React.ComponentType<{ className?: string }>; updates className composition accordingly.
Import path & mock change
apps/desktop/src/components/task-manager.tsx, apps/desktop/src/mocks/updater.ts
Updates checkForUpdate import to the new ota/task location; changes exported mock check to reference check_2 instead of check_1.
Transcript merge logic & tests
apps/desktop/src/utils/transcript.ts, apps/desktop/src/utils/transcript.test.ts
Changes transcript logic to merge consecutive words from the same speaker into single segments; replaces inline test with a testcases array and adds scenarios covering adjacent-word merging and multi-channel separation.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UpdateChecker
    participant useOTA
    participant updateStore
    participant TauriUpdater

    User->>UpdateChecker: open menu (idle)
    UpdateChecker->>useOTA: subscribe / render
    useOTA->>updateStore: subscribe

    User->>UpdateChecker: "Check for updates"
    UpdateChecker->>useOTA: handleCheckForUpdate()
    useOTA->>updateStore: setState(checking)
    useOTA->>TauriUpdater: check()
    TauriUpdater-->>useOTA: response (update | null) or error

    alt update available
        useOTA->>updateStore: checkSuccess(update) -> state: available
        User->>UpdateChecker: "Download"
        UpdateChecker->>useOTA: handleStartDownload()
        useOTA->>updateStore: setState(downloading)
        TauriUpdater-->>useOTA: progress events
        useOTA->>updateStore: downloadProgress(%)
        TauriUpdater-->>useOTA: finished
        useOTA->>updateStore: downloadFinished -> state: ready
        User->>UpdateChecker: "Install"
        UpdateChecker->>useOTA: handleInstall()
        useOTA->>updateStore: setInstalling() -> state: installing
        useOTA->>TauriUpdater: installAndRelaunch()
    else no update
        useOTA->>updateStore: checkSuccess(null) -> state: noUpdate
    else error
        useOTA->>updateStore: checkError(err) -> state: error
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Multiple new files split responsibilities (state, tasks, UI) and require reading each piece plus integration points (Tauri updater, XState store). Transcript and MenuItem changes are straightforward.

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 2 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The PR title "OTA UI final" uses vague and non-descriptive language that fails to clearly communicate the actual changes in the pull request. While the title does reference "OTA UI" and is technically related to the changeset, the term "final" is meaningless without context and provides no insight into what was actually modified. A reader scanning the commit history would not understand whether this is a refactoring, a bug fix, a feature addition, or something else entirely. The title is too generic to effectively summarize the primary changes, which involve reorganizing the OTA component into a modular structure with separate files for state management and business logic.
Description Check ❓ Inconclusive No pull request description was provided by the author, leaving the changeset completely undocumented. While the pre-merge check for descriptions is meant to be lenient, it requires that a description exist and be related to the changeset in some way. An empty description cannot satisfy this criterion because there is no information present to evaluate for relevance. This makes it impossible to assess whether the author's intent aligns with the changes or whether additional context exists for understanding the refactoring.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7211209 and 805e4e2.

📒 Files selected for processing (1)
  • apps/desktop/src/components/main/sidebar/profile/ota/store.ts (1 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/components/main/sidebar/profile/shared.tsx (1)

34-39: Use array notation with cn per coding guidelines.

The cn utility should receive an array of class segments for consistency.

As per coding guidelines.

Apply this diff:

             <span
-              className={cn(
+              className={cn([
                 "rounded-full",
                 "px-2 py-0.5",
                 "bg-red-500",
                 "text-xs font-semibold text-white",
-              )}
+              ])}
             >
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a3e90ea and 7211209.

📒 Files selected for processing (9)
  • apps/desktop/src/components/main/sidebar/profile/ota.tsx (0 hunks)
  • apps/desktop/src/components/main/sidebar/profile/ota/index.tsx (1 hunks)
  • apps/desktop/src/components/main/sidebar/profile/ota/store.ts (1 hunks)
  • apps/desktop/src/components/main/sidebar/profile/ota/task.ts (1 hunks)
  • apps/desktop/src/components/main/sidebar/profile/shared.tsx (2 hunks)
  • apps/desktop/src/components/task-manager.tsx (1 hunks)
  • apps/desktop/src/mocks/updater.ts (1 hunks)
  • apps/desktop/src/utils/transcript.test.ts (1 hunks)
  • apps/desktop/src/utils/transcript.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • apps/desktop/src/components/main/sidebar/profile/ota.tsx
🧰 Additional context used
📓 Path-based instructions (1)
apps/desktop/**/*.{tsx,jsx}

📄 CodeRabbit inference engine (apps/desktop/.cursor/rules/style.mdc)

apps/desktop/**/*.{tsx,jsx}: When there are many className values with conditional logic in React components, use the cn utility imported as import { cn } from "@hypr/ui/lib/utils"
When using cn, always pass an array of class segments (e.g., cn([ ... ]))
When using cn, split the array by logical grouping of classes for readability

Files:

  • apps/desktop/src/components/task-manager.tsx
  • apps/desktop/src/components/main/sidebar/profile/ota/index.tsx
  • apps/desktop/src/components/main/sidebar/profile/shared.tsx
🧠 Learnings (2)
📚 Learning: 2025-10-20T06:06:12.693Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-20T06:06:12.693Z
Learning: Applies to apps/desktop/**/*.{tsx,jsx} : When there are many className values with conditional logic in React components, use the `cn` utility imported as `import { cn } from "hypr/ui/lib/utils"`

Applied to files:

  • apps/desktop/src/components/main/sidebar/profile/shared.tsx
📚 Learning: 2025-10-20T06:06:12.693Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-20T06:06:12.693Z
Learning: Applies to apps/desktop/**/*.{tsx,jsx} : When using `cn`, split the array by logical grouping of classes for readability

Applied to files:

  • apps/desktop/src/components/main/sidebar/profile/shared.tsx
🧬 Code graph analysis (5)
apps/desktop/src/utils/transcript.ts (1)
crates/whisper-local/src/model/actual.rs (1)
  • segments (178-181)
apps/desktop/src/utils/transcript.test.ts (1)
apps/desktop/src/utils/transcript.ts (1)
  • buildSegments (8-31)
apps/desktop/src/components/main/sidebar/profile/ota/index.tsx (3)
apps/desktop/src/components/main/sidebar/profile/ota/task.ts (1)
  • useOTA (28-38)
apps/desktop/src/components/main/sidebar/profile/shared.tsx (1)
  • MenuItem (3-49)
packages/utils/src/cn.ts (1)
  • cn (20-22)
apps/desktop/src/components/main/sidebar/profile/ota/task.ts (1)
apps/desktop/src/components/main/sidebar/profile/ota/store.ts (1)
  • updateStore (25-106)
apps/desktop/src/components/main/sidebar/profile/shared.tsx (1)
packages/utils/src/cn.ts (1)
  • cn (20-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: ci (macos, macos-14)
🔇 Additional comments (20)
apps/desktop/src/utils/transcript.ts (1)

19-28: LGTM! Clean segment merging implementation.

The logic correctly consolidates adjacent words from the same speaker into single segments, with proper edge-case handling for empty arrays and clean word separation.

apps/desktop/src/mocks/updater.ts (1)

19-19: LGTM! Mock now simulates update available.

Switching from check_1 (no update) to check_2 (update available) allows testing the full OTA download and install flow.

apps/desktop/src/utils/transcript.test.ts (1)

28-76: LGTM! Comprehensive test coverage for segment merging.

The testcases array structure improves maintainability and the new scenarios thoroughly validate the speaker merging logic:

  • Empty input handling
  • Single word segment creation
  • Adjacent words from same speaker merging
  • Different speakers creating separate segments
apps/desktop/src/components/main/sidebar/profile/shared.tsx (2)

1-1: LGTM! Correct styling utility import.

Switching from clsx to cn aligns with the project's styling utilities and enables Tailwind class merging.


11-14: LGTM! Tightened prop types improve type safety.

Explicitly typing icon and suffixIcon as React.ComponentType<{ className?: string }> ensures proper component contracts and enables className forwarding.

apps/desktop/src/components/main/sidebar/profile/ota/store.ts (3)

1-23: LGTM! Well-structured state and context types.

The state enumeration and context shape provide clear contracts for the OTA update flow, with appropriate fields for tracking update status, payload, error, and download progress.


25-61: LGTM! Clean state transition handlers.

The handlers for setState, checkSuccess, checkError, and startDownload correctly manage context updates and state transitions.


76-105: LGTM! Complete lifecycle management.

The remaining handlers (downloadFinished, cancelDownload, setInstalling, reset) correctly manage state cleanup and transitions for the complete update lifecycle.

apps/desktop/src/components/task-manager.tsx (1)

6-6: LGTM! Import path updated for modular OTA structure.

The import now points to the dedicated task module, aligning with the refactored OTA directory structure.

apps/desktop/src/components/main/sidebar/profile/ota/index.tsx (6)

21-29: LGTM! Clean checking state UI.

The MenuItem correctly shows a spinner and disables interaction during update checks.


31-39: LGTM! Clear no-update feedback.

The success indicator and message appropriately communicate that the software is current.


68-77: LGTM! Clear download prompt with visual indicator.

The download button clearly displays the version and uses a red dot badge to draw attention to the available update.


79-108: LGTM! Excellent download progress UI with cancel option.

The progress bar correctly binds to downloadProgress.percentage, and the cancel button provides good UX. The cn usage with array notation follows coding guidelines.


110-119: LGTM! Clear install prompt with animated indicator.

The pulsing red dot effectively draws attention to the ready-to-install state.


121-139: LGTM! Complete state coverage for OTA flow.

The installing and idle states provide appropriate feedback and interaction points for the full update lifecycle.

apps/desktop/src/components/main/sidebar/profile/ota/task.ts (5)

7-26: LGTM! Robust update check with auto-reset.

The check logic correctly handles both success and error cases. The defensive state check before auto-resetting noUpdate (lines 16-17) prevents race conditions if the user manually triggers another action during the timeout.


28-38: LGTM! Clean hook interface for OTA state.

The useOTA hook effectively exposes both the store context and action handlers, providing a simple API for components.


40-68: LGTM! Complete download flow with progress tracking.

The download handler correctly processes all three event types (Started, Progress, Finished) and properly initializes contentLength from the Started event before accumulating chunk progress.


70-81: LGTM! Safe cancel with error suppression.

Catching and logging (rather than propagating) the close error is appropriate since cancellation should succeed regardless of cleanup failures.


83-101: LGTM! Development-safe install handler.

Skipping install and relaunch in development (line 93) prevents disrupting the dev environment while still allowing state transitions to be tested.

@yujonglee yujonglee merged commit 1af60ee into main Oct 21, 2025
3 of 4 checks passed
@yujonglee yujonglee deleted the ota-final-ui branch October 21, 2025 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant