Skip to content

fix: file size calculation and race condition#24

Merged
alhaymex merged 1 commit intomainfrom
feature/fix/migration-state-race
Mar 19, 2026
Merged

fix: file size calculation and race condition#24
alhaymex merged 1 commit intomainfrom
feature/fix/migration-state-race

Conversation

@alhaymex
Copy link
Copy Markdown
Owner

@alhaymex alhaymex commented Mar 19, 2026

Summary by CodeRabbit

Bug Fixes

  • Corrected download throughput reporting to accurately accumulate transfer data during progress updates
  • Enhanced library migration to treat both completion and error states consistently for proper state handling
  • Added timestamp-based ordering to prevent out-of-order migration state updates during concurrent operations

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 19, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 19602b5e-bdbd-46aa-b49f-82f1c482a95a

📥 Commits

Reviewing files that changed from the base of the PR and between 408bf9b and 3e90300.

📒 Files selected for processing (4)
  • apps/desktop/src/bun/utils/download-manager.ts
  • apps/desktop/src/mainview/lib/electroview.ts
  • apps/desktop/src/mainview/screens/Settings.tsx
  • apps/desktop/src/mainview/store/index.ts

📝 Walkthrough

Walkthrough

These changes implement timestamp-based state synchronization for library migration to prevent stale updates from overwriting newer state, and correct download throughput calculations by properly accumulating bytes and adjusting rate scaling. The migration store now tracks lastUpdatedAt and conditionally updates based on source timestamps.

Changes

Cohort / File(s) Summary
Download Throughput Tracking
apps/desktop/src/bun/utils/download-manager.ts
Accumulated bytesSinceLastReport across chunks instead of overwriting, and scaled bytesPerSec calculation from * 100 to * 1000 for accurate rate reporting.
Library Migration Timestamp Sync
apps/desktop/src/mainview/lib/electroview.ts, apps/desktop/src/mainview/store/index.ts, apps/desktop/src/mainview/screens/Settings.tsx
Added timestamp-based state versioning: store tracks lastUpdatedAt and only updates when sourceTimestamp >= lastUpdatedAt. Message handler passes Date.now() to setState. Settings component treats both "completed" and "error" as terminal migration states with appropriate cleanup logic.

Sequence Diagram

sequenceDiagram
    participant Component as Settings Component
    participant Handler as Migration Handler
    participant Store as LibraryMigrationStore
    participant State as Store State

    Component->>Handler: onLibraryMigrationProgress(payload)
    Handler->>Store: setState(payload.state, Date.now())
    
    rect rgba(100, 150, 200, 0.5)
    Store->>Store: sourceTimestamp >= lastUpdatedAt?
    end
    
    alt Timestamp Valid
        Store->>State: Update migration state
        Store->>State: Set lastUpdatedAt = sourceTimestamp
        Store-->>Component: State updated
    else Stale Update
        Store-->>Component: State unchanged (ignored)
    end
    
    Component->>Component: Check terminal state (completed/error)
    Component->>Store: Cleanup if needed
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Timestamps now guard the state's decree,
No stale updates shall overwrite thee,
Bytes accumulate with truthful measure,
Migration flows with ordered pleasure!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/fix/migration-state-race
📝 Coding Plan
  • Generate coding plan for human review comments

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.

@alhaymex alhaymex merged commit f3ce43f into main Mar 19, 2026
4 of 5 checks passed
@alhaymex alhaymex deleted the feature/fix/migration-state-race branch March 19, 2026 09:40
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