Skip to content

Commit d368458

Browse files
wesmclaude
andcommitted
fix(cache): Fix edits not persisting to cache in filtered view mode
Root cause: In handle_commit_result(), the `edits` parameter was the same list object as `self.data_manager.pending_edits`. When we called `pending_edits.clear()` BEFORE the cache update, it emptied the `edits` list (Python passes lists by reference), so the cache update applied zero edits. The fix moves `pending_edits.clear()` to AFTER the cache update completes. Also improved handling when cache tiers can't be loaded in filtered view: - Previously: silently skipped cache update entirely - Now: updates whichever tier(s) can be loaded, with detailed logging Added tests: - 5 new cache persistence tests in test_cache.py - 2 new partial cache update tests in test_app_controller.py Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 21efe4b commit d368458

File tree

3 files changed

+313
-6
lines changed

3 files changed

+313
-6
lines changed

moneyflow/app_controller.py

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,11 +1651,10 @@ def handle_commit_result(
16511651
bulk_merchant_renames,
16521652
)
16531653

1654-
# Clear pending edits on success
1655-
self.data_manager.pending_edits.clear()
1656-
logger.info("Cleared pending edits")
1657-
16581654
# Update cache with edited data (if caching is enabled)
1655+
# NOTE: Must happen BEFORE clearing pending_edits because `edits` parameter
1656+
# is the same list object (passed by reference), so clearing pending_edits
1657+
# would empty the edits list before we can use it for cache updates!
16591658
if self.cache_manager and cache_filters:
16601659
try:
16611660
if is_filtered_view:
@@ -1664,10 +1663,54 @@ def handle_commit_result(
16641663
hot_df = self.cache_manager.load_hot_cache()
16651664
cold_df = self.cache_manager.load_cold_cache()
16661665
if hot_df is None or cold_df is None:
1666+
# This can happen if cache files don't exist, are corrupted,
1667+
# or encryption mode changed since they were created.
1668+
# We'll try to recover by applying edits to whichever tier
1669+
# loaded successfully, and preserve the other.
16671670
logger.warning(
1668-
"Filtered view detected but cache tiers are unavailable; "
1669-
"skipping cache update to avoid corruption"
1671+
"Filtered view: cache tier(s) unavailable (hot=%s, cold=%s). "
1672+
"Attempting partial update to preserve edits.",
1673+
hot_df is not None,
1674+
cold_df is not None,
16701675
)
1676+
if hot_df is not None:
1677+
updated_hot = CommitOrchestrator.apply_edits_to_dataframe(
1678+
hot_df,
1679+
edits,
1680+
self.data_manager.categories,
1681+
self.data_manager.apply_category_groups,
1682+
bulk_merchant_renames,
1683+
)
1684+
self.cache_manager.save_hot_cache(
1685+
hot_df=updated_hot,
1686+
categories=self.data_manager.categories,
1687+
category_groups=self.data_manager.category_groups,
1688+
)
1689+
logger.info("Updated hot cache with edits (cold unavailable)")
1690+
if cold_df is not None:
1691+
updated_cold = CommitOrchestrator.apply_edits_to_dataframe(
1692+
cold_df,
1693+
edits,
1694+
self.data_manager.categories,
1695+
self.data_manager.apply_category_groups,
1696+
bulk_merchant_renames,
1697+
)
1698+
self.cache_manager.save_cold_cache(cold_df=updated_cold)
1699+
logger.info("Updated cold cache with edits (hot unavailable)")
1700+
if hot_df is None and cold_df is None:
1701+
# Neither tier available - cache is likely corrupted or missing
1702+
# Save current view data as last resort
1703+
logger.error(
1704+
"Neither cache tier could be loaded! "
1705+
"Saving current view data - historical transactions may be lost."
1706+
)
1707+
self.cache_manager.save_cache(
1708+
transactions_df=self.data_manager.df,
1709+
categories=self.data_manager.categories,
1710+
category_groups=self.data_manager.category_groups,
1711+
year=cache_filters.get("year"),
1712+
since=cache_filters.get("since"),
1713+
)
16711714
else:
16721715
logger.info("Filtered view detected - updating cached tiers with edits")
16731716
updated_hot = CommitOrchestrator.apply_edits_to_dataframe(
@@ -1704,6 +1747,10 @@ def handle_commit_result(
17041747
# Cache update failed - not critical, just log
17051748
logger.warning(f"Cache update failed: {e}", exc_info=True)
17061749

1750+
# Clear pending edits on success (after cache update to preserve edits list)
1751+
self.data_manager.pending_edits.clear()
1752+
logger.info("Cleared pending edits")
1753+
17071754
# Refresh to show updated data (smooth update)
17081755
# Note: View already restored in app.py before commit started
17091756
logger.debug(

tests/test_app_controller.py

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,123 @@ def save_cache(
647647
updated_merchant = stub_cache.saved_hot.filter(pl.col("id") == edit_id)["merchant"][0]
648648
assert updated_merchant == "Edited"
649649

650+
async def test_filtered_view_partial_cache_update_when_cold_unavailable(self, controller):
651+
"""When cold cache is unavailable, hot should still be updated."""
652+
from moneyflow.state import TransactionEdit
653+
654+
class StubCacheManager:
655+
def __init__(self, hot_df):
656+
self._hot_df = hot_df
657+
self.saved_hot = None
658+
self.saved_cold = None
659+
self.saved_full = None
660+
661+
def load_hot_cache(self):
662+
return self._hot_df.clone() if self._hot_df is not None else None
663+
664+
def load_cold_cache(self):
665+
return None # Simulate cold cache unavailable
666+
667+
def save_hot_cache(self, hot_df, categories, category_groups):
668+
self.saved_hot = hot_df
669+
670+
def save_cold_cache(self, cold_df):
671+
self.saved_cold = cold_df
672+
673+
def save_cache(
674+
self, transactions_df, categories, category_groups, year=None, since=None
675+
):
676+
self.saved_full = transactions_df
677+
678+
full_df = controller.data_manager.df
679+
hot_df = full_df.head(3)
680+
681+
edit_id = hot_df["id"][0]
682+
old_merchant = hot_df["merchant"][0]
683+
edits = [TransactionEdit(edit_id, "merchant", old_merchant, "Edited", datetime.now())]
684+
controller.data_manager.pending_edits = edits.copy()
685+
controller.data_manager.df = hot_df.head(1).clone()
686+
687+
stub_cache = StubCacheManager(hot_df)
688+
controller.cache_manager = stub_cache
689+
690+
saved_state = controller.state.save_view_state()
691+
controller.handle_commit_result(
692+
success_count=1,
693+
failure_count=0,
694+
edits=edits,
695+
saved_state=saved_state,
696+
cache_filters={"year": None, "since": None},
697+
is_filtered_view=True,
698+
)
699+
700+
# Hot should be saved with edits, cold should not be touched
701+
assert stub_cache.saved_hot is not None, "Hot cache should be updated"
702+
assert stub_cache.saved_cold is None, "Cold cache should not be updated (was unavailable)"
703+
assert stub_cache.saved_full is None, "Full cache should not be called"
704+
705+
updated_merchant = stub_cache.saved_hot.filter(pl.col("id") == edit_id)["merchant"][0]
706+
assert updated_merchant == "Edited"
707+
708+
async def test_filtered_view_partial_cache_update_when_hot_unavailable(self, controller):
709+
"""When hot cache is unavailable, cold should still be updated."""
710+
from moneyflow.state import TransactionEdit
711+
712+
class StubCacheManager:
713+
def __init__(self, cold_df):
714+
self._cold_df = cold_df
715+
self.saved_hot = None
716+
self.saved_cold = None
717+
self.saved_full = None
718+
719+
def load_hot_cache(self):
720+
return None # Simulate hot cache unavailable
721+
722+
def load_cold_cache(self):
723+
return self._cold_df.clone() if self._cold_df is not None else None
724+
725+
def save_hot_cache(self, hot_df, categories, category_groups):
726+
self.saved_hot = hot_df
727+
728+
def save_cold_cache(self, cold_df):
729+
self.saved_cold = cold_df
730+
731+
def save_cache(
732+
self, transactions_df, categories, category_groups, year=None, since=None
733+
):
734+
self.saved_full = transactions_df
735+
736+
full_df = controller.data_manager.df
737+
cold_df = full_df.tail(3)
738+
739+
# Edit a transaction that's in cold cache
740+
edit_id = cold_df["id"][0]
741+
old_merchant = cold_df["merchant"][0]
742+
edits = [TransactionEdit(edit_id, "merchant", old_merchant, "ColdEdited", datetime.now())]
743+
controller.data_manager.pending_edits = edits.copy()
744+
controller.data_manager.df = cold_df.head(1).clone()
745+
746+
stub_cache = StubCacheManager(cold_df)
747+
controller.cache_manager = stub_cache
748+
749+
saved_state = controller.state.save_view_state()
750+
controller.handle_commit_result(
751+
success_count=1,
752+
failure_count=0,
753+
edits=edits,
754+
saved_state=saved_state,
755+
cache_filters={"year": None, "since": None},
756+
is_filtered_view=True,
757+
)
758+
759+
# Cold should be saved with edits, hot should not be touched
760+
assert stub_cache.saved_cold is not None, "Cold cache should be updated"
761+
assert stub_cache.saved_hot is None, "Hot cache should not be updated (was unavailable)"
762+
assert stub_cache.saved_full is None, "Full cache should not be called"
763+
764+
updated_merchant = stub_cache.saved_cold.filter(pl.col("id") == edit_id)["merchant"][0]
765+
assert updated_merchant == "ColdEdited"
766+
650767

651768
class TestEditQueueing:
652769
"""

tests/test_cache.py

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,3 +1570,146 @@ def test_filter_mtd_scenario(self):
15701570

15711571
assert len(filtered) == 2
15721572
assert all(d.month == 12 for d in filtered["date"].to_list())
1573+
1574+
1575+
class TestCachePersistenceAfterEdits:
1576+
"""
1577+
Test that edits are actually persisted to cache files.
1578+
1579+
This addresses the bug where changes committed to the backend
1580+
were not persisted to the local cache.
1581+
"""
1582+
1583+
def test_save_cache_persists_edits(self, cache_manager, sample_categories, sample_category_groups):
1584+
"""Test that changes to transactions are persisted after save_cache."""
1585+
# Create initial data
1586+
df = create_mixed_transactions_df()
1587+
original_merchant = df.filter(pl.col("id") == df["id"][0])["merchant"][0]
1588+
cache_manager.save_cache(df, sample_categories, sample_category_groups)
1589+
1590+
# Simulate an edit (like what happens after a commit)
1591+
edited_df = df.with_columns(
1592+
pl.when(pl.col("id") == df["id"][0])
1593+
.then(pl.lit("EDITED_MERCHANT"))
1594+
.otherwise(pl.col("merchant"))
1595+
.alias("merchant")
1596+
)
1597+
1598+
# Save the edited data
1599+
cache_manager.save_cache(edited_df, sample_categories, sample_category_groups)
1600+
1601+
# Load and verify the edit was persisted
1602+
result = cache_manager.load_cache()
1603+
assert result is not None
1604+
loaded_df, _, _, _ = result
1605+
1606+
edited_row = loaded_df.filter(pl.col("id") == df["id"][0])
1607+
assert len(edited_row) == 1
1608+
assert edited_row["merchant"][0] == "EDITED_MERCHANT"
1609+
assert edited_row["merchant"][0] != original_merchant
1610+
1611+
def test_save_hot_cache_persists_edits(
1612+
self, cache_manager, sample_categories, sample_category_groups
1613+
):
1614+
"""Test that edits to hot cache are persisted."""
1615+
# Create initial data with both tiers
1616+
df = create_mixed_transactions_df()
1617+
cache_manager.save_cache(df, sample_categories, sample_category_groups)
1618+
1619+
# Load the hot cache
1620+
hot_df = cache_manager.load_hot_cache()
1621+
assert hot_df is not None
1622+
assert len(hot_df) > 0
1623+
1624+
# Edit a hot transaction
1625+
hot_id = hot_df["id"][0]
1626+
edited_hot = hot_df.with_columns(
1627+
pl.when(pl.col("id") == hot_id)
1628+
.then(pl.lit("HOT_EDITED"))
1629+
.otherwise(pl.col("merchant"))
1630+
.alias("merchant")
1631+
)
1632+
1633+
# Save hot cache only
1634+
cache_manager.save_hot_cache(edited_hot, sample_categories, sample_category_groups)
1635+
1636+
# Load and verify
1637+
reloaded_hot = cache_manager.load_hot_cache()
1638+
edited_row = reloaded_hot.filter(pl.col("id") == hot_id)
1639+
assert edited_row["merchant"][0] == "HOT_EDITED"
1640+
1641+
# Verify cold cache is unchanged
1642+
reloaded_cold = cache_manager.load_cold_cache()
1643+
assert reloaded_cold is not None
1644+
1645+
def test_save_cold_cache_persists_edits(
1646+
self, cache_manager, sample_categories, sample_category_groups
1647+
):
1648+
"""Test that edits to cold cache are persisted."""
1649+
# Create initial data with both tiers
1650+
df = create_mixed_transactions_df()
1651+
cache_manager.save_cache(df, sample_categories, sample_category_groups)
1652+
1653+
# Load the cold cache
1654+
cold_df = cache_manager.load_cold_cache()
1655+
assert cold_df is not None
1656+
assert len(cold_df) > 0
1657+
1658+
# Edit a cold transaction
1659+
cold_id = cold_df["id"][0]
1660+
edited_cold = cold_df.with_columns(
1661+
pl.when(pl.col("id") == cold_id)
1662+
.then(pl.lit("COLD_EDITED"))
1663+
.otherwise(pl.col("merchant"))
1664+
.alias("merchant")
1665+
)
1666+
1667+
# Save cold cache only
1668+
cache_manager.save_cold_cache(edited_cold)
1669+
1670+
# Load and verify
1671+
reloaded_cold = cache_manager.load_cold_cache()
1672+
edited_row = reloaded_cold.filter(pl.col("id") == cold_id)
1673+
assert edited_row["merchant"][0] == "COLD_EDITED"
1674+
1675+
# Verify hot cache is unchanged
1676+
reloaded_hot = cache_manager.load_hot_cache()
1677+
assert reloaded_hot is not None
1678+
1679+
def test_atomic_write_creates_no_temp_files_on_success(
1680+
self, cache_manager, sample_categories, sample_category_groups
1681+
):
1682+
"""Verify no temp files are left behind after successful saves."""
1683+
df = create_mixed_transactions_df()
1684+
cache_manager.save_cache(df, sample_categories, sample_category_groups)
1685+
1686+
# Check for leftover temp files
1687+
cache_dir = cache_manager.cache_dir
1688+
temp_files = list(cache_dir.glob(".tmp_*"))
1689+
assert len(temp_files) == 0, f"Temp files left behind: {temp_files}"
1690+
1691+
def test_multiple_sequential_saves_persist_correctly(
1692+
self, cache_manager, sample_categories, sample_category_groups
1693+
):
1694+
"""Test that multiple sequential saves all persist correctly."""
1695+
df = create_mixed_transactions_df()
1696+
cache_manager.save_cache(df, sample_categories, sample_category_groups)
1697+
1698+
# Perform multiple edits and saves
1699+
for i in range(5):
1700+
hot_df = cache_manager.load_hot_cache()
1701+
if hot_df is not None and len(hot_df) > 0:
1702+
edited_hot = hot_df.with_columns(
1703+
pl.when(pl.col("id") == hot_df["id"][0])
1704+
.then(pl.lit(f"EDIT_{i}"))
1705+
.otherwise(pl.col("merchant"))
1706+
.alias("merchant")
1707+
)
1708+
cache_manager.save_hot_cache(edited_hot, sample_categories, sample_category_groups)
1709+
1710+
# Verify final state
1711+
final_hot = cache_manager.load_hot_cache()
1712+
final_row = final_hot.filter(pl.col("id") == df["id"][0])
1713+
# The merchant should be the last edit
1714+
if len(final_row) > 0:
1715+
assert "EDIT_" in final_row["merchant"][0]

0 commit comments

Comments
 (0)