Skip to content

feat: group DM conversations — lifecycle + members (1/3)#56

Merged
jackparnell merged 1 commit into
TheColonyCC:mainfrom
arch-colony:feat/group-dm-lifecycle
May 27, 2026
Merged

feat: group DM conversations — lifecycle + members (1/3)#56
jackparnell merged 1 commit into
TheColonyCC:mainfrom
arch-colony:feat/group-dm-lifecycle

Conversation

@arch-colony
Copy link
Copy Markdown
Collaborator

Summary

First of three PRs adding group-DM coverage to the SDK. Wraps 13 endpoints under /api/v1/messages/groups/*:

Lifecycle: create_group_conversation, list_group_templates, create_group_from_template, get_group_conversation, update_group_conversation, send_group_message

Members: list_group_members, add_group_member, remove_group_member, set_group_admin, transfer_group_creator, respond_to_group_invite, mark_group_all_read

All three clients (ColonyClient, AsyncColonyClient, MockColonyClient) get the new methods.

Why three PRs

Group-DM coverage is ~30 endpoints across lifecycle, state (mute/snooze/receipts/pin), avatar, search, per-message ops (read/reactions/edit/star/forward), and attachments. Splitting keeps review surface manageable:

  • PR 1 (this) — lifecycle + members
  • PR 2 — state + avatar + search (mute/snooze/receipts/pin/search/avatar)
  • PR 3 — per-message ops + attachments

Per author request, no version bump until all three are in.

Test plan

  • 53 new tests added (TestGroupConversationsLifecycle, TestGroupMembership, async + mock counterparts)
  • ruff check + ruff format --check clean
  • mypy src/ clean
  • All 559 unit tests pass
  • 100% line coverage preserved (sync + async + mock)

Notes for review

  • The server takes most state-changing inputs as query params (its choice for v1 simplicity). The SDK URL-encodes them; tests pin the exact wire format.
  • Booleans are serialised as the lowercase strings "true" / "false" — FastAPI's bool query coercion rejects Python's capitalised str(bool) default. There's an explicit test for this.
  • send_group_message accepts idempotency_key on the sync client only. The async _raw_request doesn't yet thread the Idempotency-Key header through — same gap as the existing async send_message. Documented in the docstring; will close in a follow-up that fixes both at once.
  • update_group_conversation(conv_id, description="") deliberately sends description= on the wire (clears the description), distinct from omitting the kwarg (no change). Pinned by test_update_group_conversation_empty_clears_description.

🤖 Generated with Claude Code

First of three PRs adding group-DM coverage to the SDK. Wraps the 13
endpoints under /api/v1/messages/groups/* that handle group creation,
metadata, message-send, and member management.

Lifecycle:
  create_group_conversation(title, members)
  list_group_templates()
  create_group_from_template(template, members, title_override=None)
  get_group_conversation(conv_id, limit=50, offset=0)
  update_group_conversation(conv_id, title=None, description=None)
  send_group_message(conv_id, body, reply_to_message_id=None,
                     idempotency_key=None)

Members:
  list_group_members(conv_id)
  add_group_member(conv_id, username)
  remove_group_member(conv_id, user_id)
  set_group_admin(conv_id, user_id, is_admin)
  transfer_group_creator(conv_id, new_creator_username)
  respond_to_group_invite(conv_id, accept)
  mark_group_all_read(conv_id)

Per the user's instruction, no version bump in this PR — the version
will move once the remaining two PRs (per-message ops + attachments)
have landed.

idempotency_key is sync-only for now: the async _raw_request doesn't
yet thread the Idempotency-Key header through. This matches the
existing async send_message gap; both will close together in a
follow-up.

53 new tests across sync (TestGroupConversationsLifecycle,
TestGroupMembership), async (TestAsyncGroupConversationsLifecycle,
TestAsyncGroupMembership), and mock-client coverage. 100% line
coverage preserved. Lint + format + mypy + 559 tests all green.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@ColonistOne ColonistOne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed end-to-end against main. CI is fully green and the surface looks ready to merge.

Strengths

  • Three-client parity is tight. Every sync method has a matching async + mock entry with the same signature (one deliberate exception, see below). MockColonyClient reuses the existing _respond recording protocol — zero new mock machinery.
  • Three known traps explicitly pinned by tests:
    • description="" clears vs description=None omits — test_update_group_conversation_empty_clears_description guards against accidental falsy-collapse.
    • FastAPI boolean coercion: is_admin=true/false lowercase, never str(bool) capitalised — pinned in test_set_group_admin_demote.
    • Idempotency-Key header threading on sync send_group_message — pinned in test_send_group_message_with_idempotency_key.
  • Wire-format tests cover both shape and encodingtest_create_group_conversation_escapes_special_chars confirms title=R%26D+Lab rather than ampersand-as-separator. The right depth for an SDK whose entire job is producing the correct wire bytes.
  • Documented deliberate gaps rather than silently divergent: PR description calls out the three-PR plan + no-version-bump, the async send_group_message docstring acknowledges the missing idempotency_key (matching the pre-existing 1:1 send_message precedent), and the CHANGELOG mirrors both.

Minor observations (none blocking)

  1. from urllib.parse import urlencode is repeated inline inside each async method — but this matches the pre-existing style in async_client.py (lines 476, 632, 989, 1016, 1042), so the PR is consistent with the codebase rather than introducing a new wart. Worth a separate cleanup sweep someday.
  2. No client-side validation of bounds (title 1..100, description 0..500, members 1..49) — all server-validated. Fine for v1; could fail-fast in client later if useful.
  3. update_group_conversation with both fields omitted PATCHes a query-less URL and lets the server 400. Symmetric with the PATCH-omits-no-change contract, so I'd leave it.

Approving. Holding the merge button for a human reviewer per the TheColonyCC/* convention.

@jackparnell jackparnell merged commit 70a3417 into TheColonyCC:main May 27, 2026
7 checks passed
jackparnell pushed a commit that referenced this pull request May 27, 2026
Second of three PRs adding group-DM coverage. Layers conversation
state + within-group search on top of the lifecycle methods landed
in #56.

State (per-participant — affects only the caller's row):
  mute_group_conversation(conv_id, until=None)
  unmute_group_conversation(conv_id)
  snooze_group_conversation(conv_id, duration)
  unsnooze_group_conversation(conv_id)
  set_group_read_receipts(conv_id, show=None)  # 3-state override

Pins (group-wide, admin-only):
  pin_group_message(conv_id, msg_id)
  unpin_group_message(conv_id, msg_id)

Search:
  search_group_messages(conv_id, q, limit=50, offset=0)

Group-avatar upload + serve were originally planned for this PR but
pulled out: they need multipart/form-data, which the SDK doesn't yet
support, and the work belongs alongside attachment uploads (PR 3
already needs the same transport). Keeps PR 2's surface focused on
query-string endpoints we can land cleanly with the existing
infrastructure.

Per the user's instruction, still no version bump.

35 new tests covering the three-state set-receipts surface (true /
false / cleared), the lowercase-bool quirk that FastAPI's query
coercion enforces, query-string escaping for special characters,
and pagination defaults. 100% line coverage preserved across sync,
async, and mock clients. 594 tests + lint + format + mypy green.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants