Remove unused code

[romaintb]

most of it was for the old architecture

Summary by CodeRabbit

• Refactor
  • Simplified syncing to a single explicit action with consistent error handling.
  • Sync now proceeds even if some section data fails to load (with a warning).
• UX
  • Removed per-task cancellation; only “cancel all” is available.
  • Reduced on-screen status details; the aggregate status overview and manual render trigger are no longer exposed.
• Notes
  • No new features introduced; overall API surface streamlined without changing core task/project operations.

[coderabbitai]

Walkthrough

Public API surface was reduced across icons, sync, and UI modules by removing several getters, helper methods, a status struct, and sync-related utilities. Core sync entrypoint and main operations remain. No new functionality was added.

Changes

Cohort / File(s)	Summary of edits
Icons API reductions src/icons.rs	Removed four public getters on IconService: tasks_title, projects_title, error, success. No replacements introduced; callers must use icons() -> UiIcons directly.
SyncService API trimming src/sync.rs	Deleted public methods has_local_data, should_sync, sync_if_needed, clear_local_data, get_sync_stats; removed public SyncStats type and its impl; dropped Duration import; simplified to use sync() as sole sync path. Core CRUD/listing and force_sync remain.
UI API reductions src/ui/core/task_manager.rs, src/ui/layout.rs, src/ui/renderer.rs	TaskManager: removed active_tasks() and cancel_task(id). LayoutManager: removed layout helpers (main_layout, top_pane_layout, right_pane_layout, calculate_status_height, help_panel_dimensions, sidebar_constraints, task_content_constraints); kept centered_rect helpers. Renderer: removed AppStatus struct, get_status(), and request_render().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Perform an initial fetch when starting the app #4 — Uses sync-related public APIs (e.g., has_local_data, SyncStats) and UI accessors that are removed here.
• Icons/set ascii as default #3 — Modifies the same IconService getters later removed in this PR, indicating direct overlap.
• Feature: Task edition (only change name for now) #5 — Adjusts SyncService public API in src/sync.rs while this PR removes several sync methods/types.

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title “Remove unused code” succinctly captures the primary change of deleting obsolete code tied to the old architecture and avoids extraneous details. It is clear, concise, and directly reflects the main focus of the changeset.
Description Check	✅ Passed	The description notes that most removed code was part of the old architecture, which directly relates to the deletions made in this pull request. Although terse, it correctly ties the changes to their context and remains on-topic.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch enhance/remove_dead_code

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[romaintb]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

src/sync.rs (1)

269-291: Ensure local state is refreshed after updating a task
In src/ui/app_component.rs (around line 816), invoke sync_service.force_sync().await? (or sync()) immediately after update_task_content succeeds to update local storage and prevent stale content.

🧹 Nitpick comments (7)

src/sync.rs (7)

47-52: Avoid formatting cost when logger is None.

Gate string construction by taking a closure; no behavior change, small perf win in hot paths.

-fn log_debug(&self, message: String) {
-    if let Some(ref logger) = self.debug_logger {
-        logger.log(message);
-    }
-}
+fn log_debug<F>(&self, msg: F)
+where
+    F: FnOnce() -> String,
+{
+    if let Some(ref logger) = self.debug_logger {
+        logger.log(msg());
+    }
+}

Example call site change:

• self.log_debug(format!("API: Creating label '{}'...", name));

• self.log_debug(|| format!("API: Creating label '{}'...", name));

---

15-15: Use AtomicBool for sync gate instead of Mutex.

Reduces lock contention, simpler logic, and no await points around the gate.

@@
-use std::sync::Arc;
+use std::sync::Arc;
+use std::sync::atomic::{AtomicBool, Ordering};
@@
-    sync_in_progress: Arc<Mutex<bool>>,
+    sync_in_progress: AtomicBool,
@@
-        let sync_in_progress = Arc::new(Mutex::new(false));
+        let sync_in_progress = AtomicBool::new(false);
@@
-    pub async fn is_syncing(&self) -> bool {
-        *self.sync_in_progress.lock().await
-    }
+    pub async fn is_syncing(&self) -> bool {
+        self.sync_in_progress.load(Ordering::Relaxed)
+    }
@@
-        // Check if sync is already in progress and acquire lock
-        let mut sync_guard = self.sync_in_progress.lock().await;
-        if *sync_guard {
-            return Ok(SyncStatus::InProgress);
-        }
-        *sync_guard = true;
-
-        // Release the lock before performing sync to avoid holding it during the long operation
-        drop(sync_guard);
+        // Fast path: acquire the sync gate
+        if self
+            .sync_in_progress
+            .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed)
+            .is_err()
+        {
+            return Ok(SyncStatus::InProgress);
+        }
@@
-        // Release sync lock
-        {
-            let mut sync_guard = self.sync_in_progress.lock().await;
-            *sync_guard = false;
-        }
+        // Release sync gate
+        self.sync_in_progress.store(false, Ordering::Release);

Also applies to: 122-126, 409-426

---

371-404: Defaulting to complete when task not found locally is risky.

Could complete an unintended task if local cache is stale/partial. Prefer fetching the task from API or returning a recoverable error prompting a full sync.

---

489-529: Consider a single transaction for storing projects/tasks/labels/sections.

Prevents partial writes if later steps fail; wrap stores in a DB transaction if LocalStorage supports it.

---

127-131: Nit: avoid magic string for sync scope.

Extract "projects" into a const to reduce typo risk and aid discoverability.

---

96-108: Filtering by label name can collide on renames/duplicates.

If available, prefer label ID matching to ensure stable lookups.

---

229-236: Add immediate local deletion in delete_label for UX parity. LocalStorage currently lacks a delete_label method, so labels remain visible until the next sync, unlike tasks/projects which are removed locally immediately. Implement in src/storage.rs:

pub async fn delete_label(&self, label_id: &str) -> Result<()> {
    sqlx::query("DELETE FROM labels WHERE id = ?")
        .bind(label_id)
        .execute(&self.pool)
        .await?;
    Ok(())
}

Then in src/sync.rs’s delete_label, after the API call add:

let storage = self.storage.lock().await;
storage.delete_label(label_id).await?;

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 198edcb and 55b412f.

📒 Files selected for processing (5)

• src/icons.rs (0 hunks)
• src/sync.rs (1 hunks)
• src/ui/core/task_manager.rs (0 hunks)
• src/ui/layout.rs (0 hunks)
• src/ui/renderer.rs (0 hunks)

💤 Files with no reviewable changes (4)

• src/ui/core/task_manager.rs
• src/icons.rs
• src/ui/renderer.rs
• src/ui/layout.rs

🔇 Additional comments (2)

src/sync.rs (2)

2-2: LGTM: streamlined chrono import.

Dropping unused items (e.g., Duration) aligns with the “remove dead code” goal.

---

27-27: No lingering sync API references found: verified removal of has_local_data, should_sync, sync_if_needed, clear_local_data, get_sync_stats, and SyncStats across the codebase.