Skip to content

Conversation

richiemcilroy
Copy link
Member

@richiemcilroy richiemcilroy commented Oct 17, 2025

This pull request removes support for the "New uploader" experimental feature flag from both the backend and frontend codebases. All logic that conditionally enabled legacy or new upload flows based on the enableNewUploader setting has been deleted, resulting in a simplified upload implementation that always uses the new uploader. Additionally, related UI elements and settings for toggling the feature have been removed.

Summary by CodeRabbit

  • Refactor

    • Removed the "New uploader" experimental toggle from settings.
    • Upload flows now exclusively use the modern multipart upload implementation.
    • Simplified upload resumption logic on startup.
  • Chores

    • Updated type definitions and formatting.

Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Walkthrough

The PR removes the enable_new_uploader feature flag from the codebase, eliminating conditional branches in upload logic, removing the experimental settings toggle, and updating type definitions. The uploader now always uses the modern multipart flow.

Changes

Cohort / File(s) Summary
Backend: Settings field removal
apps/desktop/src-tauri/src/general_settings.rs
Removed enable_new_uploader field and its default initializer from GeneralSettingsStore struct
Backend: Upload flow consolidation
apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/upload.rs
Removed feature flag checks that branched to legacy upload paths; upload now always proceeds through modern multipart/singlepart flows without conditional gating
Frontend: Settings UI
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx, apps/desktop/src/routes/(window-chrome)/settings/general.tsx
Removed experimental "New uploader" toggle UI component and enableNewUploader field from GeneralSettingsStore default initialization
Frontend: Type definitions
apps/desktop/src/utils/tauri.ts
Removed optional hasCompletedStartup?: boolean property from exported GeneralSettingsStore type
Other
crates/recording/src/sources/audio_mixer.rs
Reformatted silence_duration computation to span multiple lines; no logic changes

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Changes are predominantly homogeneous removals of a single feature flag across multiple files with clear, consistent intent. Minimal logic density; mostly straightforward deletions of conditional branches and UI components. One unrelated cosmetic formatting change.

Possibly related PRs

Suggested labels

codex

Poem

🐰 The feature flag hops away,
One uploader wins the day,
No more branches, clean and true,
Modern uploads shine right through!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "feat: Remove legacy uploader flag" directly and clearly aligns with the primary objective stated in the PR summary: removing support for the "New uploader" experimental feature flag across backend and frontend code. The title is specific and concise, accurately capturing the main change across all modified files (general_settings.rs, lib.rs, upload.rs, experimental.tsx, general.tsx, and tauri.ts), which collectively eliminate conditional logic based on the enableNewUploader setting and its related UI components. The title uses appropriate conventional commit formatting and avoids vague language, making the primary change immediately clear to reviewers scanning pull request history.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch remove-uploader-flag

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e08ba68 and 30a748a.

📒 Files selected for processing (7)
  • apps/desktop/src-tauri/src/general_settings.rs (0 hunks)
  • apps/desktop/src-tauri/src/lib.rs (1 hunks)
  • apps/desktop/src-tauri/src/upload.rs (0 hunks)
  • apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx (0 hunks)
  • apps/desktop/src/routes/(window-chrome)/settings/general.tsx (0 hunks)
  • apps/desktop/src/utils/tauri.ts (1 hunks)
  • crates/recording/src/sources/audio_mixer.rs (1 hunks)
💤 Files with no reviewable changes (4)
  • apps/desktop/src-tauri/src/general_settings.rs
  • apps/desktop/src-tauri/src/upload.rs
  • apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/general.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format Rust code using rustfmt and ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.

Files:

  • crates/recording/src/sources/audio_mixer.rs
  • apps/desktop/src-tauri/src/lib.rs
crates/*/src/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Rust crates should place tests within the src/ and/or a sibling tests/ directory for each crate inside crates/*.

Files:

  • crates/recording/src/sources/audio_mixer.rs
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by running pnpm format.

Use strict TypeScript and avoid any; leverage shared types

Files:

  • apps/desktop/src/utils/tauri.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g., user-menu.tsx).
Use PascalCase for React/Solid components.

Files:

  • apps/desktop/src/utils/tauri.ts
**/tauri.ts

📄 CodeRabbit inference engine (AGENTS.md)

Do not edit auto-generated files named tauri.ts.

NEVER EDIT auto-generated IPC bindings file: tauri.ts

Files:

  • apps/desktop/src/utils/tauri.ts
apps/desktop/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/desktop/src/**/*.{ts,tsx}: Desktop icons are auto-imported (unplugin-icons); do not import icons manually
Desktop IPC: Call generated tauri_specta commands/events; listen to generated events and use typed interfaces
Use @tanstack/solid-query for server state in the desktop app

Files:

  • apps/desktop/src/utils/tauri.ts
🧬 Code graph analysis (1)
crates/recording/src/sources/audio_mixer.rs (1)
crates/media-info/src/lib.rs (1)
  • rate (125-127)
⏰ 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). (3)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Analyze (rust)
🔇 Additional comments (3)
crates/recording/src/sources/audio_mixer.rs (1)

312-314: Formatting aligns with Rust style conventions.

The refactored multi-line format for the silence_duration calculation adheres to the 100-character line width maximum and uses block-style indentation, maintaining readability without changing the underlying logic.

apps/desktop/src/utils/tauri.ts (1)

396-396: Auto-generated file correctly reflects Rust type changes.

This file is auto-generated by tauri-specta (as noted in line 2), and the type definition correctly mirrors the removal of the enable_new_uploader field from the Rust GeneralSettingsStore. No manual changes needed here.

apps/desktop/src-tauri/src/lib.rs (1)

2219-2222: LGTM! Unconditional upload resumption simplifies startup flow.

The removal of the feature flag check means uploads now always resume on startup, which aligns with the PR objective. The implementation is safe:

  • Error handling logs warnings without blocking startup
  • The resume_uploads function gracefully handles missing directories and failed recordings

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.

@richiemcilroy richiemcilroy merged commit 598d31d into main Oct 17, 2025
15 checks passed
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