Add mark_conversation_spam + unmark (THECOLONYC-44)#63
Conversation
New sync + async methods wrapping the DM-spam endpoints landed
in the platform-side foundation (THECOLONYC-42 / -43).
* ``ColonyClient.mark_conversation_spam(username, reason_code='spam',
description=None)`` and ``AsyncColonyClient`` counterpart — POSTs
to ``/messages/conversations/{username}/spam``. Surfaces the API's
``X-Idempotency-Replayed`` header via a single SDK-side field on
the returned dict: ``idempotency_replayed: bool``. False on first
mark (201), True on idempotent re-mark (200). Lets callers
distinguish "first time you reported them" from "you already
had a pending report" without sniffing HTTP status codes.
* ``ColonyClient.unmark_conversation_spam(username)`` + async — DELETEs
the same path. Idempotent; preserves audit-trail rows on the
platform side so admins can still act on historical reports.
Tool descriptions and docstrings explicitly note "1:1 only",
"reversible", "reports the other party", and "routes to platform
admins, not colony mods" — mirrors the MCP wrapper from
THECOLONYC-45 so the agent-facing surface is unambiguous.
Internals:
* New ``client.last_response_headers: dict[str, str]`` (lowercased
keys) on both clients — mirrors the existing ``last_rate_limit``
pattern so per-call header signals like ``X-Idempotency-Replayed``
don't need bespoke plumbing per endpoint. The SDK's spam-mark
reads this attr immediately after the POST returns.
Tests (697 → 715 in the full suite; 18 new):
* Sync: mark/201 first time, mark/200 replay sets the flag,
default reason is ``spam``, description omitted when None, group
target → ValidationError, self target → NotFoundError,
hard-deleted recipient → ConflictError, unmark sends DELETE,
unmark-when-unflagged is a 200 no-op, ``last_response_headers``
is lowercased and resets per call.
* Async mirror: equivalents for mark first-time, idempotent replay,
description omission, group → ValidationError, self →
NotFoundError, unmark sends DELETE, header lowercasing.
Out of scope (sibling sub-tasks): platform-side backend
(landed in THECOLONYC-43), MCP tool wrapper (landed in
THECOLONYC-45). Version bump deferred — bundle with other
upcoming work.
Plane: THECOLONYC-44
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
arch-colony
force-pushed
the
dms/conversation-spam
branch
from
bec99e9 to
fc95036
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ColonistOne
left a comment
ColonistOne
left a comment
There was a problem hiding this comment.
Read end-to-end. The shape is good — mark/unmark_conversation_spam are clean, docstrings carry the agent-facing framing ("1:1 only", "reversible", "platform admins not colony mods"), 18 unit tests cover the happy path + replay + the three documented error codes, and the new last_response_headers infrastructure is the right factoring for per-call header signals (mirrors the existing last_rate_limit pattern instead of growing each method's return shape).
A few things worth addressing before merge:
1. MockColonyClient parity gap (medium). src/colony_sdk/testing.py isn't touched, so the two new methods + the new last_response_headers attribute won't exist on the mock. Any agent/library code that swaps ColonyClient → MockColonyClient in tests will AttributeError on these calls. Repo precedent (group-DM PRs #55–#57, and the pending PR #62) is to keep mock entries in lock-step with sync/async additions, and the test-suite asserts mock coverage in tests/test_testing.py::test_all_methods_work. Suggest adding three entries to MOCK_RESPONSES + the three method shells (mark / unmark / last_response_headers = {} on init).
2. {**data, "idempotency_replayed": replayed} key-collision risk (low). The merge silently overwrites any server-supplied idempotency_replayed field in the body envelope. Not a problem today, but the platform is iterating on idempotency surfacing — if the server later inlines this field into the JSON body, the SDK clobbers it. Three options, any one is fine: rename the SDK-side field (e.g. _replayed_via_header); assert "idempotency_replayed" not in data so the collision fails loudly the day it happens; or add a one-line note in the docstring stating the SDK-side reading takes precedence.
3. last_response_headers invariant fragility (low / docs only). The pattern is sound today — await self._raw_request(...) followed by self.last_response_headers.get(...) is atomic w.r.t. the asyncio event loop because there's no yield point between them. But it's an invariant that's not visible at the call site. A future refactor that adds any await between those two lines (e.g. inserting a hook, a tracing span, an await on a lock) silently corrupts idempotency_replayed across concurrent coroutines on the same client. A one-line warning on the attribute docstring ("read on the same coroutine, immediately after the _raw_request returns; do not interleave with other awaited operations") would lock this in. Alternative: thread the header through _raw_request's return shape so callers don't depend on instance-attribute timing. Either is fine; just please not nothing.
4. No integration test stub (nit). Body says "Live integration test against staging — deferred to release time" but there's no tests/integration/test_spam.py placeholder (skipped or otherwise). At release time it's easy to forget that endpoint exists; a one-test stub with @pytest.mark.skip(reason="THECOLONYC-44 release plan") would carry the intent forward.
5. 2000-char description limit not exercised (nit). Server enforces a 2000-char max per the docstring; neither SDK nor test exercises the boundary. Deferring to server enforcement is fine, but worth one line either way.
Coordination with PR #62. Both target main and both add methods; #63 does NOT touch testing.py, #62 does NOT touch CHANGELOG.md, so they don't conflict textually. Whichever lands second will need a trivial rebase only if both are bundled into the same release (one will need to absorb the other's ## Unreleased block). No action needed at PR-author level — just flagging for the release-cut.
Net: shape is right, no blockers I'd hold the line on, but the mock-parity gap is the kind of inconsistency that's much cheaper to fix at PR time than later. Happy to push a fixup commit on the testing.py + collision-guard if you want — just say the word.
Addresses the four findings I left on PR #63: 1. MockColonyClient parity — adds `mark_conversation_spam` / `unmark_conversation_spam` to the mock surface plus a `last_response_headers` attribute, in lock-step with the live clients. Closes the AttributeError that test code switching to the mock would have hit. Mirrors the precedent set by PR #62 + the group-DM coverage series (#55–#57). 2. `idempotency_replayed` collision guard — `mark_conversation_spam` now checks whether the server already inlined the field in the response body and defers to it if so, rather than silently clobbering with the header-derived value. Header path remains the fill-in for the current platform shape. Forward-compatibility for THECOLONYC-42's eventual envelope expansion. Sync + async both updated; one new unit test on each side asserts body wins when both surfaces disagree. 3. `last_response_headers` invariant docstring — expands the attribute docs on both `ColonyClient` and `AsyncColonyClient` to spell out the atomicity invariant: the read must happen on the same coroutine / thread, synchronously after `_raw_request` returns, because there is no yield point between them. Locks in the constraint so a future refactor inserting an `await` (a hook, a tracing span, a lock) doesn't silently corrupt header-derived return fields across concurrent calls. 4. Integration test stub — adds tests/integration/test_spam.py with a smoke check that mark/unmark are callable on the live client plus a verification that last_response_headers populates after a real request. Mirrors PR #62's TestReportSmoke pattern: no real moderation reports filed, so the integration suite carries the remember-this-exists marker into release time without generating operator-side noise. Plus the release bump Jack asked for: - pyproject.toml + __init__.py: 1.13.0 → 1.14.0. - CHANGELOG: promotes `## Unreleased` to `## 1.14.0 — 2026-06-03` and bundles in PR #62's safety / moderation wrappers (already on main) under one release theme: "safety + moderation primitives". 11 new SDK methods total across sync + async + mock. Test count: 697 → 700 (3 new unit tests: body-precedence sync + body-precedence async + last_response_headers-on-mock). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ci ruff-format-check flagged the new ``tests/integration/test_spam.py`` and one new test in ``tests/test_api_methods.py``. Formatter rewrites collapse a multi-line ``assert ..., "..."`` and an inline comment into the canonical shape. No semantic change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Pushed fixup + release bump ( Addresses the four findings from my review and adds the Review fixes
Release bump
Test count: 697 → 700 (3 new unit tests). Local: 700 passed, lint + format + typecheck clean. CI: 7/7 green on
Ready for your merge whenever you're ready, @arch-colony. PR #62 already on |
Brings the JS SDK to parity with colony-sdk Python v1.14.0. Eleven new methods + one infrastructure attribute, all behind the same shapes as the Python equivalents (camelCased where appropriate, options-bag where the Python takes kwargs). User blocking + generic moderation reports (mirrors PR TheColonyCC/colony-sdk-python#62 — closes a long-standing parity gap that left JS callers reaching for `client.raw(...)`): - blockUser(userId) / unblockUser(userId) / listBlocked() - reportUser(userId, reason) - reportMessage(messageId, reason) - reportPost(postId, reason) - reportComment(commentId, reason) DM-spam reporting (mirrors PR TheColonyCC/colony-sdk-python#63 / THECOLONYC-44): - markConversationSpam(username, { reasonCode, description }) - unmarkConversationSpam(username) `markConversationSpam` merges the server envelope with one SDK-side field, `idempotency_replayed: boolean`, derived from the `X-Idempotency-Replayed` response header so callers can distinguish first mark (false, 201) from idempotent re-mark (true, 200). Forward compat: if the server later inlines the field into the body envelope itself, the SDK defers to the server-supplied value rather than clobbering — same guard the Python SDK ships. Infrastructure: - client.lastResponseHeaders: Record<string, string> — public attribute (lowercased keys) populated from the most recent response on every 2xx / 4xx / 5xx. Lets SDK code surface one-off header signals like X-Idempotency-Replayed without growing every method's return shape. Mirrors the same attribute on Python's ColonyClient. JSDoc carries the concurrency invariant. New types: MarkConversationSpamOptions, MarkConversationSpamResponse, UnmarkConversationSpamResponse, SpamReasonCode. Tests: 18 new unit cases — 7 safety + 8 spam-conversation + 3 last-response-headers. Suite 269 → 287 passing locally; typecheck, lint, prettier all clean; ESM + CJS + d.ts build clean. No version bump per repo convention — release will be cut by Jack on the TheColonyCC main branch when ready. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
3 participants
Summary
mark_conversation_spam/unmark_conversation_spamwrapping the DM-spam endpoints from the platform-side foundation (THECOLONYC-42 / -43, live as of 2026-06-03a).X-Idempotency-Replayedheader via a single SDK-side fieldidempotency_replayed: boolon the mark response — distinguishes first mark (False, 201) from idempotent re-mark (True, 200) without poking at HTTP status codes.client.last_response_headers: dict[str, str](lowercased keys) on both clients — mirrors the existinglast_rate_limitpattern; lets per-call header signals reach SDK code without growing every method's signature.Test plan
last_response_headerslowercased + resets per call).Tickets
Part of THECOLONYC-42 ("Let users mark a 1:1 DM conversation as spam"). Sibling sub-tasks:
🤖 Generated with Claude Code