fix(cache): Address Review #1580 findings - prevent data loss

moneyflow/app_controller.py

@@ -1698,18 +1698,15 @@ def handle_commit_result(
                                 self.cache_manager.save_cold_cache(cold_df=updated_cold)
                                 logger.info("Updated cold cache with edits (hot unavailable)")
                             if hot_df is None and cold_df is None:
-                                # Neither tier available - cache is likely corrupted or missing
-                                # Save current view data as last resort
+                                # Neither tier available - cache is corrupted or missing.
+                                # In filtered view, data_manager.df only contains the filtered
+                                # subset, so we CANNOT safely save it as the full cache (would
+                                # lose historical data). Just log the error - edits are already
+                                # saved to backend, so next --refresh will restore consistency.
                                 logger.error(
-                                    "Neither cache tier could be loaded! "
-                                    "Saving current view data - historical transactions may be lost."
-                                )
-                                self.cache_manager.save_cache(
-                                    transactions_df=self.data_manager.df,
-                                    categories=self.data_manager.categories,
-                                    category_groups=self.data_manager.category_groups,
-                                    year=cache_filters.get("year"),
-                                    since=cache_filters.get("since"),
+                                    "Neither cache tier could be loaded in filtered view! "
+                                    "Cache may be corrupted. Edits saved to backend but not to "
+                                    "local cache. Use --refresh to rebuild cache from backend."
                                 )
                         else:
                             logger.info("Filtered view detected - updating cached tiers with edits")

tests/test_app_controller.py

@@ -764,6 +764,66 @@ def save_cache(
         updated_merchant = stub_cache.saved_cold.filter(pl.col("id") == edit_id)["merchant"][0]
         assert updated_merchant == "ColdEdited"
 
+    async def test_filtered_view_both_tiers_unavailable_does_not_corrupt_cache(self, controller):
+        """When both cache tiers are unavailable, we should NOT write partial data.
+
+        In filtered view, data_manager.df only contains the filtered subset.
+        Writing it as the full cache would lose historical transactions.
+        Instead, we just log an error - edits are saved to backend.
+        """
+        from moneyflow.state import TransactionEdit
+
+        class StubCacheManager:
+            def __init__(self):
+                self.saved_hot = None
+                self.saved_cold = None
+                self.saved_full = None
+
+            def load_hot_cache(self):
+                return None  # Both tiers unavailable
+
+            def load_cold_cache(self):
+                return None  # Both tiers unavailable
+
+            def save_hot_cache(self, hot_df, categories, category_groups):
+                self.saved_hot = hot_df
+
+            def save_cold_cache(self, cold_df):
+                self.saved_cold = cold_df
+
+            def save_cache(
+                self, transactions_df, categories, category_groups, year=None, since=None
+            ):
+                self.saved_full = transactions_df
+
+        # Set up a filtered view with only partial data
+        full_df = controller.data_manager.df
+        filtered_df = full_df.head(1)  # Only 1 transaction (simulates MTD filter)
+
+        edit_id = filtered_df["id"][0]
+        old_merchant = filtered_df["merchant"][0]
+        edits = [TransactionEdit(edit_id, "merchant", old_merchant, "Edited", datetime.now())]
+        controller.data_manager.pending_edits = edits.copy()
+        controller.data_manager.df = filtered_df.clone()
+
+        stub_cache = StubCacheManager()
+        controller.cache_manager = stub_cache
+
+        saved_state = controller.state.save_view_state()
+        controller.handle_commit_result(
+            success_count=1,
+            failure_count=0,
+            edits=edits,
+            saved_state=saved_state,
+            cache_filters={"year": None, "since": None},
+            is_filtered_view=True,
+        )
+
+        # CRITICAL: No cache writes should happen to avoid data loss
+        assert stub_cache.saved_hot is None, "Hot cache should NOT be written"
+        assert stub_cache.saved_cold is None, "Cold cache should NOT be written"
+        assert stub_cache.saved_full is None, "Full cache should NOT be written (would lose data)"
+
 
 class TestEditQueueing:
     """

tests/test_cache.py

@@ -1697,21 +1697,26 @@ def test_multiple_sequential_saves_persist_correctly(
         df = create_mixed_transactions_df()
         cache_manager.save_cache(df, sample_categories, sample_category_groups)
 
+        # Get an ID that's definitely in the hot cache
+        hot_df = cache_manager.load_hot_cache()
+        assert hot_df is not None and len(hot_df) > 0, "Hot cache should have data"
+        target_id = hot_df["id"][0]
+
         # Perform multiple edits and saves
         for i in range(5):
             hot_df = cache_manager.load_hot_cache()
-            if hot_df is not None and len(hot_df) > 0:
-                edited_hot = hot_df.with_columns(
-                    pl.when(pl.col("id") == hot_df["id"][0])
-                    .then(pl.lit(f"EDIT_{i}"))
-                    .otherwise(pl.col("merchant"))
-                    .alias("merchant")
-                )
-                cache_manager.save_hot_cache(edited_hot, sample_categories, sample_category_groups)
-
-        # Verify final state
+            edited_hot = hot_df.with_columns(
+                pl.when(pl.col("id") == target_id)
+                .then(pl.lit(f"EDIT_{i}"))
+                .otherwise(pl.col("merchant"))
+                .alias("merchant")
+            )
+            cache_manager.save_hot_cache(edited_hot, sample_categories, sample_category_groups)
+
+        # Verify final state - should be the last edit (EDIT_4)
         final_hot = cache_manager.load_hot_cache()
-        final_row = final_hot.filter(pl.col("id") == df["id"][0])
-        # The merchant should be the last edit
-        if len(final_row) > 0:
-            assert "EDIT_" in final_row["merchant"][0]
+        final_row = final_hot.filter(pl.col("id") == target_id)
+        assert len(final_row) == 1, f"Target ID {target_id} should exist in hot cache"
+        assert final_row["merchant"][0] == "EDIT_4", (
+            f"Expected merchant to be 'EDIT_4', got '{final_row['merchant'][0]}'"
+        )