fix(stats): categorize ticket stats by state name, not state_type_id

mcp_zammad/server.py

@@ -102,14 +102,6 @@ def model_dump(self) -> dict[str, Any]: ...  # codacy: ignore E704
 CHARACTER_LIMIT = 25000  # Maximum response size per MCP best practices
 ARTICLE_BODY_TRUNCATE_LENGTH = 500  # Maximum length for article body in markdown formatting
 
-# Zammad state type IDs (from Zammad API)
-STATE_TYPE_NEW = 1
-STATE_TYPE_OPEN = 2
-STATE_TYPE_CLOSED = 3
-STATE_TYPE_PENDING_REMINDER = 4
-STATE_TYPE_PENDING_CLOSE = 5
-
-
 # Tool annotation constants
 def _read_only_annotations(title: str) -> ToolAnnotations:
     """Create read-only tool annotations with title."""
@@ -1848,8 +1840,6 @@ def clear_caches(self) -> None:
             del self._states_cache
         if hasattr(self, "_priorities_cache"):
             del self._priorities_cache
-        if hasattr(self, "_state_type_mapping"):
-            del self._state_type_mapping
 
     @staticmethod
     def _extract_state_name(ticket: dict[str, Any]) -> str:
@@ -1884,16 +1874,21 @@ def _is_ticket_escalated(ticket: dict[str, Any]) -> bool:
             or ticket.get("update_escalation_at")
         )
 
-    def _get_state_type_mapping(self) -> dict[str, int]:
-        """Get mapping of state names to state_type_id.
-
-        Returns:
-            Dictionary mapping state name to state_type_id
-        """
-        if not hasattr(self, "_state_type_mapping"):
-            states = self._get_cached_states()
-            self._state_type_mapping = {state.name: state.state_type_id for state in states}
-        return self._state_type_mapping
+    # Semantic ticket state names that map to the open/closed/pending buckets.
+    # Categorization is keyed on the state *name*, not the numeric state_type_id:
+    # state_type_id is assigned per instance and is not stable across Zammad
+    # versions or installations, so matching on it produced incorrect counts on
+    # instances whose state set differs from the defaults (e.g. a renumbered
+    # "closed" state or an extra custom state such as "merged").
+    _STATE_NAME_OPEN = frozenset({"new", "open"})
+    _STATE_NAME_CLOSED = frozenset({"closed"})
+    _STATE_NAME_PENDING = frozenset({"pending", "pending reminder", "pending close"})
+    # Fallback prefix for custom states that Zammad's own UI treats as pending
+    # (e.g. a user-defined "pending refund"). A space-terminated word match is
+    # deliberately narrower than a bare prefix or substring: it catches
+    # "pending anything" without misfiring on near-miss names such as
+    # "pendingly" or "pending-approval", which are not reliably pending states.
+    _STATE_NAME_PENDING_PREFIX = "pending "
 
     def _categorize_ticket_state(self, state_name: str) -> tuple[int, int, int]:
         """Categorize a ticket state into open/closed/pending counters.
@@ -1905,20 +1900,21 @@ def _categorize_ticket_state(self, state_name: str) -> tuple[int, int, int]:
             Tuple of (open_increment, closed_increment, pending_increment)
 
         Note:
-            Uses state_type_id from Zammad instead of string matching:
-            - 1 (new), 2 (open) -> open
-            - 3 (closed) -> closed
-            - 4 (pending reminder), 5 (pending close) -> pending
+            Categorizes by the semantic state name (case-insensitive) rather
+            than the numeric state_type_id, which is per-instance and unstable:
+            - "new", "open" -> open
+            - "closed" -> closed
+            - "pending", "pending reminder", "pending close", or any custom
+              state starting with "pending " (space-terminated) -> pending
+            Any other state (e.g. "merged") is counted in the total but not in
+            any bucket.
         """
-        state_type_mapping = self._get_state_type_mapping()
-        state_type_id = state_type_mapping.get(state_name, 0)
-
-        # Categorize based on state_type_id
-        if state_type_id in [STATE_TYPE_NEW, STATE_TYPE_OPEN]:
+        name = state_name.strip().casefold()
+        if name in self._STATE_NAME_OPEN:
             return (1, 0, 0)
-        if state_type_id == STATE_TYPE_CLOSED:
+        if name in self._STATE_NAME_CLOSED:
             return (0, 1, 0)
-        if state_type_id in [STATE_TYPE_PENDING_REMINDER, STATE_TYPE_PENDING_CLOSE]:
+        if name in self._STATE_NAME_PENDING or name.startswith(self._STATE_NAME_PENDING_PREFIX):
             return (0, 0, 1)
         return (0, 0, 0)
 
@@ -2084,7 +2080,8 @@ def zammad_get_ticket_stats(params: GetTicketStatsParams) -> TicketStats:
             Note:
                 Uses pagination to scan tickets without loading all into memory.
                 May take several seconds for large ticket databases (>10k tickets).
-                State categorization uses state_type_id: new/open=open, closed=closed, pending=pending.
+                State categorization is by semantic state name: new/open=open,
+                closed=closed, pending reminder/pending close=pending.
                 Date filtering (start_date, end_date) not yet implemented - shows warning if provided.
                 Processes up to 100,000 tickets (1000 pages x 100 per page).
             """
@@ -2222,7 +2219,8 @@ def zammad_list_ticket_states(params: ListParams) -> str:
                 Results are cached in memory for performance (cleared on server restart).
                 All states are returned in a single response (no pagination needed).
                 Use state 'name' field when creating/updating tickets, not ID.
-                State types: 1=new, 2=open, 3=closed, 4=pending reminder, 5=pending close.
+                The state_type_id values shown are per-instance and are not
+                meaningful across different Zammad installations.
             """
             states = self._get_cached_states()
 

tests/test_server.py

@@ -1398,6 +1398,89 @@ def test_get_ticket_stats_tool(mock_zammad_client, decorator_capturer):
         mock_logger.warning.assert_called_with("Date filtering not yet implemented - ignoring date parameters")
 
 
+def test_categorize_ticket_state_uses_name_not_type_id():
+    """Categorize by state name, not numeric state_type_id (per-instance and unstable)."""
+    # Regression guard: on instances with renumbered built-in states or custom
+    # states, the numeric state_type_id does not line up with the defaults, so
+    # only the semantic name can be trusted.
+    server_inst = ZammadMCPServer()
+
+    assert server_inst._categorize_ticket_state("new") == (1, 0, 0)
+    assert server_inst._categorize_ticket_state("open") == (1, 0, 0)
+    assert server_inst._categorize_ticket_state("closed") == (0, 1, 0)
+    assert server_inst._categorize_ticket_state("pending reminder") == (0, 0, 1)
+    assert server_inst._categorize_ticket_state("pending close") == (0, 0, 1)
+    # The bare "pending" state (exact match) is pending.
+    assert server_inst._categorize_ticket_state("pending") == (0, 0, 1)
+    # Custom "pending " states fall into pending via the space-terminated
+    # word fallback, so user-defined states (e.g. "pending refund") are not
+    # silently dropped.
+    assert server_inst._categorize_ticket_state("pending refund") == (0, 0, 1)
+    # The fallback is deliberately narrow: near-miss names are NOT auto-
+    # pending (a bare prefix or substring would miscount these).
+    assert server_inst._categorize_ticket_state("pendingly") == (0, 0, 0)
+    assert server_inst._categorize_ticket_state("pending-approval") == (0, 0, 0)
+    assert server_inst._categorize_ticket_state("PENDING") == (0, 0, 1)
+    assert server_inst._categorize_ticket_state("PENDING APPROVAL") == (0, 0, 1)
+    # A state that merely *contains* "pending" elsewhere is NOT auto-pending.
+    assert server_inst._categorize_ticket_state("reviewed pending approval") == (0, 0, 0)
+    # Other custom states: counted in the total, excluded from every bucket.
+    assert server_inst._categorize_ticket_state("merged") == (0, 0, 0)
+    # Case-insensitive, tolerant of surrounding whitespace.
+    assert server_inst._categorize_ticket_state("  Closed ") == (0, 1, 0)
+
+
+def test_get_ticket_stats_uses_state_name_not_state_type_id(mock_zammad_client, decorator_capturer):
+    """Ticket stats stay correct when state_type_id values are non-default."""
+    # Replicates a real Zammad instance where the built-in states are renumbered
+    # (closed=5, pending reminder=3) and a custom "merged" state (id 6) exists.
+    # The old implementation compared state_type_id against hardcoded defaults
+    # (1-5), which scrambled the counts on such an instance.
+    mock_instance, _ = mock_zammad_client
+
+    tickets = [
+        {"id": 1, "state": {"id": 1, "name": "new", "state_type_id": 1}},
+        {"id": 2, "state": {"id": 2, "name": "open", "state_type_id": 2}},
+        # Non-default: "pending reminder" is state_type_id 3 here (not 4).
+        {"id": 3, "state": {"id": 3, "name": "pending reminder", "state_type_id": 3}},
+        # Non-default: "closed" is state_type_id 5 here (not 3).
+        {"id": 4, "state": {"id": 5, "name": "closed", "state_type_id": 5}},
+        {"id": 5, "state": {"id": 4, "name": "pending close", "state_type_id": 4}},
+        # Custom state: counted in total, excluded from every bucket.
+        {"id": 6, "state": {"id": 6, "name": "merged", "state_type_id": 6}},
+    ]
+    mock_instance.search_tickets.side_effect = [tickets, []]
+    mock_instance.get_ticket_states.return_value = [
+        {"id": 1, "name": "new", "state_type_id": 1, "created_at": "2024-01-01", "updated_at": "2024-01-01"},
+        {"id": 2, "name": "open", "state_type_id": 2, "created_at": "2024-01-01", "updated_at": "2024-01-01"},
+        {
+            "id": 3,
+            "name": "pending reminder",
+            "state_type_id": 3,
+            "created_at": "2024-01-01",
+            "updated_at": "2024-01-01",
+        },
+        {"id": 5, "name": "closed", "state_type_id": 5, "created_at": "2024-01-01", "updated_at": "2024-01-01"},
+        {"id": 4, "name": "pending close", "state_type_id": 4, "created_at": "2024-01-01", "updated_at": "2024-01-01"},
+        {"id": 6, "name": "merged", "state_type_id": 6, "created_at": "2024-01-01", "updated_at": "2024-01-01"},
+    ]
+
+    server_inst = ZammadMCPServer()
+    server_inst.client = mock_instance
+    test_tools, capture_tool = decorator_capturer(server_inst.mcp.tool)
+    server_inst.mcp.tool = capture_tool  # type: ignore[method-assign, assignment]
+    server_inst.get_client = lambda: server_inst.client  # type: ignore[method-assign, assignment, return-value]
+    server_inst._setup_system_tools()
+
+    stats = test_tools["zammad_get_ticket_stats"](GetTicketStatsParams())
+
+    assert stats.total_count == 6
+    assert stats.open_count == 2  # new + open
+    assert stats.closed_count == 1  # closed (state_type_id 5, not 3)
+    assert stats.pending_count == 2  # pending reminder (id 3) + pending close (id 4)
+    # "merged" is in the total but in no bucket: 2 + 1 + 2 = 5, + 1 custom = 6.
+
+
 def test_resource_handlers(decorator_capturer):
     """Test resource handler registration and execution."""
     server = ZammadMCPServer()