fix: cached value and cached sorting issues

picard/ui/itemviews/__init__.py

@@ -418,6 +418,8 @@ def update_colums_text(self, color=None, bgcolor=None):
                     self.setTextAlignment(i, QtCore.Qt.AlignmentFlag.AlignRight | QtCore.Qt.AlignmentFlag.AlignVCenter)
 
                 if isinstance(column, CustomColumn):
+                    # Invalidate caches for this object to reflect tag changes
+                    column.invalidate_cache(self.obj)
                     # Hide custom column values for container rows like
                     # "Clusters" and "Unclustered Files".
                     # These represent unrelated collections and should not

picard/ui/itemviews/basetreeview.py

@@ -462,8 +462,15 @@ def _on_header_updated(self):
         recognized = set(get_recognized_view_columns().values())
         if self.columns not in recognized:
             return
+        # Invalidate provider and item sort caches to avoid incompatible keys
+        self._invalidate_view_caches()
         self._refresh_header_labels()
         self._refresh_all_items_data()
+        # Trigger resort on current column if sorting is enabled
+        if self.isSortingEnabled():
+            current = self.sortColumn()
+            if current >= 0:
+                self.sortByColumn(current, self.header().sortIndicatorOrder())
 
     def _init_header(self):
         # Load any persisted user-defined custom columns before header setup
@@ -483,6 +490,25 @@ def _init_header(self):
         self.restore_default_columns()
         self.restore_state()
 
+    def _invalidate_view_caches(self):
+        """Clear provider caches and per-item sort keys for this view."""
+        # Invalidate provider caches for custom columns in this view
+        from picard.ui.itemviews.custom_columns import CustomColumn  # local import to avoid cycles
+
+        for col in self.columns:
+            if isinstance(col, CustomColumn):
+                col.invalidate_cache()
+
+        # Clear cached sort keys on all items
+        def _clear_item_sort_cache(item):
+            if hasattr(item, "_sortkeys") and isinstance(item._sortkeys, dict):
+                item._sortkeys.clear()
+            for i in range(item.childCount()):
+                _clear_item_sort_cache(item.child(i))
+
+        for i in range(self.topLevelItemCount()):
+            _clear_item_sort_cache(self.topLevelItem(i))
+
     def supportedDropActions(self):
         return QtCore.Qt.DropAction.CopyAction | QtCore.Qt.DropAction.MoveAction
 

picard/ui/itemviews/custom_columns/column.py

@@ -28,6 +28,7 @@
     ColumnSortType,
 )
 from picard.ui.itemviews.custom_columns.protocols import (
+    CacheInvalidatable,
     ColumnValueProvider,
     SortKeyProvider,
 )
@@ -81,3 +82,15 @@ def __init__(
             status_icon=False,
         )
         self.provider = provider
+
+    def invalidate_cache(self, obj=None):  # pragma: no cover - UI-driven
+        """Invalidate any caches on the provider if supported.
+
+        Parameters
+        ----------
+        obj
+            Optional item to invalidate for; if omitted, clears entire cache.
+        """
+        if isinstance(self.provider, CacheInvalidatable):
+            # type: ignore[attr-defined]
+            self.provider.invalidate(obj)  # noqa: PGH003

picard/ui/itemviews/custom_columns/protocols.py

@@ -30,6 +30,20 @@
 from picard.item import Item
 
 
+@runtime_checkable
+class CacheInvalidatable(Protocol):
+    def invalidate(self, obj: Item | None = None) -> None:  # pragma: no cover - optional
+        """Invalidate any cached values.
+
+        Parameters
+        ----------
+        obj
+            If provided, invalidate cache entries related to this item only.
+            If ``None``, invalidate the entire cache.
+        """
+        ...
+
+
 @runtime_checkable
 class ColumnValueProvider(Protocol):
     def evaluate(self, obj: Item) -> str:

picard/ui/itemviews/custom_columns/script_provider.py

@@ -24,6 +24,7 @@
 
 from collections import deque
 from collections.abc import Callable
+import contextlib
 import re
 from time import perf_counter
 from weakref import WeakKeyDictionary
@@ -154,3 +155,33 @@ def __repr__(self) -> str:  # pragma: no cover - debug helper
             f"{cls_name}(script={self._script!r}, max_runtime_ms={self._max_runtime_ms}, "
             f"cache_size={self._id_cache_max})"
         )
+
+    # Optional cache invalidation API (duck-typed via protocol)
+    def invalidate(self, obj: Item | None = None) -> None:
+        """Invalidate cached values.
+
+        Parameters
+        ----------
+        obj
+            If provided, invalidate cache for this specific item; otherwise
+            clear the entire cache.
+        """
+        if obj is None:
+            # Clear all caches
+            self._cache.clear()
+            self._id_cache.clear()
+            self._id_order.clear()
+            return
+
+        # Try to remove from weak cache
+        with contextlib.suppress(TypeError):  # Not weakrefable; fall back to id-cache removal
+            if obj in self._cache:
+                del self._cache[obj]
+
+        # Remove from id-based cache if present
+        obj_id = id(obj)
+        if obj_id in self._id_cache:
+            self._id_cache.pop(obj_id, None)
+            with contextlib.suppress(ValueError):
+                # Remove one occurrence from order deque
+                self._id_order.remove(obj_id)

picard/ui/itemviews/custom_columns/sorting_adapters.py

@@ -28,6 +28,7 @@
 from picard.item import Item
 
 from picard.ui.itemviews.custom_columns.protocols import (
+    CacheInvalidatable,
     ColumnValueProvider,
     SortKeyProvider,
 )
@@ -62,6 +63,18 @@ def evaluate(self, obj: Item) -> str:
         """Return evaluated text value for item."""
         return self._base.evaluate(obj)
 
+    def invalidate(self, obj: Item | None = None) -> None:  # pragma: no cover - simple delegation
+        """Forward cache invalidation to the wrapped provider if supported.
+
+        Parameters
+        ----------
+        obj
+            Optional item to invalidate for; if omitted, clears entire cache.
+        """
+        # Not all providers implement `invalidate`; the check avoids `AttributeError`
+        if isinstance(self._base, CacheInvalidatable):
+            self._base.invalidate(obj)
+
     def __repr__(self) -> str:  # pragma: no cover - debug helper
         return f"{self.__class__.__name__}(base={self._base!r})"
 
@@ -298,6 +311,20 @@ def sort_key(self, obj: Item):  # pragma: no cover - thin wrapper
             pass
         return key
 
+    # Optional cache invalidation API for adapter
+    def invalidate(self, obj: Item | None = None) -> None:  # pragma: no cover - simple
+        if obj is None:
+            self._cache.clear()
+        else:
+            # Attempt precise removal; fall back to full clear if not weakrefable
+            try:
+                del self._cache[obj]
+            except (KeyError, TypeError):
+                # Not present or not weakrefable; best effort clear all to avoid stale keys
+                self._cache.clear()
+        # Also forward invalidation to the wrapped provider if it supports it
+        super().invalidate(obj)
+
 
 class ReverseAdapter(_AdapterBase):
     """Provide reversed sorting for string or numeric sort keys."""

test/columns_api/__init__.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+#
+# Picard, the next-generation MusicBrainz tagger
+#
+# Copyright (C) 2025 The MusicBrainz Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+"""Tests for public API for custom columns: types, factories and registry."""

test/columns_api/test_custom_columns_invalidation.py

@@ -0,0 +1,186 @@
+# -*- coding: utf-8 -*-
+#
+# Picard, the next-generation MusicBrainz tagger
+#
+# Copyright (C) 2025 The MusicBrainz Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+from __future__ import annotations
+
+from collections.abc import Callable
+from dataclasses import dataclass
+
+from picard.item import Item
+from picard.metadata import Metadata
+
+import pytest
+
+from picard.ui.columns import (
+    ColumnAlign,
+    ColumnSortType,
+)
+from picard.ui.itemviews.custom_columns import CustomColumn
+from picard.ui.itemviews.custom_columns.protocols import ColumnValueProvider
+from picard.ui.itemviews.custom_columns.script_provider import ChainedValueProvider
+from picard.ui.itemviews.custom_columns.sorting_adapters import CachedSortAdapter
+
+
+# --- Fixtures and helpers ----------------------------------------------------
+
+
+@dataclass
+class _FakeItem:
+    values: dict[str, str]
+
+    def column(self, key: str) -> str:
+        return self.values.get(key, "")
+
+    @property
+    def metadata(self) -> Metadata:
+        md = Metadata()
+        for k, v in self.values.items():
+            md[k] = v
+        return md
+
+
+class _NonWeakRefable:
+    def __init__(self, artist: str) -> None:
+        self.artist = artist
+
+    def column(self, key: str) -> str:
+        return self.artist if key == "artist" else ""
+
+    @property
+    def metadata(self) -> Metadata:
+        md = Metadata()
+        md["artist"] = self.artist
+        return md
+
+
+@pytest.fixture(params=["weak", "nonweak"])
+def item_factory(request: pytest.FixtureRequest) -> Callable[[str], object]:
+    def _make(initial_value: str) -> object:
+        if request.param == "weak":
+            return _FakeItem(values={"artist": initial_value})
+        return _NonWeakRefable(initial_value)
+
+    return _make
+
+
+# --- Tests: provider invalidation --------------------------------------------
+
+
+def test_chained_provider_invalidate_single_and_all(item_factory: Callable[[str], Item]) -> None:
+    provider = ChainedValueProvider("%artist%", max_runtime_ms=1000)
+    obj = item_factory("Artist A")
+
+    # Initial evaluation and cache
+    assert provider.evaluate(obj) == "Artist A"
+
+    # Change underlying value; still cached until invalidated
+    if isinstance(obj, _FakeItem):
+        obj.values["artist"] = "Artist B"
+    else:
+        obj.artist = "Artist B"  # type: ignore[attr-defined]
+    assert provider.evaluate(obj) == "Artist A"
+
+    # Invalidate for this object only
+    provider.invalidate(obj)
+    assert provider.evaluate(obj) == "Artist B"
+
+    # Cache again; change value to C
+    if isinstance(obj, _FakeItem):
+        obj.values["artist"] = "Artist C"
+    else:
+        obj.artist = "Artist C"  # type: ignore[attr-defined]
+    assert provider.evaluate(obj) == "Artist B"
+
+    # Invalidate all and confirm new value
+    provider.invalidate(None)
+    assert provider.evaluate(obj) == "Artist C"
+
+
+# --- Tests: CustomColumn.invalidate_cache delegation -------------------------
+
+
+class _SpyProvider(ChainedValueProvider):
+    def __init__(self, script: str) -> None:
+        super().__init__(script)
+        self.calls: list[object | None] = []
+
+    def invalidate(self, obj: object | None = None) -> None:  # type: ignore[override]
+        self.calls.append(obj)
+        super().invalidate(obj)  # keep behavior intact
+
+
+def test_custom_column_invalidate_cache_delegates() -> None:
+    spy = _SpyProvider("%artist%")
+    col = CustomColumn(
+        title="T",
+        key="k",
+        provider=spy,
+        width=None,
+        align=ColumnAlign.LEFT,
+        sort_type=ColumnSortType.TEXT,
+        always_visible=False,
+    )
+
+    # Call without obj and with obj
+    col.invalidate_cache(None)
+    fake = _FakeItem(values={"artist": "A"})
+    col.invalidate_cache(fake)
+
+    assert spy.calls == [None, fake]
+
+
+# --- Tests: CachedSortAdapter invalidation -----------------------------------
+
+
+class _ValueItem:
+    def __init__(self, value: str) -> None:
+        self.value = value
+
+
+class _ValueProvider(ColumnValueProvider):
+    def evaluate(self, obj: _ValueItem) -> str:
+        return obj.value
+
+
+def test_cached_sort_adapter_invalidate_single_and_all() -> None:
+    provider = _ValueProvider()
+    adapter = CachedSortAdapter(provider)
+    a_item = _ValueItem("B")
+    b_item = _ValueItem("a")
+
+    # Populate cache
+    assert adapter.sort_key(a_item) == "b"
+    assert adapter.sort_key(b_item) == "a"
+
+    # Change values; still cached
+    a_item.value = "Z"
+    b_item.value = "y"
+    assert adapter.sort_key(a_item) == "b"
+    assert adapter.sort_key(b_item) == "a"
+
+    # Invalidate single
+    adapter.invalidate(a_item)
+    assert adapter.sort_key(a_item) == "z"
+    # b still cached
+    assert adapter.sort_key(b_item) == "a"
+
+    # Invalidate all
+    adapter.invalidate(None)
+    assert adapter.sort_key(b_item) == "y"