refactor(BA-5528): rename misleading pop* methods on chat cache/config

Python's `pop` convention (`dict.pop`, `list.pop`) implies the popped value
is returned, but these methods return a plain `bool` because every caller
only needs "did anything actually get removed?" Renaming so the method
names match what the calls do:

- `DeploymentChatCache.pop`        → `delete`        (removes the entry)
- `DeploymentChatConfig.pop`       → `delete`        (removes the entry)
- `DeploymentChatConfig.pop_token` → `clear_token`   (nulls the field, drops the entry only when both fields are unset)
- `DeploymentChatConfig.pop_model` → `clear_model`   (same shape as `clear_token`)

`pop_token`/`pop_model` were already misnomers — they null one field rather
than fully popping the entry, so `clear_*` reflects the actual behavior.
Return types stay `bool` since no caller uses the popped value.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: jopemachine

src/ai/backend/client/cli/v2/deployment/chat/commands.py

@@ -191,7 +191,7 @@ async def _run() -> None:
                 # hint instead of silently re-sending a stale key. Other
                 # BackendAPIErrors fall through to ``_run_async`` which formats
                 # the manager-style status/title/msg payload generically.
-                if token is not None and chat_config.pop_token(deployment_id):
+                if token is not None and chat_config.clear_token(deployment_id):
                     chat_config.save()
                 raise click.ClickException(
                     f"The inference endpoint rejected the configured token for "
@@ -305,7 +305,7 @@ def clear(deployment_id: DeploymentID) -> None:
     ``./bai deployment chat-cache clear`` to drop it immediately.
     """
     config = DeploymentChatConfig.load()
-    if config.pop(deployment_id):
+    if config.delete(deployment_id):
         config.save()
         print(f"Removed config entry for deployment {deployment_id}.")
     else:
@@ -350,7 +350,7 @@ def cache_clear(deployment_id: DeploymentID) -> None:
     user-managed config entry (token + model) is left alone.
     """
     cache = DeploymentChatCache.load()
-    if cache.pop(deployment_id):
+    if cache.delete(deployment_id):
         cache.save()
         print(f"Removed cache entry for deployment {deployment_id}.")
     else:

src/ai/backend/client/cli/v2/deployment/chat/types.py

@@ -59,7 +59,8 @@ def get(self, deployment_id: DeploymentID) -> DeploymentChatCacheEntry | None:
     def set(self, deployment_id: DeploymentID, entry: DeploymentChatCacheEntry) -> None:
         self.deployments[deployment_id] = entry
 
-    def pop(self, deployment_id: DeploymentID) -> bool:
+    def delete(self, deployment_id: DeploymentID) -> bool:
+        """Remove the cache entry for ``deployment_id``; return True when an entry was removed."""
         return self.deployments.pop(deployment_id, None) is not None
 
     @classmethod
@@ -124,7 +125,11 @@ def set_token(self, deployment_id: DeploymentID, token: str) -> None:
     def set_model(self, deployment_id: DeploymentID, model: str) -> None:
         self.deployments[deployment_id].model = model
 
-    def pop_token(self, deployment_id: DeploymentID) -> bool:
+    def clear_token(self, deployment_id: DeploymentID) -> bool:
+        """Null the token field for ``deployment_id``; return True when a token was cleared.
+
+        Drops the entry entirely if both ``token`` and ``model`` end up unset.
+        """
         entry = self.deployments.get(deployment_id)
         if entry is None or entry.token is None:
             return False
@@ -133,7 +138,11 @@ def pop_token(self, deployment_id: DeploymentID) -> bool:
             del self.deployments[deployment_id]
         return True
 
-    def pop_model(self, deployment_id: DeploymentID) -> bool:
+    def clear_model(self, deployment_id: DeploymentID) -> bool:
+        """Null the model field for ``deployment_id``; return True when a model was cleared.
+
+        Drops the entry entirely if both ``token`` and ``model`` end up unset.
+        """
         entry = self.deployments.get(deployment_id)
         if entry is None or entry.model is None:
             return False
@@ -142,7 +151,8 @@ def pop_model(self, deployment_id: DeploymentID) -> bool:
             del self.deployments[deployment_id]
         return True
 
-    def pop(self, deployment_id: DeploymentID) -> bool:
+    def delete(self, deployment_id: DeploymentID) -> bool:
+        """Remove the config entry for ``deployment_id``; return True when an entry was removed."""
         return self.deployments.pop(deployment_id, None) is not None
 
     @classmethod

tests/unit/client/cli/test_deployment_chat_types.py

@@ -66,20 +66,20 @@ def test_set_overwrites_existing_entry(
         assert stored is not None
         assert stored.default_model == "m2"
 
-    def test_pop_returns_true_when_present(
+    def test_delete_returns_true_when_present(
         self,
         cache: DeploymentChatCache,
         cache_entry: DeploymentChatCacheEntry,
         deployment_id: UUID,
     ) -> None:
         cache.set(deployment_id, cache_entry)
-        assert cache.pop(deployment_id) is True
+        assert cache.delete(deployment_id) is True
         assert cache.get(deployment_id) is None
 
-    def test_pop_returns_false_when_absent(
+    def test_delete_returns_false_when_absent(
         self, cache: DeploymentChatCache, deployment_id: UUID
     ) -> None:
-        assert cache.pop(deployment_id) is False
+        assert cache.delete(deployment_id) is False
 
 
 class TestConfigTokenStore:
@@ -96,24 +96,24 @@ def test_set_overwrites_existing_token(
         chat_config.set_token(deployment_id, "sk-new")
         assert chat_config.get_token(deployment_id) == "sk-new"
 
-    def test_pop_token_returns_true_when_present(
+    def test_clear_token_returns_true_when_present(
         self, chat_config: DeploymentChatConfig, deployment_id: UUID
     ) -> None:
         chat_config.set_token(deployment_id, "sk-x")
-        assert chat_config.pop_token(deployment_id) is True
+        assert chat_config.clear_token(deployment_id) is True
         assert chat_config.get_token(deployment_id) is None
 
-    def test_pop_token_returns_false_when_absent(
+    def test_clear_token_returns_false_when_absent(
         self, chat_config: DeploymentChatConfig, deployment_id: UUID
     ) -> None:
-        assert chat_config.pop_token(deployment_id) is False
+        assert chat_config.clear_token(deployment_id) is False
 
-    def test_pop_token_keeps_entry_when_model_remains(
+    def test_clear_token_keeps_entry_when_model_remains(
         self, chat_config: DeploymentChatConfig, deployment_id: UUID
     ) -> None:
         chat_config.set_token(deployment_id, "sk-x")
         chat_config.set_model(deployment_id, "llama-3-8b")
-        assert chat_config.pop_token(deployment_id) is True
+        assert chat_config.clear_token(deployment_id) is True
         # Model side of the entry survives token removal.
         entry = chat_config.get(deployment_id)
         assert entry is not None
@@ -135,17 +135,17 @@ def test_set_overwrites_existing_model(
         chat_config.set_model(deployment_id, "new-model")
         assert chat_config.get_model(deployment_id) == "new-model"
 
-    def test_pop_model_returns_true_when_present(
+    def test_clear_model_returns_true_when_present(
         self, chat_config: DeploymentChatConfig, deployment_id: UUID
     ) -> None:
         chat_config.set_model(deployment_id, "llama-3-8b")
-        assert chat_config.pop_model(deployment_id) is True
+        assert chat_config.clear_model(deployment_id) is True
         assert chat_config.get_model(deployment_id) is None
 
-    def test_pop_model_returns_false_when_absent(
+    def test_clear_model_returns_false_when_absent(
         self, chat_config: DeploymentChatConfig, deployment_id: UUID
     ) -> None:
-        assert chat_config.pop_model(deployment_id) is False
+        assert chat_config.clear_model(deployment_id) is False
 
     def test_token_and_model_share_one_entry(
         self, chat_config: DeploymentChatConfig, deployment_id: UUID
@@ -156,19 +156,19 @@ def test_token_and_model_share_one_entry(
         assert entry == DeploymentChatConfigEntry(token="sk-x", model="llama-3-8b")
 
 
-class TestConfigPop:
-    def test_pop_removes_whole_entry(
+class TestConfigDelete:
+    def test_delete_removes_whole_entry(
         self, chat_config: DeploymentChatConfig, deployment_id: UUID
     ) -> None:
         chat_config.set_token(deployment_id, "sk-x")
         chat_config.set_model(deployment_id, "llama-3-8b")
-        assert chat_config.pop(deployment_id) is True
+        assert chat_config.delete(deployment_id) is True
         assert chat_config.get(deployment_id) is None
 
-    def test_pop_returns_false_when_absent(
+    def test_delete_returns_false_when_absent(
         self, chat_config: DeploymentChatConfig, deployment_id: UUID
     ) -> None:
-        assert chat_config.pop(deployment_id) is False
+        assert chat_config.delete(deployment_id) is False
 
 
 class TestHistoryAppendSlice: