refactor(deck): address PR #826 review feedback

- Modernize the new models (DeckCardSummary, DeckCommentSummary,
  StackOverview, BoardOverviewResponse + the loosened unions) to PEP 604
  syntax (list[...] / X | None), per CLAUDE.md.
- Make status="done" exclude archived cards so open/done/archived partition
  the board with no overlap (a done+archived card is reported only as
  "archived"); document the semantics in docstrings and docs/deck.md, add a
  partition unit test.
- deck_get_archived_stacks: pass through label/assigned_to filters (status
  stays archived-only by definition); note the limitation in the docstring.
- Rename _validate_description_max_length → _validate_positive_length (now a
  generic positive-length guard).
- Soften deck_get_board_overview docstring: it views board state and omits
  the ACL/user/label-management fields deck_get_board exposes.

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

docs/deck.md

@@ -59,7 +59,7 @@ default** and support filtering so you fetch only what you need.
 | Parameter | Default | Effect |
 |-----------|---------|--------|
 | `detail` | `summary` | `summary` returns compact rows (id, title, stackId, labels as titles, assignee UIDs, due/done, counts, a short `descriptionPreview`); `full` returns the complete card objects (the pre-0.92 shape). |
-| `status` | `open` | Filter before serialization: `open` (excludes archived **and** done), `done`, `archived`, or `all`. |
+| `status` | `open` | Filter before serialization: `open`, `done`, `archived`, or `all`. The first three **partition** the board (no overlap) — a card that is both done and archived is reported only under `archived`. |
 | `label` | – | Only cards carrying a label with this exact title. |
 | `assigned_to` | – | Only cards assigned to this user UID. |
 | `description_max_length` | – | In `detail="full"`, truncate each description. |

nextcloud_mcp_server/models/deck.py

@@ -145,24 +145,24 @@ class DeckCardSummary(BaseModel):
     title: str
     stackId: int
     archived: bool = False
-    duedate: Optional[datetime] = None
+    duedate: datetime | None = None
-    done: Optional[datetime] = None
+    done: datetime | None = None
-    labels: List[str] = Field(
+    labels: list[str] = Field(
         default_factory=list, description="Label titles assigned to the card"
     )
-    assignedUsers: List[str] = Field(
+    assignedUsers: list[str] = Field(
         default_factory=list, description="UIDs of users assigned to the card"
     )
-    attachmentCount: Optional[int] = Field(
+    attachmentCount: int | None = Field(
         default=None, description="Number of attachments on the card"
     )
-    commentsUnread: Optional[int] = Field(
+    commentsUnread: int | None = Field(
         default=None, description="Number of unread comments (Deck exposes no total)"
     )
     hasDescription: bool = Field(
         default=False, description="Whether the card has a non-empty description"
     )
-    descriptionPreview: Optional[str] = Field(
+    descriptionPreview: str | None = Field(
         default=None, description="Truncated preview of the card description"
     )
 
@@ -176,7 +176,7 @@ class DeckStack(BaseModel):
     lastModified: Optional[int] = None
     # Cards may be projected to DeckCardSummary when a tool is called with
     # detail="summary" (the default for list tools).
-    cards: Optional[List[Union[DeckCard, DeckCardSummary]]] = None
+    cards: list[DeckCard | DeckCardSummary] | None = None
     etag: Optional[str] = Field(default=None, alias="ETag")
 
 
@@ -279,9 +279,9 @@ class StackOverview(BaseModel):
 
     id: int = Field(description="Stack ID")
     title: str = Field(description="Stack title")
-    order: Optional[int] = Field(default=None, description="Stack sort order")
+    order: int | None = Field(default=None, description="Stack sort order")
     card_count: int = Field(description="Number of cards returned for this stack")
-    cards: List[DeckCardSummary] = Field(
+    cards: list[DeckCardSummary] = Field(
         default_factory=list, description="Compact card rows in this stack"
     )
 
@@ -295,10 +295,10 @@ class BoardOverviewResponse(BaseResponse):
 
     board_id: int = Field(description="Board ID")
     title: str = Field(description="Board title")
-    labels: List[str] = Field(
+    labels: list[str] = Field(
         default_factory=list, description="Board label titles (legend)"
     )
-    stacks: List[StackOverview] = Field(
+    stacks: list[StackOverview] = Field(
         default_factory=list, description="Stacks with compact card rows"
     )
     total_cards: int = Field(description="Total cards across all returned stacks")
@@ -353,7 +353,7 @@ class CreateLabelResponse(BaseResponse):
 class ListCardsResponse(BaseResponse):
     """Response model for listing deck cards."""
 
-    cards: list[Union[DeckCard, DeckCardSummary]] = Field(
+    cards: list[DeckCard | DeckCardSummary] = Field(
         description="List of deck cards (summaries unless detail='full')"
     )
     total: int = Field(description="Total number of cards")
@@ -379,7 +379,7 @@ class LabelOperationResponse(StatusResponse):
 class ListCardCommentsResponse(BaseResponse):
     """Response model for listing card comments."""
 
-    results: list[Union[DeckComment, DeckCommentSummary]] = Field(
+    results: list[DeckComment | DeckCommentSummary] = Field(
         description="Card comments in this page (summaries unless detail='full')"
     )
     count: int = Field(

nextcloud_mcp_server/server/deck.py

@@ -51,7 +51,7 @@ DetailLevel = Literal["summary", "full"]
 _DEFAULT_DESCRIPTION_PREVIEW = 140
 
 
-def _validate_description_max_length(
+def _validate_positive_length(
     value: int | None, name: str = "description_max_length"
 ) -> None:
     """Tool-layer guard: reject zero/negative length thresholds.
@@ -113,12 +113,16 @@ def _filter_cards(
 
     The upstream Deck API returns every card (including archived ones) inline,
     so this filtering reduces the tokens the caller sees but not network
-    bandwidth. ``status="open"`` excludes archived and explicitly-done cards.
+    bandwidth.
+
+    ``open``/``done``/``archived`` partition the cards (no overlap): a card
+    that is both done and archived is reported only under ``archived``, since
+    archiving is the stronger "off the active board" state.
     """
     if status == "open":
         cards = [c for c in cards if not c.archived and c.done is None]
     elif status == "done":
-        cards = [c for c in cards if c.done is not None]
+        cards = [c for c in cards if c.done is not None and not c.archived]
     elif status == "archived":
         cards = [c for c in cards if c.archived]
     # status == "all": no status filter
@@ -473,8 +477,9 @@ def configure_deck_tools(mcp: FastMCP):
                 via deck_get_cards.
             detail: "summary" (default) returns compact card rows; "full"
                 returns the complete card objects (the old behavior).
-            status: Which cards to include — "open" (default; excludes
+            status: Which cards to include — "open" (default), "done",
-                archived and done), "done", "archived", or "all".
+                "archived", or "all". The first three partition the board
+                (a card that is both done and archived counts as "archived").
             label: If set, only cards carrying a label with this exact title.
             assigned_to: If set, only cards assigned to this user UID.
             description_max_length: In detail="full", truncate each card's
@@ -482,8 +487,8 @@ def configure_deck_tools(mcp: FastMCP):
             description_preview_length: In detail="summary", length of the
                 description preview carried on each card (default 140).
         """
-        _validate_description_max_length(description_max_length)
+        _validate_positive_length(description_max_length)
-        _validate_description_max_length(
+        _validate_positive_length(
             description_preview_length, "description_preview_length"
         )
         client = await get_client(ctx)
@@ -531,14 +536,15 @@ def configure_deck_tools(mcp: FastMCP):
             stack_id: The ID of the stack
             include_cards: Include cards in the stack (default True).
             detail: "summary" (default) or "full".
-            status: "open" (default), "done", "archived", or "all".
+            status: "open" (default), "done", "archived", or "all"
+                (non-overlapping; a done+archived card counts as "archived").
             label: If set, only cards carrying a label with this exact title.
             assigned_to: If set, only cards assigned to this user UID.
             description_max_length: In detail="full", truncate descriptions.
             description_preview_length: In detail="summary", preview length.
         """
-        _validate_description_max_length(description_max_length)
+        _validate_positive_length(description_max_length)
-        _validate_description_max_length(
+        _validate_positive_length(
             description_preview_length, "description_preview_length"
         )
         client = await get_client(ctx)
@@ -564,6 +570,8 @@ def configure_deck_tools(mcp: FastMCP):
         ctx: Context,
         board_id: int,
         detail: DetailLevel = "summary",
+        label: str | None = None,
+        assigned_to: str | None = None,
         description_max_length: int | None = None,
         description_preview_length: int = _DEFAULT_DESCRIPTION_PREVIEW,
     ) -> ListStacksResponse:
@@ -576,30 +584,35 @@ def configure_deck_tools(mcp: FastMCP):
 
         Cards are always included on the returned stacks (an archived stack
         without its cards would have no audit value) and returned as compact
-        summaries by default.
+        summaries by default. There is no ``status`` filter — every card here
+        is archived by definition — but ``label``/``assigned_to`` narrow the
+        set just like the active-stack tools.
 
         Args:
             board_id: The ID of the board
             detail: "summary" (default) or "full".
+            label: If set, only cards carrying a label with this exact title.
+            assigned_to: If set, only cards assigned to this user UID.
             description_max_length: In detail="full", truncate descriptions.
             description_preview_length: In detail="summary", preview length.
         """
-        _validate_description_max_length(description_max_length)
+        _validate_positive_length(description_max_length)
-        _validate_description_max_length(
+        _validate_positive_length(
             description_preview_length, "description_preview_length"
         )
         client = await get_client(ctx)
         stacks = await client.deck.get_archived_stacks(board_id)
         # All cards in archived stacks are themselves archived; status="all"
         # keeps them (an "open"/"done" filter would drop the whole point).
+        # label/assigned_to still apply for targeted audits.
         stacks = [
             _apply_stack_filters(
                 stack,
                 include_cards=True,
                 detail=detail,
                 status="all",
-                label=None,
+                label=label,
-                assigned_to=None,
+                assigned_to=assigned_to,
                 description_max_length=description_max_length,
                 description_preview_length=description_preview_length,
             )
@@ -637,15 +650,16 @@ def configure_deck_tools(mcp: FastMCP):
             stack_id: The ID of the stack
             detail: "summary" (default) returns compact card rows; "full"
                 returns the complete card objects.
-            status: "open" (default; excludes archived and done), "done",
+            status: "open" (default), "done", "archived", or "all". The first
-                "archived", or "all".
+                three partition the board (a done+archived card counts as
+                "archived").
             label: If set, only cards carrying a label with this exact title.
             assigned_to: If set, only cards assigned to this user UID.
             description_max_length: In detail="full", truncate descriptions.
             description_preview_length: In detail="summary", preview length.
         """
-        _validate_description_max_length(description_max_length)
+        _validate_positive_length(description_max_length)
-        _validate_description_max_length(
+        _validate_positive_length(
             description_preview_length, "description_preview_length"
         )
         client = await get_client(ctx)
@@ -678,21 +692,23 @@ def configure_deck_tools(mcp: FastMCP):
         """Get a compact, whole-board snapshot in a single call.
 
         Returns the board title, its label legend, and every stack with its
-        cards projected to compact summary rows. This is the token-efficient
+        cards projected to compact summary rows. Prefer it for "show me the
-        replacement for calling deck_get_board + deck_get_stacks when you just
+        board" / "what's in progress" style requests on large boards — it is
-        need to see the state of the board — prefer it for "show me the
+        the token-efficient way to view board *state*. It intentionally omits
-        board" / "what's in progress" style requests on large boards.
+        the board-management fields (ACL, user list, full label objects) that
+        deck_get_board exposes; reach for deck_get_board when you need those.
 
         Args:
             board_id: The ID of the board
-            status: Which cards to include — "open" (default; excludes
+            status: Which cards to include — "open" (default), "done",
-                archived and done), "done", "archived", or "all".
+                "archived", or "all". The first three partition the board
+                (a card that is both done and archived counts as "archived").
             label: If set, only cards carrying a label with this exact title.
             assigned_to: If set, only cards assigned to this user UID.
             description_preview_length: Length of the description preview
                 carried on each card summary (default 140).
         """
-        _validate_description_max_length(
+        _validate_positive_length(
             description_preview_length, "description_preview_length"
         )
         client = await get_client(ctx)
@@ -1324,7 +1340,7 @@ def configure_deck_tools(mcp: FastMCP):
             order: "newest" (default) or "oldest" — sort the page by creation
                 time.
         """
-        _validate_description_max_length(message_max_length, "message_max_length")
+        _validate_positive_length(message_max_length, "message_max_length")
         client = await get_client(ctx)
         comments = await client.deck.get_comments(card_id, limit=limit, offset=offset)
         shaped = _shape_comments(

tests/unit/test_deck_server.py

@@ -28,7 +28,7 @@ from nextcloud_mcp_server.server.deck import (
     _summarize_card,
     _truncate_card_descriptions,
     _truncate_comment_message,
-    _validate_description_max_length,
+    _validate_positive_length,
 )
 
 pytestmark = pytest.mark.unit
@@ -182,30 +182,30 @@ def test_truncate_card_descriptions_shorter_than_limit_no_ellipsis():
     assert cards[0].description == "hello"
 
 
-# _validate_description_max_length ----------------------------------------
+# _validate_positive_length ----------------------------------------
 
 
-def test_validate_description_max_length_accepts_none():
+def test_validate_positive_length_accepts_none():
     """None is the documented sentinel for "no truncation"."""
-    _validate_description_max_length(None)
+    _validate_positive_length(None)
 
 
-def test_validate_description_max_length_accepts_positive():
+def test_validate_positive_length_accepts_positive():
     """Positive values pass through silently."""
-    _validate_description_max_length(1)
+    _validate_positive_length(1)
-    _validate_description_max_length(1000)
+    _validate_positive_length(1000)
 
 
-def test_validate_description_max_length_rejects_zero():
+def test_validate_positive_length_rejects_zero():
     """Zero would wipe descriptions to a single ellipsis — reject at the boundary."""
     with pytest.raises(ValueError, match="must be positive"):
-        _validate_description_max_length(0)
+        _validate_positive_length(0)
 
 
-def test_validate_description_max_length_rejects_negative():
+def test_validate_positive_length_rejects_negative():
     """Negative values produce surprising slice semantics — reject at the boundary."""
     with pytest.raises(ValueError, match="must be positive"):
-        _validate_description_max_length(-10)
+        _validate_positive_length(-10)
 
 
 # _apply_board_filters ------------------------------------------------------
@@ -299,8 +299,8 @@ def test_filter_cards_open_excludes_archived_and_done():
     assert [c.id for c in result] == [1]
 
 
-def test_filter_cards_done_keeps_only_done():
+def test_filter_cards_done_keeps_only_done_and_not_archived():
-    """status="done" keeps only cards with a done timestamp."""
+    """status="done" keeps done cards that are not archived."""
     done_at = datetime(2024, 1, 1, tzinfo=timezone.utc)
     cards = [_make_card(1), _make_card(2, done=done_at)]
     result = _filter_cards(cards, status="done", label=None, assigned_to=None)
@@ -314,6 +314,27 @@ def test_filter_cards_archived_keeps_only_archived():
     assert [c.id for c in result] == [2]
 
 
+def test_filter_cards_statuses_partition_the_board():
+    """open/done/archived are non-overlapping; a done+archived card counts
+    only as "archived", not "done"."""
+    done_at = datetime(2024, 1, 1, tzinfo=timezone.utc)
+    open_card = _make_card(1)
+    done_card = _make_card(2, done=done_at)
+    done_and_archived = _make_card(3, done=done_at, archived=True)
+    cards = [open_card, done_card, done_and_archived]
+
+    assert [
+        c.id for c in _filter_cards(cards, status="open", label=None, assigned_to=None)
+    ] == [1]
+    assert [
+        c.id for c in _filter_cards(cards, status="done", label=None, assigned_to=None)
+    ] == [2]
+    assert [
+        c.id
+        for c in _filter_cards(cards, status="archived", label=None, assigned_to=None)
+    ] == [3]
+
+
 def test_filter_cards_all_keeps_everything():
     """status="all" applies no status filter."""
     cards = [_make_card(1), _make_card(2, archived=True)]